Merge pull request #8692 from kareem-wolfssl/zd19563_verify

Update wolfSSL_X509_verify_cert to retry all certs until a valid chain is found.

Author: douzzer

certs/intermediate/ca-ecc-bad-aki.der

@@ -0,0 +1,67 @@
+Certificate:
+    Data:
+        Version: 3 (0x2)
+        Serial Number: 4113 (0x1011)
+        Signature Algorithm: sha256WithRSAEncryption
+        Issuer: C = US, ST = Washington, L = Seattle, O = wolfSSL, OU = Development, CN = wolfSSL Intermediate CA, emailAddress = info@wolfssl.com
+        Validity
+            Not Before: Jun 18 22:52:02 2025 GMT
+            Not After : Jun 13 22:52:02 2045 GMT
+        Subject: C = US, ST = Washington, L = Seattle, O = wolfSSL, OU = Development, CN = www.wolfssl.com, emailAddress = info@wolfssl.com
+        Subject Public Key Info:
+            Public Key Algorithm: id-ecPublicKey
+                Public-Key: (256 bit)
+                pub:
+                    04:02:d3:d9:6e:d6:01:8e:45:c8:b9:90:31:e5:c0:
+                    4c:e3:9e:ad:29:38:98:ba:10:d6:e9:09:2a:80:a9:
+                    2e:17:2a:b9:8a:bf:33:83:46:e3:95:0b:e4:77:40:
+                    b5:3b:43:45:33:0f:61:53:7c:37:44:c1:cb:fc:80:
+                    ca:e8:43:ea:a7
+                ASN1 OID: prime256v1
+                NIST CURVE: P-256
+        X509v3 extensions:
+            X509v3 Subject Key Identifier: 
+                56:8E:9A:C3:F0:42:DE:18:B9:45:55:6E:F9:93:CF:EA:C3:F3:A5:21
+            X509v3 Authority Key Identifier: 
+                EF:69:E0:F7:D5:1D:E6:99:EC:DC:6D:D0:F7:E2:B9:5C:64:71:83:35
+            X509v3 Basic Constraints: critical
+                CA:TRUE, pathlen:1
+            X509v3 Key Usage: critical
+                Digital Signature, Certificate Sign, CRL Sign
+    Signature Algorithm: sha256WithRSAEncryption
+    Signature Value:
+        43:55:80:10:fb:06:b8:58:4c:02:3f:43:f7:bb:fd:46:ae:83:
+        c7:fe:d3:b9:5c:58:00:49:b1:4c:ed:17:84:14:72:02:05:93:
+        d7:87:b0:27:ff:bf:8a:50:50:26:41:b5:6b:83:8e:eb:46:ab:
+        bb:da:f8:42:b2:df:3c:41:54:11:18:09:1c:a6:6e:63:56:be:
+        7a:20:0d:08:d2:c0:25:ce:a4:d0:3d:09:02:fb:7b:41:59:49:
+        b5:e1:f7:72:84:b4:c7:10:c8:a0:07:64:73:6b:80:06:7a:31:
+        62:ad:49:92:53:ef:d7:d6:b4:89:9c:15:20:a5:c4:ed:c0:39:
+        7c:68:f2:19:e0:cf:e5:bb:5a:16:10:d5:de:80:da:0f:0e:91:
+        0b:39:73:d6:a7:73:b2:b6:2b:c6:fb:bc:33:e6:fd:d9:1c:dc:
+        48:3d:1e:8b:6b:9f:8f:60:26:69:53:3b:17:ed:62:bd:34:ab:
+        8c:e4:4c:17:f4:c3:bc:81:63:ad:67:c1:5d:e3:72:ac:a5:8a:
+        bc:6f:0c:2e:33:81:81:92:20:d4:4b:e0:a3:22:12:d6:b4:27:
+        1f:37:14:a2:c4:76:c0:3c:29:44:4d:a9:35:67:21:1d:11:7f:
+        76:98:02:f7:5a:f9:05:cb:2d:3b:39:45:e9:9d:82:9a:20:b0:
+        c6:56:1c:d4
+-----BEGIN CERTIFICATE-----
+MIIDTzCCAjegAwIBAgICEBEwDQYJKoZIhvcNAQELBQAwgZ8xCzAJBgNVBAYTAlVT
+MRMwEQYDVQQIDApXYXNoaW5ndG9uMRAwDgYDVQQHDAdTZWF0dGxlMRAwDgYDVQQK
+DAd3b2xmU1NMMRQwEgYDVQQLDAtEZXZlbG9wbWVudDEgMB4GA1UEAwwXd29sZlNT
+TCBJbnRlcm1lZGlhdGUgQ0ExHzAdBgkqhkiG9w0BCQEWEGluZm9Ad29sZnNzbC5j
+b20wHhcNMjUwNjE4MjI1MjAyWhcNNDUwNjEzMjI1MjAyWjCBlzELMAkGA1UEBhMC
+VVMxEzARBgNVBAgMCldhc2hpbmd0b24xEDAOBgNVBAcMB1NlYXR0bGUxEDAOBgNV
+BAoMB3dvbGZTU0wxFDASBgNVBAsMC0RldmVsb3BtZW50MRgwFgYDVQQDDA93d3cu
+d29sZnNzbC5jb20xHzAdBgkqhkiG9w0BCQEWEGluZm9Ad29sZnNzbC5jb20wWTAT
+BgcqhkjOPQIBBggqhkjOPQMBBwNCAAQC09lu1gGORci5kDHlwEzjnq0pOJi6ENbp
+CSqAqS4XKrmKvzODRuOVC+R3QLU7Q0UzD2FTfDdEwcv8gMroQ+qno2YwZDAdBgNV
+HQ4EFgQUVo6aw/BC3hi5RVVu+ZPP6sPzpSEwHwYDVR0jBBgwFoAU72ng99Ud5pns
+3G3Q9+K5XGRxgzUwEgYDVR0TAQH/BAgwBgEB/wIBATAOBgNVHQ8BAf8EBAMCAYYw
+DQYJKoZIhvcNAQELBQADggEBAENVgBD7BrhYTAI/Q/e7/Uaug8f+07lcWABJsUzt
+F4QUcgIFk9eHsCf/v4pQUCZBtWuDjutGq7va+EKy3zxBVBEYCRymbmNWvnogDQjS
+wCXOpNA9CQL7e0FZSbXh93KEtMcQyKAHZHNrgAZ6MWKtSZJT79fWtImcFSClxO3A
+OXxo8hngz+W7WhYQ1d6A2g8OkQs5c9anc7K2K8b7vDPm/dkc3Eg9Hotrn49gJmlT
+OxftYr00q4zkTBf0w7yBY61nwV3jcqylirxvDC4zgYGSINRL4KMiEta0Jx83FKLE
+dsA8KURNqTVnIR0Rf3aYAvda+QXLLTs5RemdgpogsMZWHNQ=
+-----END CERTIFICATE-----

certs/intermediate/ca-ecc-bad-aki.pem

@@ -313,6 +313,9 @@ create_cert wolfssl_int2_ecc wolfssl_int2_ecc ./certs/ecc-key.pem server-int-ecc
 echo "Create ECC Client Certificate signed by intermediate2"
 create_cert wolfssl_int2_ecc wolfssl_int2_ecc ./certs/ecc-client-key.pem client-int-ecc-cert usr_cert "wolfSSL Client Chain ECC" 3650
 
+echo "Create alt CA with intentionally invalid AKI"
+create_cert wolfssl_root_ecc wolfssl_int ./certs/ca-ecc-key.pem ca-ecc-bad-aki v3_intermediate_ca "www.wolfssl.com" 7300
+
 echo "Generate CRLs for new certificates"
 openssl ca -config ./certs/intermediate/wolfssl_root_ecc.cnf -gencrl -crldays 1000 -out ./certs/crl/ca-int-ecc.pem -keyfile ./certs/intermediate/ca-int-ecc-key.pem -cert ./certs/intermediate/ca-int-ecc-cert.pem
 check_result $?

certs/intermediate/genintcerts.sh

@@ -4,6 +4,8 @@
 
 EXTRA_DIST += \
 	     certs/intermediate/genintcerts.sh \
+	     certs/intermediate/ca-ecc-bad-aki.der \
+	     certs/intermediate/ca-ecc-bad-aki.pem \
 	     certs/intermediate/ca-int-cert.der \
 	     certs/intermediate/ca-int-cert.pem \
 	     certs/intermediate/ca-int-ecc-cert.der \

certs/intermediate/include.am

@@ -6141,6 +6141,55 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify)
     return ret == 0 ? WOLFSSL_SUCCESS : ret;
 }
 
+/* Removes the CA with the passed in subject hash from the
+   cert manager's CA cert store. */
+int RemoveCA(WOLFSSL_CERT_MANAGER* cm, byte* hash, int type)
+{
+    Signer* current;
+    Signer** prev;
+    int     ret = WC_NO_ERR_TRACE(WOLFSSL_FAILURE);
+    word32  row;
+
+    WOLFSSL_MSG("Removing a CA");
+
+    if (cm == NULL || hash == NULL) {
+        return BAD_FUNC_ARG;
+    }
+
+    row = HashSigner(hash);
+
+    if (wc_LockMutex(&cm->caLock) != 0) {
+        return BAD_MUTEX_E;
+    }
+    current = cm->caTable[row];
+    prev = &cm->caTable[row];
+    while (current) {
+        byte* subjectHash;
+
+    #ifndef NO_SKID
+        subjectHash = current->subjectKeyIdHash;
+    #else
+        subjectHash = current->subjectNameHash;
+    #endif
+
+        if ((current->type == type) &&
+            (XMEMCMP(hash, subjectHash, SIGNER_DIGEST_SIZE) == 0)) {
+            *prev = current->next;
+            FreeSigner(current, cm->heap);
+            ret = WOLFSSL_SUCCESS;
+            break;
+        }
+        prev = &current->next;
+        current = current->next;
+    }
+    wc_UnLockMutex(&cm->caLock);
+
+    WOLFSSL_LEAVE("RemoveCA", ret);
+
+    return ret;
+}
+
+
 /* Sets the CA with the passed in subject hash
    to the provided type. */
 int SetCAType(WOLFSSL_CERT_MANAGER* cm, byte* hash, int type)

src/ssl.c

@@ -404,6 +404,64 @@ static int addAllButSelfSigned(WOLF_STACK_OF(WOLFSSL_X509)*to,
     return ret;
 }
 
+static int X509StoreRemoveCa(WOLFSSL_X509_STORE* store,
+                                            WOLFSSL_X509* x509, int type) {
+    int result = WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR);
+    byte          hash[KEYID_SIZE];
+
+    if (store != NULL && x509 != NULL && x509->derCert != NULL) {
+        result = GetHashId(x509->subjKeyId, (int)x509->subjKeyIdSz,
+                    hash, HashIdAlg(x509->sigOID));
+        if (result) {
+            result = WOLFSSL_FATAL_ERROR;
+        } else {
+            result = RemoveCA(store->cm, hash, type);
+        }
+    }
+
+    return result;
+}
+
+static int X509StoreMoveCert(WOLFSSL_STACK *certs_stack,
+                             WOLFSSL_STACK *dest_stack,
+                             WOLFSSL_X509 *cert) {
+    int i;
+
+    if (certs_stack == NULL || dest_stack == NULL || cert == NULL)
+        return WOLFSSL_FATAL_ERROR;
+
+    for (i = 0; i < wolfSSL_sk_X509_num(certs_stack); i++) {
+        if (wolfSSL_sk_X509_value(certs_stack, i) == cert) {
+            wolfSSL_sk_X509_push(dest_stack,
+                                 (WOLFSSL_X509*)wolfSSL_sk_pop_node(certs_stack, i));
+            return WOLFSSL_SUCCESS;
+        }
+    }
+
+    return WOLFSSL_FAILURE;
+}
+
+
+/* Current certificate failed, but it is possible there is an
+ * alternative cert with the same subject key which will work.
+ * Retry until all possible candidate certs are exhausted. */
+static int X509VerifyCertSetupRetry(WOLFSSL_X509_STORE_CTX* ctx,
+    WOLF_STACK_OF(WOLFSSL_X509)* certs, WOLF_STACK_OF(WOLFSSL_X509)* failed,
+    int* depth, int origDepth) {
+    int ret = WC_NO_ERR_TRACE(WOLFSSL_FAILURE);
+
+    WOLFSSL_MSG("X509_verify_cert current cert failed, "
+                "retrying with other certs.");
+    ret = X509StoreRemoveCa(ctx->store, ctx->current_cert,
+                            WOLFSSL_TEMP_CA);
+    X509StoreMoveCert(certs, failed, ctx->current_cert);
+    ctx->current_cert = wolfSSL_sk_X509_pop(ctx->chain);
+    if (*depth < origDepth)
+        *depth += 1;
+
+    return ret;
+}
+
 /* Verifies certificate chain using WOLFSSL_X509_STORE_CTX
  * returns 1 on success or <= 0 on failure.
  */
@@ -414,11 +472,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     int added = 0;
     int i = 0;
     int numInterAdd = 0;
+    int numFailedCerts = 0;
     int depth = 0;
+    int origDepth = 0;
     WOLFSSL_X509 *issuer = NULL;
     WOLFSSL_X509 *orig = NULL;
     WOLF_STACK_OF(WOLFSSL_X509)* certs = NULL;
     WOLF_STACK_OF(WOLFSSL_X509)* certsToUse = NULL;
+    WOLF_STACK_OF(WOLFSSL_X509)* failedCerts = NULL;
     WOLFSSL_ENTER("wolfSSL_X509_verify_cert");
 
     if (ctx == NULL || ctx->store == NULL || ctx->store->cm == NULL
@@ -427,6 +488,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     }
 
     certs = ctx->store->certs;
+
     if (ctx->setTrustedSk != NULL) {
         certs = ctx->setTrustedSk;
     }
@@ -450,6 +512,10 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     }
     ctx->chain = wolfSSL_sk_X509_new_null();
 
+    failedCerts = wolfSSL_sk_X509_new_null();
+    if (!failedCerts)
+        return WOLFSSL_FATAL_ERROR;
+
     if (ctx->depth > 0) {
         depth = ctx->depth + 1;
     }
@@ -458,6 +524,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     }
 
     orig = ctx->current_cert;
+    origDepth = depth;
     while(done == 0 && depth > 0) {
         issuer = NULL;
 
@@ -486,23 +553,28 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
                         goto exit;
                     }
                 }
-            } else {
+            } else
         #endif
-            ret = X509StoreAddCa(ctx->store, issuer,
-                                            WOLFSSL_TEMP_CA);
-            if (ret != WOLFSSL_SUCCESS) {
-                goto exit;
-            }
-            added = 1;
-            ret = X509StoreVerifyCert(ctx);
-            if (ret != WOLFSSL_SUCCESS) {
-                goto exit;
-            }
-            /* Add it to the current chain and look at the issuer cert next */
-            wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert);
-        #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT)
+            {
+                ret = X509StoreAddCa(ctx->store, issuer,
+                                                WOLFSSL_TEMP_CA);
+                if (ret != WOLFSSL_SUCCESS) {
+                    X509VerifyCertSetupRetry(ctx, certs, failedCerts,
+                        &depth, origDepth);
+                    continue;
+                }
+                added = 1;
+                ret = X509StoreVerifyCert(ctx);
+                if (ret != WOLFSSL_SUCCESS) {
+                    if ((origDepth - depth) <= 1)
+                        added = 0;
+                    X509VerifyCertSetupRetry(ctx, certs, failedCerts,
+                        &depth, origDepth);
+                    continue;
+                }
+                /* Add it to the current chain and look at the issuer cert next */
+                wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert);
             }
-        #endif
             ctx->current_cert = issuer;
         }
         else if (ret == WC_NO_ERR_TRACE(WOLFSSL_FAILURE)) {
@@ -515,8 +587,11 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
                     (added == 1)) {
                     wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert);
                     ret = WOLFSSL_SUCCESS;
+                } else {
+                    X509VerifyCertSetupRetry(ctx, certs, failedCerts,
+                        &depth, origDepth);
+                    continue;
                 }
-                goto exit;
             }
 
             /* Cert verified, finish building the chain */
@@ -551,6 +626,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     }
 
 exit:
+    /* Copy back failed certs. */
+    numFailedCerts = wolfSSL_sk_X509_num(failedCerts);
+    for (i = 0; i < numFailedCerts; i++)
+    {
+        wolfSSL_sk_X509_push(certs, wolfSSL_sk_X509_pop(failedCerts));
+    }
+    wolfSSL_sk_X509_pop_free(failedCerts, NULL);
+
     /* Remove additional intermediates from init from the store */
     if (ctx != NULL && numInterAdd > 0) {
         for (i = 0; i < numInterAdd; i++) {
@@ -1390,7 +1473,6 @@ static int X509StoreAddCa(WOLFSSL_X509_STORE* store,
     return result;
 }
 
-
 int wolfSSL_X509_STORE_add_cert(WOLFSSL_X509_STORE* store, WOLFSSL_X509* x509)
 {
     int result = WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR);

src/x509_str.c

@@ -20546,6 +20546,50 @@ static int test_wolfSSL_X509_STORE_CTX_ex11(X509_STORE_test_data *testData)
     X509_STORE_free(store);
     return EXPECT_RESULT();
 }
+
+static int test_wolfSSL_X509_STORE_CTX_ex12(void)
+{
+    EXPECT_DECLS;
+#ifdef HAVE_ECC
+    X509_STORE* store = NULL;
+    X509_STORE_CTX* ctx = NULL;
+    STACK_OF(X509)* chain = NULL;
+    X509* rootEccX509 = NULL;
+    X509* badAkiX509 = NULL;
+    X509* ca1X509 = NULL;
+
+    const char* intCARootECCFile    = "./certs/ca-ecc-cert.pem";
+    const char* intCA1ECCFile       = "./certs/intermediate/ca-int-ecc-cert.pem";
+    const char* intCABadAKIECCFile = "./certs/intermediate/ca-ecc-bad-aki.pem";
+
+    /* Test case 12, multiple CAs with the same SKI including 1 with intentionally
+       bad/unregistered AKI.  x509_verify_cert should still form a valid chain
+       using the valid CA, ignoring the bad CA. Developed from customer provided
+       reproducer. */
+
+    ExpectNotNull(store = X509_STORE_new());
+    ExpectNotNull(rootEccX509 = test_wolfSSL_X509_STORE_CTX_ex_helper(intCARootECCFile));
+    ExpectIntEQ(X509_STORE_add_cert(store, rootEccX509), 1);
+    ExpectNotNull(badAkiX509 = test_wolfSSL_X509_STORE_CTX_ex_helper(intCABadAKIECCFile));
+    ExpectNotNull(ctx = X509_STORE_CTX_new());
+    ExpectIntEQ(X509_STORE_CTX_init(ctx, store, badAkiX509, NULL), 1);
+    ExpectIntEQ(X509_verify_cert(ctx), 0);
+    X509_STORE_CTX_cleanup(ctx);
+
+    ExpectIntEQ(X509_STORE_add_cert(store, badAkiX509), 1);
+    ExpectNotNull(ca1X509 = test_wolfSSL_X509_STORE_CTX_ex_helper(intCA1ECCFile));
+    ExpectIntEQ(X509_STORE_CTX_init(ctx, store, ca1X509, NULL), 1);
+    ExpectIntEQ(X509_verify_cert(ctx), 1);
+    ExpectNotNull(chain = X509_STORE_CTX_get_chain(ctx));
+
+    X509_STORE_CTX_free(ctx);
+    X509_STORE_free(store);
+    X509_free(rootEccX509);
+    X509_free(badAkiX509);
+    X509_free(ca1X509);
+#endif
+    return EXPECT_RESULT();
+}
 #endif
 
 static int test_wolfSSL_X509_STORE_CTX_ex(void)
@@ -20585,6 +20629,7 @@ static int test_wolfSSL_X509_STORE_CTX_ex(void)
     ExpectIntEQ(test_wolfSSL_X509_STORE_CTX_ex9(&testData), 1);
     ExpectIntEQ(test_wolfSSL_X509_STORE_CTX_ex10(&testData), 1);
     ExpectIntEQ(test_wolfSSL_X509_STORE_CTX_ex11(&testData), 1);
+    test_wolfSSL_X509_STORE_CTX_ex12();
 
     if(testData.x509Ca) {
         X509_free(testData.x509Ca);

tests/api.c

@@ -14375,7 +14375,7 @@ int CalcHashId_ex(const byte* data, word32 len, byte* hash, int hashAlg)
  * @return  0 on success.
  * @return  MEMORY_E when dynamic memory allocation fails.
  */
-static int GetHashId(const byte* id, int length, byte* hash, int hashAlg)
+int GetHashId(const byte* id, int length, byte* hash, int hashAlg)
 {
     int ret;