fix: X509 Credential was serializing its identity whereas according t…

…o RFC9420 it mustn't

openmls/src/credentials/mod.rs

@@ -35,6 +35,7 @@ mod codec;
 #[cfg(test)]
 mod tests;
 use errors::*;
+use openmls_x509_credential::X509Ext;
 use x509_cert::{der::Decode, PkiPath};
 
 use crate::ciphersuite::SignaturePublicKey;
@@ -140,24 +141,60 @@ impl From<CredentialType> for u16 {
 ///     opaque cert_data<V>;
 /// } Certificate;
 /// ```
-#[derive(
-    Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize,
-)]
+#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 pub struct Certificate {
-    pub identity: VLBytes,
-    pub cert_data: Vec<VLBytes>,
+    // TLS transient
+    pub identity: Vec<u8>,
+    pub certificates: Vec<VLBytes>,
+}
+
+impl tls_codec::Size for Certificate {
+    fn tls_serialized_len(&self) -> usize {
+        self.certificates.tls_serialized_len()
+    }
+}
+
+impl tls_codec::Serialize for Certificate {
+    fn tls_serialize<W: Write>(&self, writer: &mut W) -> Result<usize, tls_codec::Error> {
+        self.certificates.tls_serialize(writer)
+    }
+}
+
+impl tls_codec::Deserialize for Certificate {
+    fn tls_deserialize<R: Read>(bytes: &mut R) -> Result<Self, tls_codec::Error>
+    where
+        Self: Sized,
+    {
+        let certificates = Vec::<Vec<u8>>::tls_deserialize(bytes)?;
+        // we should not do this in a deserializer but otherwise we have to deal with a `identity: Option<Vec<u8>>` everywhere
+        Certificate::try_new(certificates).map_err(|_| tls_codec::Error::InvalidInput)
+    }
 }
 
 impl Certificate {
     pub(crate) fn pki_path(&self) -> Result<PkiPath, CredentialError> {
-        self.cert_data.iter().try_fold(
+        self.certificates.iter().try_fold(
             PkiPath::new(),
             |mut acc, cert_data| -> Result<PkiPath, CredentialError> {
                 acc.push(x509_cert::Certificate::from_der(cert_data.as_slice())?);
                 Ok(acc)
             },
         )
     }
+
+    fn try_new(certificates: Vec<Vec<u8>>) -> Result<Self, CredentialError> {
+        let leaf = certificates
+            .get(0)
+            .ok_or(CredentialError::InvalidCertificateChain)?;
+        let leaf = x509_cert::Certificate::from_der(leaf)?;
+        let identity = leaf
+            .identity()
+            .map_err(|_| CredentialError::InvalidCertificateChain)?;
+        Ok(Self {
+            identity,
+            certificates: certificates.into_iter().map(|c| c.into()).collect(),
+        })
+    }
 }
 
 /// MlsCredentialType.
@@ -222,25 +259,18 @@ impl Credential {
     /// Creates and returns a new X509 [`Credential`] for the given identity.
     /// If the credential holds key material, this is generated and stored in
     /// the key store.
-    pub fn new_x509(identity: Vec<u8>, cert_data: Vec<Vec<u8>>) -> Result<Self, CredentialError> {
-        if cert_data.len() < 2 {
-            return Err(CredentialError::IncompleteCertificateChain);
-        }
+    pub fn new_x509(certificates: Vec<Vec<u8>>) -> Result<Self, CredentialError> {
         Ok(Self {
             credential_type: CredentialType::X509,
-            credential: MlsCredentialType::X509(Certificate {
-                identity: identity.into(),
-                cert_data: cert_data.into_iter().map(|c| c.into()).collect(),
-            }),
+            credential: MlsCredentialType::X509(Certificate::try_new(certificates)?),
         })
     }
 
     /// Returns the identity of a given credential.
     pub fn identity(&self) -> &[u8] {
         match &self.credential {
             MlsCredentialType::Basic(basic_credential) => basic_credential.identity.as_slice(),
-            // TODO: implement getter for identity for X509 certificates. See issue #134.
-            MlsCredentialType::X509(cred) => cred.identity.as_slice(),
+            MlsCredentialType::X509(cert) => cert.identity.as_slice(),
         }
     }
 }
@@ -350,9 +380,7 @@ pub mod test_utils {
     ) -> (CredentialWithKey, SignatureKeyPair) {
         let credential = match credential_type {
             CredentialType::Basic => Credential::new_basic(identity.into()),
-            CredentialType::X509 => {
-                Credential::new_x509(identity.into(), cert_data.unwrap()).unwrap()
-            }
+            CredentialType::X509 => Credential::new_x509(cert_data.unwrap()).unwrap(),
             CredentialType::Unknown(_) => unimplemented!(),
         };
         let signature_keys = SignatureKeyPair::new(

x509_credential/Cargo.toml

@@ -13,6 +13,8 @@ async-trait = { workspace = true }
 serde = "1.0"
 openmls_basic_credential = { version = "0.2.0", path = "../basic_credential" }
 fluvio-wasm-timer = "0.2"
+base64 = "0.21"
+uuid = "1.4"
 
 # Rust Crypto
 secrecy = { version = "0.8", features = ["serde"] }

x509_credential/src/lib.rs

@@ -2,6 +2,7 @@
 //!
 //! An implementation of the x509 credential from the MLS spec.
 
+use base64::Engine;
 use openmls_basic_credential::SignatureKeyPair;
 use x509_cert::der::Decode;
 use x509_cert::Certificate;
@@ -100,6 +101,8 @@ pub trait X509Ext {
         backend: &impl OpenMlsCrypto,
         issuer: &Certificate,
     ) -> Result<(), CryptoError>;
+
+    fn identity(&self) -> Result<Vec<u8>, CryptoError>;
 }
 
 impl X509Ext for Certificate {
@@ -180,4 +183,48 @@ impl X509Ext for Certificate {
             )
             .map_err(|_| CryptoError::InvalidSignature)
     }
+
+    fn identity(&self) -> Result<Vec<u8>, CryptoError> {
+        let extensions = self
+            .tbs_certificate
+            .extensions
+            .as_ref()
+            .ok_or(CryptoError::InvalidCertificate)?;
+        let san = extensions
+            .iter()
+            .find(|e| {
+                e.extn_id.as_bytes() == oid_registry::OID_X509_EXT_SUBJECT_ALT_NAME.as_bytes()
+            })
+            .and_then(|e| {
+                x509_cert::ext::pkix::SubjectAltName::from_der(e.extn_value.as_bytes()).ok()
+            })
+            .ok_or(CryptoError::InvalidCertificate)?;
+        san.0
+            .iter()
+            .filter_map(|n| match n {
+                x509_cert::ext::pkix::name::GeneralName::UniformResourceIdentifier(ia5_str) => {
+                    Some(ia5_str.as_str())
+                }
+                _ => None,
+            })
+            .filter(|n| n.starts_with("im:wireapp="))
+            .find_map(parse_client_id)
+            .map(|i| i.as_bytes().to_vec())
+            .ok_or(CryptoError::InvalidCertificate)
+    }
+}
+
+fn parse_client_id(client_id: &str) -> Option<&str> {
+    let (user_id, rest) = client_id.split_once('/')?;
+    parse_user_id(user_id)?;
+    let (device_id, _domain) = rest.split_once('@')?;
+    u64::from_str_radix(device_id, 16).ok()?;
+    Some(client_id)
+}
+
+fn parse_user_id(user_id: impl AsRef<[u8]>) -> Option<uuid::Uuid> {
+    let user_id = base64::prelude::BASE64_URL_SAFE_NO_PAD
+        .decode(user_id)
+        .ok()?;
+    uuid::Uuid::from_slice(&user_id).ok()
 }