check_chain_extensions(): Add check that AKID and SKID are not marked critical

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/12478)

diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 2f9890d8be94b8a35b01ad216c09ad6e99501e01..bced482df4f915b5086e721617d5b569529918a6 100644 (file)
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -401,7 +401,6 @@ int x509v3_cache_extensions(X509 *x)
     ASN1_BIT_STRING *usage;
     ASN1_BIT_STRING *ns;
     EXTENDED_KEY_USAGE *extusage;
-    X509_EXTENSION *ex;
     int i;
     int res;
 
@@ -588,17 +587,30 @@ int x509v3_cache_extensions(X509 *x)
         x->ex_flags |= EXFLAG_INVALID;
 #endif
     for (i = 0; i < X509_get_ext_count(x); i++) {
-        ex = X509_get_ext(x, i);
-        if (OBJ_obj2nid(X509_EXTENSION_get_object(ex)) == NID_freshest_crl)
+        X509_EXTENSION *ex = X509_get_ext(x, i);
+        int nid = OBJ_obj2nid(X509_EXTENSION_get_object(ex));
+
+        if (nid == NID_freshest_crl)
             x->ex_flags |= EXFLAG_FRESHEST;
         if (!X509_EXTENSION_get_critical(ex))
             continue;
-        if (OBJ_obj2nid(X509_EXTENSION_get_object(ex)) == NID_basic_constraints)
-            x->ex_flags |= EXFLAG_BCONS_CRITICAL;
         if (!X509_supported_extension(ex)) {
             x->ex_flags |= EXFLAG_CRITICAL;
             break;
         }
+        switch (nid) {
+        case NID_basic_constraints:
+            x->ex_flags |= EXFLAG_BCONS_CRITICAL;
+            break;
+        case NID_authority_key_identifier:
+            x->ex_flags |= EXFLAG_AKID_CRITICAL;
+            break;
+        case NID_subject_key_identifier:
+            x->ex_flags |= EXFLAG_SKID_CRITICAL;
+            break;
+        default:
+            break;
+        }
     }
 
     /* Set x->siginf, ignoring errors due to unsupported algos */

diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index 042211e7feec288fd778ee5543193987d516b07a..d4bf31685e16055e86183c824aef20434f4cded0 100644 (file)
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -200,6 +200,10 @@ const char *X509_verify_cert_error_string(long n)
         return "Empty Subject Alternative Name extension";
     case X509_V_ERR_CA_BCONS_NOT_CRITICAL:
         return "Basic Constraints of CA cert not marked critical";
+    case X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL:
+        return "Authority Key Identifier marked critical";
+    case X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL:
+        return "Subject Key Identifier marked critical";
 
     default:
         /* Printing an error number into a static buffer is not thread-safe */

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index d058401b2b6ae108e98cc36317dbc0abf85467aa..48c0a2d58d0a8215915863584b9c5f63a7e503f7 100644 (file)
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -562,6 +562,10 @@ static int check_chain_extensions(X509_STORE_CTX *ctx)
             /* Check sig alg consistency acc. to RFC 5280 section 4.1.1.2 */
             if (X509_ALGOR_cmp(&x->sig_alg, &x->cert_info.signature) != 0)
                 ctx->error = X509_V_ERR_SIGNATURE_ALGORITHM_INCONSISTENCY;
+            if (x->akid != NULL && (x->ex_flags & EXFLAG_AKID_CRITICAL) != 0)
+                ctx->error = X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL;
+            if (x->skid != NULL && (x->ex_flags & EXFLAG_SKID_CRITICAL) != 0)
+                ctx->error = X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL;
             if (X509_get_version(x) >= 2) { /* at least X.509v3 */
                 /* Check AKID presence acc. to RFC 5280 section 4.2.1.1 */
                 if (i + 1 < num /*
@@ -570,11 +574,9 @@ static int check_chain_extensions(X509_STORE_CTX *ctx)
                                  */
                         && (x->akid == NULL || x->akid->keyid == NULL))
                     ctx->error = X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER;
-                /* TODO check that AKID extension is not critical */
                 /* Check SKID presence acc. to RFC 5280 section 4.2.1.2 */
                 if ((x->ex_flags & EXFLAG_CA) != 0 && x->skid == NULL)
                     ctx->error = X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER;
-                /* TODO check that SKID extension is not be critical */
             }
         }
         if (ctx->error != X509_V_OK)

diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index e00d51e06f66e39e45a0d5deffb46a8e1c585f02..c568b0541c9c9adfc5a47d41ab31409cb66dee0a 100644 (file)
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -229,6 +229,8 @@ X509_LOOKUP_ctrl_with_libctx((x), X509_L_ADD_STORE, (name), 0, NULL,           \
 # define X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER       86
 # define X509_V_ERR_EMPTY_SUBJECT_ALT_NAME               87
 # define X509_V_ERR_CA_BCONS_NOT_CRITICAL                88
+# define X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL    89
+# define X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL      90
 
 /* Certificate verify flags */
 # ifndef OPENSSL_NO_DEPRECATED_1_1_0

diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index 4faca1a2ee62462f87d221987a92eee938ec1ba4..93f9347ac8b3356dfe3a9e4c83740fde9670ec36 100644 (file)
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -377,6 +377,8 @@ struct ISSUING_DIST_POINT_st {
 # define EXFLAG_SS               0x2000 /* cert is apparently self-signed */
 
 # define EXFLAG_BCONS_CRITICAL   0x10000
+# define EXFLAG_AKID_CRITICAL    0x20000
+# define EXFLAG_SKID_CRITICAL    0x40000
 
 # define KU_DIGITAL_SIGNATURE    0x0080
 # define KU_NON_REPUDIATION      0x0040