fix(tls): use right method to get cn and san attributes

Signed-off-by: Florentin Dubois <florentin.dubois@clever-cloud.com>

command/src/certificate.rs

@@ -1,18 +1,69 @@
-use std::{error, fmt, str::FromStr};
+use std::{collections::HashSet, error, fmt, str::FromStr};
 
 use hex::FromHex;
-use pem::parse;
 use serde::de::{self, Visitor};
 use sha2::{Digest, Sha256};
+use x509_parser::{
+    oid_registry::{OID_X509_COMMON_NAME, OID_X509_EXT_SUBJECT_ALT_NAME},
+    pem::{parse_x509_pem, Pem},
+};
 
 use crate::proto::command::TlsVersion;
 
-#[derive(thiserror::Error, Debug)]
+// -----------------------------------------------------------------------------
+// CertificateError
+
+#[derive(thiserror::Error, Clone, Debug)]
 pub enum CertificateError {
     #[error("Could not parse PEM certificate from bytes: {0}")]
     InvalidCertificate(String),
+    #[error("failed to parse certificate common name attribute, {0}")]
+    InvalidCommonName(String),
+    #[error("failed to parse certificate subject alternate names attribute, {0}")]
+    InvalidSubjectAlternateNames(String),
+}
+
+// -----------------------------------------------------------------------------
+// parse
+
+pub fn parse(certificate: &[u8]) -> Result<Pem, CertificateError> {
+    let (_, pem) = parse_x509_pem(certificate)
+        .map_err(|err| CertificateError::InvalidCertificate(err.to_string()))?;
+
+    Ok(pem)
+}
+
+// -----------------------------------------------------------------------------
+// get_cn_and_san_attributes
+
+pub fn get_cn_and_san_attributes(pem: &Pem) -> Result<HashSet<String>, CertificateError> {
+    let x509 = pem
+        .parse_x509()
+        .map_err(|err| CertificateError::InvalidCertificate(err.to_string()))?;
+
+    let mut names: HashSet<String> = HashSet::new();
+    for name in x509.subject().iter_by_oid(&OID_X509_COMMON_NAME) {
+        names.insert(
+            name.as_str()
+                .map(String::from)
+                .map_err(|err| CertificateError::InvalidCommonName(err.to_string()))?,
+        );
+    }
+
+    for name in x509.subject().iter_by_oid(&OID_X509_EXT_SUBJECT_ALT_NAME) {
+        names.insert(
+            name.as_str()
+                .map(String::from)
+                .map_err(|err| CertificateError::InvalidSubjectAlternateNames(err.to_string()))?,
+        );
+    }
+
+    Ok(names)
 }
 
+// -----------------------------------------------------------------------------
+// ParseErrorTlsVersion
+
 #[derive(Debug)]
 pub struct ParseErrorTlsVersion;
 
@@ -48,6 +99,9 @@ impl FromStr for TlsVersion {
     }
 }
 
+// -----------------------------------------------------------------------------
+// Fingerprint
+
 //FIXME: make fixed size depending on hash algorithm
 /// A TLS certificates, encoded in bytes
 #[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
@@ -102,18 +156,15 @@ impl<'de> serde::Deserialize<'de> for Fingerprint {
     }
 }
 
+pub fn calculate_fingerprint_from_der(certificate: &[u8]) -> Vec<u8> {
+    Sha256::digest(certificate).iter().cloned().collect()
+}
+
 pub fn calculate_fingerprint(certificate: &[u8]) -> Result<Vec<u8>, CertificateError> {
     let parsed_certificate = parse(certificate)
         .map_err(|parse_error| CertificateError::InvalidCertificate(parse_error.to_string()))?;
-    let fingerprint = Sha256::digest(parsed_certificate.contents())
-        .iter()
-        .cloned()
-        .collect();
-    Ok(fingerprint)
-}
 
-pub fn calculate_fingerprint_from_der(certificate: &[u8]) -> Vec<u8> {
-    Sha256::digest(certificate).iter().cloned().collect()
+    Ok(calculate_fingerprint_from_der(&parsed_certificate.contents))
 }
 
 pub fn split_certificate_chain(mut chain: String) -> Vec<String> {

command/src/state.rs

@@ -9,7 +9,7 @@ use std::{
 };
 
 use crate::{
-    certificate::{calculate_fingerprint, Fingerprint},
+    certificate::{self, calculate_fingerprint, Fingerprint},
     proto::{
         command::{
             request::RequestType, ActivateListener, AddBackend, AddCertificate, CertificateAndKey,

lib/src/tls.rs

@@ -16,14 +16,11 @@ use rustls::{
     Certificate, PrivateKey,
 };
 use sha2::{Digest, Sha256};
-use x509_parser::{
-    oid_registry::{OID_X509_COMMON_NAME, OID_X509_EXT_SUBJECT_ALT_NAME},
-    pem::{parse_x509_pem, Pem},
-};
+use x509_parser::pem::{parse_x509_pem, Pem};
 
 use crate::router::trie::*;
 use sozu_command::{
-    certificate::Fingerprint,
+    certificate::{get_cn_and_san_attributes, CertificateError, Fingerprint},
     proto::command::{AddCertificate, CertificateAndKey, ReplaceCertificate, TlsVersion},
 };
 
@@ -152,8 +149,8 @@ impl Clone for ParsedCertificateAndKey {
 
 #[derive(thiserror::Error, Clone, Debug)]
 pub enum GenericCertificateResolverError {
-    #[error("failed to parse certificate common name, {0}")]
-    InvalidCommonName(String),
+    #[error("failed to get common name and subject alternate names from pem, {0}")]
+    InvalidCommonNameAndSubjectAlternateNames(CertificateError),
     #[error("failed to parse pem certificate, {0}")]
     InvalidPem(String),
     #[error("failed to parse der certificate, {0}")]
@@ -291,34 +288,8 @@ impl CertificateResolverHelper for GenericCertificateResolver {
             }
         }
 
-        let x509 = pem
-            .parse_x509()
-            .map_err(|err| GenericCertificateResolverError::InvalidDer(err.to_string()))?;
-
-        let mut names: HashSet<String> = HashSet::new();
-        for name in x509.subject().iter_by_oid(&OID_X509_COMMON_NAME) {
-            names.insert(
-                name.attr_value()
-                    .as_str()
-                    .map(String::from)
-                    .map_err(|err| {
-                        GenericCertificateResolverError::InvalidCommonName(err.to_string())
-                    })?,
-            );
-        }
-
-        for name in x509.subject().iter_by_oid(&OID_X509_EXT_SUBJECT_ALT_NAME) {
-            names.insert(
-                name.attr_value()
-                    .as_str()
-                    .map(String::from)
-                    .map_err(|err| {
-                        GenericCertificateResolverError::InvalidCommonName(err.to_string())
-                    })?,
-            );
-        }
-
-        Ok(names)
+        get_cn_and_san_attributes(pem)
+            .map_err(GenericCertificateResolverError::InvalidCommonNameAndSubjectAlternateNames)
     }
 
     fn fingerprint(pem: &Pem) -> Fingerprint {
@@ -328,8 +299,9 @@ impl CertificateResolverHelper for GenericCertificateResolver {
     fn parse(
         certificate_and_key: &CertificateAndKey,
     ) -> Result<ParsedCertificateAndKey, Self::Error> {
-        let (_, certificate) = parse_x509_pem(certificate_and_key.certificate.as_bytes())
-            .map_err(|err| GenericCertificateResolverError::InvalidPem(err.to_string()))?;
+        let certificate =
+            sozu_command::certificate::parse(certificate_and_key.certificate.as_bytes())
+                .map_err(|err| GenericCertificateResolverError::InvalidPem(err.to_string()))?;
 
         let mut chains = vec![];
         for chain in &certificate_and_key.certificate_chain {
@@ -389,6 +361,7 @@ impl CertificateResolverHelper for GenericCertificateResolver {
                         .iter()
                         .filter_map(|v| TlsVersion::from_i32(*v))
                         .collect();
+
                     return Ok(ParsedCertificateAndKey {
                         certificate,
                         chain: chains,