From 31708c080597b1e176cd5d892bfd44496bfdbf36 Mon Sep 17 00:00:00 2001 From: Christopher Chong Date: Mon, 11 Dec 2023 06:29:50 +0300 Subject: [PATCH] Make tls optional between prover and notary. (#387) * Make tls optional between prover and notary. * Parameterize test, flatten code. --- notary-server/Cargo.toml | 1 + notary-server/config/config.yaml | 3 +- notary-server/src/config.rs | 10 +- notary-server/src/lib.rs | 2 +- notary-server/src/server.rs | 139 +++++++++++++----------- notary-server/tests/integration_test.rs | 115 ++++++++++++++------ 6 files changed, 166 insertions(+), 104 deletions(-) diff --git a/notary-server/Cargo.toml b/notary-server/Cargo.toml index 43a58117fd..d2a6991a34 100644 --- a/notary-server/Cargo.toml +++ b/notary-server/Cargo.toml @@ -19,6 +19,7 @@ http = "0.2.9" hyper = { version = "0.14", features = ["client", "http1", "server", "tcp"] } opentelemetry = { version = "0.19" } p256 = "0.13" +rstest = "0.18" rustls = { version = "0.21" } rustls-pemfile = { version = "1.0.2" } serde = { version = "1.0.147", features = ["derive"] } diff --git a/notary-server/config/config.yaml b/notary-server/config/config.yaml index 9d917cfbee..7efd88b268 100644 --- a/notary-server/config/config.yaml +++ b/notary-server/config/config.yaml @@ -6,7 +6,8 @@ server: notarization: max-transcript-size: 16384 -tls-signature: +tls: + enabled: true private-key-pem-path: "./fixture/tls/notary.key" certificate-pem-path: "./fixture/tls/notary.crt" diff --git a/notary-server/src/config.rs b/notary-server/src/config.rs index 2057a9e7cc..396984c3cb 100644 --- a/notary-server/src/config.rs +++ b/notary-server/src/config.rs @@ -7,8 +7,8 @@ pub struct NotaryServerProperties { pub server: ServerProperties, /// Setting for notarization pub notarization: NotarizationProperties, - /// File path of private key and certificate (in PEM format) used for establishing TLS with prover - pub tls_signature: TLSSignatureProperties, + /// Setting for TLS connection between prover and notary + pub tls: TLSProperties, /// File path of private key (in PEM format) used to sign the notarization pub notary_signature: NotarySignatureProperties, /// Setting for logging/tracing @@ -23,7 +23,7 @@ pub struct AuthorizationProperties { /// Switch to turn on or off auth middleware pub enabled: bool, /// File path of the whitelist API key csv - pub whitelist_csv_path: Option, + pub whitelist_csv_path: String, } #[derive(Clone, Debug, Deserialize)] @@ -44,7 +44,9 @@ pub struct ServerProperties { #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] -pub struct TLSSignatureProperties { +pub struct TLSProperties { + /// Flag to turn on/off TLS between prover and notary (should always be turned on unless TLS is handled by external setup e.g. reverse proxy, cloud) + pub enabled: bool, pub private_key_pem_path: String, pub certificate_pem_path: String, } diff --git a/notary-server/src/lib.rs b/notary-server/src/lib.rs index c411eff92c..e1d2a80384 100644 --- a/notary-server/src/lib.rs +++ b/notary-server/src/lib.rs @@ -9,7 +9,7 @@ mod util; pub use config::{ AuthorizationProperties, NotarizationProperties, NotaryServerProperties, - NotarySignatureProperties, ServerProperties, TLSSignatureProperties, TracingProperties, + NotarySignatureProperties, ServerProperties, TLSProperties, TracingProperties, }; pub use domain::{ cli::CliFields, diff --git a/notary-server/src/server.rs b/notary-server/src/server.rs index 7432fece81..8eb5f7e92b 100644 --- a/notary-server/src/server.rs +++ b/notary-server/src/server.rs @@ -28,7 +28,7 @@ use tower::MakeService; use tracing::{debug, error, info}; use crate::{ - config::{NotaryServerProperties, NotarySignatureProperties, TLSSignatureProperties}, + config::{NotaryServerProperties, NotarySignatureProperties}, domain::{ auth::{authorization_whitelist_vec_into_hashmap, AuthorizationWhitelistRecord}, notary::NotaryGlobals, @@ -40,69 +40,68 @@ use crate::{ util::parse_csv_file, }; -/// Start a TLS-secured TCP server to accept notarization request for both TCP and WebSocket clients +/// Start a TCP server (with or without TLS) to accept notarization request for both TCP and WebSocket clients #[tracing::instrument(skip(config))] pub async fn run_server(config: &NotaryServerProperties) -> Result<(), NotaryServerError> { - // Load the private key and cert needed for TLS connection from fixture folder — can be swapped out when we stop using static self signed cert - let (tls_private_key, tls_certificates) = load_tls_key_and_cert(&config.tls_signature).await?; - // Load the private key for notarized transcript signing from fixture folder — can be swapped out when we use proper ephemeral signing key + // Load the private key for notarized transcript signing let notary_signing_key = load_notary_signing_key(&config.notary_signature).await?; + // Build TLS acceptor if it is turned on + let tls_acceptor = if !config.tls.enabled { + debug!("Skipping TLS setup as it is turned off."); + None + } else { + let (tls_private_key, tls_certificates) = load_tls_key_and_cert( + &config.tls.private_key_pem_path, + &config.tls.certificate_pem_path, + ) + .await?; + + let mut server_config = ServerConfig::builder() + .with_safe_defaults() + .with_no_client_auth() + .with_single_cert(tls_certificates, tls_private_key) + .map_err(|err| eyre!("Failed to instantiate notary server tls config: {err}"))?; + + // Set the http protocols we support + server_config.alpn_protocols = vec![b"http/1.1".to_vec()]; + let tls_config = Arc::new(server_config); + Some(TlsAcceptor::from(tls_config)) + }; + // Load the authorization whitelist csv if it is turned on let authorization_whitelist = if !config.authorization.enabled { debug!("Skipping authorization as it is turned off."); None } else { - // Get the path of whitelist csv from config - let whitelist_csv_path = config - .authorization - .whitelist_csv_path - .as_ref() - .ok_or(eyre!( - "Failed to load authorization whitelist as its csv path is absent in config" - ))?; // Load the csv - let whitelist_csv = parse_csv_file::(whitelist_csv_path) - .map_err(|err| eyre!("Failed to parse authorization whitelist csv: {:?}", err))?; + let whitelist_csv = parse_csv_file::( + &config.authorization.whitelist_csv_path, + ) + .map_err(|err| eyre!("Failed to parse authorization whitelist csv: {:?}", err))?; // Convert the whitelist record into hashmap for faster lookup Some(authorization_whitelist_vec_into_hashmap(whitelist_csv)) }; - // Build a TCP listener with TLS enabled - let mut server_config = ServerConfig::builder() - .with_safe_defaults() - .with_no_client_auth() - .with_single_cert(tls_certificates, tls_private_key) - .map_err(|err| eyre!("Failed to instantiate notary server tls config: {err}"))?; - - // Set the http protocols we support - server_config.alpn_protocols = vec![b"http/1.1".to_vec()]; - let tls_config = Arc::new(server_config); - let notary_address = SocketAddr::new( IpAddr::V4(config.server.host.parse().map_err(|err| { eyre!("Failed to parse notary host address from server config: {err}") })?), config.server.port, ); - - let acceptor = TlsAcceptor::from(tls_config); let listener = TcpListener::bind(notary_address) .await .map_err(|err| eyre!("Failed to bind server address to tcp listener: {err}"))?; let mut listener = AddrIncoming::from_listener(listener) .map_err(|err| eyre!("Failed to build hyper tcp listener: {err}"))?; - info!( - "Listening for TLS-secured TCP traffic at {}", - notary_address - ); + info!("Listening for TCP traffic at {}", notary_address); let protocol = Arc::new(Http::new()); let notary_globals = NotaryGlobals::new( notary_signing_key, config.notarization.clone(), - authorization_whitelist.map(Arc::new), // Use Arc to prevent cloning the whitelist for every request + authorization_whitelist.map(Arc::new), ); // Parameters needed for the info endpoint @@ -155,34 +154,48 @@ pub async fn run_server(config: &NotaryServerProperties) -> Result<(), NotarySer }; debug!(?prover_address, "Received a prover's TCP connection"); - let acceptor = acceptor.clone(); + let tls_acceptor = tls_acceptor.clone(); let protocol = protocol.clone(); let service = MakeService::<_, Request>::make_service(&mut app, &stream); // Spawn a new async task to handle the new connection tokio::spawn(async move { - match acceptor.accept(stream).await { - Ok(stream) => { - info!( - ?prover_address, - "Accepted prover's TLS-secured TCP connection", - ); - // Serve different requests using the same hyper protocol and axum router - let _ = protocol - // Can unwrap because it's infallible - .serve_connection(stream, service.await.unwrap()) - // use with_upgrades to upgrade connection to websocket for websocket clients - // and to extract tcp connection for tcp clients - .with_upgrades() - .await; - } - Err(err) => { - error!( - ?prover_address, - "{}", - NotaryServerError::Connection(err.to_string()) - ); + // When TLS is enabled + if let Some(acceptor) = tls_acceptor { + match acceptor.accept(stream).await { + Ok(stream) => { + info!( + ?prover_address, + "Accepted prover's TLS-secured TCP connection", + ); + // Serve different requests using the same hyper protocol and axum router + let _ = protocol + // Can unwrap because it's infallible + .serve_connection(stream, service.await.unwrap()) + // use with_upgrades to upgrade connection to websocket for websocket clients + // and to extract tcp connection for tcp clients + .with_upgrades() + .await; + } + Err(err) => { + error!( + ?prover_address, + "{}", + NotaryServerError::Connection(err.to_string()) + ); + } } + } else { + // When TLS is disabled + info!(?prover_address, "Accepted prover's TCP connection",); + // Serve different requests using the same hyper protocol and axum router + let _ = protocol + // Can unwrap because it's infallible + .serve_connection(stream, service.await.unwrap()) + // use with_upgrades to upgrade connection to websocket for websocket clients + // and to extract tcp connection for tcp clients + .with_upgrades() + .await; } }); } @@ -207,11 +220,12 @@ pub async fn read_pem_file(file_path: &str) -> Result> { /// Load notary tls private key and cert from static files async fn load_tls_key_and_cert( - config: &TLSSignatureProperties, + private_key_pem_path: &str, + certificate_pem_path: &str, ) -> Result<(PrivateKey, Vec)> { debug!("Loading notary server's tls private key and certificate"); - let mut private_key_file_reader = read_pem_file(&config.private_key_pem_path).await?; + let mut private_key_file_reader = read_pem_file(private_key_pem_path).await?; let mut private_keys = rustls_pemfile::pkcs8_private_keys(&mut private_key_file_reader)?; ensure!( private_keys.len() == 1, @@ -219,7 +233,7 @@ async fn load_tls_key_and_cert( ); let private_key = PrivateKey(private_keys.remove(0)); - let mut certificate_file_reader = read_pem_file(&config.certificate_pem_path).await?; + let mut certificate_file_reader = read_pem_file(certificate_pem_path).await?; let certificates = rustls_pemfile::certs(&mut certificate_file_reader)? .into_iter() .map(Certificate) @@ -235,11 +249,10 @@ mod test { #[tokio::test] async fn test_load_notary_key_and_cert() { - let config = TLSSignatureProperties { - private_key_pem_path: "./fixture/tls/notary.key".to_string(), - certificate_pem_path: "./fixture/tls/notary.crt".to_string(), - }; - let result: Result<(PrivateKey, Vec)> = load_tls_key_and_cert(&config).await; + let private_key_pem_path = "./fixture/tls/notary.key"; + let certificate_pem_path = "./fixture/tls/notary.crt"; + let result: Result<(PrivateKey, Vec)> = + load_tls_key_and_cert(private_key_pem_path, certificate_pem_path).await; assert!(result.is_ok(), "Could not load tls private key and cert"); } diff --git a/notary-server/tests/integration_test.rs b/notary-server/tests/integration_test.rs index 0b6ead318b..4fa8d1dedc 100644 --- a/notary-server/tests/integration_test.rs +++ b/notary-server/tests/integration_test.rs @@ -8,6 +8,7 @@ use hyper::{ Body, Client, Request, StatusCode, }; use hyper_tls::HttpsConnector; +use rstest::rstest; use rustls::{Certificate, ClientConfig, RootCertStore}; use std::{ net::{IpAddr, SocketAddr}, @@ -16,7 +17,11 @@ use std::{ }; use tls_server_fixture::{bind_test_server_hyper, CA_CERT_DER, SERVER_DOMAIN}; use tlsn_prover::tls::{Prover, ProverConfig}; -use tokio_rustls::TlsConnector; +use tokio::{ + io::{AsyncRead, AsyncWrite}, + net::TcpStream, +}; +use tokio_rustls::{client::TlsStream, TlsConnector}; use tokio_util::compat::{FuturesAsyncReadCompatExt, TokioAsyncReadCompatExt}; use tracing::debug; use ws_stream_tungstenite::WsStream; @@ -24,14 +29,14 @@ use ws_stream_tungstenite::WsStream; use notary_server::{ read_pem_file, run_server, AuthorizationProperties, NotarizationProperties, NotarizationSessionRequest, NotarizationSessionResponse, NotaryServerProperties, - NotarySignatureProperties, ServerProperties, TLSSignatureProperties, TracingProperties, + NotarySignatureProperties, ServerProperties, TLSProperties, TracingProperties, }; const NOTARY_CA_CERT_PATH: &str = "./fixture/tls/rootCA.crt"; const NOTARY_CA_CERT_BYTES: &[u8] = include_bytes!("../fixture/tls/rootCA.crt"); -async fn setup_config_and_server(sleep_ms: u64, port: u16) -> NotaryServerProperties { - let notary_config = NotaryServerProperties { +fn get_server_config(port: u16, tls_enabled: bool) -> NotaryServerProperties { + NotaryServerProperties { server: ServerProperties { name: "tlsnotaryserver.io".to_string(), host: "127.0.0.1".to_string(), @@ -40,7 +45,8 @@ async fn setup_config_and_server(sleep_ms: u64, port: u16) -> NotaryServerProper notarization: NotarizationProperties { max_transcript_size: 1 << 14, }, - tls_signature: TLSSignatureProperties { + tls: TLSProperties { + enabled: tls_enabled, private_key_pem_path: "./fixture/tls/notary.key".to_string(), certificate_pem_path: "./fixture/tls/notary.crt".to_string(), }, @@ -53,9 +59,17 @@ async fn setup_config_and_server(sleep_ms: u64, port: u16) -> NotaryServerProper }, authorization: AuthorizationProperties { enabled: false, - whitelist_csv_path: None, + whitelist_csv_path: "./fixture/auth/whitelist.csv".to_string(), }, - }; + } +} + +async fn setup_config_and_server( + sleep_ms: u64, + port: u16, + tls_enabled: bool, +) -> NotaryServerProperties { + let notary_config = get_server_config(port, tls_enabled); let _ = tracing_subscriber::fmt::try_init(); @@ -72,10 +86,22 @@ async fn setup_config_and_server(sleep_ms: u64, port: u16) -> NotaryServerProper notary_config } -#[tokio::test] -async fn test_tcp_prover() { - // Notary server configuration setup - let notary_config = setup_config_and_server(100, 7048).await; +async fn tcp_socket(notary_config: NotaryServerProperties) -> TcpStream { + tokio::net::TcpStream::connect(SocketAddr::new( + IpAddr::V4(notary_config.server.host.parse().unwrap()), + notary_config.server.port, + )) + .await + .unwrap() +} + +async fn tls_socket(notary_config: NotaryServerProperties) -> TlsStream { + let notary_tcp_socket = tokio::net::TcpStream::connect(SocketAddr::new( + IpAddr::V4(notary_config.server.host.parse().unwrap()), + notary_config.server.port, + )) + .await + .unwrap(); // Connect to the Notary via TLS-TCP let mut certificate_file_reader = read_pem_file(NOTARY_CA_CERT_PATH).await.unwrap(); @@ -93,29 +119,47 @@ async fn test_tcp_prover() { .with_safe_defaults() .with_root_certificates(root_store) .with_no_client_auth(); - let notary_connector = TlsConnector::from(Arc::new(client_notary_config)); - let notary_host = notary_config.server.host.clone(); - let notary_port = notary_config.server.port; - let notary_socket = tokio::net::TcpStream::connect(SocketAddr::new( - IpAddr::V4(notary_host.parse().unwrap()), - notary_port, - )) - .await - .unwrap(); - - let notary_tls_socket = notary_connector + let notary_connector = TlsConnector::from(Arc::new(client_notary_config)); + notary_connector .connect( notary_config.server.name.as_str().try_into().unwrap(), - notary_socket, + notary_tcp_socket, ) .await - .unwrap(); + .unwrap() +} - // Attach the hyper HTTP client to the notary TLS connection to send request to the /session endpoint to configure notarization and obtain session id - let (mut request_sender, connection) = hyper::client::conn::handshake(notary_tls_socket) - .await - .unwrap(); +#[rstest] +#[case::with_tls( + setup_config_and_server(100, 7048, true), + tls_socket(get_server_config(7048, true)) +)] +#[case::without_tls( + setup_config_and_server(100, 7049, false), + tcp_socket(get_server_config(7049, false)) +)] +#[awt] +#[tokio::test] +async fn test_tcp_prover( + #[future] + #[case] + notary_config: NotaryServerProperties, + #[future] + #[case] + notary_socket: S, +) { + let notary_host = notary_config.server.host; + let notary_port = notary_config.server.port; + let http_scheme = if notary_config.tls.enabled { + "https" + } else { + "http" + }; + + // Attach the hyper HTTP client to the notary connection to send request to the /session endpoint to configure notarization and obtain session id + let (mut request_sender, connection) = + hyper::client::conn::handshake(notary_socket).await.unwrap(); // Spawn the HTTP task to be run concurrently let connection_task = tokio::spawn(connection.without_shutdown()); @@ -127,7 +171,9 @@ async fn test_tcp_prover() { }) .unwrap(); let request = Request::builder() - .uri(format!("https://{notary_host}:{notary_port}/session")) + .uri(format!( + "{http_scheme}://{notary_host}:{notary_port}/session" + )) .method("POST") .header("Host", notary_host.clone()) // Need to specify application/json for axum to parse it as json @@ -158,7 +204,7 @@ async fn test_tcp_prover() { // Need to specify the session_id so that notary server knows the right configuration to use // as the configuration is set in the previous HTTP call .uri(format!( - "https://{}:{}/notarize?sessionId={}", + "{http_scheme}://{}:{}/notarize?sessionId={}", notary_host, notary_port, notarization_response.session_id.clone() @@ -181,10 +227,9 @@ async fn test_tcp_prover() { debug!("Switched protocol OK"); - // Claim back the TCP socket after HTTP exchange is done so that client can use it for notarization + // Claim back the socket after HTTP exchange is done so that client can use it for notarization let Parts { - io: notary_tls_socket, - .. + io: notary_socket, .. } = connection_task.await.unwrap().unwrap(); // Connect to the Server @@ -206,7 +251,7 @@ async fn test_tcp_prover() { // Bind the Prover to the sockets let prover = Prover::new(prover_config) - .setup(notary_tls_socket.compat()) + .setup(notary_socket.compat()) .await .unwrap(); let (tls_connection, prover_fut) = prover.connect(client_socket.compat()).await.unwrap(); @@ -266,7 +311,7 @@ async fn test_tcp_prover() { #[tokio::test] async fn test_websocket_prover() { // Notary server configuration setup - let notary_config = setup_config_and_server(100, 7049).await; + let notary_config = setup_config_and_server(100, 7050, true).await; let notary_host = notary_config.server.host.clone(); let notary_port = notary_config.server.port;