rework AsExtension trait?

Author: baloo

x509-cert/src/builder.rs

@@ -1,6 +1,6 @@
 //! X509 Certificate builder
 
-use alloc::vec::Vec;
+use alloc::vec;
 use der::asn1::{BitString, OctetString};
 use sha1::{Digest, Sha1};
 use spki::{AlgorithmIdentifierOwned, SubjectPublicKeyInfoOwned};
@@ -81,12 +81,13 @@ impl Profile {
         &self,
         spk: &SubjectPublicKeyInfoOwned,
         issuer_spk: &SubjectPublicKeyInfoOwned,
-    ) -> Result<Vec<Extension>> {
-        let mut extensions: Vec<Extension> = Vec::new();
+        tbs: &TbsCertificate,
+    ) -> Result<vec::Vec<Extension>> {
+        let mut extensions: vec::Vec<Extension> = vec::Vec::new();
 
         extensions.push({
             let result = Sha1::digest(spk.subject_public_key.raw_bytes());
-            SubjectKeyIdentifier(OctetString::new(result.to_vec())?).to_extension()?
+            SubjectKeyIdentifier(OctetString::new(result.to_vec())?).to_extension(tbs)?
         });
 
         // Build Authority Key Identifier
@@ -103,7 +104,7 @@ impl Profile {
                         authority_cert_issuer: None,
                         authority_cert_serial_number: None,
                     }
-                    .to_extension()?,
+                    .to_extension(tbs)?,
                 );
             }
         }
@@ -114,20 +115,20 @@ impl Profile {
                 ca: true,
                 path_len_constraint: None,
             }
-            .to_extension()?,
+            .to_extension(tbs)?,
             Profile::SubCA {
                 path_len_constraint,
                 ..
             } => BasicConstraints {
                 ca: true,
                 path_len_constraint: *path_len_constraint,
             }
-            .to_extension()?,
+            .to_extension(tbs)?,
             Profile::Leaf { .. } => BasicConstraints {
                 ca: false,
                 path_len_constraint: None,
             }
-            .to_extension()?,
+            .to_extension(tbs)?,
         });
 
         // Build Key Usage extension
@@ -137,7 +138,7 @@ impl Profile {
                     KeyUsage(
                         KeyUsages::DigitalSignature | KeyUsages::KeyCertSign | KeyUsages::CRLSign,
                     )
-                    .to_extension()?,
+                    .to_extension(tbs)?,
                 );
             }
             Profile::Leaf { .. } => {
@@ -230,18 +231,7 @@ impl From<CertificateVersion> for Version {
 /// .expect("Create certificate");
 /// ```
 pub struct CertificateBuilder<'s, S: Signer> {
-    version: Version,
-    serial_number: SerialNumber,
-    signature_alg: AlgorithmIdentifierOwned,
-    issuer: Name,
-    validity: Validity,
-    subject: Name,
-    subject_public_key_info: SubjectPublicKeyInfoOwned,
-    issuer_unique_id: Option<BitString>,
-    subject_unique_id: Option<BitString>,
-
-    extensions: Vec<Extension>,
-
+    tbs: TbsCertificate,
     signer: &'s S,
 }
 
@@ -264,63 +254,59 @@ impl<'s, S: Signer> CertificateBuilder<'s, S> {
         validity.not_before.rfc5280_adjust_utc_time()?;
         validity.not_after.rfc5280_adjust_utc_time()?;
 
-        let (version, (issuer_unique_id, subject_unique_id), extensions) = match version {
-            CertificateVersion::V1 => (Version::V1, (None, None), Vec::new()),
-            CertificateVersion::V2(uids) => (Version::V2, uids.get_unique_ids(), Vec::new()),
-            CertificateVersion::V3(uids) => (
-                Version::V3,
-                uids.get_unique_ids(),
-                profile.build_extensions(&subject_public_key_info, &signer_pub)?,
-            ),
+        let (version, (issuer_unique_id, subject_unique_id)) = match version {
+            CertificateVersion::V1 => (Version::V1, (None, None)),
+            CertificateVersion::V2(uids) => (Version::V2, uids.get_unique_ids()),
+            CertificateVersion::V3(uids) => (Version::V3, uids.get_unique_ids()),
         };
 
-        Ok(Self {
+        let mut tbs = TbsCertificate {
             version,
-            issuer_unique_id,
-            subject_unique_id,
             serial_number,
+            signature: signature_alg,
             issuer,
-            subject,
             validity,
+            subject,
             subject_public_key_info,
-            signature_alg,
-            extensions,
-            signer,
-        })
+            issuer_unique_id,
+            subject_unique_id,
+            extensions: None,
+        };
+
+        if tbs.version == Version::V3 {
+            let extensions =
+                profile.build_extensions(&tbs.subject_public_key_info, &signer_pub, &tbs)?;
+            if !extensions.is_empty() {
+                tbs.extensions = Some(extensions);
+            }
+        }
+
+        Ok(Self { tbs, signer })
     }
 
     /// Add an extension to this certificate
     pub fn add_extension<E: AsExtension>(&mut self, extension: &E) -> Result<()> {
-        self.extensions.push(extension.to_extension()?);
+        if self.tbs.version == Version::V3 {
+            let ext = extension.to_extension(&self.tbs)?;
+
+            if let Some(extensions) = self.tbs.extensions.as_mut() {
+                extensions.push(ext);
+            } else {
+                let extensions = vec![ext];
+                self.tbs.extensions = Some(extensions);
+            }
+        }
+
         Ok(())
     }
 
     /// Run the certificate through the signer and build the end certificate.
     pub fn build(&mut self) -> Result<Certificate> {
-        let extensions = if self.version == Version::V3 {
-            Some(self.extensions.clone())
-        } else {
-            None
-        };
-
-        let tbs_certificate = TbsCertificate {
-            version: self.version,
-            serial_number: self.serial_number.clone(),
-            signature: self.signature_alg.clone(),
-            issuer: self.issuer.clone(),
-            validity: self.validity,
-            subject: self.subject.clone(),
-            subject_public_key_info: self.subject_public_key_info.clone(),
-            issuer_unique_id: self.issuer_unique_id.clone(),
-            subject_unique_id: self.subject_unique_id.clone(),
-            extensions,
-        };
-
-        let signature = BitString::from_bytes(&self.signer.sign(&tbs_certificate))?;
+        let signature = BitString::from_bytes(&self.signer.sign(&self.tbs))?;
 
         let cert = Certificate {
-            tbs_certificate,
-            signature_algorithm: self.signature_alg.clone(),
+            tbs_certificate: self.tbs.clone(),
+            signature_algorithm: self.tbs.signature.clone(),
             signature,
         };
 
@@ -346,5 +332,5 @@ pub trait Signer {
     fn public_key(&self) -> SubjectPublicKeyInfoOwned;
 
     /// The sign method should return the signature of the payload.
-    fn sign(&self, input: &TbsCertificate) -> Vec<u8>;
+    fn sign(&self, input: &TbsCertificate) -> vec::Vec<u8>;
 }

x509-cert/src/ext.rs

@@ -1,5 +1,6 @@
 //! Standardized X.509 Certificate Extensions
 
+use crate::certificate;
 use const_oid::AssociatedOid;
 use der::{asn1::OctetString, Sequence, ValueOrd};
 use spki::ObjectIdentifier;
@@ -48,15 +49,15 @@ pub type Extensions = alloc::vec::Vec<Extension>;
 /// builder.
 pub trait AsExtension: AssociatedOid + der::Encode {
     /// Should the extension be marked critical
-    const CRITICAL: bool;
+    fn critical(&self, tbs: &certificate::TbsCertificate) -> bool;
 
     /// Returns the Extension with the content encoded.
-    fn to_extension(&self) -> Result<Extension, der::Error> {
+    fn to_extension(&self, tbs: &certificate::TbsCertificate) -> Result<Extension, der::Error> {
         let content = OctetString::new(<Self as der::Encode>::to_vec(self)?)?;
 
         Ok(Extension {
             extn_id: <Self as AssociatedOid>::OID,
-            critical: Self::CRITICAL,
+            critical: self.critical(tbs),
             extn_value: content,
         })
     }

x509-cert/src/ext/pkix.rs

@@ -66,6 +66,23 @@ impl AssociatedOid for SubjectAltName {
 
 impl_newtype!(SubjectAltName, name::GeneralNames);
 
+impl crate::ext::AsExtension for SubjectAltName {
+    fn critical(&self, tbs: &crate::certificate::TbsCertificate) -> bool {
+        // https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6
+        //   Further, if the only subject identity included in the certificate is
+        //   an alternative name form (e.g., an electronic mail address), then the
+        //   subject distinguished name MUST be empty (an empty sequence), and the
+        //   subjectAltName extension MUST be present.  If the subject field
+        //   contains an empty sequence, then the issuing CA MUST include a
+        //   subjectAltName extension that is marked as critical.  When including
+        //   the subjectAltName extension in a certificate that has a non-empty
+        //   subject distinguished name, conforming CAs SHOULD mark the
+        //   subjectAltName extension as non-critical.
+
+        tbs.subject.is_empty()
+    }
+}
+
 /// IssuerAltName as defined in [RFC 5280 Section 4.2.1.7].
 ///
 /// ```text

x509-cert/src/ext/pkix/certpolicy.rs

@@ -22,6 +22,12 @@ impl AssociatedOid for CertificatePolicies {
 }
 
 impl_newtype!(CertificatePolicies, Vec<PolicyInformation>);
+// TODO
+//   If this extension is
+//   critical, the path validation software MUST be able to interpret this
+//   extension (including the optional qualifier), or MUST reject the
+//   certificate.
+impl_extension!(CertificatePolicies);
 
 /// PolicyInformation as defined in [RFC 5280 Section 4.2.1.4].
 ///

x509-cert/src/ext/pkix/constraints/basic.rs

@@ -22,3 +22,17 @@ pub struct BasicConstraints {
 impl AssociatedOid for BasicConstraints {
     const OID: ObjectIdentifier = ID_CE_BASIC_CONSTRAINTS;
 }
+
+// TODO
+//   Conforming CAs MUST include this extension in all CA certificates
+//   that contain public keys used to validate digital signatures on
+//   certificates and MUST mark the extension as critical in such
+//   certificates.  This extension MAY appear as a critical or non-
+//   critical extension in CA certificates that contain public keys used
+//   exclusively for purposes other than validating digital signatures on
+//   certificates.  Such CA certificates include ones that contain public
+//   keys used exclusively for validating digital signatures on CRLs and
+//   ones that contain key management public keys used with certificate
+//   enrollment protocols.  This extension MAY appear as a critical or
+//   non-critical extension in end entity certificates.
+impl_extension!(BasicConstraints, critical = true);

x509-cert/src/ext/pkix/constraints/name.rs

@@ -31,6 +31,8 @@ impl AssociatedOid for NameConstraints {
     const OID: ObjectIdentifier = ID_CE_NAME_CONSTRAINTS;
 }
 
+impl_extension!(NameConstraints, critical = true);
+
 /// GeneralSubtrees as defined in [RFC 5280 Section 4.2.1.10].
 ///
 /// ```text

x509-cert/src/ext/pkix/keyusage.rs

@@ -78,6 +78,20 @@ impl AssociatedOid for ExtendedKeyUsage {
 }
 
 impl_newtype!(ExtendedKeyUsage, Vec<ObjectIdentifier>);
+// TODO
+//   This extension MAY, at the option of the certificate issuer, be
+//   either critical or non-critical.
+//
+//   If a CA includes extended key usages to satisfy such applications,
+//   but does not wish to restrict usages of the key, the CA can include
+//   the special KeyPurposeId anyExtendedKeyUsage in addition to the
+//   particular key purposes required by the applications.  Conforming CAs
+//   SHOULD NOT mark this extension as critical if the anyExtendedKeyUsage
+//   KeyPurposeId is present.  Applications that require the presence of a
+//   particular purpose MAY reject certificates that include the
+//   anyExtendedKeyUsage OID but not the particular OID expected for the
+//   application.
+impl_extension!(ExtendedKeyUsage, critical = true);
 
 /// PrivateKeyUsagePeriod as defined in [RFC 3280 Section 4.2.1.4].
 ///

x509-cert/src/macros.rs

@@ -86,7 +86,9 @@ macro_rules! impl_extension {
     };
     ($newtype:ty, critical = $critical:expr) => {
         impl crate::ext::AsExtension for $newtype {
-            const CRITICAL: bool = $critical;
+            fn critical(&self, _tbs: &crate::certificate::TbsCertificate) -> bool {
+                $critical
+            }
         }
     };
 }

x509-cert/src/name.rs

@@ -43,6 +43,11 @@ impl RdnSequence {
 
         RdnSequence(out).to_vec()
     }
+
+    /// Is this [`RdnSequence`] empty?
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
 }
 
 /// Serializes the structure according to the rules in [RFC 4514].