Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tls] Allow refreshing TLS configuration information at runtime #502

Merged
merged 4 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be mistaken, but I think this is the first breaking change in this list. If that's the case, can you please create two sections as below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, updated in 219ff7a

* 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`.

Expand Down
2 changes: 1 addition & 1 deletion dropshot/examples/pagination-multiple-sorts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 32 additions & 9 deletions dropshot/src/server.rs
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
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
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
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