diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index b8f0377a..f6467ac0 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -17,6 +17,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of co === Breaking Changes +* https://github.com/oxidecomputer/dropshot/pull/504[#504] Dropshot allows TLS configuration to be supplied either by path or as bytes. For compatibility, the `AsFile` variant of `ConfigTls` contains the `cert_file` and `key_file` fields, and may be used similarly to the old variant. * 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 diff --git a/README.adoc b/README.adoc index 5244b548..ce2b266c 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 diff --git a/dropshot/examples/https.rs b/dropshot/examples/https.rs index 4ea7d384..60c946d0 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 012e247a..a32e3ebc 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 3f2dd594..c5652030 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 1518ccd8..93e8951d 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 599998a1..cc8ad629 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 ca15549a..32faf137 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(), }),