x509-cert: builder updates (#1001)

- Per RFC5280, DigitalSignature 'is asserted when the subject public key
  is used for verifying digital signatures, other than signatures on
  certificates (bit 5) and CRLs (bit 6)'.
  Using CA keys to sign random data would definitely be a bad practice and
  should be avoided. Thus remove the DigitalSignature keyUsage from these
  certificates.
- RSA PSS implements DynSignatureAlgorithmIdentifier only for the
  SigningKey, not for the verifying key. To allow using CertificateBuilder
  with RSA PSS keys require DynSignatureAlgorithmIdentifier implementation
  on S rather than on S::VerifyingKey.
- Signer (unlike SignerMut) is not expected to be mutable. Don't require
mutability of the Signer argument.
- ECDSA keys can not be used for keyEncipherment. Make this keyUsage bit
  optional.
- Follow the rules from RFC 5280 Section 4.1.2.1 to set the certificate's
  version depending on the presence of the extensions and identifiers.
- Remove unused conversion when building RDN fields.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Author: lumag

x509-cert/Cargo.toml

@@ -22,7 +22,7 @@ spki = { version = "0.7.1", features = ["alloc"] }
 # optional dependencies
 arbitrary = { version = "1.3", features = ["derive"], optional = true }
 sha1 = { version = "0.10.0", optional = true }
-signature = { version = "2.1.0", features = ["digest"], optional = true }
+signature = { version = "2.1.0", optional = true }
 
 [dev-dependencies]
 hex-literal = "0.4"

x509-cert/src/builder.rs

@@ -36,6 +36,9 @@ pub enum Error {
     Signature(signature::Error),
 }
 
+#[cfg(feature = "std")]
+impl std::error::Error for Error {}
+
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -87,6 +90,8 @@ pub enum Profile {
         issuer: Name,
         /// should the key agreement flag of KeyUsage be enabled
         enable_key_agreement: bool,
+        /// should the key encipherment flag of KeyUsage be enabled
+        enable_key_encipherment: bool,
     },
     #[cfg(feature = "hazmat")]
     /// Opt-out of the default extensions
@@ -161,20 +166,18 @@ impl Profile {
         // Build Key Usage extension
         match self {
             Profile::Root | Profile::SubCA { .. } => {
-                extensions.push(
-                    KeyUsage(
-                        KeyUsages::DigitalSignature | KeyUsages::KeyCertSign | KeyUsages::CRLSign,
-                    )
-                    .to_extension(tbs)?,
-                );
+                extensions
+                    .push(KeyUsage(KeyUsages::KeyCertSign | KeyUsages::CRLSign).to_extension(tbs)?);
             }
             Profile::Leaf {
                 enable_key_agreement,
+                enable_key_encipherment,
                 ..
             } => {
-                let mut key_usage = KeyUsages::DigitalSignature
-                    | KeyUsages::NonRepudiation
-                    | KeyUsages::KeyEncipherment;
+                let mut key_usage = KeyUsages::DigitalSignature | KeyUsages::NonRepudiation;
+                if *enable_key_encipherment {
+                    key_usage |= KeyUsages::KeyEncipherment;
+                }
                 if *enable_key_agreement {
                     key_usage |= KeyUsages::KeyAgreement;
                 }
@@ -194,7 +197,6 @@ impl Profile {
 /// ```
 /// use der::Decode;
 /// use x509_cert::spki::SubjectPublicKeyInfoOwned;
-/// use x509_cert::certificate::Version;
 /// use x509_cert::builder::{CertificateBuilder, Profile};
 /// use x509_cert::name::Name;
 /// use x509_cert::serial_number::SerialNumber;
@@ -223,35 +225,32 @@ impl Profile {
 /// let mut signer = rsa_signer();
 /// let mut builder = CertificateBuilder::new(
 ///     profile,
-///     Version::V3,
 ///     serial_number,
 ///     validity,
 ///     subject,
 ///     pub_key,
-///     &mut signer,
+///     &signer,
 /// )
 /// .expect("Create certificate");
 /// ```
 pub struct CertificateBuilder<'s, S> {
     tbs: TbsCertificate,
-    signer: &'s mut S,
+    signer: &'s S,
 }
 
 impl<'s, S> CertificateBuilder<'s, S>
 where
-    S: Keypair,
+    S: Keypair + DynSignatureAlgorithmIdentifier,
     S::VerifyingKey: EncodePublicKey,
-    S::VerifyingKey: DynSignatureAlgorithmIdentifier,
 {
     /// Creates a new certificate builder
     pub fn new<Signature>(
         profile: Profile,
-        version: Version,
         serial_number: SerialNumber,
         mut validity: Validity,
         subject: Name,
         subject_public_key_info: SubjectPublicKeyInfoOwned,
-        signer: &'s mut S,
+        signer: &'s S,
     ) -> Result<Self>
     where
         S: Signer<Signature>,
@@ -261,14 +260,14 @@ where
             .to_public_key_der()?
             .decode_msg::<SubjectPublicKeyInfoOwned>()?;
 
-        let signature_alg = verifying_key.signature_algorithm_identifier()?;
+        let signature_alg = signer.signature_algorithm_identifier()?;
         let issuer = profile.get_issuer(&subject);
 
         validity.not_before.rfc5280_adjust_utc_time()?;
         validity.not_after.rfc5280_adjust_utc_time()?;
 
         let mut tbs = TbsCertificate {
-            version,
+            version: Version::V3,
             serial_number,
             signature: signature_alg,
             issuer,
@@ -286,15 +285,13 @@ where
             subject_unique_id: None,
         };
 
-        if tbs.version == Version::V3 {
-            let extensions = profile.build_extensions(
-                tbs.subject_public_key_info.owned_to_ref(),
-                signer_pub.owned_to_ref(),
-                &tbs,
-            )?;
-            if !extensions.is_empty() {
-                tbs.extensions = Some(extensions);
-            }
+        let extensions = profile.build_extensions(
+            tbs.subject_public_key_info.owned_to_ref(),
+            signer_pub.owned_to_ref(),
+            &tbs,
+        )?;
+        if !extensions.is_empty() {
+            tbs.extensions = Some(extensions);
         }
 
         Ok(Self { tbs, signer })
@@ -317,17 +314,24 @@ where
     }
 
     /// Run the certificate through the signer and build the end certificate.
-    pub fn build<Signature>(&mut self) -> Result<Certificate>
+    pub fn build<Signature>(mut self) -> Result<Certificate>
     where
         S: Signer<Signature>,
         Signature: SignatureEncoding,
     {
+        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;
+            } else {
+                self.tbs.version = Version::V1;
+            }
+        }
         let signature = self.signer.try_sign(&self.tbs.to_der()?)?;
         let signature = BitString::from_bytes(signature.to_bytes().as_ref())?;
 
         let cert = Certificate {
             tbs_certificate: self.tbs.clone(),
-            signature_algorithm: self.tbs.signature.clone(),
+            signature_algorithm: self.tbs.signature,
             signature,
         };
 

x509-cert/test-support/src/zlint.rs

@@ -11,7 +11,7 @@ use std::{
 };
 use tempfile::tempdir;
 
-#[derive(Debug, Copy, Clone, PartialEq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum Status {
     NotApplicable,
     NotEffective,
@@ -29,7 +29,7 @@ impl Status {
     }
 }
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct LintStatus {
     pub status: Status,
     pub details: Option<String>,

x509-cert/tests/builder.rs

@@ -9,7 +9,6 @@ use spki::SubjectPublicKeyInfoOwned;
 use std::{str::FromStr, time::Duration};
 use x509_cert::{
     builder::{CertificateBuilder, Profile},
-    certificate::Version,
     name::Name,
     serial_number::SerialNumber,
     time::Validity,
@@ -31,17 +30,10 @@ fn root_ca_certificate() {
     let pub_key =
         SubjectPublicKeyInfoOwned::try_from(RSA_2048_DER_EXAMPLE).expect("get rsa pub key");
 
-    let mut signer = rsa_signer();
-    let mut builder = CertificateBuilder::new(
-        profile,
-        Version::V3,
-        serial_number,
-        validity,
-        subject,
-        pub_key,
-        &mut signer,
-    )
-    .expect("Create certificate");
+    let signer = rsa_signer();
+    let builder =
+        CertificateBuilder::new(profile, serial_number, validity, subject, pub_key, &signer)
+            .expect("Create certificate");
 
     let certificate = builder.build().unwrap();
 
@@ -57,33 +49,26 @@ fn sub_ca_certificate() {
     let serial_number = SerialNumber::from(42u32);
     let validity = Validity::from_now(Duration::new(5, 0)).unwrap();
 
-    let issuer = Name::from_str("CN=World domination corporation,O=World domination Inc,C=US")
-        .unwrap()
-        .to_der()
-        .unwrap();
-    let issuer = Name::from_der(&issuer).unwrap();
+    let issuer =
+        Name::from_str("CN=World domination corporation,O=World domination Inc,C=US").unwrap();
     let profile = Profile::SubCA {
         issuer,
         path_len_constraint: Some(0),
     };
 
-    let subject = Name::from_str("CN=World domination task force,O=World domination Inc,C=US")
-        .unwrap()
-        .to_der()
-        .unwrap();
-    let subject = Name::from_der(&subject).unwrap();
+    let subject =
+        Name::from_str("CN=World domination task force,O=World domination Inc,C=US").unwrap();
     let pub_key =
         SubjectPublicKeyInfoOwned::try_from(RSA_2048_DER_EXAMPLE).expect("get rsa pub key");
 
-    let mut signer = ecdsa_signer();
-    let mut builder = CertificateBuilder::new::<ecdsa::Signature<NistP256>>(
+    let signer = ecdsa_signer();
+    let builder = CertificateBuilder::new::<ecdsa::Signature<NistP256>>(
         profile,
-        Version::V3,
         serial_number,
         validity,
         subject,
         pub_key,
-        &mut signer,
+        &signer,
     )
     .expect("Create certificate");
 
@@ -108,33 +93,26 @@ fn leaf_certificate() {
     let serial_number = SerialNumber::from(42u32);
     let validity = Validity::from_now(Duration::new(5, 0)).unwrap();
 
-    let issuer = Name::from_str("CN=World domination corporation,O=World domination Inc,C=US")
-        .unwrap()
-        .to_der()
-        .unwrap();
-    let issuer = Name::from_der(&issuer).unwrap();
+    let issuer =
+        Name::from_str("CN=World domination corporation,O=World domination Inc,C=US").unwrap();
     let profile = Profile::Leaf {
         issuer,
         enable_key_agreement: false,
+        enable_key_encipherment: false,
     };
 
-    let subject = Name::from_str("CN=service.domination.world")
-        .unwrap()
-        .to_der()
-        .unwrap();
-    let subject = Name::from_der(&subject).unwrap();
+    let subject = Name::from_str("CN=service.domination.world").unwrap();
     let pub_key =
         SubjectPublicKeyInfoOwned::try_from(RSA_2048_DER_EXAMPLE).expect("get rsa pub key");
 
-    let mut signer = ecdsa_signer();
-    let mut builder = CertificateBuilder::new::<ecdsa::Signature<NistP256>>(
+    let signer = ecdsa_signer();
+    let builder = CertificateBuilder::new::<ecdsa::Signature<NistP256>>(
         profile,
-        Version::V3,
         serial_number,
         validity,
         subject,
         pub_key,
-        &mut signer,
+        &signer,
     )
     .expect("Create certificate");