x509-cert: adds a CertReq builder

baloo · baloo · commit 3a1fc2a4ba4e · 2023-05-01T12:46:36.000-07:00
diff --git a/x509-cert/src/builder.rs b/x509-cert/src/builder.rs
@@ -15,9 +15,10 @@ use crate::{
         pkix::{
             AuthorityKeyIdentifier, BasicConstraints, KeyUsage, KeyUsages, SubjectKeyIdentifier,
         },
-        AsExtension, Extension,
+        AsExtension, Extension, Extensions,
     },
     name::Name,
+    request::{CertReq, CertReqInfo, ExtensionReq},
     serial_number::SerialNumber,
     time::Validity,
 };
@@ -142,15 +143,19 @@ impl Profile {
                 include_subject_key_identifier: false,
                 ..
             } => {}
-            _ => extensions.push(SubjectKeyIdentifier::try_from(spk)?.to_extension(tbs)?),
+            _ => extensions.push(
+                SubjectKeyIdentifier::try_from(spk)?.to_extension(&tbs.subject, &extensions)?,
+            ),
         }
 
         // Build Authority Key Identifier
         match self {
             Profile::Root => {}
             _ => {
-                extensions
-                    .push(AuthorityKeyIdentifier::try_from(issuer_spk.clone())?.to_extension(tbs)?);
+                extensions.push(
+                    AuthorityKeyIdentifier::try_from(issuer_spk.clone())?
+                        .to_extension(&tbs.subject, &extensions)?,
+                );
             }
         }
 
@@ -160,29 +165,31 @@ impl Profile {
                 ca: true,
                 path_len_constraint: None,
             }
-            .to_extension(tbs)?,
+            .to_extension(&tbs.subject, &extensions)?,
             Profile::SubCA {
                 path_len_constraint,
                 ..
             } => BasicConstraints {
                 ca: true,
                 path_len_constraint: *path_len_constraint,
             }
-            .to_extension(tbs)?,
+            .to_extension(&tbs.subject, &extensions)?,
             Profile::Leaf { .. } => BasicConstraints {
                 ca: false,
                 path_len_constraint: None,
             }
-            .to_extension(tbs)?,
+            .to_extension(&tbs.subject, &extensions)?,
             #[cfg(feature = "hazmat")]
             Profile::Manual { .. } => unreachable!(),
         });
 
         // Build Key Usage extension
         match self {
             Profile::Root | Profile::SubCA { .. } => {
-                extensions
-                    .push(KeyUsage(KeyUsages::KeyCertSign | KeyUsages::CRLSign).to_extension(tbs)?);
+                extensions.push(
+                    KeyUsage(KeyUsages::KeyCertSign | KeyUsages::CRLSign)
+                        .to_extension(&tbs.subject, &extensions)?,
+                );
             }
             Profile::Leaf {
                 enable_key_agreement,
@@ -197,7 +204,7 @@ impl Profile {
                     key_usage |= KeyUsages::KeyAgreement;
                 }
 
-                extensions.push(KeyUsage(key_usage).to_extension(tbs)?);
+                extensions.push(KeyUsage(key_usage).to_extension(&tbs.subject, &extensions)?);
             }
             #[cfg(feature = "hazmat")]
             Profile::Manual { .. } => unreachable!(),
@@ -250,6 +257,7 @@ impl Profile {
 /// ```
 pub struct CertificateBuilder<'s, S> {
     tbs: TbsCertificate,
+    extensions: Extensions,
     signer: &'s S,
 }
 
@@ -302,30 +310,26 @@ where
             signer_pub.owned_to_ref(),
             &tbs,
         )?;
-        if !extensions.is_empty() {
-            tbs.extensions = Some(extensions);
-        }
-
-        Ok(Self { tbs, signer })
+        Ok(Self {
+            tbs,
+            extensions,
+            signer,
+        })
     }
 
     /// Add an extension to this certificate
     pub fn add_extension<E: AsExtension>(&mut self, extension: &E) -> Result<()> {
-        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);
-            }
-        }
+        let ext = extension.to_extension(&self.tbs.subject, &self.extensions)?;
+        self.extensions.push(ext);
 
         Ok(())
     }
 
     fn finalize(&mut self) {
+        if !self.extensions.is_empty() {
+            self.tbs.extensions = Some(self.extensions.clone());
+        }
+
         if self.tbs.extensions.is_none() {
             if self.tbs.issuer_unique_id.is_some() || self.tbs.subject_unique_id.is_some() {
                 self.tbs.version = Version::V2;
@@ -375,3 +379,95 @@ where
         Ok(cert)
     }
 }
+
+pub struct RequestBuilder<'s, S> {
+    info: CertReqInfo,
+    extension_req: ExtensionReq,
+    signer: &'s S,
+}
+
+impl<'s, S> RequestBuilder<'s, S>
+where
+    S: Keypair + DynSignatureAlgorithmIdentifier,
+    S::VerifyingKey: EncodePublicKey,
+{
+    /// Creates a new certificate request builder
+    pub fn new(subject: Name, signer: &'s S) -> Result<Self> {
+        let version = Default::default();
+        let verifying_key = signer.verifying_key();
+        let public_key = verifying_key
+            .to_public_key_der()?
+            .decode_msg::<SubjectPublicKeyInfoOwned>()?;
+        let attributes = Default::default();
+        let extension_req = Default::default();
+
+        Ok(Self {
+            info: CertReqInfo {
+                version,
+                subject,
+                public_key,
+                attributes,
+            },
+            extension_req,
+            signer,
+        })
+    }
+
+    /// Add an extension to this certificate request
+    pub fn add_extension<E: AsExtension>(&mut self, extension: &E) -> Result<()> {
+        let ext = extension.to_extension(&self.info.subject, &self.extension_req.0)?;
+
+        self.extension_req.0.push(ext);
+
+        Ok(())
+    }
+
+    fn finalize(&mut self) -> Result<()> {
+        self.info
+            .attributes
+            .add(self.extension_req.clone().try_into()?);
+        Ok(())
+    }
+
+    /// Run the certificate through the signer and build the end certificate.
+    pub fn build<Signature>(mut self) -> Result<CertReq>
+    where
+        S: Signer<Signature>,
+        Signature: SignatureEncoding,
+    {
+        self.finalize();
+
+        let algorithm = self.signer.signature_algorithm_identifier()?;
+        let signature = self.signer.try_sign(&self.info.to_der()?)?;
+        let signature = BitString::from_bytes(signature.to_bytes().as_ref())?;
+
+        let req = CertReq {
+            info: self.info,
+            algorithm,
+            signature,
+        };
+
+        Ok(req)
+    }
+
+    /// Run the certificate through the signer and build the end certificate.
+    pub fn build_with_rng<Signature>(mut self, rng: &mut impl CryptoRngCore) -> Result<CertReq>
+    where
+        S: RandomizedSigner<Signature>,
+        Signature: SignatureEncoding,
+    {
+        self.finalize();
+
+        let algorithm = self.signer.signature_algorithm_identifier()?;
+        let signature = self.signer.try_sign_with_rng(rng, &self.info.to_der()?)?;
+        let signature = BitString::from_bytes(signature.to_bytes().as_ref())?;
+
+        let req = CertReq {
+            info: self.info,
+            algorithm,
+            signature,
+        };
+
+        Ok(req)
+    }
+}
diff --git a/x509-cert/src/ext.rs b/x509-cert/src/ext.rs
@@ -1,6 +1,5 @@
 //! Standardized X.509 Certificate Extensions
 
-use crate::certificate;
 use const_oid::AssociatedOid;
 use der::{asn1::OctetString, Sequence, ValueOrd};
 use spki::ObjectIdentifier;
@@ -49,15 +48,19 @@ pub type Extensions = alloc::vec::Vec<Extension>;
 /// builder.
 pub trait AsExtension: AssociatedOid + der::Encode {
     /// Should the extension be marked critical
-    fn critical(&self, tbs: &certificate::TbsCertificate) -> bool;
+    fn critical(&self, subject: &crate::name::Name, extensions: &[Extension]) -> bool;
 
     /// Returns the Extension with the content encoded.
-    fn to_extension(&self, tbs: &certificate::TbsCertificate) -> Result<Extension, der::Error> {
+    fn to_extension(
+        &self,
+        subject: &crate::name::Name,
+        extensions: &[Extension],
+    ) -> Result<Extension, der::Error> {
         let content = OctetString::new(<Self as der::Encode>::to_der(self)?)?;
 
         Ok(Extension {
             extn_id: <Self as AssociatedOid>::OID,
-            critical: self.critical(tbs),
+            critical: self.critical(subject, extensions),
             extn_value: content,
         })
     }
diff --git a/x509-cert/src/ext/pkix.rs b/x509-cert/src/ext/pkix.rs
@@ -71,7 +71,7 @@ impl AssociatedOid for SubjectAltName {
 impl_newtype!(SubjectAltName, name::GeneralNames);
 
 impl crate::ext::AsExtension for SubjectAltName {
-    fn critical(&self, tbs: &crate::certificate::TbsCertificate) -> bool {
+    fn critical(&self, subject: &crate::name::Name, _extensions: &[super::Extension]) -> 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
@@ -83,7 +83,7 @@ impl crate::ext::AsExtension for SubjectAltName {
         //   subject distinguished name, conforming CAs SHOULD mark the
         //   subjectAltName extension as non-critical.
 
-        tbs.subject.is_empty()
+        subject.is_empty()
     }
 }
 
diff --git a/x509-cert/src/ext/pkix/constraints/basic.rs b/x509-cert/src/ext/pkix/constraints/basic.rs
@@ -24,7 +24,11 @@ impl AssociatedOid for BasicConstraints {
 }
 
 impl crate::ext::AsExtension for BasicConstraints {
-    fn critical(&self, _tbs: &crate::certificate::TbsCertificate) -> bool {
+    fn critical(
+        &self,
+        _subject: &crate::name::Name,
+        _extensions: &[crate::ext::Extension],
+    ) -> bool {
         // https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.9
         //   Conforming CAs MUST include this extension in all CA certificates
         //   that contain public keys used to validate digital signatures on
diff --git a/x509-cert/src/ext/pkix/keyusage.rs b/x509-cert/src/ext/pkix/keyusage.rs
@@ -139,7 +139,11 @@ impl AssociatedOid for ExtendedKeyUsage {
 impl_newtype!(ExtendedKeyUsage, Vec<ObjectIdentifier>);
 
 impl crate::ext::AsExtension for ExtendedKeyUsage {
-    fn critical(&self, _tbs: &crate::certificate::TbsCertificate) -> bool {
+    fn critical(
+        &self,
+        _subject: &crate::name::Name,
+        _extensions: &[crate::ext::Extension],
+    ) -> bool {
         // https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.12
         //   This extension MAY, at the option of the certificate issuer, be
         //   either critical or non-critical.
diff --git a/x509-cert/src/macros.rs b/x509-cert/src/macros.rs
@@ -86,7 +86,11 @@ macro_rules! impl_extension {
     };
     ($newtype:ty, critical = $critical:expr) => {
         impl crate::ext::AsExtension for $newtype {
-            fn critical(&self, _tbs: &crate::certificate::TbsCertificate) -> bool {
+            fn critical(
+                &self,
+                _subject: &crate::name::Name,
+                _extensions: &[crate::ext::Extension],
+            ) -> bool {
                 $critical
             }
         }
diff --git a/x509-cert/src/request.rs b/x509-cert/src/request.rs
@@ -17,11 +17,12 @@ use der::pem::PemLabel;
 /// Version identifier for certification request information.
 ///
 /// (RFC 2986 designates `0` as the only valid version)
-#[derive(Clone, Debug, Copy, PartialEq, Eq, Enumerated)]
+#[derive(Clone, Debug, Copy, PartialEq, Eq, Enumerated, Default)]
 #[asn1(type = "INTEGER")]
 #[repr(u8)]
 pub enum Version {
     /// Denotes PKCS#8 v1
+    #[default]
     V1 = 0,
 }
 
@@ -96,11 +97,28 @@ impl<'a> TryFrom<&'a [u8]> for CertReq {
 /// ```
 ///
 /// [RFC 5272 Section 3.1]: https://datatracker.ietf.org/doc/html/rfc5272#section-3.1
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq, Default)]
 pub struct ExtensionReq(pub Vec<Extension>);
 
 impl AssociatedOid for ExtensionReq {
     const OID: ObjectIdentifier = ID_EXTENSION_REQ;
 }
 
 impl_newtype!(ExtensionReq, Vec<Extension>);
+
+use crate::attr::{Attribute, AttributeValue};
+use der::asn1::{Any, SetOfVec};
+
+impl TryFrom<ExtensionReq> for Attribute {
+    type Error = der::Error;
+
+    fn try_from(extension_req: ExtensionReq) -> der::Result<Attribute> {
+        let mut values: SetOfVec<AttributeValue> = Default::default();
+        values.add(Any::encode_from(&extension_req.0.clone())?);
+
+        Ok(Attribute {
+            oid: ExtensionReq::OID,
+            values,
+        })
+    }
+}
diff --git a/x509-cert/test-support/src/openssl.rs b/x509-cert/test-support/src/openssl.rs
@@ -33,3 +33,33 @@ pub fn check_certificate(pem: &[u8]) -> String {
 
     String::from_utf8(output_buf.clone()).unwrap()
 }
+
+// TODO(baloo) factorize that out
+pub fn check_request(pem: &[u8]) -> String {
+    let tmp_dir = tempdir().expect("create tempdir");
+    let cert_path = tmp_dir.path().join("cert.pem");
+
+    let mut cert_file = File::create(&cert_path).expect("create pem file");
+    cert_file.write_all(pem).expect("Create pem file");
+
+    let mut child = Command::new("openssl")
+        .arg("req")
+        .arg("-in")
+        .arg(&cert_path)
+        //        .arg("-noout")
+        .arg("-text")
+        .stderr(Stdio::inherit())
+        .stdout(Stdio::piped())
+        .spawn()
+        .expect("zlint failed");
+    let mut stdout = child.stdout.take().unwrap();
+    let exit_status = child.wait().expect("get openssl x509 req status");
+
+    assert!(exit_status.success(), "openssl failed");
+    let mut output_buf = Vec::new();
+    stdout
+        .read_to_end(&mut output_buf)
+        .expect("read openssl output");
+
+    String::from_utf8(output_buf.clone()).unwrap()
+}
diff --git a/x509-cert/tests/builder.rs b/x509-cert/tests/builder.rs

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ impl AssociatedOid for SubjectAltName {`
`71`	`71`	`impl_newtype!(SubjectAltName, name::GeneralNames);`
`72`	`72`
`73`	`73`	`impl crate::ext::AsExtension for SubjectAltName {`
`74`		`- fn critical(&self, tbs: &crate::certificate::TbsCertificate) -> bool {`
	`74`	`+ fn critical(&self, subject: &crate::name::Name, _extensions: &[super::Extension]) -> bool {`
`75`	`75`	`// https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6`
`76`	`76`	`// Further, if the only subject identity included in the certificate is`
`77`	`77`	`// an alternative name form (e.g., an electronic mail address), then the`
`@@ -83,7 +83,7 @@ impl crate::ext::AsExtension for SubjectAltName {`
`83`	`83`	`// subject distinguished name, conforming CAs SHOULD mark the`
`84`	`84`	`// subjectAltName extension as non-critical.`
`85`	`85`
`86`		`- tbs.subject.is_empty()`
	`86`	`+ subject.is_empty()`
`87`	`87`	`}`
`88`	`88`	`}`
`89`	`89`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,11 @@ macro_rules! impl_extension {`
`86`	`86`	`};`
`87`	`87`	`($newtype:ty, critical = $critical:expr) => {`
`88`	`88`	`impl crate::ext::AsExtension for $newtype {`
`89`		`- fn critical(&self, _tbs: &crate::certificate::TbsCertificate) -> bool {`
	`89`	`+ fn critical(`
	`90`	`+ &self,`
	`91`	`+ _subject: &crate::name::Name,`
	`92`	`+ _extensions: &[crate::ext::Extension],`
	`93`	`+ ) -> bool {`
`90`	`94`	`$critical`
`91`	`95`	`}`
`92`	`96`	`}`