From ef0c01ac5030149521b4396536aa04e159859f74 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 6 Dec 2022 13:56:25 -0500 Subject: [PATCH 1/6] [tls] Allow refreshing TLS configuration information at runtime Fixes https://github.com/oxidecomputer/dropshot/issues/491 --- .../examples/pagination-multiple-sorts.rs | 2 +- dropshot/src/server.rs | 36 ++++-- dropshot/src/websocket.rs | 2 +- dropshot/tests/common/mod.rs | 116 +++++++++++++----- dropshot/tests/test_tls.rs | 92 +++++++++++++- 5 files changed, 202 insertions(+), 46 deletions(-) diff --git a/dropshot/examples/pagination-multiple-sorts.rs b/dropshot/examples/pagination-multiple-sorts.rs index 935512548..e475877ff 100644 --- a/dropshot/examples/pagination-multiple-sorts.rs +++ b/dropshot/examples/pagination-multiple-sorts.rs @@ -377,7 +377,7 @@ impl ProjectCollection { let name = format!("project{:03}", n); let project = Arc::new(Project { name: name.clone(), - mtime: Utc.timestamp_millis(timestamp), + mtime: Utc.timestamp_millis_opt(timestamp).unwrap(), }); /* * To make this dataset at least somewhat interesting in terms of diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 7e005531b..199624b60 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -68,8 +68,14 @@ pub struct DropshotState { pub log: Logger, /** bound local address for the server. */ pub local_addr: SocketAddr, - /** are requests served over HTTPS */ - pub tls: bool, + /** Identifies how to accept TLS connections */ + pub(crate) tls_acceptor: Option>>, +} + +impl DropshotState { + pub fn using_tls(&self) -> bool { + self.tls_acceptor.is_some() + } } /** @@ -255,7 +261,7 @@ impl InnerHttpServerStarter { router: api.into_router(), log: log.new(o!("local_addr" => local_addr)), local_addr, - tls: false, + tls_acceptor: None, }); let make_service = ServerConnectionHandler::new(app_state.clone()); @@ -337,7 +343,7 @@ struct HttpsAcceptor { impl HttpsAcceptor { pub fn new( log: slog::Logger, - tls_acceptor: TlsAcceptor, + tls_acceptor: Arc>, tcp_listener: TcpListener, ) -> HttpsAcceptor { HttpsAcceptor { @@ -351,7 +357,7 @@ impl HttpsAcceptor { fn new_stream( log: slog::Logger, - tls_acceptor: TlsAcceptor, + tls_acceptor: Arc>, tcp_listener: TcpListener, ) -> impl Stream> { stream! { @@ -402,6 +408,8 @@ impl HttpsAcceptor { }; let tls_negotiation = tls_acceptor + .lock() + .await .accept(socket) .map_ok(move |stream| TlsConn::new(stream, addr)); tls_negotiations.push(tls_negotiation); @@ -481,11 +489,11 @@ impl InnerHttpsServerStarter { private: C, log: &Logger, ) -> Result, GenericError> { - let acceptor = TlsAcceptor::from(Arc::new( + let acceptor = Arc::new(Mutex::new(TlsAcceptor::from(Arc::new( // Unwrap is safe here because we cannot enter this code path // without a TLS configuration rustls::ServerConfig::try_from(config.tls.as_ref().unwrap())?, - )); + )))); let tcp = { let listener = std::net::TcpListener::bind(&config.bind_address)?; @@ -498,7 +506,8 @@ impl InnerHttpsServerStarter { let local_addr = tcp.local_addr()?; let logger = log.new(o!("local_addr" => local_addr)); - let https_acceptor = HttpsAcceptor::new(logger.clone(), acceptor, tcp); + let https_acceptor = + HttpsAcceptor::new(logger.clone(), acceptor.clone(), tcp); let app_state = Arc::new(DropshotState { private, @@ -506,7 +515,7 @@ impl InnerHttpsServerStarter { router: api.into_router(), log: logger, local_addr, - tls: true, + tls_acceptor: Some(acceptor), }); let make_service = ServerConnectionHandler::new(Arc::clone(&app_state)); @@ -555,6 +564,15 @@ impl HttpServer { &self.app_state.private } + /// Update TLS certificates for a running HTTPS server. + pub async fn refresh_tls(&self, config: &ConfigTls) { + if let Some(acceptor) = &self.app_state.tls_acceptor { + *acceptor.lock().await = TlsAcceptor::from(Arc::new( + rustls::ServerConfig::try_from(config).unwrap(), + )); + } + } + /** * Signals the currently running server to stop and waits for it to exit. */ diff --git a/dropshot/src/websocket.rs b/dropshot/src/websocket.rs index ff8029d6d..0b111b957 100644 --- a/dropshot/src/websocket.rs +++ b/dropshot/src/websocket.rs @@ -334,7 +334,7 @@ mod tests { IpAddr::V6(Ipv6Addr::LOCALHOST), 8080, ), - tls: false, + tls_acceptor: None, }), request: Arc::new(Mutex::new( Request::builder() diff --git a/dropshot/tests/common/mod.rs b/dropshot/tests/common/mod.rs index fbc04447e..1518ccd8e 100644 --- a/dropshot/tests/common/mod.rs +++ b/dropshot/tests/common/mod.rs @@ -43,43 +43,91 @@ pub fn create_log_context(test_name: &str) -> LogContext { LogContext::new(test_name, &log_config) } +pub struct TestCertificateChain { + root_cert: rustls::Certificate, + intermediate_cert: rustls::Certificate, + intermediate_keypair: rcgen::Certificate, + end_cert: rustls::Certificate, + end_keypair: rcgen::Certificate, +} + +impl TestCertificateChain { + pub fn new() -> Self { + let mut root_params = rcgen::CertificateParams::new(vec![]); + root_params.is_ca = + rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); + let root_keypair = rcgen::Certificate::from_params(root_params) + .expect("failed to generate root keys"); + + let mut intermediate_params = rcgen::CertificateParams::new(vec![]); + intermediate_params.is_ca = + rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); + let intermediate_keypair = + rcgen::Certificate::from_params(intermediate_params) + .expect("failed to generate intermediate keys"); + + let end_keypair = rcgen::Certificate::from_params( + rcgen::CertificateParams::new(vec!["localhost".into()]), + ) + .expect("failed to generate end-entity keys"); + + let root_cert = rustls::Certificate( + root_keypair + .serialize_der() + .expect("failed to serialize root cert"), + ); + let intermediate_cert = rustls::Certificate( + intermediate_keypair + .serialize_der_with_signer(&root_keypair) + .expect("failed to serialize intermediate cert"), + ); + let end_cert = rustls::Certificate( + end_keypair + .serialize_der_with_signer(&intermediate_keypair) + .expect("failed to serialize end-entity cert"), + ); + + Self { + root_cert, + intermediate_cert, + intermediate_keypair, + end_cert, + end_keypair, + } + } + + pub fn end_cert_private_key(&self) -> rustls::PrivateKey { + rustls::PrivateKey(self.end_keypair.serialize_private_key_der()) + } + + pub fn cert_chain(&self) -> Vec { + vec![ + self.end_cert.clone(), + self.intermediate_cert.clone(), + self.root_cert.clone(), + ] + } + + pub fn generate_new_end_cert(&mut self) { + let end_keypair = rcgen::Certificate::from_params( + rcgen::CertificateParams::new(vec!["localhost".into()]), + ) + .expect("failed to generate end-entity keys"); + let end_cert = rustls::Certificate( + end_keypair + .serialize_der_with_signer(&self.intermediate_keypair) + .expect("failed to serialize end-entity cert"), + ); + self.end_keypair = end_keypair; + self.end_cert = end_cert; + } +} + /// Generate a TLS key and a certificate chain containing a certificate for /// the key, an intermediate cert, and a self-signed root cert. pub fn generate_tls_key() -> (Vec, rustls::PrivateKey) { - let mut root_params = rcgen::CertificateParams::new(vec![]); - root_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); - let root_keypair = rcgen::Certificate::from_params(root_params) - .expect("failed to generate root keys"); - - let mut intermediate_params = rcgen::CertificateParams::new(vec![]); - intermediate_params.is_ca = - rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); - let intermediate_keypair = - rcgen::Certificate::from_params(intermediate_params) - .expect("failed to generate intermediate keys"); - - let end_keypair = - rcgen::Certificate::from_params(rcgen::CertificateParams::new(vec![ - "localhost".into(), - ])) - .expect("failed to generate end-entity keys"); - - let root_cert = rustls::Certificate( - root_keypair.serialize_der().expect("failed to serialize root cert"), - ); - let intermediate_cert = rustls::Certificate( - intermediate_keypair - .serialize_der_with_signer(&root_keypair) - .expect("failed to serialize intermediate cert"), - ); - let end_cert = rustls::Certificate( - end_keypair - .serialize_der_with_signer(&intermediate_keypair) - .expect("failed to serialize end-entity cert"), - ); - - let key = rustls::PrivateKey(end_keypair.serialize_private_key_der()); - (vec![end_cert, intermediate_cert, root_cert], key) + let ca = TestCertificateChain::new(); + (ca.cert_chain(), ca.end_cert_private_key()) } fn make_temp_file() -> std::io::Result { diff --git a/dropshot/tests/test_tls.rs b/dropshot/tests/test_tls.rs index 3d0e4df7b..21f61db97 100644 --- a/dropshot/tests/test_tls.rs +++ b/dropshot/tests/test_tls.rs @@ -194,6 +194,96 @@ async fn test_tls_only() { logctx.cleanup_successful(); } +#[tokio::test] +async fn test_tls_refresh_certificates() { + let logctx = create_log_context("test_tls_refresh_certificates"); + let log = logctx.log.new(o!()); + + // Generate key for the server + let ca = common::TestCertificateChain::new(); + let (certs, key) = (ca.cert_chain(), ca.end_cert_private_key()); + let (cert_file, key_file) = common::tls_key_to_file(&certs, &key); + + let server = make_server(&log, cert_file.path(), key_file.path()).start(); + let port = server.local_addr().port(); + + let https_uri: hyper::Uri = + format!("https://localhost:{}/", port).parse().unwrap(); + + let https_request_maker = || { + hyper::Request::builder() + .method(http::method::Method::GET) + .uri(&https_uri) + .body(hyper::Body::empty()) + .unwrap() + }; + + let make_cert_verifier = |certs: Vec| { + CertificateVerifier(Box::new( + move |end_entity: &rustls::Certificate, + intermediates: &[rustls::Certificate], + server_name: &rustls::ServerName, + _scts: &mut dyn Iterator, + _ocsp_response: &[u8], + _now: SystemTime| + -> Result< + rustls::client::ServerCertVerified, + rustls::Error, + > { + // Verify we're seeing the right cert chain from the server + if *end_entity != certs[0] { + return Err(rustls::Error::InvalidCertificateData( + "Invalid end cert".to_string(), + )); + } + if intermediates != &certs[1..3] { + return Err(rustls::Error::InvalidCertificateData( + "Invalid intermediates".to_string(), + )); + } + if *server_name + != rustls::ServerName::try_from("localhost").unwrap() + { + return Err(rustls::Error::InvalidCertificateData( + "Invalid name".to_string(), + )); + } + Ok(rustls::client::ServerCertVerified::assertion()) + }, + )) + }; + + // Make an HTTPS request successfully with the original certificate chain. + let https_client = make_https_client(make_cert_verifier(certs.clone())); + https_client.request(https_request_maker()).await.unwrap(); + + // Create a brand new certificate chain. + let ca = common::TestCertificateChain::new(); + let (new_certs, new_key) = (ca.cert_chain(), ca.end_cert_private_key()); + let (cert_file, key_file) = common::tls_key_to_file(&new_certs, &new_key); + let config = ConfigTls { + cert_file: cert_file.path().to_path_buf(), + key_file: key_file.path().to_path_buf(), + }; + + // Refresh the server to use the new certificate chain. + server.refresh_tls(&config).await; + + // Client requests which have already been accepted should succeed. + https_client.request(https_request_maker()).await.unwrap(); + + // New client requests using the old certificate chain should fail. + let https_client = make_https_client(make_cert_verifier(certs.clone())); + https_client.request(https_request_maker()).await.unwrap_err(); + + // New client requests using the new certificate chain should succeed. + let https_client = make_https_client(make_cert_verifier(new_certs.clone())); + https_client.request(https_request_maker()).await.unwrap(); + + server.close().await.unwrap(); + logctx.cleanup_successful(); +} + #[tokio::test] async fn test_tls_aborted_negotiation() { let logctx = create_log_context("test_tls_aborted_negotiation"); @@ -261,7 +351,7 @@ async fn tls_check_handler( rqctx: Arc>, query: dropshot::Query, ) -> Result, dropshot::HttpError> { - if rqctx.server.tls != query.into_inner().tls { + if rqctx.server.using_tls() != query.into_inner().tls { return Err(dropshot::HttpError::for_bad_request( None, "mismatch between expected and actual tls state".to_string(), From e9e3a15ca9b4a77d435ba487f5e42e79bbcd96d8 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 6 Dec 2022 14:38:18 -0500 Subject: [PATCH 2/6] Handle error better, update CHANGELOG --- CHANGELOG.adoc | 1 + dropshot/src/server.rs | 17 +++++++++++------ dropshot/tests/test_tls.rs | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 37f0e898f..255d0a978 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,6 +15,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of commits] +* https://github.com/oxidecomputer/dropshot/pull/502[#502] Dropshot exposes a `refresh_tls` method to update the TLS certificates being used by a running server. * https://github.com/oxidecomputer/dropshot/pull/452[#452] Dropshot no longer enables the `slog` cargo features `max_level_trace` and `release_max_level_debug`. Previously, clients were unable to set a release log level of `trace`; now they can. However, clients that did not select their own max log levels will see behavior change from the levels Dropshot was choosing to the default levels of `slog` itself (`debug` for debug builds and `info` for release builds). * https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See `HttpResponseFound`, `HttpResponseSeeOther`, and `HttpResponseTemporaryRedirect`. diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 199624b60..3f2dd594a 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -565,12 +565,17 @@ impl HttpServer { } /// Update TLS certificates for a running HTTPS server. - pub async fn refresh_tls(&self, config: &ConfigTls) { - if let Some(acceptor) = &self.app_state.tls_acceptor { - *acceptor.lock().await = TlsAcceptor::from(Arc::new( - rustls::ServerConfig::try_from(config).unwrap(), - )); - } + pub async fn refresh_tls(&self, config: &ConfigTls) -> Result<(), String> { + let acceptor = &self + .app_state + .tls_acceptor + .as_ref() + .ok_or_else(|| "Not configured for TLS".to_string())?; + + *acceptor.lock().await = TlsAcceptor::from(Arc::new( + rustls::ServerConfig::try_from(config).unwrap(), + )); + Ok(()) } /** diff --git a/dropshot/tests/test_tls.rs b/dropshot/tests/test_tls.rs index 21f61db97..ca15549af 100644 --- a/dropshot/tests/test_tls.rs +++ b/dropshot/tests/test_tls.rs @@ -267,7 +267,7 @@ async fn test_tls_refresh_certificates() { }; // Refresh the server to use the new certificate chain. - server.refresh_tls(&config).await; + server.refresh_tls(&config).await.unwrap(); // Client requests which have already been accepted should succeed. https_client.request(https_request_maker()).await.unwrap(); From 867b587a0e17f11fa95642914ef25e6f4e41406c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 6 Dec 2022 22:27:20 -0500 Subject: [PATCH 3/6] Allow TLS certificate and key to come from bytes Fixes https://github.com/oxidecomputer/dropshot/issues/490 --- dropshot/examples/https.rs | 2 +- dropshot/src/config.rs | 74 ++++++++++++++++++++---- dropshot/src/server.rs | 37 +++++------- dropshot/tests/common/mod.rs | 36 +++++++++--- dropshot/tests/test_config.rs | 103 ++++++++++++++++++++++++++++------ dropshot/tests/test_tls.rs | 6 +- 6 files changed, 196 insertions(+), 62 deletions(-) diff --git a/dropshot/examples/https.rs b/dropshot/examples/https.rs index 4ea7d3842..60c946d0e 100644 --- a/dropshot/examples/https.rs +++ b/dropshot/examples/https.rs @@ -72,7 +72,7 @@ async fn main() -> Result<(), String> { * In addition, we'll make this an HTTPS server. */ let config_dropshot = ConfigDropshot { - tls: Some(ConfigTls { + tls: Some(ConfigTls::AsFile { cert_file: cert_file.path().to_path_buf(), key_file: key_file.path().to_path_buf(), }), diff --git a/dropshot/src/config.rs b/dropshot/src/config.rs index 012e247ad..a32e3ebc8 100644 --- a/dropshot/src/config.rs +++ b/dropshot/src/config.rs @@ -34,6 +34,7 @@ use std::path::PathBuf; * request_body_max_bytes = 1024 * ## Optional, to enable TLS * [http_api_server.tls] + * type = "AsFile" * cert_file = "/path/to/certs.pem" * key_file = "/path/to/key.pem" * @@ -61,17 +62,68 @@ pub struct ConfigDropshot { } #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -pub struct ConfigTls { - /** Path to a PEM file containing a certificate chain for the - * server to identify itself with. The first certificate is the - * end-entity certificate, and the remaining are intermediate - * certificates on the way to a trusted CA. - */ - pub cert_file: PathBuf, - /** Path to a PEM-encoded PKCS #8 file containing the private key the - * server will use. - */ - pub key_file: PathBuf, +#[serde(tag = "type")] +pub enum ConfigTls { + AsFile { + /** Path to a PEM file containing a certificate chain for the + * server to identify itself with. The first certificate is the + * end-entity certificate, and the remaining are intermediate + * certificates on the way to a trusted CA. + */ + cert_file: PathBuf, + /** Path to a PEM-encoded PKCS #8 file containing the private key the + * server will use. + */ + key_file: PathBuf, + }, + AsBytes { + certs: Vec, + key: Vec, + }, +} + +impl ConfigTls { + pub(crate) fn cert_reader( + &self, + ) -> std::io::Result> { + match self { + ConfigTls::AsFile { cert_file, .. } => { + let certfile = std::fs::File::open(cert_file).map_err(|e| { + std::io::Error::new( + std::io::ErrorKind::Other, + format!( + "failed to open {}: {}", + cert_file.display(), + e + ), + ) + })?; + Ok(Box::new(std::io::BufReader::new(certfile))) + } + ConfigTls::AsBytes { certs, .. } => { + Ok(Box::new(std::io::BufReader::new(certs.as_slice()))) + } + } + } + + pub(crate) fn key_reader( + &self, + ) -> std::io::Result> { + match self { + ConfigTls::AsFile { key_file, .. } => { + let keyfile = std::fs::File::open(key_file).map_err(|e| { + std::io::Error::new( + std::io::ErrorKind::Other, + format!("failed to open {}: {}", key_file.display(), e), + ) + })?; + Ok(Box::new(std::io::BufReader::new(keyfile))) + } + ConfigTls::AsBytes { key, .. } => { + Ok(Box::new(std::io::BufReader::new(key.as_slice()))) + } + } + } } impl Default for ConfigDropshot { diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 3f2dd594a..c5652030d 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -30,7 +30,6 @@ use std::convert::TryFrom; use std::future::Future; use std::net::SocketAddr; use std::num::NonZeroU32; -use std::path::Path; use std::pin::Pin; use std::sync::Arc; use std::task::{Context, Poll}; @@ -445,8 +444,8 @@ impl TryFrom<&ConfigTls> for rustls::ServerConfig { type Error = std::io::Error; fn try_from(config: &ConfigTls) -> std::io::Result { - let certs = load_certs(&config.cert_file)?; - let private_key = load_private_key(&config.key_file)?; + let certs = load_certs(&config)?; + let private_key = load_private_key(&config)?; let mut cfg = rustls::ServerConfig::builder() // TODO: We may want to expose protocol configuration in our // config @@ -914,37 +913,31 @@ impl Service> for ServerRequestHandler { } } -fn error(err: String) -> std::io::Error { +fn io_error(err: String) -> std::io::Error { std::io::Error::new(std::io::ErrorKind::Other, err) } -// Load public certificate from file. -fn load_certs(filename: &Path) -> std::io::Result> { - // Open certificate file. - let certfile = std::fs::File::open(filename).map_err(|e| { - error(format!("failed to open {}: {}", filename.display(), e)) - })?; - let mut reader = std::io::BufReader::new(certfile); +// Load public certificate from config. +fn load_certs(config: &ConfigTls) -> std::io::Result> { + let mut reader = config.cert_reader()?; // Load and return certificate. rustls_pemfile::certs(&mut reader) - .map_err(|err| error(format!("failed to load certificate: {err}"))) + .map_err(|err| io_error(format!("failed to load certificate: {err}"))) .map(|mut chain| chain.drain(..).map(rustls::Certificate).collect()) } -// Load private key from file. -fn load_private_key(filename: &Path) -> std::io::Result { - // Open keyfile. - let keyfile = std::fs::File::open(filename).map_err(|e| { - error(format!("failed to open {}: {}", filename.display(), e)) - })?; - let mut reader = std::io::BufReader::new(keyfile); +// Load private key from config. +fn load_private_key(config: &ConfigTls) -> std::io::Result { + let mut reader = config.key_reader()?; // Load and return a single private key. - let keys = rustls_pemfile::pkcs8_private_keys(&mut reader) - .map_err(|err| error(format!("failed to load private key: {err}")))?; + let keys = + rustls_pemfile::pkcs8_private_keys(&mut reader).map_err(|err| { + io_error(format!("failed to load private key: {err}")) + })?; if keys.len() != 1 { - return Err(error("expected a single private key".into())); + return Err(io_error("expected a single private key".into())); } Ok(rustls::PrivateKey(keys[0].clone())) } diff --git a/dropshot/tests/common/mod.rs b/dropshot/tests/common/mod.rs index 1518ccd8e..93e8951d8 100644 --- a/dropshot/tests/common/mod.rs +++ b/dropshot/tests/common/mod.rs @@ -134,27 +134,47 @@ fn make_temp_file() -> std::io::Result { tempfile::Builder::new().prefix("dropshot-test-").rand_bytes(5).tempfile() } -/// Write keys to a temporary file for passing to the server config -pub fn tls_key_to_file( +pub fn tls_key_to_buffer( certs: &Vec, key: &rustls::PrivateKey, -) -> (NamedTempFile, NamedTempFile) { - let mut cert_file = make_temp_file().expect("failed to create cert_file"); +) -> (Vec, Vec) { + let mut serialized_certs = vec![]; + let mut cert_writer = std::io::BufWriter::new(&mut serialized_certs); for cert in certs { let encoded_cert = pem::encode(&pem::Pem { tag: "CERTIFICATE".to_string(), contents: cert.0.clone(), }); - cert_file + cert_writer .write(encoded_cert.as_bytes()) - .expect("failed to write cert_file"); + .expect("failed to serialize cert"); } + drop(cert_writer); - let mut key_file = make_temp_file().expect("failed to create key_file"); + let mut serialized_key = vec![]; + let mut key_writer = std::io::BufWriter::new(&mut serialized_key); let encoded_key = pem::encode(&pem::Pem { tag: "PRIVATE KEY".to_string(), contents: key.0.clone(), }); - key_file.write(encoded_key.as_bytes()).expect("failed to write key_file"); + key_writer.write(encoded_key.as_bytes()).expect("failed to serialize key"); + drop(key_writer); + + (serialized_certs, serialized_key) +} + +/// Write keys to a temporary file for passing to the server config +pub fn tls_key_to_file( + certs: &Vec, + key: &rustls::PrivateKey, +) -> (NamedTempFile, NamedTempFile) { + let mut cert_file = make_temp_file().expect("failed to create cert_file"); + let mut key_file = make_temp_file().expect("failed to create key_file"); + + let (certs, key) = tls_key_to_buffer(certs, key); + + cert_file.write(certs.as_slice()).expect("Failed to write certs"); + key_file.write(key.as_slice()).expect("Failed to write key"); + (cert_file, key_file) } diff --git a/dropshot/tests/test_config.rs b/dropshot/tests/test_config.rs index 599998a1f..cc8ad6292 100644 --- a/dropshot/tests/test_config.rs +++ b/dropshot/tests/test_config.rs @@ -8,6 +8,7 @@ use dropshot::{ConfigDropshot, ConfigTls}; use dropshot::{HttpServer, HttpServerStarter}; use slog::o; use slog::Logger; +use std::str::FromStr; use tempfile::NamedTempFile; pub mod common; @@ -87,7 +88,7 @@ fn test_config_bad_request_body_max_bytes_too_large() { fn test_config_bad_key_file_garbage() { let error = read_config::( "bad_key_file_garbage", - "[tls]\ncert_file = ''\nkey_file = 23", + "[tls]\ntype = 'AsFile'\ncert_file = ''\nkey_file = 23", ) .unwrap_err() .to_string(); @@ -102,7 +103,7 @@ fn test_config_bad_key_file_garbage() { fn test_config_bad_cert_file_garbage() { let error = read_config::( "bad_cert_file_garbage", - "[tls]\ncert_file = 23\nkey_file=''", + "[tls]\ntype = 'AsFile'\ncert_file = 23\nkey_file=''", ) .unwrap_err() .to_string(); @@ -125,7 +126,7 @@ fn test_config_bad_tls_garbage() { fn test_config_bad_tls_incomplete() { let error = read_config::( "bad_tls_incomplete", - "[tls]\ncert_file = ''", + "[tls]\ntype = 'AsFile'\ncert_file = ''", ) .unwrap_err() .to_string(); @@ -133,7 +134,7 @@ fn test_config_bad_tls_incomplete() { let error = read_config::( "bad_tls_incomplete", - "[tls]\nkey_file = ''", + "[tls]\ntype = 'AsFile'\nkey_file = ''", ) .unwrap_err() .to_string(); @@ -153,19 +154,14 @@ fn make_config( bind_port: u16, tls: Option, ) -> ConfigDropshot { - let mut config_text = format!( - "bind_address = \"{}:{}\"\nrequest_body_max_bytes = 1024", - bind_ip_str, bind_port, - ); - if let Some(tls) = tls { - config_text = format!( - "{}\n[tls]\ncert_file = '{}'\nkey_file = '{}'", - config_text, - tls.cert_file.to_str().unwrap(), - tls.key_file.to_str().unwrap(), - ); + ConfigDropshot { + bind_address: std::net::SocketAddr::new( + std::net::IpAddr::from_str(bind_ip_str).unwrap(), + bind_port, + ), + request_body_max_bytes: 1024, + tls, } - read_config::("bind_address", &config_text).unwrap() } // Trait for abstracting out test case specific properties from the common bind @@ -319,7 +315,7 @@ async fn test_config_bind_address_https() { } fn make_server(&self, bind_port: u16) -> HttpServer { - let tls = Some(ConfigTls { + let tls = Some(ConfigTls::AsFile { cert_file: self.cert_file.path().to_path_buf(), key_file: self.key_file.path().to_path_buf(), }); @@ -347,3 +343,76 @@ async fn test_config_bind_address_https() { logctx.cleanup_successful(); } + +#[tokio::test] +async fn test_config_bind_address_https_buffer() { + struct ConfigBindServerHttps { + log: slog::Logger, + certs: Vec, + serialized_certs: Vec, + serialized_key: Vec, + } + + impl + TestConfigBindServer< + hyper_rustls::HttpsConnector, + > for ConfigBindServerHttps + { + fn make_client( + &self, + ) -> hyper::Client< + hyper_rustls::HttpsConnector, + > { + // Configure TLS to trust the self-signed cert + let mut root_store = rustls::RootCertStore { roots: vec![] }; + root_store + .add(&self.certs[self.certs.len() - 1]) + .expect("adding root cert"); + + let tls_config = rustls::ClientConfig::builder() + .with_safe_defaults() + .with_root_certificates(root_store) + .with_no_client_auth(); + let https_connector = hyper_rustls::HttpsConnectorBuilder::new() + .with_tls_config(tls_config) + .https_only() + .enable_http1() + .build(); + hyper::Client::builder().build(https_connector) + } + + fn make_uri(&self, bind_port: u16) -> hyper::Uri { + format!("https://localhost:{}/", bind_port).parse().unwrap() + } + + fn make_server(&self, bind_port: u16) -> HttpServer { + let tls = Some(ConfigTls::AsBytes { + certs: self.serialized_certs.clone(), + key: self.serialized_key.clone(), + }); + let config = make_config("127.0.0.1", bind_port, tls); + make_server(&config, &self.log).start() + } + + fn log(&self) -> &Logger { + &self.log + } + } + + let logctx = create_log_context("config_bind_address_https_buffer"); + let log = logctx.log.new(o!()); + + // Generate key for the server + let (certs, key) = common::generate_tls_key(); + let (serialized_certs, serialized_key) = + common::tls_key_to_buffer(&certs, &key); + let test_config = + ConfigBindServerHttps { log, certs, serialized_certs, serialized_key }; + + /* This must be different than the bind_port used in the http test. */ + let bind_port = 12218; + test_config_bind_server::<_, ConfigBindServerHttps>(test_config, bind_port) + .await; + + logctx.cleanup_successful(); +} diff --git a/dropshot/tests/test_tls.rs b/dropshot/tests/test_tls.rs index ca15549af..32faf1374 100644 --- a/dropshot/tests/test_tls.rs +++ b/dropshot/tests/test_tls.rs @@ -73,7 +73,7 @@ fn make_server( let config = ConfigDropshot { bind_address: "127.0.0.1:0".parse().unwrap(), request_body_max_bytes: 1024, - tls: Some(ConfigTls { + tls: Some(ConfigTls::AsFile { cert_file: cert_file.to_path_buf(), key_file: key_file.to_path_buf(), }), @@ -261,7 +261,7 @@ async fn test_tls_refresh_certificates() { let ca = common::TestCertificateChain::new(); let (new_certs, new_key) = (ca.cert_chain(), ca.end_cert_private_key()); let (cert_file, key_file) = common::tls_key_to_file(&new_certs, &new_key); - let config = ConfigTls { + let config = ConfigTls::AsFile { cert_file: cert_file.path().to_path_buf(), key_file: key_file.path().to_path_buf(), }; @@ -372,7 +372,7 @@ async fn test_server_is_https() { let config = ConfigDropshot { bind_address: "127.0.0.1:0".parse().unwrap(), request_body_max_bytes: 1024, - tls: Some(ConfigTls { + tls: Some(ConfigTls::AsFile { cert_file: cert_file.path().to_path_buf(), key_file: key_file.path().to_path_buf(), }), From 0024c749682afbad9e25c1cfa1fd1aded7eb8ca4 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 6 Dec 2022 22:28:43 -0500 Subject: [PATCH 4/6] Update CHANGELOG --- CHANGELOG.adoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 255d0a978..421923c06 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,6 +15,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of commits] +* https://github.com/oxidecomputer/dropshot/pull/504[#504] Dropshot allows TLS configuration to be supplied either by path or as bytes. * https://github.com/oxidecomputer/dropshot/pull/502[#502] Dropshot exposes a `refresh_tls` method to update the TLS certificates being used by a running server. * https://github.com/oxidecomputer/dropshot/pull/452[#452] Dropshot no longer enables the `slog` cargo features `max_level_trace` and `release_max_level_debug`. Previously, clients were unable to set a release log level of `trace`; now they can. However, clients that did not select their own max log levels will see behavior change from the levels Dropshot was choosing to the default levels of `slog` itself (`debug` for debug builds and `info` for release builds). * https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See `HttpResponseFound`, `HttpResponseSeeOther`, and `HttpResponseTemporaryRedirect`. From 1b7f420f1c40ce6d9912c4dee63f0dbc8553c22a Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 6 Dec 2022 22:36:22 -0500 Subject: [PATCH 5/6] Update README too --- README.adoc | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/README.adoc b/README.adoc index 5244b548d..ce2b266c9 100644 --- a/README.adoc +++ b/README.adoc @@ -50,16 +50,30 @@ include: |No |Specifies the maximum number of bytes allowed in a request body. Larger requests will receive a 400 error. Defaults to 1024. +|`tls.type` +|`"AsFile", "AsBytes"` +|No +|Specifies if and how TLS certificate and key information is provided. + |`tls.cert_file` |`"/path/to/cert.pem"` -|Only if `tls.key_file` is set +|Only if `tls.type = AsFile` |Specifies the path to a PEM file containing a certificate chain for the server to identify itself with. The first certificate is the end-entity certificate, and the remaining are intermediate certificates on the way to a trusted CA. If specified, the server will only listen for TLS connections. |`tls.key_file` |`"/path/to/key.pem"` -|Only if `tls.cert_file` is set +|Only if `tls.type = AsFile` |Specifies the path to a PEM-encoded PKCS #8 file containing the private key the server will use. If specified, the server will only listen for TLS connections. +|`tls.certs` +|`Vec of certificate data` +|Only if `tls.type = AsBytes` +|Identical to `tls.cert_file`, but provided as a buffer. + +|`tls.key` +|`Vec of key data` +|Only if `tls.type = AsBytes` +|Identical to `tls.key_file`, but provided as a buffer. |=== === Logging From 219ff7af4890eed9227d1cb9d5cb9931a5ee16d0 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 14 Dec 2022 14:55:37 -0500 Subject: [PATCH 6/6] Breaking explanation --- CHANGELOG.adoc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index dffad00e2..b8f0377a8 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,7 +15,12 @@ https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of commits] -* https://github.com/oxidecomputer/dropshot/pull/502[#502] Dropshot exposes a `refresh_tls` method to update the TLS certificates being used by a running server. +=== Breaking Changes + +* https://github.com/oxidecomputer/dropshot/pull/502[#502] Dropshot exposes a `refresh_tls` method to update the TLS certificates being used by a running server. If you previously tried to access `DropshotState.tls`, you can access the `DropshotState.using_tls()` method instead. + +=== Other notable Changes + * https://github.com/oxidecomputer/dropshot/pull/452[#452] Dropshot no longer enables the `slog` cargo features `max_level_trace` and `release_max_level_debug`. Previously, clients were unable to set a release log level of `trace`; now they can. However, clients that did not select their own max log levels will see behavior change from the levels Dropshot was choosing to the default levels of `slog` itself (`debug` for debug builds and `info` for release builds). * https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See `HttpResponseFound`, `HttpResponseSeeOther`, and `HttpResponseTemporaryRedirect`. * https://github.com/oxidecomputer/dropshot/pull/503[#503] Add an optional `deprecated` field to the `#[endpoint]` macro.