store certificates in parsed form in CertificateResolver

The GenericCertificateResolver would store certs
in a ParsedCertificateAndKey form, after parsing and checking,
but this form had to be re-parsed into rustls::sign::CertifiedKey
at every handshake, diminishing performance.
This commit comes back to the feature of 0.13.6: storing
CertifiedKey directly in the certificate resolver.

rename trait CertificateResolver to ResolveCertificate
rename GenericCertificateResolver to CertificateResolver
create type StoredCertificate (wraps CertifiedKey)
remove trait GenericCertificateResolverHelper
additional parse functions in command::certificate module
add documenting comments
apply clippy suggestions

Co-Authored-By: Emmanuel Bosquet <[email protected]>

command/src/certificate.rs

@@ -4,8 +4,10 @@ use hex::FromHex;
 use serde::de::{self, Visitor};
 use sha2::{Digest, Sha256};
 use x509_parser::{
+    certificate::X509Certificate,
     extensions::{GeneralName, ParsedExtension},
     oid_registry::{OID_X509_COMMON_NAME, OID_X509_EXT_SUBJECT_ALT_NAME},
+    parse_x509_certificate,
     pem::{parse_x509_pem, Pem},
 };
 
@@ -27,21 +29,26 @@ pub enum CertificateError {
 
 /// parse a pem file encoded as binary and convert it into the right structure
 /// (a.k.a [`Pem`])
-pub fn parse(certificate: &[u8]) -> Result<Pem, CertificateError> {
+pub fn parse_pem(certificate: &[u8]) -> Result<Pem, CertificateError> {
     let (_, pem) = parse_x509_pem(certificate)
         .map_err(|err| CertificateError::InvalidCertificate(err.to_string()))?;
 
     Ok(pem)
 }
 
+pub fn parse_x509(pem_bytes: &[u8]) -> Result<X509Certificate, CertificateError> {
+    parse_x509_certificate(pem_bytes)
+        .map_err(|nom_e| CertificateError::InvalidCertificate(nom_e.to_string()))
+        .map(|t| t.1)
+}
+
 // -----------------------------------------------------------------------------
 // get_cn_and_san_attributes
 
-/// Retrieve from the [`Pem`] structure the common name (a.k.a `CN`) and the
+/// Retrieve from the pem (as bytes) the common name (a.k.a `CN`) and the
 /// subject alternate names (a.k.a `SAN`)
-pub fn get_cn_and_san_attributes(pem: &Pem) -> Result<HashSet<String>, CertificateError> {
-    let x509 = pem
-        .parse_x509()
+pub fn get_cn_and_san_attributes(pem_bytes: &[u8]) -> Result<HashSet<String>, CertificateError> {
+    let x509 = parse_x509(pem_bytes)
         .map_err(|err| CertificateError::InvalidCertificate(err.to_string()))?;
 
     let mut names: HashSet<String> = HashSet::new();
@@ -150,7 +157,7 @@ pub fn calculate_fingerprint_from_der(certificate: &[u8]) -> Vec<u8> {
 
 /// Compute fingerprint from a certificate that is encoded in pem format
 pub fn calculate_fingerprint(certificate: &[u8]) -> Result<Vec<u8>, CertificateError> {
-    let parsed_certificate = parse(certificate)
+    let parsed_certificate = parse_pem(certificate)
         .map_err(|parse_error| CertificateError::InvalidCertificate(parse_error.to_string()))?;
 
     Ok(calculate_fingerprint_from_der(&parsed_certificate.contents))

command/src/state.rs

@@ -1253,10 +1253,10 @@ impl ConfigState {
                 .flat_map(|hash_map| hash_map.iter())
                 .flat_map(|(fingerprint, cert)| {
                     if cert.names.is_empty() {
-                        let pem = certificate::parse(cert.certificate.as_bytes()).ok()?;
+                        let pem = certificate::parse_pem(cert.certificate.as_bytes()).ok()?;
                         let mut c = cert.to_owned();
 
-                        c.names = certificate::get_cn_and_san_attributes(&pem)
+                        c.names = certificate::get_cn_and_san_attributes(&pem.contents)
                             .ok()?
                             .into_iter()
                             .collect();
@@ -1276,10 +1276,10 @@ impl ConfigState {
                 .filter(|(fingerprint, _cert)| fingerprint.to_string() == f)
                 .flat_map(|(fingerprint, cert)| {
                     if cert.names.is_empty() {
-                        let pem = certificate::parse(cert.certificate.as_bytes()).ok()?;
+                        let pem = certificate::parse_pem(cert.certificate.as_bytes()).ok()?;
                         let mut c = cert.to_owned();
 
-                        c.names = certificate::get_cn_and_san_attributes(&pem)
+                        c.names = certificate::get_cn_and_san_attributes(&pem.contents)
                             .ok()?
                             .into_iter()
                             .collect();
@@ -1297,10 +1297,10 @@ impl ConfigState {
                 .flat_map(|hash_map| hash_map.iter())
                 .flat_map(|(fingerprint, cert)| {
                     if cert.names.is_empty() {
-                        let pem = certificate::parse(cert.certificate.as_bytes()).ok()?;
+                        let pem = certificate::parse_pem(cert.certificate.as_bytes()).ok()?;
                         let mut c = cert.to_owned();
 
-                        c.names = certificate::get_cn_and_san_attributes(&pem)
+                        c.names = certificate::get_cn_and_san_attributes(&pem.contents)
                             .ok()?
                             .into_iter()
                             .collect();

e2e/src/tests/tests.rs

@@ -390,7 +390,7 @@ pub fn try_tls_endpoint() -> State {
     let certificate_and_key = CertificateAndKey {
         certificate: String::from(include_str!("../../../lib/assets/local-certificate.pem")),
         key: String::from(include_str!("../../../lib/assets/local-key.pem")),
-        certificate_chain: vec![],
+        certificate_chain: vec![], // in config.toml the certificate chain would be the same as the certificate
         versions: vec![],
         names: vec![],
     };

lib/src/https.rs

@@ -62,7 +62,7 @@ use crate::{
     server::{ListenSession, ListenToken, ProxyChannel, Server, SessionManager, SessionToken},
     socket::{server_bind, FrontRustls},
     timer::TimeoutContainer,
-    tls::{CertificateResolver, MutexWrappedCertificateResolver, ParsedCertificateAndKey},
+    tls::{MutexWrappedCertificateResolver, ResolveCertificate, StoredCertificate},
     util::UnwrapLog,
     AcceptError, CachedTags, FrontendFromRequestError, L7ListenerHandler, L7Proxy, ListenerError,
     ListenerHandler, Protocol, ProxyConfiguration, ProxyError, ProxySession, SessionIsToBeClosed,
@@ -597,10 +597,10 @@ impl L7ListenerHandler for HttpsListener {
     }
 }
 
-impl CertificateResolver for HttpsListener {
+impl ResolveCertificate for HttpsListener {
     type Error = ListenerError;
 
-    fn get_certificate(&self, fingerprint: &Fingerprint) -> Option<ParsedCertificateAndKey> {
+    fn get_certificate(&self, fingerprint: &Fingerprint) -> Option<StoredCertificate> {
         let resolver = self
             .resolver
             .0
@@ -641,7 +641,7 @@ impl HttpsListener {
         config: HttpsListenerConfig,
         token: Token,
     ) -> Result<HttpsListener, ListenerError> {
-        let resolver = Arc::new(MutexWrappedCertificateResolver::new());
+        let resolver = Arc::new(MutexWrappedCertificateResolver::default());
 
         let server_config = Arc::new(Self::create_rustls_context(&config, resolver.to_owned())?);
 
@@ -1640,7 +1640,7 @@ mod tests {
 
         let address: StdSocketAddr = FromStr::from_str("127.0.0.1:1032")
             .expect("test address 127.0.0.1:1032 should be parsed");
-        let resolver = Arc::new(MutexWrappedCertificateResolver::new());
+        let resolver = Arc::new(MutexWrappedCertificateResolver::default());
 
         let server_config = ServerConfig::builder()
             .with_safe_default_cipher_suites()

lib/src/lib.rs

@@ -371,7 +371,7 @@ use mio::{net::TcpStream, Interest, Token};
 use protocol::http::parser::Method;
 use router::RouterError;
 use time::{Duration, Instant};
-use tls::GenericCertificateResolverError;
+use tls::CertificateResolverError;
 
 use sozu_command::{
     proto::command::{Cluster, ListenerType, RequestHttpFrontend},
@@ -658,7 +658,7 @@ pub enum ListenerError {
     #[error("failed to acquire the lock, {0}")]
     Lock(String),
     #[error("failed to handle certificate request, got a resolver error, {0}")]
-    Resolver(GenericCertificateResolverError),
+    Resolver(CertificateResolverError),
     #[error("failed to parse pem, {0}")]
     PemParse(String),
     #[error("failed to build rustls context, {0}")]

lib/src/metrics/local_drain.rs

@@ -359,7 +359,7 @@ impl LocalDrain {
                 .iter()
                 .find(|backend_metrics| backend_metrics.backend_id == backend_id)
             {
-                return Ok(backend_metrics.to_filtered_metrics(metric_names)?);
+                return backend_metrics.to_filtered_metrics(metric_names);
             }
         }
 
@@ -486,7 +486,7 @@ impl LocalDrain {
         match self.cluster_metrics.entry(cluster_id.to_owned()) {
             Entry::Vacant(entry) => {
                 let mut metrics = BTreeMap::new();
-                metrics.insert(metric_name.to_owned(), aggregated_metric.clone());
+                metrics.insert(metric_name.to_owned(), aggregated_metric);
                 let backends = [LocalBackendMetrics {
                     backend_id: backend_id.to_owned(),
                     metrics,
@@ -497,11 +497,10 @@ impl LocalDrain {
                     cluster: BTreeMap::new(),
                     backends,
                 });
-                return;
             }
             Entry::Occupied(mut entry) => {
                 for backend_metrics in &mut entry.get_mut().backends {
-                    if &backend_metrics.backend_id == backend_id {
+                    if backend_metrics.backend_id == backend_id {
                         if let Some(existing_metric) = backend_metrics.metrics.get_mut(metric_name)
                         {
                             existing_metric.update(metric_name, metric);

lib/src/router/pattern_trie.rs

@@ -222,11 +222,10 @@ impl<V: Debug + Clone> TrieNode<V> {
                     if pos > 0 {
                         let mut remove_result = RemoveResult::NotFound;
                         for t in self.regexps.iter_mut() {
-                            if t.0.as_str() == s {
-                                if t.1.remove_recursive(&partial_key[..pos - 1]) == RemoveResult::Ok
-                                {
-                                    remove_result = RemoveResult::Ok;
-                                }
+                            if t.0.as_str() == s
+                                && t.1.remove_recursive(&partial_key[..pos - 1]) == RemoveResult::Ok
+                            {
+                                remove_result = RemoveResult::Ok;
                             }
                         }
                         return remove_result;
@@ -257,10 +256,10 @@ impl<V: Debug + Clone> TrieNode<V> {
                     if child.is_empty() {
                         self.children.remove(suffix);
                     }
-                    return RemoveResult::Ok;
+                    RemoveResult::Ok
                 }
             },
-            None => return RemoveResult::NotFound,
+            None => RemoveResult::NotFound,
         }
     }
 

lib/src/router/trie.rs

@@ -307,6 +307,12 @@ impl<V: Debug + Clone> TrieNode<V> {
     }
 }
 
+impl<T: Clone + Debug> Default for TrieNode<T> {
+    fn default() -> Self {
+        Self::root()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

lib/src/socket.rs

@@ -333,7 +333,7 @@ impl SocketHandler for FrontRustls {
         let mut is_error = false;
         let mut is_closed = false;
 
-        match self.session.writer().write_vectored(&bufs) {
+        match self.session.writer().write_vectored(bufs) {
             Ok(0) => {}
             Ok(sz) => {
                 buffered_size += sz;