diff --git a/src/rust/cryptography-x509/src/certificate.rs b/src/rust/cryptography-x509/src/certificate.rs index bb9a666f5f78..59960242b202 100644 --- a/src/rust/cryptography-x509/src/certificate.rs +++ b/src/rust/cryptography-x509/src/certificate.rs @@ -4,6 +4,7 @@ use crate::common; use crate::extensions; +use crate::extensions::Extensions; use crate::name; #[derive(asn1::Asn1Read, asn1::Asn1Write, Hash, PartialEq, Clone)] @@ -31,7 +32,13 @@ pub struct TbsCertificate<'a> { #[implicit(2)] pub subject_unique_id: Option>, #[explicit(3)] - pub extensions: Option>, + pub raw_extensions: Option>, +} + +impl<'a> TbsCertificate<'a> { + pub fn extensions(&'a self) -> Result>, asn1::ObjectIdentifier> { + Extensions::from_raw_extensions(self.raw_extensions.as_ref()) + } } #[derive(asn1::Asn1Read, asn1::Asn1Write, Hash, PartialEq, Clone)] diff --git a/src/rust/cryptography-x509/src/crl.rs b/src/rust/cryptography-x509/src/crl.rs index 3a47e0a37727..c81a3c4a95fd 100644 --- a/src/rust/cryptography-x509/src/crl.rs +++ b/src/rust/cryptography-x509/src/crl.rs @@ -2,7 +2,11 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::{common, extensions, name}; +use crate::{ + common, + extensions::{self}, + name, +}; pub type ReasonFlags<'a> = Option, asn1::OwnedBitString>>; @@ -31,14 +35,14 @@ pub struct TBSCertList<'a> { pub next_update: Option, pub revoked_certificates: RevokedCertificates<'a>, #[explicit(0)] - pub crl_extensions: Option>, + pub raw_crl_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Hash, Clone)] pub struct RevokedCertificate<'a> { pub user_certificate: asn1::BigUint<'a>, pub revocation_date: common::Time, - pub crl_entry_extensions: Option>, + pub raw_crl_entry_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] diff --git a/src/rust/cryptography-x509/src/csr.rs b/src/rust/cryptography-x509/src/csr.rs index c23d22d0fd11..d2cf9b5e2739 100644 --- a/src/rust/cryptography-x509/src/csr.rs +++ b/src/rust/cryptography-x509/src/csr.rs @@ -26,7 +26,7 @@ pub struct CertificationRequestInfo<'a> { impl CertificationRequestInfo<'_> { pub fn get_extension_attribute( &self, - ) -> Result>, asn1::ParseError> { + ) -> Result>, asn1::ParseError> { for attribute in self.attributes.unwrap_read().clone() { if attribute.type_id == oid::EXTENSION_REQUEST || attribute.type_id == oid::MS_EXTENSION_REQUEST diff --git a/src/rust/cryptography-x509/src/extensions.rs b/src/rust/cryptography-x509/src/extensions.rs index 0728633d4adb..b1138fec206e 100644 --- a/src/rust/cryptography-x509/src/extensions.rs +++ b/src/rust/cryptography-x509/src/extensions.rs @@ -2,16 +2,62 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. +use std::collections::HashSet; + use crate::common; use crate::crl; use crate::name; -pub type Extensions<'a> = common::Asn1ReadableOrWritable< +pub type RawExtensions<'a> = common::Asn1ReadableOrWritable< 'a, asn1::SequenceOf<'a, Extension<'a>>, asn1::SequenceOfWriter<'a, Extension<'a>, Vec>>, >; +/// An invariant-enforcing wrapper for `RawExtensions`. +/// +/// In particular, an `Extensions` cannot be constructed from a `RawExtensions` +/// that contains duplicated extensions (by OID). +pub struct Extensions<'a>(RawExtensions<'a>); + +impl<'a> Extensions<'a> { + /// Create an `Extensions` from the given `RawExtensions`. + /// + /// Returns an `Err` variant containing the first duplicated extension's + /// OID, if there are any duplicates. + pub fn from_raw_extensions( + raw: Option<&RawExtensions<'a>>, + ) -> Result, asn1::ObjectIdentifier> { + match raw { + Some(raw_exts) => { + let mut seen_oids = HashSet::new(); + + for ext in raw_exts.unwrap_read().clone() { + if !seen_oids.insert(ext.extn_id.clone()) { + return Err(ext.extn_id); + } + } + + Ok(Some(Self(raw_exts.clone()))) + } + None => Ok(None), + } + } + + /// Retrieves the extension identified by the given OID, + /// or None if the extension is not present (or no extensions are present). + pub fn get_extension(&self, oid: &asn1::ObjectIdentifier) -> Option { + let mut extensions = self.0.unwrap_read().clone(); + + extensions.find(|ext| &ext.extn_id == oid) + } + + /// Returns a reference to the underlying extensions. + pub fn as_raw(&self) -> &RawExtensions<'_> { + &self.0 + } +} + #[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Eq, Hash, Clone)] pub struct Extension<'a> { pub extn_id: asn1::ObjectIdentifier, @@ -174,3 +220,38 @@ pub struct BasicConstraints { pub ca: bool, pub path_length: Option, } + +#[cfg(test)] +mod tests { + use asn1::SequenceOfWriter; + + use crate::oid::{AUTHORITY_KEY_IDENTIFIER_OID, BASIC_CONSTRAINTS_OID}; + + use super::{BasicConstraints, Extension, Extensions}; + + #[test] + fn test_get_extension() { + let extension_value = BasicConstraints { + ca: true, + path_length: Some(3), + }; + let extension = Extension { + extn_id: BASIC_CONSTRAINTS_OID, + critical: true, + extn_value: &asn1::write_single(&extension_value).unwrap(), + }; + let extensions = SequenceOfWriter::new(vec![extension]); + + let der = asn1::write_single(&extensions).unwrap(); + + let extensions: Extensions = + Extensions::from_raw_extensions(Some(&asn1::parse_single(&der).unwrap())) + .unwrap() + .unwrap(); + + assert!(&extensions.get_extension(&BASIC_CONSTRAINTS_OID).is_some()); + assert!(&extensions + .get_extension(&AUTHORITY_KEY_IDENTIFIER_OID) + .is_none()); + } +} diff --git a/src/rust/cryptography-x509/src/ocsp_req.rs b/src/rust/cryptography-x509/src/ocsp_req.rs index 1e096e71f1da..ba54d391f506 100644 --- a/src/rust/cryptography-x509/src/ocsp_req.rs +++ b/src/rust/cryptography-x509/src/ocsp_req.rs @@ -2,7 +2,11 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::{common, extensions, name}; +use crate::{ + common, + extensions::{self}, + name, +}; #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct TBSRequest<'a> { @@ -17,14 +21,14 @@ pub struct TBSRequest<'a> { asn1::SequenceOfWriter<'a, Request<'a>>, >, #[explicit(2)] - pub request_extensions: Option>, + pub raw_request_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct Request<'a> { pub req_cert: CertID<'a>, #[explicit(0)] - pub single_request_extensions: Option>, + pub single_request_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] diff --git a/src/rust/cryptography-x509/src/ocsp_resp.rs b/src/rust/cryptography-x509/src/ocsp_resp.rs index f7620f6aa601..21f01e2c7375 100644 --- a/src/rust/cryptography-x509/src/ocsp_resp.rs +++ b/src/rust/cryptography-x509/src/ocsp_resp.rs @@ -2,7 +2,11 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::{certificate, common, crl, extensions, name, ocsp_req}; +use crate::{ + certificate, common, crl, + extensions::{self}, + name, ocsp_req, +}; #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct OCSPResponse<'a> { @@ -47,7 +51,7 @@ pub struct ResponseData<'a> { asn1::SequenceOfWriter<'a, SingleResponse<'a>, Vec>>, >, #[explicit(1)] - pub response_extensions: Option>, + pub raw_response_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] @@ -66,7 +70,7 @@ pub struct SingleResponse<'a> { #[explicit(0)] pub next_update: Option, #[explicit(1)] - pub single_extensions: Option>, + pub raw_single_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] diff --git a/src/rust/src/x509/certificate.rs b/src/rust/src/x509/certificate.rs index 03d8ae883256..3784b1c9a4b0 100644 --- a/src/rust/src/x509/certificate.rs +++ b/src/rust/src/x509/certificate.rs @@ -9,13 +9,13 @@ use crate::error::{CryptographyError, CryptographyResult}; use crate::x509::{extensions, sct, sign}; use crate::{exceptions, x509}; use cryptography_x509::common::Asn1ReadableOrWritable; +use cryptography_x509::extensions::Extension; use cryptography_x509::extensions::{ AuthorityKeyIdentifier, BasicConstraints, DisplayText, DistributionPoint, DistributionPointName, MSCertificateTemplate, NameConstraints, PolicyConstraints, - PolicyInformation, PolicyQualifierInfo, Qualifier, SequenceOfAccessDescriptions, + PolicyInformation, PolicyQualifierInfo, Qualifier, RawExtensions, SequenceOfAccessDescriptions, SequenceOfSubtrees, UserNotice, }; -use cryptography_x509::extensions::{Extension, Extensions}; use cryptography_x509::{common, name, oid}; use once_cell::sync::Lazy; use pyo3::{IntoPy, ToPyObject}; @@ -193,9 +193,9 @@ impl Certificate { let val = self.raw.borrow_value(); let mut tbs_precert = val.tbs_cert.clone(); // Remove the SCT list extension - match tbs_precert.extensions { - Some(extensions) => { - let readable_extensions = extensions.unwrap_read().clone(); + match val.tbs_cert.extensions() { + Ok(Some(extensions)) => { + let readable_extensions = extensions.as_raw().unwrap_read().clone(); let ext_count = readable_extensions.len(); let filtered_extensions: Vec> = readable_extensions .filter(|x| x.extn_id != oid::PRECERT_SIGNED_CERTIFICATE_TIMESTAMPS_OID) @@ -207,18 +207,26 @@ impl Certificate { ), )); } - let filtered_extensions: Extensions<'_> = Asn1ReadableOrWritable::new_write( + let filtered_extensions: RawExtensions<'_> = Asn1ReadableOrWritable::new_write( asn1::SequenceOfWriter::new(filtered_extensions), ); - tbs_precert.extensions = Some(filtered_extensions); + tbs_precert.raw_extensions = Some(filtered_extensions); let result = asn1::write_single(&tbs_precert)?; Ok(pyo3::types::PyBytes::new(py, &result)) } - None => Err(CryptographyError::from( + Ok(None) => Err(CryptographyError::from( pyo3::exceptions::PyValueError::new_err( "Could not find any extensions in TBS certificate", ), )), + Err(oid) => { + let oid_obj = oid_to_py_oid(py, &oid)?; + Err(exceptions::DuplicateExtension::new_err(( + format!("Duplicate {} extension found", oid), + oid_obj.into_py(py), + )) + .into()) + } } } @@ -360,7 +368,7 @@ impl Certificate { x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.raw.borrow_value().tbs_cert.extensions, + &self.raw.borrow_value().tbs_cert.raw_extensions, |oid, ext_data| match *oid { oid::PRECERT_POISON_OID => { asn1::parse_single::<()>(ext_data)?; @@ -1035,7 +1043,7 @@ fn create_x509_certificate( spki: asn1::parse_single(spki_bytes)?, issuer_unique_id: None, subject_unique_id: None, - extensions: x509::common::encode_extensions( + raw_extensions: x509::common::encode_extensions( py, builder.getattr(pyo3::intern!(py, "_extensions"))?, extensions::encode_extension, diff --git a/src/rust/src/x509/common.rs b/src/rust/src/x509/common.rs index 571963e36b63..94ae58d386c5 100644 --- a/src/rust/src/x509/common.rs +++ b/src/rust/src/x509/common.rs @@ -6,11 +6,10 @@ use crate::asn1::{oid_to_py_oid, py_oid_to_oid}; use crate::error::{CryptographyError, CryptographyResult}; use crate::{exceptions, x509}; use cryptography_x509::common::{Asn1ReadableOrWritable, AttributeTypeValue, RawTlv}; -use cryptography_x509::extensions::{AccessDescription, Extension, Extensions}; +use cryptography_x509::extensions::{AccessDescription, Extension, Extensions, RawExtensions}; use cryptography_x509::name::{GeneralName, Name, OtherName, UnvalidatedIA5String}; use pyo3::types::IntoPyDict; use pyo3::{IntoPy, ToPyObject}; -use std::collections::HashSet; /// Parse all sections in a PEM file and return the first matching section. /// If no matching sections are found, return an error. @@ -391,27 +390,30 @@ pub(crate) fn parse_and_cache_extensions< >( py: pyo3::Python<'p>, cached_extensions: &mut Option, - raw_exts: &Option>, + raw_extensions: &Option>, parse_ext: F, ) -> pyo3::PyResult { if let Some(cached) = cached_extensions { return Ok(cached.clone_ref(py)); } + let extensions = match Extensions::from_raw_extensions(raw_extensions.as_ref()) { + Ok(extensions) => extensions, + Err(oid) => { + let oid_obj = oid_to_py_oid(py, &oid)?; + return Err(exceptions::DuplicateExtension::new_err(( + format!("Duplicate {} extension found", oid), + oid_obj.into_py(py), + ))); + } + }; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; let exts = pyo3::types::PyList::empty(py); - let mut seen_oids = HashSet::new(); - if let Some(raw_exts) = raw_exts { - for raw_ext in raw_exts.unwrap_read().clone() { + if let Some(extensions) = extensions { + for raw_ext in extensions.as_raw().unwrap_read().clone() { let oid_obj = oid_to_py_oid(py, &raw_ext.extn_id)?; - if seen_oids.contains(&raw_ext.extn_id) { - return Err(exceptions::DuplicateExtension::new_err(( - format!("Duplicate {} extension found", raw_ext.extn_id), - oid_obj.into_py(py), - ))); - } - let extn_value = match parse_ext(&raw_ext.extn_id, raw_ext.extn_value)? { Some(e) => e, None => x509_module.call_method1( @@ -424,7 +426,6 @@ pub(crate) fn parse_and_cache_extensions< (oid_obj, raw_ext.critical, extn_value), )?; exts.append(ext_obj)?; - seen_oids.insert(raw_ext.extn_id); } } let extensions = x509_module @@ -445,7 +446,7 @@ pub(crate) fn encode_extensions< py: pyo3::Python<'p>, py_exts: &'p pyo3::PyAny, encode_ext: F, -) -> pyo3::PyResult>> { +) -> pyo3::PyResult>> { let unrecognized_extension_type: &pyo3::types::PyType = py .import(pyo3::intern!(py, "cryptography.x509"))? .getattr(pyo3::intern!(py, "UnrecognizedExtension"))? diff --git a/src/rust/src/x509/crl.rs b/src/rust/src/x509/crl.rs index e2c4b9c09b9e..6bb08779a0a2 100644 --- a/src/rust/src/x509/crl.rs +++ b/src/rust/src/x509/crl.rs @@ -260,11 +260,13 @@ impl CertificateRevocationList { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { + let tbs_cert_list = &self.owned.borrow_value().tbs_cert_list; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.owned.borrow_value().tbs_cert_list.crl_extensions, + &tbs_cert_list.raw_crl_extensions, |oid, ext_data| match *oid { oid::CRL_NUMBER_OID => { let bignum = asn1::parse_single::>(ext_data)?; @@ -498,7 +500,7 @@ impl RevokedCertificate { x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.owned.borrow_value().crl_entry_extensions, + &self.owned.borrow_value().raw_crl_entry_extensions, |oid, ext_data| parse_crl_entry_ext(py, oid.clone(), ext_data), ) } @@ -594,7 +596,7 @@ fn create_x509_crl( user_certificate: asn1::BigUint::new(py_uint_to_big_endian_bytes(py, serial_number)?) .unwrap(), revocation_date: x509::certificate::time_from_py(py, py_revocation_date)?, - crl_entry_extensions: x509::common::encode_extensions( + raw_crl_entry_extensions: x509::common::encode_extensions( py, py_revoked_cert.getattr(pyo3::intern!(py, "extensions"))?, extensions::encode_extension, @@ -618,7 +620,7 @@ fn create_x509_crl( asn1::SequenceOfWriter::new(revoked_certs), )) }, - crl_extensions: x509::common::encode_extensions( + raw_crl_extensions: x509::common::encode_extensions( py, builder.getattr(pyo3::intern!(py, "_extensions"))?, extensions::encode_extension, diff --git a/src/rust/src/x509/csr.rs b/src/rust/src/x509/csr.rs index 35aee5c9e501..7ceed3511daa 100644 --- a/src/rust/src/x509/csr.rs +++ b/src/rust/src/x509/csr.rs @@ -211,7 +211,7 @@ impl CertificateSigningRequest { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { - let exts = self + let raw_exts = self .raw .borrow_value() .csr_info @@ -222,9 +222,12 @@ impl CertificateSigningRequest { ) })?; - x509::parse_and_cache_extensions(py, &mut self.cached_extensions, &exts, |oid, ext_data| { - certificate::parse_cert_ext(py, oid.clone(), ext_data) - }) + x509::parse_and_cache_extensions( + py, + &mut self.cached_extensions, + &raw_exts, + |oid, ext_data| certificate::parse_cert_ext(py, oid.clone(), ext_data), + ) } #[getter] diff --git a/src/rust/src/x509/ocsp_req.rs b/src/rust/src/x509/ocsp_req.rs index 235ac6ee10c5..bd5aecad0ec7 100644 --- a/src/rust/src/x509/ocsp_req.rs +++ b/src/rust/src/x509/ocsp_req.rs @@ -108,11 +108,13 @@ impl OCSPRequest { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { + let tbs_request = &self.raw.borrow_value().tbs_request; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.raw.borrow_value().tbs_request.request_extensions, + &tbs_request.raw_request_extensions, |oid, value| { match *oid { oid::NONCE_OID => { @@ -228,7 +230,7 @@ fn create_ocsp_request( request_list: common::Asn1ReadableOrWritable::new_write(asn1::SequenceOfWriter::new( &reqs, )), - request_extensions: extensions, + raw_request_extensions: extensions, }, optional_signature: None, }; diff --git a/src/rust/src/x509/ocsp_resp.rs b/src/rust/src/x509/ocsp_resp.rs index 942822b48168..728eb92cef38 100644 --- a/src/rust/src/x509/ocsp_resp.rs +++ b/src/rust/src/x509/ocsp_resp.rs @@ -316,20 +316,22 @@ impl OCSPResponse { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { self.requires_successful_response()?; + + let response_data = &self + .raw + .borrow_value() + .response_bytes + .as_ref() + .unwrap() + .response + .get() + .tbs_response_data; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self - .raw - .borrow_value() - .response_bytes - .as_ref() - .unwrap() - .response - .get() - .tbs_response_data - .response_extensions, + &response_data.raw_response_extensions, |oid, ext_data| { match oid { &oid::NONCE_OID => { @@ -362,11 +364,12 @@ impl OCSPResponse { .response .get(), )?; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_single_extensions, - &single_resp.single_extensions, + &single_resp.raw_single_extensions, |oid, ext_data| match oid { &oid::SIGNED_CERTIFICATE_TIMESTAMPS_OID => { let contents = asn1::parse_single::<&[u8]>(ext_data)?; @@ -628,7 +631,7 @@ fn create_ocsp_response( cert_status, next_update, this_update, - single_extensions: None, + raw_single_extensions: None, }]; borrowed_cert = responder_cert.borrow(); @@ -669,7 +672,7 @@ fn create_ocsp_response( responses: common::Asn1ReadableOrWritable::new_write(asn1::SequenceOfWriter::new( responses, )), - response_extensions: x509::common::encode_extensions( + raw_response_extensions: x509::common::encode_extensions( py, builder.getattr(pyo3::intern!(py, "_extensions"))?, extensions::encode_extension, diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py index a32dfca930cf..b33e09ce518f 100644 --- a/tests/x509/test_x509.py +++ b/tests/x509/test_x509.py @@ -1000,6 +1000,20 @@ def test_tbs_certificate_bytes(self, backend): cert.signature_hash_algorithm, ) + def test_tbs_precertificate_bytes_duplicate_extensions_raises( + self, backend + ): + cert = _load_cert( + os.path.join("x509", "custom", "two_basic_constraints.pem"), + x509.load_pem_x509_certificate, + ) + + with pytest.raises( + x509.DuplicateExtension, + match="Duplicate 2.5.29.19 extension found", + ): + cert.tbs_precertificate_bytes + def test_tbs_precertificate_bytes_no_extensions_raises(self, backend): cert = _load_cert( os.path.join("x509", "v1_cert.pem"),