Skip to content

Commit

Permalink
Use san instead of domain component
Browse files Browse the repository at this point in the history
  • Loading branch information
@augustocdias
augustocdias committed Jul 19, 2023
1 parent 153621e commit 1b6c431
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 32 deletions.
77 changes: 54 additions & 23 deletions crypto/src/mls/credential/trust_anchor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ use mls_crypto_provider::MlsCryptoProvider;
use openmls::prelude::group_context::GroupContext;
use openmls_traits::OpenMlsCryptoProvider;
use openmls_x509_credential::X509Ext;
use x509_cert::{
der::{Decode, Encode},
Certificate, PkiPath,
};
use x509_cert::{der::Decode, Certificate, PkiPath};

use crate::{
mls::{
Expand Down Expand Up @@ -92,8 +89,8 @@ impl PerDomainTrustAnchor {
let leaf_cert = certificate_chain.first().ok_or(CryptoError::InvalidCertificateChain)?;

// validate domain in the leaf matches with the one supplied
let domain_name = extract_domain_name(leaf_cert)?;
if domain_name != self.domain_name {
let domain_names = extract_domain_names(leaf_cert)?;
if !domain_names.contains(&self.domain_name) {
return Err(CryptoError::DomainNamesDontMatch);
}

Expand Down Expand Up @@ -310,18 +307,52 @@ impl MlsCentral {
}
}

pub(crate) fn extract_domain_name(certificate: &Certificate) -> CryptoResult<String> {
for attr in certificate.tbs_certificate.subject.0.iter().flat_map(|n| n.0.iter()) {
// according to the RFC implementations must be prepared to receive the domain component
if attr.oid.as_bytes() == oid_registry::OID_DOMAIN_COMPONENT.as_bytes() {
return Ok(std::str::from_utf8(attr.value.value())?.to_string());
}
// but the common name seems to be standard to have the domain field
if attr.oid.as_bytes() == oid_registry::OID_X509_COMMON_NAME.as_bytes() {
return Ok(std::str::from_utf8(attr.value.value())?.to_string());
}
pub(crate) fn extract_domain_names(certificate: &Certificate) -> CryptoResult<Vec<String>> {
let common_name = certificate
.tbs_certificate
.subject
.0
.iter()
.flat_map(|n| n.0.iter())
.find_map(|attr| {
if attr.oid.as_bytes() == oid_registry::OID_X509_COMMON_NAME.as_bytes() {
Some(attr.value.value())
} else {
None
}
})
.map(|bytes| String::from_utf8(bytes.to_owned()))
.transpose()?;

let san = if let Some(extensions) = certificate.tbs_certificate.extensions.as_ref() {
extensions
.iter()
.find(|e| e.extn_id.as_bytes() == oid_registry::OID_X509_EXT_SUBJECT_ALT_NAME.as_bytes())
.map(|e| x509_cert::ext::pkix::SubjectAltName::from_der(e.extn_value.as_bytes()))
.transpose()?
} else {
None
};

let dns_names: Vec<_> = san
.into_iter()
.flat_map(|san| {
san.0
.iter()
.filter_map(|n| match n {
x509_cert::ext::pkix::name::GeneralName::DnsName(ia5_str) => Some(ia5_str.to_string()),
_ => None,
})
.collect::<Vec<_>>()
})
.chain(common_name.into_iter())
.collect();

if dns_names.is_empty() {
Err(CryptoError::DomainNameNotFound)
} else {
Ok(dns_names)
}
Err(CryptoError::DomainNameNotFound)
}

#[cfg(test)]
Expand All @@ -341,7 +372,7 @@ mod tests {
};

mod domain_name_extraction {
use crate::{mls::credential::trust_anchor::extract_domain_name, test_utils::TestCase};
use crate::{mls::credential::trust_anchor::extract_domain_names, test_utils::TestCase};

use super::*;
use crate::{
Expand All @@ -368,8 +399,8 @@ mod tests {
false,
);

let domain_name = extract_domain_name(&cert).unwrap();
assert_eq!(domain_name, "wire.com");
let domain_names = extract_domain_names(&cert).unwrap();
assert_eq!(domain_names[0], "wire.com");
}

#[apply(all_cred_cipher)]
Expand All @@ -387,8 +418,8 @@ mod tests {
false,
);

let domain_name = extract_domain_name(&cert).unwrap();
assert_eq!(domain_name, "wire.com");
let domain_names = extract_domain_names(&cert).unwrap();
assert_eq!(domain_names[0], "wire.com");
}

#[apply(all_cred_cipher)]
Expand All @@ -406,7 +437,7 @@ mod tests {
false,
);

let err = extract_domain_name(&cert).unwrap_err();
let err = extract_domain_names(&cert).unwrap_err();
assert!(matches!(err, CryptoError::DomainNameNotFound));
}
}
Expand Down
15 changes: 6 additions & 9 deletions crypto/src/test_utils/x509.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use x509_cert::{
Certificate, PkiPath,
};

use crate::mls::credential::trust_anchor::{extract_domain_name, PerDomainTrustAnchor};
use crate::mls::credential::trust_anchor::{extract_domain_names, PerDomainTrustAnchor};

/// Params for generating the Certificate chain
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -36,24 +36,24 @@ impl PerDomainTrustAnchor {

impl From<PkiPath> for PerDomainTrustAnchor {
fn from(chain: PkiPath) -> Self {
let domain = extract_domain_name(&chain[0]).unwrap_or_default();
let domains = extract_domain_names(&chain[0]).unwrap_or_default();
let pems = chain
.iter()
.map(|c| pem::Pem::new("CERTIFICATE", c.to_der().unwrap()))
.collect::<Vec<_>>();
Self {
domain_name: domain,
domain_name: domains.get(0).cloned().unwrap_or_default(),
intermediate_certificate_chain: pem::encode_many(&pems),
}
}
}

impl From<Certificate> for PerDomainTrustAnchor {
fn from(cert: Certificate) -> Self {
let domain = extract_domain_name(&cert).unwrap_or_default();
let mut domains = extract_domain_names(&cert).unwrap_or_default();
let pem = pem::Pem::new("CERTIFICATE", cert.to_der().unwrap());
Self {
domain_name: domain,
domain_name: domains.remove(0),
intermediate_certificate_chain: pem::encode(&pem),
}
}
Expand Down Expand Up @@ -118,10 +118,7 @@ fn new_cert_params(
dn.push(rcgen::DnType::CommonName, common_name);
}
if let Some(domain) = cert_params.domain {
dn.push(
rcgen::DnType::CustomDnType(oid_registry::OID_DOMAIN_COMPONENT.iter().unwrap().collect()),
domain,
);
params.subject_alt_names = vec![rcgen::SanType::DnsName(domain)];
}

if is_ca {
Expand Down

0 comments on commit 1b6c431

Please sign in to comment.