check_chain_extensions(): Add check that Basic Constraints of CA cert are marked...

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 4bde90f277db440002dca72a66e7bdc6d2ccdcc7..2f9890d8be94b8a35b01ad216c09ad6e99501e01 100644 (file)
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -470,7 +470,7 @@ int x509v3_cache_extensions(X509 *x)
         x->ex_flags |= EXFLAG_INVALID;
     }
 
-    /* Handle basic key usage */
+    /* Handle (basic) key usage */
     if ((usage = X509_get_ext_d2i(x, NID_key_usage, &i, NULL)) != NULL) {
         x->ex_kusage = 0;
         if (usage->length > 0) {
@@ -593,6 +593,8 @@ int x509v3_cache_extensions(X509 *x)
             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;

diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index 4bb16545ef9a12d9656055bb83ab42a454536b88..042211e7feec288fd778ee5543193987d516b07a 100644 (file)
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -198,6 +198,8 @@ const char *X509_verify_cert_error_string(long n)
         return "Missing Subject Key Identifier";
     case X509_V_ERR_EMPTY_SUBJECT_ALT_NAME:
         return "Empty Subject Alternative Name extension";
+    case X509_V_ERR_CA_BCONS_NOT_CRITICAL:
+        return "Basic Constraints of CA cert not 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 c6717c53c84bf8a4b50d814908d579889eb1106f..d058401b2b6ae108e98cc36317dbc0abf85467aa 100644 (file)
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -528,7 +528,14 @@ static int check_chain_extensions(X509_STORE_CTX *ctx)
                 if ((x->ex_kusage & KU_KEY_CERT_SIGN) == 0)
                     ctx->error = X509_V_ERR_PATHLEN_WITHOUT_KU_KEY_CERT_SIGN;
             }
-            /* TODO check basic constrains of CA cert are marked critical */
+            /*
+             * Check Basic Constraints of CA cert are marked critical,
+             * TODO should be only if cert is intended for verifying other certs
+             */
+            if ((x->ex_flags & EXFLAG_CA) != 0
+                    && (x->ex_flags & EXFLAG_BCONS) != 0
+                    && (x->ex_flags & EXFLAG_BCONS_CRITICAL) == 0)
+                ctx->error = X509_V_ERR_CA_BCONS_NOT_CRITICAL;
             /* Check keyCertSign according to RFC 5280 section 4.2.1.3 */
             if ((x->ex_flags & EXFLAG_CA) == 0
                     && (x->ex_kusage & KU_KEY_CERT_SIGN) != 0)

diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index ec2021357b39d5e970e948b5b8988aaa623a6485..e00d51e06f66e39e45a0d5deffb46a8e1c585f02 100644 (file)
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -228,9 +228,9 @@ X509_LOOKUP_ctrl_with_libctx((x), X509_L_ADD_STORE, (name), 0, NULL,           \
 # define X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER     85
 # 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
 
 /* Certificate verify flags */
-
 # ifndef OPENSSL_NO_DEPRECATED_1_1_0
 #  define X509_V_FLAG_CB_ISSUER_CHECK             0x0   /* Deprecated */
 # endif

diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index 24f5a361d074abafad6226489ee4e8db140351fb..4faca1a2ee62462f87d221987a92eee938ec1ba4 100644 (file)
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -364,8 +364,7 @@ struct ISSUING_DIST_POINT_st {
 # define EXFLAG_NSCERT           0x8
 
 # define EXFLAG_CA               0x10
-/* Really self issued not necessarily self signed */
-# define EXFLAG_SI               0x20
+# define EXFLAG_SI               0x20 /* self-issued, maybe not self-signed */
 # define EXFLAG_V1               0x40
 # define EXFLAG_INVALID          0x80
 /* EXFLAG_SET is set to indicate that some values have been precomputed */
@@ -375,8 +374,9 @@ struct ISSUING_DIST_POINT_st {
 
 # define EXFLAG_INVALID_POLICY   0x800
 # define EXFLAG_FRESHEST         0x1000
-/* Self signed */
-# define EXFLAG_SS               0x2000
+# define EXFLAG_SS               0x2000 /* cert is apparently self-signed */
+
+# define EXFLAG_BCONS_CRITICAL   0x10000
 
 # define KU_DIGITAL_SIGNATURE    0x0080
 # define KU_NON_REPUDIATION      0x0040