From: Tomas Mraz <tomas@openssl.org>
Date: Mon, 22 Mar 2021 08:51:52 +0000 (+0000)
Subject: check_chain_extensions: Do not override error return value by check_curve
X-Git-Tag: OpenSSL_1_1_1k~7
X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=2a40b7bc7b94dd7de897a74571e7024f0cf0d63b

check_chain_extensions: Do not override error return value by check_curve

The X509_V_FLAG_X509_STRICT flag enables additional security checks of the
certificates present in a certificate chain. It is not set by default.

Starting from OpenSSL version 1.1.1h a check to disallow certificates with
explicitly encoded elliptic curve parameters in the chain was added to the
strict checks.

An error in the implementation of this check meant that the result of a
previous check to confirm that certificates in the chain are valid CA
certificates was overwritten. This effectively bypasses the check
that non-CA certificates must not be able to issue other certificates.

If a "purpose" has been configured then a subsequent check that the
certificate is consistent with that purpose also checks that it is a
valid CA. Therefore where a purpose is set the certificate chain will
still be rejected even when the strict flag has been used. A purpose is
set by default in libssl client and server certificate verification
routines, but it can be overriden by an application.

Affected applications explicitly set the X509_V_FLAG_X509_STRICT
verification flag and either do not set a purpose for the certificate
verification or, in the case of TLS client or server applications,
override the default purpose to make it not set.

CVE-2021-3450

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
---

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 0c71b2e8b4..20a36e763c 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -524,15 +524,19 @@ static int check_chain_extensions(X509_STORE_CTX *ctx)
                 ret = 1;
             break;
         }
-        if ((ctx->param->flags & X509_V_FLAG_X509_STRICT) && num > 1) {
+        if (ret > 0
+            && (ctx->param->flags & X509_V_FLAG_X509_STRICT) && num > 1) {
             /* Check for presence of explicit elliptic curve parameters */
             ret = check_curve(x);
-            if (ret < 0)
+            if (ret < 0) {
                 ctx->error = X509_V_ERR_UNSPECIFIED;
-            else if (ret == 0)
+                ret = 0;
+            } else if (ret == 0) {
                 ctx->error = X509_V_ERR_EC_KEY_EXPLICIT_PARAMS;
+            }
         }
-        if ((x->ex_flags & EXFLAG_CA) == 0
+        if (ret > 0
+            && (x->ex_flags & EXFLAG_CA) == 0
             && x->ex_pathlen != -1
             && (ctx->param->flags & X509_V_FLAG_X509_STRICT)) {
             ctx->error = X509_V_ERR_INVALID_EXTENSION;
diff --git a/test/verify_extra_test.c b/test/verify_extra_test.c
index 010403e74a..b9959e0c66 100644
--- a/test/verify_extra_test.c
+++ b/test/verify_extra_test.c
@@ -140,10 +140,22 @@ static int test_alt_chains_cert_forgery(void)
 
     i = X509_verify_cert(sctx);
 
-    if (i == 0 && X509_STORE_CTX_get_error(sctx) == X509_V_ERR_INVALID_CA) {
+    if (i != 0 || X509_STORE_CTX_get_error(sctx) != X509_V_ERR_INVALID_CA)
+        goto err;
+
+    /* repeat with X509_V_FLAG_X509_STRICT */
+    X509_STORE_CTX_cleanup(sctx);
+    X509_STORE_set_flags(store, X509_V_FLAG_X509_STRICT);
+
+    if (!X509_STORE_CTX_init(sctx, store, x, untrusted))
+        goto err;
+
+    i = X509_verify_cert(sctx);
+
+    if (i == 0 && X509_STORE_CTX_get_error(sctx) == X509_V_ERR_INVALID_CA)
         /* This is the result we were expecting: Test passed */
         ret = 1;
-    }
+
  err:
     X509_STORE_CTX_free(sctx);
     X509_free(x);