From a6fd25b94e0c03c1be265a4454390d7225e81129 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Wed, 8 Apr 2026 16:27:07 -0500 Subject: [PATCH 1/3] Fix partial chain verification --- src/x509_str.c | 90 ++++++++++++++++++++++- tests/api/test_ossl_x509_str.c | 127 +++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 1 deletion(-) diff --git a/src/x509_str.c b/src/x509_str.c index 90113caede2..01e97519878 100644 --- 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 index 2e1821c068e..9acc2cfee69 100644 --- 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 From 2b503dae543d09c63a08b1e24238e0e9ce1ac6c5 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Tue, 14 Apr 2026 07:41:30 -0500 Subject: [PATCH 2/3] Fix from review --- tests/api/test_ossl_x509_str.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/api/test_ossl_x509_str.c b/tests/api/test_ossl_x509_str.c index 9acc2cfee69..b0756f8bbbc 100644 --- a/tests/api/test_ossl_x509_str.c +++ b/tests/api/test_ossl_x509_str.c @@ -854,6 +854,7 @@ static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_mixed( 1); /* Must verify: chain terminates at trusted intermediate in the store. */ ExpectIntEQ(X509_verify_cert(ctx), 1); + ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_OK); X509_STORE_CTX_free(ctx); X509_STORE_free(store); @@ -899,6 +900,8 @@ static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_untrusted_terminal( /* 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); + 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); From c873f3f77da8273e160612468f08892b2ba0ed99 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Tue, 14 Apr 2026 07:58:43 -0500 Subject: [PATCH 3/3] Fix from review --- src/x509_str.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/x509_str.c b/src/x509_str.c index 01e97519878..67e3fcae61b 100644 --- a/src/x509_str.c +++ b/src/x509_str.c @@ -765,6 +765,10 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) X509StoreCertIsTrusted(ctx->store, ctx->current_cert, origTrustedSk)) { wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert); + /* Clear error set by the failed X509StoreVerifyCert + * attempt; the partial-chain fallback accepted the + * chain at a caller-trusted certificate. */ + ctx->error = 0; ret = WOLFSSL_SUCCESS; } else { X509VerifyCertSetupRetry(ctx, certs, failedCerts,