Skip to content

Commit

Permalink
[tls] Allow refreshing TLS configuration information at runtime (#502)
Browse files Browse the repository at this point in the history 
Adds a `refresh_tls` method to `HttpServer`, which allows TLS information to be updated for a running server.

Fixes #491
  • Loading branch information
@smklein
smklein authored Dec 14, 2022
1 parent 640e73c commit 94967b8
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 45 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@

https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of commits]

=== 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.
Expand Down
41 changes: 32 additions & 9 deletions dropshot/src/server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,14 @@ pub struct DropshotState<C: ServerContext> {
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<Arc<Mutex<TlsAcceptor>>>,
}

impl<C: ServerContext> DropshotState<C> {
pub fn using_tls(&self) -> bool {
self.tls_acceptor.is_some()
}
}

/**
Expand Down Expand Up @@ -255,7 +261,7 @@ impl<C: ServerContext> InnerHttpServerStarter<C> {
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());
Expand Down Expand Up @@ -337,7 +343,7 @@ struct HttpsAcceptor {
impl HttpsAcceptor {
pub fn new(
log: slog::Logger,
tls_acceptor: TlsAcceptor,
tls_acceptor: Arc<Mutex<TlsAcceptor>>,
tcp_listener: TcpListener,
) -> HttpsAcceptor {
HttpsAcceptor {
Expand All @@ -351,7 +357,7 @@ impl HttpsAcceptor {

fn new_stream(
log: slog::Logger,
tls_acceptor: TlsAcceptor,
tls_acceptor: Arc<Mutex<TlsAcceptor>>,
tcp_listener: TcpListener,
) -> impl Stream<Item = std::io::Result<TlsConn>> {
stream! {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -481,11 +489,11 @@ impl<C: ServerContext> InnerHttpsServerStarter<C> {
private: C,
log: &Logger,
) -> Result<InnerHttpsServerStarterNewReturn<C>, 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)?;
Expand All @@ -498,15 +506,16 @@ impl<C: ServerContext> InnerHttpsServerStarter<C> {

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,
config: server_config,
router: api.into_router(),
log: logger,
local_addr,
tls: true,
tls_acceptor: Some(acceptor),
});

let make_service = ServerConnectionHandler::new(Arc::clone(&app_state));
Expand Down Expand Up @@ -555,6 +564,20 @@ impl<C: ServerContext> HttpServer<C> {
&self.app_state.private
}

/// Update TLS certificates for a running HTTPS server.
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(())
}

/**
* Signals the currently running server to stop and waits for it to exit.
*/
Expand Down
2 changes: 1 addition & 1 deletion dropshot/src/websocket.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ mod tests {
IpAddr::V6(Ipv6Addr::LOCALHOST),
8080,
),
tls: false,
tls_acceptor: None,
}),
request: Arc::new(Mutex::new(
Request::builder()
Expand Down
116 changes: 82 additions & 34 deletions dropshot/tests/common/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<rustls::Certificate> {
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::Certificate>, 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<NamedTempFile> {
Expand Down
92 changes: 91 additions & 1 deletion dropshot/tests/test_tls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<rustls::Certificate>| {
CertificateVerifier(Box::new(
move |end_entity: &rustls::Certificate,
intermediates: &[rustls::Certificate],
server_name: &rustls::ServerName,
_scts: &mut dyn Iterator<Item = &[u8]>,
_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.unwrap();

// 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");
Expand Down Expand Up @@ -261,7 +351,7 @@ async fn tls_check_handler(
rqctx: Arc<dropshot::RequestContext<usize>>,
query: dropshot::Query<TlsCheckArgs>,
) -> Result<HttpResponseOk<()>, 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(),
Expand Down

0 comments on commit 94967b8

Please sign in to comment.