Fix partial chain verification

embhorn · embhorn · commit a6fd25b94e0c · 2026-04-14T07:25:11.000-05:00
diff --git a/src/x509_str.c b/src/x509_str.c
@@ -552,6 +552,53 @@ static int X509VerifyCertSetupRetry(WOLFSSL_X509_STORE_CTX* ctx,
     return ret;
 }
 
+/* Returns 1 if cur and x509 have identical DER encodings, 0 otherwise. */
+static int X509DerEquals(WOLFSSL_X509* cur, WOLFSSL_X509* x509)
+{
+    if (cur == NULL || cur->derCert == NULL ||
+        x509 == NULL || x509->derCert == NULL) {
+        return 0;
+    }
+    if (cur->derCert->length != x509->derCert->length)
+        return 0;
+    return XMEMCMP(cur->derCert->buffer, x509->derCert->buffer,
+                   x509->derCert->length) == 0;
+}
+
+/* Returns 1 if x509's DER matches an entry in either origTrustedSk (an
+ * immutable snapshot of the caller's trusted set captured before any
+ * intermediates were injected for this verification call) or in
+ * store->trusted.  Returns 0 otherwise.  Used by the
+ * X509_V_FLAG_PARTIAL_CHAIN fallback to confirm that a chain actually
+ * terminates at a caller-trusted certificate. */
+static int X509StoreCertIsTrusted(WOLFSSL_X509_STORE* store,
+        WOLFSSL_X509* x509, WOLF_STACK_OF(WOLFSSL_X509)* origTrustedSk)
+{
+    int i;
+    int n;
+
+    if (x509 == NULL || x509->derCert == NULL)
+        return 0;
+
+    if (origTrustedSk != NULL) {
+        n = wolfSSL_sk_X509_num(origTrustedSk);
+        for (i = 0; i < n; i++) {
+            if (X509DerEquals(wolfSSL_sk_X509_value(origTrustedSk, i), x509))
+                return 1;
+        }
+    }
+
+    if (store != NULL && store->trusted != NULL) {
+        n = wolfSSL_sk_X509_num(store->trusted);
+        for (i = 0; i < n; i++) {
+            if (X509DerEquals(wolfSSL_sk_X509_value(store->trusted, i), x509))
+                return 1;
+        }
+    }
+
+    return 0;
+}
+
 /* Verifies certificate chain using WOLFSSL_X509_STORE_CTX
  * returns 1 on success or <= 0 on failure.
  */
@@ -570,6 +617,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     WOLF_STACK_OF(WOLFSSL_X509)* certs = NULL;
     WOLF_STACK_OF(WOLFSSL_X509)* certsToUse = NULL;
     WOLF_STACK_OF(WOLFSSL_X509)* failedCerts = NULL;
+    WOLF_STACK_OF(WOLFSSL_X509)* origTrustedSk = NULL;
     WOLFSSL_ENTER("wolfSSL_X509_verify_cert");
 
     if (ctx == NULL || ctx->store == NULL || ctx->store->cm == NULL
@@ -586,9 +634,37 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     if (certs == NULL &&
         wolfSSL_sk_X509_num(ctx->ctxIntermediates) > 0) {
         certsToUse = wolfSSL_sk_X509_new_null();
+        if (certsToUse == NULL) {
+            ret = WOLFSSL_FAILURE;
+            goto exit;
+        }
         ret = addAllButSelfSigned(certsToUse, ctx->ctxIntermediates, NULL);
+        /* certsToUse holds only injected intermediates, none are trusted, so
+         * leave origTrustedSk NULL (empty snapshot). */
+        certs = certsToUse;
     }
     else {
+        /* Snapshot the caller-trusted entries before injecting the
+         * caller-supplied untrusted intermediates.  Only the entries already
+         * present count as trusted for the partial-chain check below, and
+         * we need a stable reference because X509VerifyCertSetupRetry may
+         * remove nodes from `certs` during chain building. */
+        if (certs != NULL && wolfSSL_sk_X509_num(certs) > 0) {
+            int j;
+            int n = wolfSSL_sk_X509_num(certs);
+            origTrustedSk = wolfSSL_sk_X509_new_null();
+            if (origTrustedSk == NULL) {
+                ret = WOLFSSL_FAILURE;
+                goto exit;
+            }
+            for (j = 0; j < n; j++) {
+                if (wolfSSL_sk_X509_push(origTrustedSk,
+                        wolfSSL_sk_X509_value(certs, j)) <= 0) {
+                    ret = WOLFSSL_FAILURE;
+                    goto exit;
+                }
+            }
+        }
         /* Add the intermediates provided on init to the list of untrusted
          * intermediates to be used */
         ret = addAllButSelfSigned(certs, ctx->ctxIntermediates, &numInterAdd);
@@ -677,9 +753,17 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
              * a trusted CA in the CM */
             ret = X509StoreVerifyCert(ctx);
             if (ret != WOLFSSL_SUCCESS) {
+                /* WOLFSSL_PARTIAL_CHAIN may only terminate the chain at a
+                 * certificate the caller actually trusts.  The previous
+                 * "added == 1" guard merely confirmed that some untrusted
+                 * intermediate had been temporarily loaded into the
+                 * CertManager during chain building, which would accept
+                 * chains that never reach a trust anchor.  Verify that
+                 * ctx->current_cert is itself in the original trust set. */
                 if (((ctx->flags & WOLFSSL_PARTIAL_CHAIN) ||
                      (ctx->store->param->flags & WOLFSSL_PARTIAL_CHAIN)) &&
-                    (added == 1)) {
+                    X509StoreCertIsTrusted(ctx->store, ctx->current_cert,
+                        origTrustedSk)) {
                     wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert);
                     ret = WOLFSSL_SUCCESS;
                 } else {
@@ -749,6 +833,10 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
     if (certsToUse != NULL) {
         wolfSSL_sk_X509_free(certsToUse);
     }
+    if (origTrustedSk != NULL) {
+        /* Shallow free: only the snapshot's stack nodes, not the X509s. */
+        wolfSSL_sk_X509_free(origTrustedSk);
+    }
 
     /* Enforce hostname / IP verification from X509_VERIFY_PARAM if set.
      * Always check against the leaf (end-entity) certificate, captured in
diff --git a/tests/api/test_ossl_x509_str.c b/tests/api/test_ossl_x509_str.c
@@ -785,6 +785,127 @@ static int test_wolfSSL_X509_STORE_CTX_ex11(X509_STORE_test_data *testData)
     return EXPECT_RESULT();
 }
 
+static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_neg(
+    X509_STORE_test_data *testData)
+{
+    EXPECT_DECLS;
+    X509_STORE* store = NULL;
+    X509_STORE_CTX* ctx = NULL;
+    STACK_OF(X509)* untrusted = NULL;
+
+    /* Negative partial-chain test: with X509_V_FLAG_PARTIAL_CHAIN set, the
+     * intermediates are supplied ONLY as untrusted (passed through the
+     * X509_STORE_CTX_init "chain" argument and never added to the store).
+     * No certificate in the chain is in the store, so verification must
+     * fail.  Pre-fix, wolfSSL_X509_verify_cert would incorrectly accept
+     * this chain because its partial-chain fallback only checked that some
+     * intermediate had been temporarily loaded into the CertManager, not
+     * that any chain certificate was actually trusted. */
+    ExpectNotNull(store = X509_STORE_new());
+    /* Intentionally do NOT add x509CaInt, x509CaInt2, or x509Ca. */
+    ExpectIntEQ(X509_STORE_set_flags(store, X509_V_FLAG_PARTIAL_CHAIN), 1);
+
+    ExpectNotNull(untrusted = sk_X509_new_null());
+    ExpectIntGT(sk_X509_push(untrusted, testData->x509CaInt2), 0);
+    ExpectIntGT(sk_X509_push(untrusted, testData->x509CaInt), 0);
+
+    ExpectNotNull(ctx = X509_STORE_CTX_new());
+    ExpectIntEQ(X509_STORE_CTX_init(ctx, store, testData->x509Leaf, untrusted),
+        1);
+    /* Must NOT verify: partial-chain does not relax the trust requirement. */
+    ExpectIntNE(X509_verify_cert(ctx), 1);
+    /* Verify the failure is specifically due to missing trust anchor, not
+     * some unrelated error. */
+    ExpectIntEQ(X509_STORE_CTX_get_error(ctx),
+        X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY);
+
+    X509_STORE_CTX_free(ctx);
+    X509_STORE_free(store);
+    sk_X509_free(untrusted);
+    return EXPECT_RESULT();
+}
+
+static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_mixed(
+    X509_STORE_test_data *testData)
+{
+    EXPECT_DECLS;
+    X509_STORE* store = NULL;
+    X509_STORE_CTX* ctx = NULL;
+    STACK_OF(X509)* untrusted = NULL;
+
+    /* Mixed trusted-store + untrusted-chain partial-chain test: the store
+     * trusts an intermediate (x509CaInt2, the leaf's direct issuer), while
+     * an additional intermediate (x509CaInt) is supplied only as untrusted
+     * via the chain argument.  With X509_V_FLAG_PARTIAL_CHAIN, verification
+     * must succeed by terminating at the trusted intermediate.  This test
+     * exercises the snapshot-based trust check in X509StoreCertIsTrusted:
+     * the untrusted intermediate injected during verification must not be
+     * treated as a trust anchor, but the intermediate already in the store
+     * must be. */
+    ExpectNotNull(store = X509_STORE_new());
+    ExpectIntEQ(X509_STORE_add_cert(store, testData->x509CaInt2), 1);
+    ExpectIntEQ(X509_STORE_set_flags(store, X509_V_FLAG_PARTIAL_CHAIN), 1);
+
+    ExpectNotNull(untrusted = sk_X509_new_null());
+    ExpectIntGT(sk_X509_push(untrusted, testData->x509CaInt), 0);
+
+    ExpectNotNull(ctx = X509_STORE_CTX_new());
+    ExpectIntEQ(X509_STORE_CTX_init(ctx, store, testData->x509Leaf, untrusted),
+        1);
+    /* Must verify: chain terminates at trusted intermediate in the store. */
+    ExpectIntEQ(X509_verify_cert(ctx), 1);
+
+    X509_STORE_CTX_free(ctx);
+    X509_STORE_free(store);
+    sk_X509_free(untrusted);
+    return EXPECT_RESULT();
+}
+
+static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_untrusted_terminal(
+    X509_STORE_test_data *testData)
+{
+    EXPECT_DECLS;
+    X509_STORE* store = NULL;
+    X509_STORE_CTX* ctx = NULL;
+    STACK_OF(X509)* untrusted = NULL;
+
+    /* Partial-chain boundary test: the store trusts a CA (x509Ca) that is
+     * NOT reachable from the leaf given the supplied untrusted intermediates,
+     * and an untrusted intermediate (x509CaInt2) IS the terminal of the
+     * (truncated) chain.  With X509_V_FLAG_PARTIAL_CHAIN set, verification
+     * must FAIL because the chain terminates at an untrusted certificate.
+     *
+     * This test specifically targets the snapshot-based trust check in
+     * X509StoreCertIsTrusted.  Before addAllButSelfSigned injects
+     * x509CaInt2, origTrustedSk is snapshotted from the caller-trusted set
+     * and contains only x509Ca.  When the chain terminates at x509CaInt2,
+     * the trust check consults origTrustedSk (not the mutated working
+     * stack) and correctly finds no match.  A regression that consulted
+     * the post-injection working stack instead of the snapshot would
+     * incorrectly mark x509CaInt2 as trusted and cause verification to
+     * succeed. */
+    ExpectNotNull(store = X509_STORE_new());
+    ExpectIntEQ(X509_STORE_add_cert(store, testData->x509Ca), 1);
+    ExpectIntEQ(X509_STORE_set_flags(store, X509_V_FLAG_PARTIAL_CHAIN), 1);
+
+    /* Only x509CaInt2 supplied as untrusted; x509CaInt is intentionally
+     * withheld so the chain cannot actually reach the trusted x509Ca. */
+    ExpectNotNull(untrusted = sk_X509_new_null());
+    ExpectIntGT(sk_X509_push(untrusted, testData->x509CaInt2), 0);
+
+    ExpectNotNull(ctx = X509_STORE_CTX_new());
+    ExpectIntEQ(X509_STORE_CTX_init(ctx, store, testData->x509Leaf, untrusted),
+        1);
+    /* Must NOT verify: the chain terminal (x509CaInt2) is not in the
+     * original trust set, even though the store is non-empty. */
+    ExpectIntNE(X509_verify_cert(ctx), 1);
+
+    X509_STORE_CTX_free(ctx);
+    X509_STORE_free(store);
+    sk_X509_free(untrusted);
+    return EXPECT_RESULT();
+}
+
 #ifdef HAVE_ECC
 static int test_wolfSSL_X509_STORE_CTX_ex12(void)
 {
@@ -870,6 +991,12 @@ 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);
+    ExpectIntEQ(test_wolfSSL_X509_STORE_CTX_ex_partial_chain_neg(&testData), 1);
+    ExpectIntEQ(test_wolfSSL_X509_STORE_CTX_ex_partial_chain_mixed(&testData),
+        1);
+    ExpectIntEQ(
+        test_wolfSSL_X509_STORE_CTX_ex_partial_chain_untrusted_terminal(
+            &testData), 1);
 #ifdef HAVE_ECC
     ExpectIntEQ(test_wolfSSL_X509_STORE_CTX_ex12(), 1);
 #endif
