Convert several additional extensions to use Asn1Operation (#12020)

Author: alex

src/rust/cryptography-x509-verification/src/lib.rs

@@ -18,6 +18,7 @@ use std::vec;
 use asn1::ObjectIdentifier;
 use cryptography_x509::extensions::{DuplicateExtensionsError, Extensions};
 use cryptography_x509::{
+    common::Asn1Read,
     extensions::{NameConstraints, SubjectAlternativeName},
     name::GeneralName,
     oid::{NAME_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID},
@@ -216,7 +217,7 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
 
     fn evaluate_constraints<B: CryptoOps>(
         &self,
-        constraints: &NameConstraints<'chain>,
+        constraints: &NameConstraints<'chain, Asn1Read>,
         budget: &mut Budget,
     ) -> ValidationResult<'chain, (), B> {
         if let Some(child) = self.child {
@@ -227,7 +228,7 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
             // If there are no applicable constraints, the SAN is considered valid so the default is true.
             let mut permit = true;
             if let Some(permitted_subtrees) = &constraints.permitted_subtrees {
-                for p in permitted_subtrees.unwrap_read().clone() {
+                for p in permitted_subtrees.clone() {
                     let status = self.evaluate_single_constraint(&p.base, &san, budget)?;
                     if status.is_applied() {
                         permit = status.is_match();
@@ -245,7 +246,7 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
             }
 
             if let Some(excluded_subtrees) = &constraints.excluded_subtrees {
-                for e in excluded_subtrees.unwrap_read().clone() {
+                for e in excluded_subtrees.clone() {
                     let status = self.evaluate_single_constraint(&e.base, &san, budget)?;
                     if status.is_match() {
                         return Err(ValidationError::new(ValidationErrorKind::Other(

src/rust/cryptography-x509-verification/src/policy/extension.rs

@@ -381,6 +381,7 @@ pub(crate) mod ee {
 pub(crate) mod ca {
     use cryptography_x509::{
         certificate::Certificate,
+        common::Asn1Read,
         extensions::{
             AuthorityKeyIdentifier, BasicConstraints, ExtendedKeyUsage, Extension, KeyUsage,
             NameConstraints,
@@ -413,7 +414,7 @@ pub(crate) mod ca {
         // some chains that are not strictly CABF compliant (e.g. ones where intermediate
         // CAs are missing AKIs), but this is a relatively minor discrepancy.
         if let Some(extn) = extn {
-            let aki: AuthorityKeyIdentifier<'_> = extn.value()?;
+            let aki: AuthorityKeyIdentifier<'_, Asn1Read> = extn.value()?;
             // 7.1.2.11.1 Authority Key Identifier:
 
             // keyIdentifier MUST be present.
@@ -478,16 +479,16 @@ pub(crate) mod ca {
         extn: Option<&Extension<'_>>,
     ) -> ValidationResult<'chain, (), B> {
         if let Some(extn) = extn {
-            let name_constraints: NameConstraints<'_> = extn.value()?;
+            let name_constraints: NameConstraints<'_, Asn1Read> = extn.value()?;
 
             let permitted_subtrees_empty = name_constraints
                 .permitted_subtrees
                 .as_ref()
-                .map_or(true, |pst| pst.unwrap_read().is_empty());
+                .map_or(true, |pst| pst.is_empty());
             let excluded_subtrees_empty = name_constraints
                 .excluded_subtrees
                 .as_ref()
-                .map_or(true, |est| est.unwrap_read().is_empty());
+                .map_or(true, |est| est.is_empty());
 
             if permitted_subtrees_empty && excluded_subtrees_empty {
                 return Err(ValidationError::new(ValidationErrorKind::Other(

src/rust/cryptography-x509/src/common.rs

@@ -265,6 +265,9 @@ impl<T: asn1::SimpleAsn1Writable, U: asn1::SimpleAsn1Writable> asn1::SimpleAsn1W
 
 pub trait Asn1Operation {
     type SequenceOfVec<'a, T>
+    where
+        T: 'a;
+    type SetOfVec<'a, T>
     where
         T: 'a;
     type OwnedBitString<'a>;
@@ -278,13 +281,21 @@ impl Asn1Operation for Asn1Read {
         = asn1::SequenceOf<'a, T>
     where
         T: 'a;
+    type SetOfVec<'a, T>
+        = asn1::SetOf<'a, T>
+    where
+        T: 'a;
     type OwnedBitString<'a> = asn1::BitString<'a>;
 }
 impl Asn1Operation for Asn1Write {
     type SequenceOfVec<'a, T>
         = asn1::SequenceOfWriter<'a, T, Vec<T>>
     where
         T: 'a;
+    type SetOfVec<'a, T>
+        = asn1::SetOfWriter<'a, T, Vec<T>>
+    where
+        T: 'a;
     type OwnedBitString<'a> = asn1::OwnedBitString;
 }
 

src/rust/cryptography-x509/src/crl.rs

@@ -43,7 +43,7 @@ pub struct RevokedCertificate<'a> {
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
 pub struct IssuingDistributionPoint<'a, Op: Asn1Operation> {
     #[explicit(0)]
-    pub distribution_point: Option<extensions::DistributionPointName<'a>>,
+    pub distribution_point: Option<extensions::DistributionPointName<'a, Op>>,
 
     #[implicit(1)]
     #[default(false)]

src/rust/cryptography-x509/src/extensions.rs

@@ -142,19 +142,15 @@ pub enum DisplayText<'a> {
     BmpString(asn1::BMPString<'a>),
 }
 
-// Needed due to clippy type complexity warning.
-pub type SequenceOfSubtrees<'a> = common::Asn1ReadableOrWritable<
-    asn1::SequenceOf<'a, GeneralSubtree<'a>>,
-    asn1::SequenceOfWriter<'a, GeneralSubtree<'a>, Vec<GeneralSubtree<'a>>>,
->;
+pub type SequenceOfSubtrees<'a, Op> = <Op as Asn1Operation>::SequenceOfVec<'a, GeneralSubtree<'a>>;
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
-pub struct NameConstraints<'a> {
+pub struct NameConstraints<'a, Op: Asn1Operation> {
     #[implicit(0)]
-    pub permitted_subtrees: Option<SequenceOfSubtrees<'a>>,
+    pub permitted_subtrees: Option<SequenceOfSubtrees<'a, Op>>,
 
     #[implicit(1)]
-    pub excluded_subtrees: Option<SequenceOfSubtrees<'a>>,
+    pub excluded_subtrees: Option<SequenceOfSubtrees<'a, Op>>,
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
@@ -179,39 +175,30 @@ pub struct MSCertificateTemplate {
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
 pub struct DistributionPoint<'a, Op: Asn1Operation> {
     #[explicit(0)]
-    pub distribution_point: Option<DistributionPointName<'a>>,
+    pub distribution_point: Option<DistributionPointName<'a, Op>>,
 
     #[implicit(1)]
     pub reasons: crl::ReasonFlags<'a, Op>,
 
     #[implicit(2)]
-    pub crl_issuer: Option<name::SequenceOfGeneralName<'a>>,
+    pub crl_issuer: Option<name::SequenceOfGeneralName<'a, Op>>,
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
-pub enum DistributionPointName<'a> {
+pub enum DistributionPointName<'a, Op: Asn1Operation> {
     #[implicit(0)]
-    FullName(name::SequenceOfGeneralName<'a>),
+    FullName(name::SequenceOfGeneralName<'a, Op>),
 
     #[implicit(1)]
-    NameRelativeToCRLIssuer(
-        common::Asn1ReadableOrWritable<
-            asn1::SetOf<'a, common::AttributeTypeValue<'a>>,
-            asn1::SetOfWriter<
-                'a,
-                common::AttributeTypeValue<'a>,
-                Vec<common::AttributeTypeValue<'a>>,
-            >,
-        >,
-    ),
+    NameRelativeToCRLIssuer(Op::SetOfVec<'a, common::AttributeTypeValue<'a>>),
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
-pub struct AuthorityKeyIdentifier<'a> {
+pub struct AuthorityKeyIdentifier<'a, Op: Asn1Operation> {
     #[implicit(0)]
     pub key_identifier: Option<&'a [u8]>,
     #[implicit(1)]
-    pub authority_cert_issuer: Option<name::SequenceOfGeneralName<'a>>,
+    pub authority_cert_issuer: Option<name::SequenceOfGeneralName<'a, Op>>,
     #[implicit(2)]
     pub authority_cert_serial_number: Option<asn1::BigUint<'a>>,
 }

src/rust/cryptography-x509/src/name.rs

@@ -2,7 +2,7 @@
 // 2.0, and the BSD License. See the LICENSE file in the root of this repository
 // for complete details.
 
-use crate::common;
+use crate::common::{self, Asn1Operation};
 
 pub type NameReadable<'a> = asn1::SequenceOf<'a, asn1::SetOf<'a, common::AttributeTypeValue<'a>>>;
 
@@ -82,7 +82,5 @@ pub enum GeneralName<'a> {
     RegisteredID(asn1::ObjectIdentifier),
 }
 
-pub(crate) type SequenceOfGeneralName<'a> = common::Asn1ReadableOrWritable<
-    asn1::SequenceOf<'a, GeneralName<'a>>,
-    asn1::SequenceOfWriter<'a, GeneralName<'a>, Vec<GeneralName<'a>>>,
->;
+pub(crate) type SequenceOfGeneralName<'a, Op> =
+    <Op as Asn1Operation>::SequenceOfVec<'a, GeneralName<'a>>;

src/rust/src/x509/certificate.rs

@@ -574,28 +574,27 @@ fn parse_cp<'p>(
 
 fn parse_general_subtrees<'p>(
     py: pyo3::Python<'p>,
-    subtrees: SequenceOfSubtrees<'_>,
+    subtrees: SequenceOfSubtrees<'_, Asn1Read>,
 ) -> CryptographyResult<pyo3::Bound<'p, pyo3::PyAny>> {
     let gns = pyo3::types::PyList::empty(py);
-    for gs in subtrees.unwrap_read().clone() {
+    for gs in subtrees {
         gns.append(x509::parse_general_name(py, gs.base)?)?;
     }
     Ok(gns.into_any())
 }
 
 pub(crate) fn parse_distribution_point_name<'p>(
     py: pyo3::Python<'p>,
-    dp: DistributionPointName<'p>,
+    dp: DistributionPointName<'p, Asn1Read>,
 ) -> CryptographyResult<(pyo3::Bound<'p, pyo3::PyAny>, pyo3::Bound<'p, pyo3::PyAny>)> {
     Ok(match dp {
         DistributionPointName::FullName(data) => (
-            x509::parse_general_names(py, data.unwrap_read())?,
+            x509::parse_general_names(py, &data)?,
             py.None().into_bound(py),
         ),
-        DistributionPointName::NameRelativeToCRLIssuer(data) => (
-            py.None().into_bound(py),
-            x509::parse_rdn(py, data.unwrap_read())?,
-        ),
+        DistributionPointName::NameRelativeToCRLIssuer(data) => {
+            (py.None().into_bound(py), x509::parse_rdn(py, &data)?)
+        }
     })
 }
 
@@ -609,7 +608,7 @@ fn parse_distribution_point<'p>(
     };
     let reasons = parse_distribution_point_reasons(py, dp.reasons.as_ref())?;
     let crl_issuer = match dp.crl_issuer {
-        Some(aci) => x509::parse_general_names(py, aci.unwrap_read())?,
+        Some(aci) => x509::parse_general_names(py, &aci)?,
         None => py.None().into_bound(py),
     };
     Ok(types::DISTRIBUTION_POINT
@@ -674,13 +673,13 @@ pub(crate) fn parse_authority_key_identifier<'p>(
     py: pyo3::Python<'p>,
     ext: &Extension<'p>,
 ) -> Result<pyo3::Bound<'p, pyo3::PyAny>, CryptographyError> {
-    let aki = ext.value::<AuthorityKeyIdentifier<'_>>()?;
+    let aki = ext.value::<AuthorityKeyIdentifier<'_, Asn1Read>>()?;
     let serial = match aki.authority_cert_serial_number {
         Some(biguint) => big_byte_slice_to_py_int(py, biguint.as_bytes())?.unbind(),
         None => py.None(),
     };
     let issuer = match aki.authority_cert_issuer {
-        Some(aci) => x509::parse_general_names(py, aci.unwrap_read())?,
+        Some(aci) => x509::parse_general_names(py, &aci)?,
         None => py.None().into_bound(py),
     };
     Ok(types::AUTHORITY_KEY_IDENTIFIER
@@ -911,7 +910,7 @@ pub fn parse_cert_ext<'p>(
             Ok(Some(types::FRESHEST_CRL.get(py)?.call1((dp,))?))
         }
         oid::NAME_CONSTRAINTS_OID => {
-            let nc = ext.value::<NameConstraints<'_>>()?;
+            let nc = ext.value::<NameConstraints<'_, Asn1Read>>()?;
             let permitted_subtrees = match nc.permitted_subtrees {
                 Some(data) => parse_general_subtrees(py, data)?,
                 None => py.None().into_bound(py),

src/rust/src/x509/extensions.rs

@@ -19,7 +19,7 @@ fn encode_general_subtrees<'a>(
     ka_bytes: &'a cryptography_keepalive::KeepAlive<pyo3::pybacked::PyBackedBytes>,
     ka_str: &'a cryptography_keepalive::KeepAlive<pyo3::pybacked::PyBackedStr>,
     subtrees: &pyo3::Bound<'a, pyo3::PyAny>,
-) -> Result<Option<extensions::SequenceOfSubtrees<'a>>, CryptographyError> {
+) -> Result<Option<extensions::SequenceOfSubtrees<'a, Asn1Write>>, CryptographyError> {
     if subtrees.is_none() {
         Ok(None)
     } else {
@@ -32,9 +32,7 @@ fn encode_general_subtrees<'a>(
                 maximum: None,
             });
         }
-        Ok(Some(common::Asn1ReadableOrWritable::new_write(
-            asn1::SequenceOfWriter::new(subtree_seq),
-        )))
+        Ok(Some(asn1::SequenceOfWriter::new(subtree_seq)))
     }
 }
 
@@ -55,9 +53,7 @@ pub(crate) fn encode_authority_key_identifier<'a>(
     let authority_cert_issuer = if let Some(authority_cert_issuer) = aki.authority_cert_issuer {
         let gns =
             x509::common::encode_general_names(py, &ka_bytes, &ka_str, &authority_cert_issuer)?;
-        Some(common::Asn1ReadableOrWritable::new_write(
-            asn1::SequenceOfWriter::new(gns),
-        ))
+        Some(asn1::SequenceOfWriter::new(gns))
     } else {
         None
     };
@@ -69,7 +65,9 @@ pub(crate) fn encode_authority_key_identifier<'a>(
         } else {
             None
         };
-    Ok(asn1::write_single(&extensions::AuthorityKeyIdentifier {
+    Ok(asn1::write_single(&extensions::AuthorityKeyIdentifier::<
+        Asn1Write,
+    > {
         authority_cert_issuer,
         authority_cert_serial_number,
         key_identifier: aki.key_identifier.as_deref(),
@@ -96,16 +94,14 @@ pub(crate) fn encode_distribution_points<'p>(
 
         let crl_issuer = if let Some(py_crl_issuer) = py_dp.crl_issuer {
             let gns = x509::common::encode_general_names(py, &ka_bytes, &ka_str, &py_crl_issuer)?;
-            Some(common::Asn1ReadableOrWritable::new_write(
-                asn1::SequenceOfWriter::new(gns),
-            ))
+            Some(asn1::SequenceOfWriter::new(gns))
         } else {
             None
         };
         let distribution_point = if let Some(py_full_name) = py_dp.full_name {
             let gns = x509::common::encode_general_names(py, &ka_bytes, &ka_str, &py_full_name)?;
             Some(extensions::DistributionPointName::FullName(
-                common::Asn1ReadableOrWritable::new_write(asn1::SequenceOfWriter::new(gns)),
+                asn1::SequenceOfWriter::new(gns),
             ))
         } else if let Some(py_relative_name) = py_dp.relative_name {
             let mut name_entries = vec![];
@@ -114,7 +110,7 @@ pub(crate) fn encode_distribution_points<'p>(
                 name_entries.push(ne);
             }
             Some(extensions::DistributionPointName::NameRelativeToCRLIssuer(
-                common::Asn1ReadableOrWritable::new_write(asn1::SetOfWriter::new(name_entries)),
+                asn1::SetOfWriter::new(name_entries),
             ))
         } else {
             None
@@ -338,7 +334,7 @@ fn encode_issuing_distribution_point(
         let py_full_name = ext.getattr(pyo3::intern!(py, "full_name"))?;
         let gns = x509::common::encode_general_names(ext.py(), &ka_bytes, &ka_str, &py_full_name)?;
         Some(extensions::DistributionPointName::FullName(
-            common::Asn1ReadableOrWritable::new_write(asn1::SequenceOfWriter::new(gns)),
+            asn1::SequenceOfWriter::new(gns),
         ))
     } else if ext
         .getattr(pyo3::intern!(py, "relative_name"))?
@@ -353,7 +349,7 @@ fn encode_issuing_distribution_point(
             name_entries.push(name_entry);
         }
         Some(extensions::DistributionPointName::NameRelativeToCRLIssuer(
-            common::Asn1ReadableOrWritable::new_write(asn1::SetOfWriter::new(name_entries)),
+            asn1::SetOfWriter::new(name_entries),
         ))
     } else {
         None
@@ -610,7 +606,7 @@ pub(crate) fn encode_extension(
 
             let permitted = ext.getattr(pyo3::intern!(py, "permitted_subtrees"))?;
             let excluded = ext.getattr(pyo3::intern!(py, "excluded_subtrees"))?;
-            let nc = extensions::NameConstraints {
+            let nc = extensions::NameConstraints::<Asn1Write> {
                 permitted_subtrees: encode_general_subtrees(
                     ext.py(),
                     &ka_bytes,