From 103f123efa15191c0125555cfc623a54ba7a5392 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 30 Jan 2024 19:45:18 -0500 Subject: [PATCH] parsing, verification: check RSA key size against WebPKI minimum (#10302) * parsing, verification: check RSA key size against WebPKI minimum Signed-off-by: William Woodruff * move key size check to permits_ca We don't enforce EE key sizes, consistent with other CABF validators. Signed-off-by: William Woodruff * limit is_rsa to key algorithms Signed-off-by: William Woodruff * is_rsa -> is_rsa_key Signed-off-by: William Woodruff * fetch-vectors: bump limbo Signed-off-by: William Woodruff * reorg, remove helper Signed-off-by: William Woodruff * Update .github/actions/fetch-vectors/action.yml Co-authored-by: Alex Gaynor --------- Signed-off-by: William Woodruff Co-authored-by: Alex Gaynor --- .github/actions/fetch-vectors/action.yml | 4 +-- src/rust/Cargo.lock | 1 + src/rust/cryptography-key-parsing/src/rsa.rs | 4 +-- .../cryptography-x509-verification/Cargo.toml | 1 + .../src/policy/mod.rs | 25 +++++++++++++++++++ tests/x509/verification/test_limbo.py | 3 +++ 6 files changed, 34 insertions(+), 4 deletions(-) diff --git a/.github/actions/fetch-vectors/action.yml b/.github/actions/fetch-vectors/action.yml index 7e5198c8094a..f9715437f878 100644 --- 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 index d4a9a31adec1..84e9d90e7eea 100644 --- a/src/rust/Cargo.lock +++ b/src/rust/Cargo.lock @@ -125,6 +125,7 @@ name = "cryptography-x509-verification" version = "0.1.0" dependencies = [ "asn1", + "cryptography-key-parsing", "cryptography-x509", "once_cell", "pem", diff --git a/src/rust/cryptography-key-parsing/src/rsa.rs b/src/rust/cryptography-key-parsing/src/rsa.rs index 066e7053cb52..5a2f57d58a6b 100644 --- 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 index 30a4e8cb7373..2ec541fb2af0 100644 --- 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 index d5fffd0d8e2a..3d8bc86b6b8b 100644 --- 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>>, @@ -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 index 194b64f1f0bd..57c429886809 100644 --- 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.