Skip to content

Commit

Permalink
verification: forbid unsupported NC GNs
Browse files Browse the repository at this point in the history
This is what we should have been doing originally, per
RFC 5280 4.2.1.10:

> If a name constraints extension that is marked as critical
> imposes constraints on a particular name form, and an instance of
> that name form appears in the subject field or subjectAltName
> extension of a subsequent certificate, then the application MUST
> either process the constraint or reject the certificate.
  • Loading branch information
woodruffw committed Mar 10, 2024
1 parent eeda511 commit b3fa9cf
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 18 deletions.
12 changes: 12 additions & 0 deletions src/rust/cryptography-x509-verification/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,18 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
))),
}
}
// All other matching pairs of (constraint, name) are currently unsupported.
(GeneralName::OtherName(_), GeneralName::OtherName(_))
| (GeneralName::X400Address(_), GeneralName::X400Address(_))
| (GeneralName::DirectoryName(_), GeneralName::DirectoryName(_))
| (GeneralName::EDIPartyName(_), GeneralName::EDIPartyName(_))
| (
GeneralName::UniformResourceIdentifier(_),
GeneralName::UniformResourceIdentifier(_),
)
| (GeneralName::RegisteredID(_), GeneralName::RegisteredID(_)) => Err(
ValidationError::Other("unsupported name constraint".to_string()),
),
_ => Ok(Skipped),
}
}
Expand Down
22 changes: 4 additions & 18 deletions src/rust/src/x509/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
// for complete details.

use cryptography_x509::{
certificate::Certificate, extensions::SubjectAlternativeName, name::GeneralName,
oid::SUBJECT_ALTERNATIVE_NAME_OID,
certificate::Certificate, extensions::SubjectAlternativeName, oid::SUBJECT_ALTERNATIVE_NAME_OID,
};
use cryptography_x509_verification::{
ops::{CryptoOps, VerificationCertificate},
Expand All @@ -21,7 +20,7 @@ use crate::x509::certificate::Certificate as PyCertificate;
use crate::x509::common::{datetime_now, datetime_to_py, py_to_datetime};
use crate::x509::sign;

use super::parse_general_name;
use super::parse_general_names;

pub(crate) struct PyCryptoOps {}

Expand Down Expand Up @@ -290,23 +289,10 @@ impl PyClientVerifier {
.unwrap();

let leaf_gns = leaf_san.value::<SubjectAlternativeName<'_>>()?;

// Instead of returning all general names, we return only ones
// that we currently have name constraint implementations for.
let filtered_gns = leaf_gns.filter(|gn| {
matches!(
gn,
GeneralName::DNSName(_) | GeneralName::IPAddress(_) | GeneralName::RFC822Name(_)
)
});

let filtered_py_gns = pyo3::types::PyList::empty(py);
for filtered_gn in filtered_gns {
filtered_py_gns.append(parse_general_name(py, filtered_gn)?)?;
}
let py_gns = parse_general_names(py, &leaf_gns)?;

Ok(PyVerifiedClient {
subjects: filtered_py_gns.into(),
subjects: py_gns,
chain: py_chain.into_py(py),
})
}
Expand Down

0 comments on commit b3fa9cf

Please sign in to comment.