certificate: add a get_extension helper (#8892)

* certificate: add a `get_extension` helper Signed-off-by: William Woodruff <[email protected]> * certificate: OID by ref Signed-off-by: William Woodruff <[email protected]> * certificate: syntax Signed-off-by: William Woodruff <[email protected]> * x509, src: `check_duplicate_extensions` Signed-off-by: William Woodruff <[email protected]> * src: simplify Signed-off-by: William Woodruff <[email protected]> * src: everyone loves newtypes Signed-off-by: William Woodruff <[email protected]> * rust: refactor-o-rama Signed-off-by: William Woodruff <[email protected]> * src: look upon my works Signed-off-by: William Woodruff <[email protected]> * src: continue blasting the code Signed-off-by: William Woodruff <[email protected]> * src/rust: actually commit my changes Signed-off-by: William Woodruff <[email protected]> * src: clippage Signed-off-by: William Woodruff <[email protected]> * relocate Signed-off-by: William Woodruff <[email protected]> * src: dedupe Signed-off-by: William Woodruff <[email protected]> * src: cleanup Signed-off-by: William Woodruff <[email protected]> * clippage Signed-off-by: William Woodruff <[email protected]> * src: dedupe Signed-off-by: William Woodruff <[email protected]> * common: cleanup Signed-off-by: William Woodruff <[email protected]> * src: unused impls Signed-off-by: William Woodruff <[email protected]> * more deletion Signed-off-by: William Woodruff <[email protected]> * clippage Signed-off-by: William Woodruff <[email protected]> * extensions: add a `get_extension` test Signed-off-by: William Woodruff <[email protected]> * extensions: unused derives Signed-off-by: William Woodruff <[email protected]> * tests/x509: dup ext check for tbs_precertificate_bytes Signed-off-by: William Woodruff <[email protected]> * certificate: remove `extensions()` Signed-off-by: William Woodruff <[email protected]> * extensions: docs Signed-off-by: William Woodruff <[email protected]> * extensions: newtype Signed-off-by: William Woodruff <[email protected]> * rust: better error types, dedupe Signed-off-by: William Woodruff <[email protected]> extensions: unwrap -> expect Signed-off-by: William Woodruff <[email protected]> * Revert "rust: better error types, dedupe" This reverts commit 212b75f. --------- Signed-off-by: William Woodruff <[email protected]>
pyca · May 10, 2023 · 1ff6208 · 1ff6208
1 parent c6887af
commit 1ff6208
Show file tree

Hide file tree

Showing 13 changed files with 193 additions and 60 deletions.
diff --git 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<asn1::BitString<'a>>,
     #[explicit(3)]
-    pub extensions: Option<extensions::Extensions<'a>>,
+    pub raw_extensions: Option<extensions::RawExtensions<'a>>,
+}
+
+impl<'a> TbsCertificate<'a> {
+    pub fn extensions(&'a self) -> Result<Option<Extensions<'a>>, 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
@@ -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<common::Asn1ReadableOrWritable<'a, asn1::BitString<'a>, asn1::OwnedBitString>>;
@@ -31,14 +35,14 @@ pub struct TBSCertList<'a> {
     pub next_update: Option<common::Time>,
     pub revoked_certificates: RevokedCertificates<'a>,
     #[explicit(0)]
-    pub crl_extensions: Option<extensions::Extensions<'a>>,
+    pub raw_crl_extensions: Option<extensions::RawExtensions<'a>>,
 }
 
 #[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<extensions::Extensions<'a>>,
+    pub raw_crl_entry_extensions: Option<extensions::RawExtensions<'a>>,
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]

diff --git 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<Option<extensions::Extensions<'_>>, asn1::ParseError> {
+    ) -> Result<Option<extensions::RawExtensions<'_>>, 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
@@ -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<Extension<'a>>>,
 >;
 
+/// 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<Option<Self>, 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<Extension> {
+        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<u64>,
 }
+
+#[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
@@ -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<extensions::Extensions<'a>>,
+    pub raw_request_extensions: Option<extensions::RawExtensions<'a>>,
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
 pub struct Request<'a> {
     pub req_cert: CertID<'a>,
     #[explicit(0)]
-    pub single_request_extensions: Option<extensions::Extensions<'a>>,
+    pub single_request_extensions: Option<extensions::RawExtensions<'a>>,
 }
 
 #[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
@@ -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<SingleResponse<'a>>>,
     >,
     #[explicit(1)]
-    pub response_extensions: Option<extensions::Extensions<'a>>,
+    pub raw_response_extensions: Option<extensions::RawExtensions<'a>>,
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
@@ -66,7 +70,7 @@ pub struct SingleResponse<'a> {
     #[explicit(0)]
     pub next_update: Option<asn1::GeneralizedTime>,
     #[explicit(1)]
-    pub single_extensions: Option<extensions::Extensions<'a>>,
+    pub raw_single_extensions: Option<extensions::RawExtensions<'a>>,
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]

diff --git 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<Extension<'_>> = 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
@@ -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<pyo3::PyObject>,
-    raw_exts: &Option<Extensions<'_>>,
+    raw_extensions: &Option<RawExtensions<'_>>,
     parse_ext: F,
 ) -> pyo3::PyResult<pyo3::PyObject> {
     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<Option<Extensions<'p>>> {
+) -> pyo3::PyResult<Option<RawExtensions<'p>>> {
     let unrecognized_extension_type: &pyo3::types::PyType = py
         .import(pyo3::intern!(py, "cryptography.x509"))?
         .getattr(pyo3::intern!(py, "UnrecognizedExtension"))?