]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
x509: remove erroneous critical extension enforcement
authorDaniel Kubec <kubec@openssl.org>
Tue, 3 Mar 2026 10:26:10 +0000 (11:26 +0100)
committerTomas Mraz <tomas@openssl.org>
Thu, 5 Mar 2026 14:21:57 +0000 (15:21 +0100)
Critical extension enforcement (introduced in #8a639b9) is incorrect. These
checks were intended as CA requirements to prevent misinterpretation by
verifiers that don't support certain extensions. However, since we do support
these extensions, we have no requirement for them to be marked critical,
enforcing that is a mistake.

As noted in: #30233 (comment)

Co-authored-by: David von Oheimb <david.von.oheimb@siemens.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
MergeDate: Thu Mar  5 14:22:07 2026
(Merged from https://github.com/openssl/openssl/pull/30249)

CHANGES.md
crypto/x509/v3_purp.c
crypto/x509/x509_vfy.c
doc/man1/openssl-verification-options.pod
include/openssl/x509v3.h.in

index e895ebccdf3ab9d9192b6f77d4701a58bb0ca48b..4ca267087386397a057698fecb7cb013f3466b94 100644 (file)
@@ -183,6 +183,15 @@ OpenSSL Releases
 
    *Bob Beck*
 
+ * Critical extension enforcement for EXFLAG_BCONS_CRITICAL,
+   EXFLAG_AKID_CRITICAL, EXFLAG_SKID_CRITICAL, and EXFLAG_SAN_CRITICAL is
+   incorrect. These checks were intended as CA requirements to prevent
+   misinterpretation by verifiers that don't support certain extensions
+   However, since we do support these extensions, there is no requirement for
+   them to be marked as critical. Enforcing that on X509_V_FLAG_X509_STRICT was a mistake.
+
+   *Daniel Kubec*
+
  * Made `X509_ATTRIBUTE` accessor functions const-correct. The functions
    `X509_ATTRIBUTE_get0_object()`, `X509_ATTRIBUTE_get0_type()`, and
    `X509_ATTRIBUTE_get0_data()` now accept `const X509_ATTRIBUTE *` and
index b62a5ea9d4bb8213f822de26d6f8de0455d40e0f..862c524732916414111ab380fb0a6d284228fc08 100644 (file)
@@ -662,22 +662,6 @@ int ossl_x509v3_cache_extensions(const X509 *const_x)
             tmp_ex_flags |= EXFLAG_CRITICAL;
             break;
         }
-        switch (nid) {
-        case NID_basic_constraints:
-            tmp_ex_flags |= EXFLAG_BCONS_CRITICAL;
-            break;
-        case NID_authority_key_identifier:
-            tmp_ex_flags |= EXFLAG_AKID_CRITICAL;
-            break;
-        case NID_subject_key_identifier:
-            tmp_ex_flags |= EXFLAG_SKID_CRITICAL;
-            break;
-        case NID_subject_alt_name:
-            tmp_ex_flags |= EXFLAG_SAN_CRITICAL;
-            break;
-        default:
-            break;
-        }
     }
 
     /* Set x->siginf, ignoring errors due to unsupported algos */
index 04fa0359cbc3ef570fc10d8b69e19c2b69e60b70..c5428dc15d8ac9a51439fbfce185fed100fc7efd 100644 (file)
@@ -674,10 +674,6 @@ static int check_extensions(X509_STORE_CTX *ctx)
                 CB_FAIL_IF((x->ex_kusage & KU_KEY_CERT_SIGN) == 0, ctx,
                     x, i, X509_V_ERR_PATHLEN_WITHOUT_KU_KEY_CERT_SIGN);
             }
-            CB_FAIL_IF((x->ex_flags & EXFLAG_CA) != 0
-                    && (x->ex_flags & EXFLAG_BCONS) != 0
-                    && (x->ex_flags & EXFLAG_BCONS_CRITICAL) == 0,
-                ctx, x, i, X509_V_ERR_CA_BCONS_NOT_CRITICAL);
             /* Check Key Usage according to RFC 5280 section 4.2.1.3 */
             if ((x->ex_flags & EXFLAG_CA) != 0) {
                 CB_FAIL_IF((x->ex_flags & EXFLAG_KUSAGE) == 0,
@@ -695,10 +691,6 @@ static int check_extensions(X509_STORE_CTX *ctx)
                            || x->altname == NULL)
                     && X509_NAME_entry_count(X509_get_subject_name(x)) == 0,
                 ctx, x, i, X509_V_ERR_SUBJECT_NAME_EMPTY);
-            CB_FAIL_IF(X509_NAME_entry_count(X509_get_subject_name(x)) == 0
-                    && x->altname != NULL
-                    && (x->ex_flags & EXFLAG_SAN_CRITICAL) == 0,
-                ctx, x, i, X509_V_ERR_EMPTY_SUBJECT_SAN_NOT_CRITICAL);
             /* Check SAN is non-empty according to RFC 5280 section 4.2.1.6 */
             CB_FAIL_IF(x->altname != NULL
                     && sk_GENERAL_NAME_num(x->altname) <= 0,
@@ -706,12 +698,6 @@ static int check_extensions(X509_STORE_CTX *ctx)
             /* Check sig alg consistency acc. to RFC 5280 section 4.1.1.2 */
             CB_FAIL_IF(X509_ALGOR_cmp(&x->sig_alg, &x->cert_info.signature) != 0,
                 ctx, x, i, X509_V_ERR_SIGNATURE_ALGORITHM_INCONSISTENCY);
-            CB_FAIL_IF(x->akid != NULL
-                    && (x->ex_flags & EXFLAG_AKID_CRITICAL) != 0,
-                ctx, x, i, X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL);
-            CB_FAIL_IF(x->skid != NULL
-                    && (x->ex_flags & EXFLAG_SKID_CRITICAL) != 0,
-                ctx, x, i, X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL);
             if (X509_get_version(x) >= X509_VERSION_3) {
                 /* Check AKID presence acc. to RFC 5280 section 4.2.1.1 */
                 /*
index 84940673caef84d5e060e12d5eda60c9bf066b7a..9b40038ace317886bac813287931dc8014afc80c 100644 (file)
@@ -316,10 +316,6 @@ among others, the following certificate well-formedness conditions are checked:
 
 =item *
 
-The basicConstraints of CA certificates must be marked critical.
-
-=item *
-
 CA certificates must explicitly include the keyUsage extension.
 
 =item *
@@ -349,11 +345,6 @@ The signatureAlgorithm field and the cert signature must be consistent.
 
 =item *
 
-Any given authorityKeyIdentifier and any given subjectKeyIdentifier
-must not be marked critical.
-
-=item *
-
 The authorityKeyIdentifier must be given for X.509v3 certs unless they
 are self-signed.
 
@@ -777,7 +768,8 @@ L<openssl-cms(1)>
 
 Since OpenSSL 1.1.0, the B<-trusted_first> option is always enabled.
 
-The checks enabled by B<-x509_strict> have been extended in OpenSSL 3.0.
+The checks enabled by B<-x509_strict> have been extended in OpenSSL 3.0,
+which has been partially reverted in OpenSSL 4.0.
 
 =head1 COPYRIGHT
 
index 99df8ac8ea05633a1c8b8859d4f180eeba8aff6b..cd88499067323e0589a20345ba9c9ca2012f81a3 100644 (file)
@@ -443,11 +443,17 @@ struct ISSUING_DIST_POINT_st {
 #define EXFLAG_FRESHEST 0x1000
 #define EXFLAG_SS 0x2000 /* cert is apparently self-signed */
 
+#define EXFLAG_NO_FINGERPRINT 0x100000
+
+/*
+ * The following flags are no longer used. On X509_V_FLAG_X509_STRICT they were
+ * previously enforced as checks on critical extensions but this behavior has
+ * been removed.
+ */
 #define EXFLAG_BCONS_CRITICAL 0x10000
 #define EXFLAG_AKID_CRITICAL 0x20000
 #define EXFLAG_SKID_CRITICAL 0x40000
 #define EXFLAG_SAN_CRITICAL 0x80000
-#define EXFLAG_NO_FINGERPRINT 0x100000
 
 /* https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.3 */
 #define KU_DIGITAL_SIGNATURE X509v3_KU_DIGITAL_SIGNATURE