parsing, verification: check RSA key size against WebPKI minimum (#10302

) * parsing, verification: check RSA key size against WebPKI minimum Signed-off-by: William Woodruff <[email protected]> * move key size check to permits_ca We don't enforce EE key sizes, consistent with other CABF validators. Signed-off-by: William Woodruff <[email protected]> * limit is_rsa to key algorithms Signed-off-by: William Woodruff <[email protected]> * is_rsa -> is_rsa_key Signed-off-by: William Woodruff <[email protected]> * fetch-vectors: bump limbo Signed-off-by: William Woodruff <[email protected]> * reorg, remove helper Signed-off-by: William Woodruff <[email protected]> * Update .github/actions/fetch-vectors/action.yml Co-authored-by: Alex Gaynor <[email protected]> --------- Signed-off-by: William Woodruff <[email protected]> Co-authored-by: Alex Gaynor <[email protected]>
pyca · Jan 31, 2024 · 103f123 · 103f123
1 parent 6b2dc96
commit 103f123
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 4 deletions.
diff --git a/.github/actions/fetch-vectors/action.yml b/.github/actions/fetch-vectors/action.yml
@@ -16,5 +16,5 @@ runs:
       with:
         repository: "C2SP/x509-limbo"
         path: "x509-limbo"
-        # Latest commit on the x509-limbo main branch, as of Jan 30, 2024.
-        ref: "dd7541dac329f03756f6358ad0c01d32e5677619" # x509-limbo-ref
+        # Latest commit on the x509-limbo main branch, as of Jan 31, 2024.
+        ref: "481b5d595b00ce55824607e1e8c2f1174539f3f8" # x509-limbo-ref
diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock
diff --git a/src/rust/cryptography-key-parsing/src/rsa.rs b/src/rust/cryptography-key-parsing/src/rsa.rs
@@ -5,8 +5,8 @@
 use crate::KeyParsingResult;
 
 #[derive(asn1::Asn1Read)]
-struct Pksc1RsaPublicKey<'a> {
-    n: asn1::BigUint<'a>,
+pub struct Pksc1RsaPublicKey<'a> {
+    pub n: asn1::BigUint<'a>,
     e: asn1::BigUint<'a>,
 }
 

diff --git a/src/rust/cryptography-x509-verification/Cargo.toml b/src/rust/cryptography-x509-verification/Cargo.toml
@@ -10,6 +10,7 @@ rust-version = "1.63.0"
 [dependencies]
 asn1 = { version = "0.16.0", default-features = false }
 cryptography-x509 = { path = "../cryptography-x509" }
+cryptography-key-parsing = { path = "../cryptography-key-parsing" }
 once_cell = "1"
 
 [dev-dependencies]

diff --git a/src/rust/cryptography-x509-verification/src/policy/mod.rs b/src/rust/cryptography-x509-verification/src/policy/mod.rs
@@ -9,6 +9,7 @@ use std::ops::Range;
 use std::sync::Arc;
 
 use asn1::ObjectIdentifier;
+use cryptography_key_parsing::rsa::Pksc1RsaPublicKey;
 use cryptography_x509::certificate::Certificate;
 use cryptography_x509::common::{
     AlgorithmIdentifier, AlgorithmParameters, EcParameters, RsaPssParameters, Time,
@@ -27,6 +28,9 @@ use crate::policy::extension::{ca, common, ee, Criticality, ExtensionPolicy, Ext
 use crate::types::{DNSName, DNSPattern, IPAddress};
 use crate::{ValidationError, VerificationCertificate};
 
+// RSA key constraints, as defined in CA/B 6.1.5.
+static WEBPKI_MINIMUM_RSA_MODULUS: usize = 2048;
+
 // SubjectPublicKeyInfo AlgorithmIdentifier constants, as defined in CA/B 7.1.3.1.
 
 // RSA
@@ -213,6 +217,10 @@ pub struct Policy<'a, B: CryptoOps> {
     /// An extended key usage that must appear in EEs validated by this policy.
     pub extended_key_usage: ObjectIdentifier,
 
+    /// The minimum RSA modulus, in bits.
+    /// This is equivalent to the public key size, e.g. 2048 for an RSA-2048 key.
+    pub minimum_rsa_modulus: usize,
+
     /// The set of permitted public key algorithms, identified by their
     /// algorithm identifiers.
     pub permitted_public_key_algorithms: Arc<HashSet<AlgorithmIdentifier<'a>>>,
@@ -240,6 +248,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
             subject,
             validation_time: time,
             extended_key_usage: EKU_SERVER_AUTH_OID.clone(),
+            minimum_rsa_modulus: WEBPKI_MINIMUM_RSA_MODULUS,
             permitted_public_key_algorithms: Arc::clone(&*WEBPKI_PERMITTED_SPKI_ALGORITHMS),
             permitted_signature_algorithms: Arc::clone(&*WEBPKI_PERMITTED_SIGNATURE_ALGORITHMS),
             ca_extension_policy: ExtensionPolicy {
@@ -488,6 +497,22 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
             )));
         }
 
+        // CA/B 6.1.5: Key sizes
+        // NOTE: We don't currently enforce that RSA moduli are divisible by 8,
+        // since other implementations don't bother.
+        let issuer_spki = &issuer.certificate().tbs_cert.spki;
+        if matches!(
+            issuer_spki.algorithm.params,
+            AlgorithmParameters::Rsa(_) | AlgorithmParameters::RsaPss(_)
+        ) {
+            let rsa_key: Pksc1RsaPublicKey<'_> =
+                asn1::parse_single(issuer_spki.subject_public_key.as_bytes())?;
+
+            if rsa_key.n.as_bytes().len() * 8 < self.minimum_rsa_modulus {
+                return Err(ValidationError::Other("RSA key is too weak".into()));
+            }
+        }
+
         let pk = issuer
             .public_key(&self.ops)
             .map_err(|_| ValidationError::Other("issuer has malformed public key".to_string()))?;

diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py
@@ -62,6 +62,9 @@
     # forbidden under CABF. This is consistent with what
     # Go's crypto/x509 and Rust's webpki crate do.
     "webpki::aki::root-with-aki-ski-mismatch",
+    # We allow RSA keys that aren't divisible by 8, which is technically
+    # forbidden under CABF. No other implementation checks this either.
+    "webpki::forbidden-rsa-key-not-divisable-by-8",
     # We disallow CAs in the leaf position, which is explicitly forbidden
     # by CABF (but implicitly permitted under RFC 5280). This is consistent
     # with what webpki and rustls do, but inconsistent with Go and OpenSSL.