From 4a2d9969aafc2c367e4db6141f1057d4d2ff972a Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 20 Aug 2024 11:42:56 -0400 Subject: [PATCH] Relax root CA AKI field checks (#11462) * Relax root CA AKI field checks Closes #11461. Signed-off-by: William Woodruff * CHANGELOG: record changes Signed-off-by: William Woodruff --------- Signed-off-by: William Woodruff --- CHANGELOG.rst | 3 +++ .../src/policy/extension.rs | 19 +++++++------------ tests/x509/verification/test_limbo.py | 6 ++++++ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9110fb78a..224747e3b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,9 @@ Changelog not be empty. * Added support for timestamp extraction to the :class:`~cryptography.fernet.MultiFernet` class. +* Relax the Authority Key Identifier requirements on root CA certificates + during X.509 verification to allow fields permitted by :rfc:`5280` but + forbidden by the CA/Browser BRs. .. _v43-0-0: diff --git a/src/rust/cryptography-x509-verification/src/policy/extension.rs b/src/rust/cryptography-x509-verification/src/policy/extension.rs index 1c8ae0067..a01eb4901 100644 --- a/src/rust/cryptography-x509-verification/src/policy/extension.rs +++ b/src/rust/cryptography-x509-verification/src/policy/extension.rs @@ -412,18 +412,13 @@ pub(crate) mod ca { )); } - // authorityCertIssuer and authorityCertSerialNumber MUST NOT be present. - if aki.authority_cert_issuer.is_some() { - return Err(ValidationError::Other( - "authorityKeyIdentifier must not contain authorityCertIssuer".to_string(), - )); - } - - if aki.authority_cert_serial_number.is_some() { - return Err(ValidationError::Other( - "authorityKeyIdentifier must not contain authorityCertSerialNumber".to_string(), - )); - } + // NOTE: CABF 7.1.2.1.3 says that Root CAs MUST NOT + // have authorityCertIdentifier or authorityCertSerialNumber, + // but these are present in practice in trust program bundles + // due to older roots that have been grandfathered in. + // Other validators are permissive of these being present, + // so we don't check for them. + // See #11461 for more information. } Ok(()) diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py index 50881eb94..d0402c4ce 100644 --- a/tests/x509/verification/test_limbo.py +++ b/tests/x509/verification/test_limbo.py @@ -67,6 +67,12 @@ LIMBO_SKIP_TESTCASES = { # 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 root CAs where the AKI contains fields other than keyIdentifier, + # which is technically forbidden under CABF. No other implementations + # enforce this requirement. + "webpki::aki::root-with-aki-authoritycertissuer", + "webpki::aki::root-with-aki-authoritycertserialnumber", + "webpki::aki::root-with-aki-all-fields", # 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-not-divisable-by-8-in-root",