From 3b482aef5e6dba3ecf9728d27119ac8c82984a77 Mon Sep 17 00:00:00 2001
From: Arthur Gautier <arthur.gautier@arista.com>
Date: Thu, 4 Jan 2024 20:50:50 -0800
Subject: [PATCH] x509-cert: fix KeyUsage on leaf certificates builder

I tried to abide by the ETSI policy.
https://www.etsi.org/deliver/etsi_en/319400_319499/31941202/02.03.01_60/en_31941202v020301p.pdf

The policy recommends to make KeyUsage bits 0, 1, (2 and/or 4) exclusive.
This is meant to avoid making signing of commitments during
authentication.

This commit goes a bit further by making bit 2 and 4 exclusive. I don't
find a use-case for requiring both at the same time (and I would think
only key agreement is required.

Fixes #1281
---
 x509-cert/src/builder.rs   | 62 ++++++++++++++++++++++++++++----------
 x509-cert/tests/builder.rs | 11 +++----
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/x509-cert/src/builder.rs b/x509-cert/src/builder.rs
index c778f1e2f..a95e2584b 100644
--- a/x509-cert/src/builder.rs
+++ b/x509-cert/src/builder.rs
@@ -89,10 +89,8 @@ pub enum Profile {
         /// issuer   Name,
         /// represents the name signing the certificate
         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,
+        /// Usage of the leaf certificate
+        usage: Usage,
         /// should the subject key identifier extension be included
         ///
         /// From [RFC 5280 Section 4.2.1.2]:
@@ -191,18 +189,8 @@ impl Profile {
                         .to_extension(&tbs.subject, &extensions)?,
                 );
             }
-            Profile::Leaf {
-                enable_key_agreement,
-                enable_key_encipherment,
-                ..
-            } => {
-                let mut key_usage = KeyUsages::DigitalSignature | KeyUsages::NonRepudiation;
-                if *enable_key_encipherment {
-                    key_usage |= KeyUsages::KeyEncipherment;
-                }
-                if *enable_key_agreement {
-                    key_usage |= KeyUsages::KeyAgreement;
-                }
+            Profile::Leaf { usage, .. } => {
+                let key_usage = usage.key_usage().into();
 
                 extensions.push(KeyUsage(key_usage).to_extension(&tbs.subject, &extensions)?);
             }
@@ -214,6 +202,48 @@ impl Profile {
     }
 }
 
+/// [`Usage`] describes the usage of a Leaf certificate.
+///
+/// This is designed in accordance with [ETSI EN 319 412-2 § 4.3.2 Key usage].
+///
+/// The various fields will refer to [RFC 5280 § 4.2.1.3 Key Usage] definitions.
+///
+/// [RFC 5280 § 4.2.1.3 Key Usage]: https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.3
+/// [ETSI EN 319 412-2 § 4.3.2 Key usage]: https://www.etsi.org/deliver/etsi_en/319400_319499/31941202/02.03.01_60/en_31941202v020301p.pdf#page=11
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum Usage {
+    /// [`Usage::NonRepudiation`] will set the NonRepudiation (also known as
+    /// contentCommitment) bit of KeyUsage.
+    NonRepudiation,
+    /// [`Usage::DigitalSignature`] will set the digitalSignature bit.
+    ///
+    /// This is meant to be used in an entity authentication service, a data
+    /// origin authentication service, and/or an integrity service.
+    DigitalSignature,
+    /// [`Usage::KeyAgreement`] will set the `keyAgreement` bit.
+    ///
+    /// This is meant to be used on Certificates when a Diffie-Hellman key is
+    /// to be used for key management.
+    KeyAgreement,
+    /// [`Usage::KeyEncipherment`] will set the `keyEncipherment` bit.
+    ///
+    /// This is meant to be used on Certificates when an RSA public
+    /// key is to be used for encrypting a symmetric content-decryption
+    /// key or an asymmetric private key.
+    KeyEncipherment,
+}
+
+impl Usage {
+    fn key_usage(&self) -> KeyUsages {
+        match self {
+            Self::NonRepudiation => KeyUsages::NonRepudiation,
+            Self::DigitalSignature => KeyUsages::DigitalSignature,
+            Self::KeyAgreement => KeyUsages::KeyAgreement,
+            Self::KeyEncipherment => KeyUsages::KeyEncipherment,
+        }
+    }
+}
+
 /// X509 Certificate builder
 ///
 /// ```
diff --git a/x509-cert/tests/builder.rs b/x509-cert/tests/builder.rs
index d31eb8c7e..a3373d474 100644
--- a/x509-cert/tests/builder.rs
+++ b/x509-cert/tests/builder.rs
@@ -8,7 +8,7 @@ use sha2::Sha256;
 use spki::SubjectPublicKeyInfoOwned;
 use std::{str::FromStr, time::Duration};
 use x509_cert::{
-    builder::{Builder, CertificateBuilder, Profile, RequestBuilder},
+    builder::{Builder, CertificateBuilder, Profile, RequestBuilder, Usage},
     ext::pkix::{
         name::{DirectoryString, GeneralName},
         SubjectAltName,
@@ -124,8 +124,7 @@ fn leaf_certificate() {
         Name::from_str("CN=World domination corporation,O=World domination Inc,C=US").unwrap();
     let profile = Profile::Leaf {
         issuer: issuer.clone(),
-        enable_key_agreement: false,
-        enable_key_encipherment: false,
+        usage: Usage::DigitalSignature,
         #[cfg(feature = "hazmat")]
         include_subject_key_identifier: true,
     };
@@ -173,8 +172,7 @@ fn leaf_certificate() {
     {
         let profile = Profile::Leaf {
             issuer,
-            enable_key_agreement: false,
-            enable_key_encipherment: false,
+            usage: Usage::DigitalSignature,
             include_subject_key_identifier: false,
         };
         let builder =
@@ -203,8 +201,7 @@ fn pss_certificate() {
         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,
+        usage: Usage::DigitalSignature,
         #[cfg(feature = "hazmat")]
         include_subject_key_identifier: true,
     };