Skip to content

Bugfix- wolf ssl add0 chain cert not incrementing certchain and causing TLS1.3 to fail #10517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Roy-Carter wants to merge 3 commits into
base: master
Choose a base branch
from
+48 −0
Open

Bugfix- wolf ssl add0 chain cert not incrementing certchain and causing TLS1.3 to fail #10517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
452622b
bugfix - in TLS 1.3 handshake we're dependent on the cert chain cnt i…
Roy-Carter May 22, 2026
f7dc5b2
Implement a simple test case to check the chain counter increases.
Roy-Carter May 22, 2026
416d317
Cosmetic - delete trailing whitespace in PR
Roy-Carter May 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ssl_load.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5218,6 +5218,7 @@ int wolfSSL_add0_chain_cert(WOLFSSL* ssl, WOLFSSL_X509* x509)
ssl->buffers.weOwnCertChain, x509->derCert->buffer,
x509->derCert->length, ssl->heap);
if (ret == 1) {
ssl->buffers.certChainCnt++;
/* We now own cert chain. */
ssl->buffers.weOwnCertChain = 1;
/* Create a stack to put certificate into. */

@dgarske dgarske Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ [Info] Increment is unconditional while CTX equivalent guards it with WOLFSSL_TLS13

The new ssl->buffers.certChainCnt++; is unconditional, whereas the analogous CTX helper wolfssl_ctx_add_to_chain (src/ssl_load.c:4968-4971) wraps ctx->certChainCnt++; in #ifdef WOLFSSL_TLS13. This is a minor stylistic inconsistency, not a bug: the certChainCnt member is always defined (under #ifndef NO_CERTS, wolfssl/internal.h:5029), so the unconditional increment always compiles, and the count is consumed both by TLS 1.3 (SendTls13Certificate) and by the non-TLS1.3 OCSP-multi path (MIN(cnt, certChainCnt + 1) at src/internal.c:26269). The unconditional form here is arguably more correct than the CTX version because it keeps the count accurate even in non-TLS1.3 builds. No change required; flagging only for awareness of the divergence between the two helpers.

Fix: Leave as-is (unconditional is fine and slightly more correct). Optionally note the divergence, or align the CTX helper to also increment unconditionally in a follow-up for consistency.

Expand Down
47 changes: 47 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3693,6 +3693,52 @@ static int test_wolfSSL_CTX_add1_chain_cert(void)
return EXPECT_RESULT();
}

static int test_wolfSSL_add0_chain_cert_increments_count(void)

@dgarske dgarske Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Test verifies the counter but not the end-to-end chain-send behavior it fixes

The new test directly asserts ssl->buffers.certChainCnt increments per chain cert, which is a good, targeted regression guard for this specific fix. It does not, however, exercise the actual symptom (a TLS 1.3 handshake emitting a full chain rather than a leaf-only Certificate message), nor the wolfSSL_add1_chain_cert path which funnels through wolfSSL_add0_chain_cert and is therefore also affected by the fix. The unit test is acceptable as-is given how much harder a full handshake assertion would be, but coverage of the add1 wrapper would be cheap and worthwhile. Also note: if SSL_add0_chain_cert ever returned 0 inside the loop, the just-loaded x509 would leak since it is unconditionally set to NULL afterward without being freed on the failure path -- only a concern under a future regression, not current behavior.

Fix: Consider adding a parallel assertion via SSL_add1_chain_cert (verifying both the count increments and the X509 ref count is bumped), and/or a comment noting the unit test stands in for the harder-to-author TLS 1.3 full-chain handshake check.

{
EXPECT_DECLS;
#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && defined(OPENSSL_EXTRA) && \
defined(KEEP_OUR_CERT) && !defined(NO_RSA) && !defined(NO_TLS) && \
!defined(NO_WOLFSSL_CLIENT)
WOLFSSL_CTX* ctx = NULL;
WOLFSSL* ssl = NULL;
const char* chainCerts[] = {
"./certs/intermediate/ca-int2-cert.pem",
"./certs/intermediate/ca-int-cert.pem",
"./certs/ca-cert.pem",
NULL
};
const char** cert;
WOLFSSL_X509* x509 = NULL;
int expectedCnt = 0;

ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()));
ExpectNotNull(ssl = wolfSSL_new(ctx));

ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(
"./certs/intermediate/client-int-cert.pem", WOLFSSL_FILETYPE_PEM));
ExpectIntEQ(SSL_add0_chain_cert(ssl, x509), 1);
/* Leaf -> ssl->buffers.certificate, not chain. certChainCnt unchanged. */
if (ssl != NULL) {
ExpectIntEQ(ssl->buffers.certChainCnt, 0);
}
x509 = NULL;
for (cert = chainCerts; EXPECT_SUCCESS() && *cert != NULL; cert++) {
ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(*cert,
WOLFSSL_FILETYPE_PEM));
ExpectIntEQ(SSL_add0_chain_cert(ssl, x509), 1);
x509 = NULL;
expectedCnt++;
if (ssl != NULL) {
ExpectIntEQ(ssl->buffers.certChainCnt, expectedCnt);
}
}

SSL_free(ssl);
SSL_CTX_free(ctx);
#endif
return EXPECT_RESULT();
}

/* Test that wolfssl_add_to_chain rejects sizes that would overflow word32.
* ZD #21241 */
static int test_wolfSSL_add_to_chain_overflow(void)
Expand Down Expand Up @@ -40634,6 +40680,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_wolfSSL_CTX_load_verify_buffer_ex),
TEST_DECL(test_wolfSSL_CTX_load_verify_chain_buffer_format),
TEST_DECL(test_wolfSSL_CTX_add1_chain_cert),
TEST_DECL(test_wolfSSL_add0_chain_cert_increments_count),
TEST_DECL(test_wolfSSL_add_to_chain_overflow),
TEST_DECL(test_wolfSSL_CTX_use_certificate_chain_buffer_format),
TEST_DECL(test_wolfSSL_CTX_use_certificate_chain_file_format),
Expand Down
Loading