From cef86f8d5b2d00f9d0e5470b1599095a13a6122f Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 15 Jun 2018 14:26:26 -1000 Subject: [PATCH] Add optional TLS client certificate authentication. (#1135) Refactor the way the TLS trust anchors are configured in preparation for the client and server authenticating each others' certificates. Make the use of client certificates optional pending the implementation of authorization policy. Signed-off-by: Brian Smith --- proxy/src/transport/tls/cert_resolver.rs | 7 +++- proxy/src/transport/tls/config.rs | 52 +++++++++++++++++------- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/proxy/src/transport/tls/cert_resolver.rs b/proxy/src/transport/tls/cert_resolver.rs index 9d4f2ab58..23ed5ddfe 100755 --- a/proxy/src/transport/tls/cert_resolver.rs +++ b/proxy/src/transport/tls/cert_resolver.rs @@ -32,7 +32,7 @@ impl CertResolver { /// TODO: Verify that the public key of the certificate matches the private /// key. pub fn new( - trust_anchors: &webpki::TLSServerTrustAnchors, + root_cert_store: &rustls::RootCertStore, cert_chain: Vec, private_key: untrusted::Input) -> Result @@ -40,6 +40,11 @@ impl CertResolver { let now = webpki::Time::try_from(SystemTime::now()) .map_err(|ring::error::Unspecified| config::Error::TimeConversionFailed)?; + let trust_anchors = root_cert_store.roots.iter() + .map(|owned_trust_anchor| owned_trust_anchor.to_trust_anchor()) + .collect::>(); + let trust_anchors = webpki::TLSServerTrustAnchors(&trust_anchors); + // Verify that we were given a valid TLS certificate that was issued by // our CA. parse_end_entity_cert(&cert_chain) diff --git a/proxy/src/transport/tls/config.rs b/proxy/src/transport/tls/config.rs index 9ec4e5036..ba3b93298 100644 --- a/proxy/src/transport/tls/config.rs +++ b/proxy/src/transport/tls/config.rs @@ -43,6 +43,7 @@ pub struct CommonSettings { /// Validated configuration common between TLS clients and TLS servers. pub struct CommonConfig { + root_cert_store: rustls::RootCertStore, cert_resolver: Arc, } @@ -110,18 +111,24 @@ impl CommonConfig { /// trust anchors file. Since filesystem operations are not atomic, we /// need to check for this consistency. fn load_from_disk(settings: &CommonSettings) -> Result { - let trust_anchor_certs = load_file_contents(&settings.trust_anchors) - .and_then(|file_contents| - rustls::internal::pemfile::certs(&mut Cursor::new(file_contents)) - .map_err(|()| Error::FailedToParseTrustAnchors(None)))?; - let mut trust_anchors = Vec::with_capacity(trust_anchor_certs.len()); - for ta in &trust_anchor_certs { - let ta = webpki::trust_anchor_util::cert_der_as_trust_anchor( - untrusted::Input::from(ta.as_ref())) - .map_err(|e| Error::FailedToParseTrustAnchors(Some(e)))?; - trust_anchors.push(ta); - } - let trust_anchors = webpki::TLSServerTrustAnchors(&trust_anchors); + let root_cert_store = load_file_contents(&settings.trust_anchors) + .and_then(|file_contents| { + let mut root_cert_store = rustls::RootCertStore::empty(); + let (added, skipped) = root_cert_store.add_pem_file(&mut Cursor::new(file_contents)) + .map_err(|err| { + error!("error parsing trust anchors file: {:?}", err); + Error::FailedToParseTrustAnchors(None) + })?; + if skipped != 0 { + warn!("skipped {} trust anchors in trust anchors file", skipped); + } + if added > 0 { + Ok(root_cert_store) + } else { + error!("no valid trust anchors in trust anchors file"); + Err(Error::FailedToParseTrustAnchors(None)) + } + })?; let end_entity_cert = load_file_contents(&settings.end_entity_cert)?; @@ -134,9 +141,10 @@ impl CommonConfig { let private_key = untrusted::Input::from(&private_key); // `CertResolver::new` is responsible for the consistency check. - let cert_resolver = CertResolver::new(&trust_anchors, cert_chain, private_key)?; + let cert_resolver = CertResolver::new(&root_cert_store, cert_chain, private_key)?; Ok(Self { + root_cert_store, cert_resolver: Arc::new(cert_resolver), }) } @@ -189,7 +197,23 @@ pub fn watch_for_config_changes(settings: Option<&CommonSettings>) impl ServerConfig { fn from(common: &CommonConfig) -> Self { - let mut config = rustls::ServerConfig::new(Arc::new(rustls::NoClientAuth)); + // Ask TLS clients for a certificate and accept any certificate issued + // by our trusted CA(s). + // + // Initially, also allow TLS clients that don't have a client + // certificate too, to minimize risk. TODO: Use + // `AllowAnyAuthenticatedClient` to require a valid certificate. + // + // XXX: Rustls's built-in verifiers don't let us tweak things as fully + // as we'd like (e.g. controlling the set of trusted signature + // algorithms), but they provide good enough defaults for now. + // TODO: lock down the verification further. + // + // TODO: Change Rustls's API to Avoid needing to clone `root_cert_store`. + let client_cert_verifier = + rustls::AllowAnyAnonymousOrAuthenticatedClient::new(common.root_cert_store.clone()); + + let mut config = rustls::ServerConfig::new(client_cert_verifier); set_common_settings(&mut config.versions); config.cert_resolver = common.cert_resolver.clone(); ServerConfig(Arc::new(config))