From 51364a4c233383775f6f52960aca53d39776c6a3 Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 19 Aug 2026 13:11:37 -0700 Subject: [PATCH 1/4] Block cert modification in callbacks based on new flag --- ChangeLog.md | 24 ++ doc/dox_comments/header_files/ssl.h | 7 + src/internal.c | 68 +++++ src/ssl_api_cert.c | 5 + src/ssl_load.c | 27 ++ tests/api/test_tls.c | 377 ++++++++++++++++++++++++++++ tests/api/test_tls.h | 8 + wolfssl/internal.h | 11 + 8 files changed, 527 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index f3107ffb0bf..623b31937bb 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -2,6 +2,30 @@ ## Behavioral Changes +* **Behavioral change (loading a certificate or key on a context from inside a + callback)**: the sni callback set with `wolfSSL_CTX_set_servername_callback()` + and the certificate setup callback set with `wolfSSL_CTX_set_cert_cb()` run in + the middle of a handshake, at which point every session made from that context + is pointing at the context's certificate, chain and key. Replacing one of + those frees what those handshakes are reading. Such a load is now refused + while the calling thread is inside one of those callbacks, and the reason, + `BAD_STATE_E`, is left where `wolfSSL_get_error()` and the OpenSSL error queue + can report it. The calls affected are `wolfSSL_CTX_use_certificate()`, its + `_file` and `_buffer` forms, `wolfSSL_CTX_use_PrivateKey_file()`, `_buffer`, + `_Id` and `_Label`, the `wolfSSL_CTX_use_AltPrivateKey_*` pair, the + `wolfSSL_CTX_use_certificate_chain_*` family, and + `wolfSSL_CTX_add0_chain_cert()`, `wolfSSL_CTX_add1_chain_cert()` and + `wolfSSL_CTX_add_extra_chain_cert()`. An application that set a certificate + this way should set it on the session instead, with + `wolfSSL_use_certificate_file()` and its relatives, or hand the session a + different context with `wolfSSL_set_SSL_CTX()`; both are untouched, as are + loads into the trust store such as `wolfSSL_CTX_load_verify_locations()` and + anything called outside a callback. The note that a callback is running is + kept per thread, so one thread's callback does not refuse another thread's + loads; on a build without thread local storage it is shared, where overlapping + callbacks can refuse a load that would have been allowed, or allow one that + would have been refused. + * **Behavioral change (`wc_PufReadSram` health tests the raw SRAM readout)**: the raw readout is now health tested before the context accepts it, and a readout that cannot be SRAM power-on noise is rejected with `PUF_READ_E` diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index 12c1f3d051f..b5f9b6d94f4 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -5980,6 +5980,13 @@ void wolfSSL_CTX_set_client_cert_cb(WOLFSSL_CTX *ctx, client_cert_cb cb); application can inspect, set or clear certificates - for example to react to a CA list sent by the peer. + Set the certificate on the WOLFSSL object, with wolfSSL_use_certificate_file + and friends, or hand it a different context with wolfSSL_set_SSL_CTX. + Loading one on the WOLFSSL_CTX the handshake is running against is refused + from inside the callback and returns failure with BAD_STATE_E: sessions + already made from that context point at its certificate, and replacing it + would free what they are reading. + \param ctx The WOLFSSL_CTX object. \param cb The callback function for certificate setup. \param arg User argument to pass to the callback. diff --git a/src/internal.c b/src/internal.c index e32fe421eb7..dd4d35e35b3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7730,6 +7730,7 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) return ret; } + #endif /* NO_CERTS */ #ifndef NO_DH @@ -7785,6 +7786,67 @@ int CopySSL_CTX_DhParams(WOLFSSL* ssl, WOLFSSL_CTX* ctx) } #endif /* !NO_DH */ +/* Context this thread is currently inside a callback for, if any. + * + * Kept per thread rather than on the context: the context is shared between + * threads, so a flag on it would race with other handshakes and would be + * written to a context the callback may have swapped out from under us. Only + * ever compared, never followed, so a context freed during the callback does + * no harm. Where the build has no thread local storage this is one shared + * pointer, which costs the guard accuracy when threads overlap but still + * cannot corrupt anything. + */ +static THREAD_LS_T WOLFSSL_CTX* inCbCtx = NULL; + +/* Note that this thread is entering a callback on a context. + * + * @param [in] ctx SSL context object the callback belongs to. + * @return What was noted before, to hand back to CtxCallbackExit(). + */ +WOLFSSL_CTX* CtxCallbackEnter(WOLFSSL_CTX* ctx) +{ + WOLFSSL_CTX* prev = inCbCtx; + + inCbCtx = ctx; + + return prev; +} + +/* Note that this thread has left the callback. + * + * @param [in] prev What CtxCallbackEnter() handed back. + */ +void CtxCallbackExit(WOLFSSL_CTX* prev) +{ + inCbCtx = prev; +} + +#ifndef NO_CERTS +/* Refuse to replace a certificate or key on a context from its own callback. + * + * Sessions made from a context point at its buffers, so replacing one frees + * what handshakes already under way are reading. Setting a certificate on the + * session alone, or handing it a different context, is what the callbacks are + * for. + * + * @param [in] ctx SSL context object. May be NULL. + * @return 0 when the load may go ahead. + * @return BAD_STATE_E while this thread is in a callback on the context. + */ +int CheckCtxCertLoad(WOLFSSL_CTX* ctx) +{ + int ret = 0; + + if ((ctx != NULL) && (ctx == inCbCtx)) { + WOLFSSL_MSG("Certificate load refused: callback running on context"); + ret = BAD_STATE_E; + WOLFSSL_ERROR_VERBOSE(ret); + } + + return ret; +} +#endif /* !NO_CERTS */ + int SetSSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup) { int ret = WOLFSSL_SUCCESS; /* set default ret */ @@ -45638,8 +45700,14 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ], /* Stunnel supports a custom sni callback to switch an SSL's ctx * when SNI is received. Call it now if exists */ if(ssl && ssl->ctx && ssl->ctx->sniRecvCb) { + WOLFSSL_CTX* prevCbCtx; + WOLFSSL_MSG("Calling custom sni callback"); + prevCbCtx = CtxCallbackEnter(ssl->ctx); sniRet = ssl->ctx->sniRecvCb(ssl, &ad, ssl->ctx->sniRecvCbArg); + /* The callback may have switched this session to another context, + * so put back what was noted rather than reading ssl->ctx again. */ + CtxCallbackExit(prevCbCtx); switch (sniRet) { case warning_return: WOLFSSL_MSG("Error in custom sni callback. Warning alert"); diff --git a/src/ssl_api_cert.c b/src/ssl_api_cert.c index 497e4f45cb7..f81c405d6a2 100644 --- a/src/ssl_api_cert.c +++ b/src/ssl_api_cert.c @@ -2827,8 +2827,13 @@ int CertSetupCbWrapper(WOLFSSL* ssl) int ret = 0; if (ssl->ctx->certSetupCb != NULL) { + WOLFSSL_CTX* prevCbCtx; + WOLFSSL_MSG("Calling user cert setup callback"); + prevCbCtx = CtxCallbackEnter(ssl->ctx); ret = ssl->ctx->certSetupCb(ssl, ssl->ctx->certSetupCbArg); + /* The callback may have switched contexts; restore what was noted. */ + CtxCallbackExit(prevCbCtx); if (ret == 1) { WOLFSSL_MSG("User cert callback returned success"); ret = 0; diff --git a/src/ssl_load.c b/src/ssl_load.c index 4d406db0786..a7097374800 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -2677,6 +2677,11 @@ int ProcessBuffer(WOLFSSL_CTX* ctx, const unsigned char* buff, long sz, if ((ret == 0) && (sz < 0)) { ret = BAD_FUNC_ARG; } + /* Sessions made from this context hold these by pointer. */ + if ((ret == 0) && (ssl == NULL) && ((type == CERT_TYPE) || + (type == PRIVATEKEY_TYPE) || (type == ALT_PRIVATEKEY_TYPE))) { + ret = CheckCtxCertLoad(ctx); + } #ifdef WOLFSSL_SMALL_STACK if (ret == 0) { @@ -4464,6 +4469,10 @@ int wolfSSL_CTX_use_PrivateKey_Id(WOLFSSL_CTX* ctx, const unsigned char* id, return 0; } + if (CheckCtxCertLoad(ctx) != 0) { + return 0; + } + /* Dispose of old private key and allocate and copy in id. */ FreeDer(&ctx->privateKey); if (AllocCopyDer(&ctx->privateKey, id, (word32)sz, PRIVATEKEY_TYPE, @@ -4542,6 +4551,10 @@ int wolfSSL_CTX_use_PrivateKey_Label(WOLFSSL_CTX* ctx, const char* label, sz = (word32)XSTRLEN(label) + 1; + if (CheckCtxCertLoad(ctx) != 0) { + return 0; + } + /* Dispose of old private key and allocate and copy in label. */ FreeDer(&ctx->privateKey); if (AllocCopyDer(&ctx->privateKey, (const byte*)label, (word32)sz, @@ -5207,6 +5220,11 @@ static int wolfssl_ctx_add_to_chain(WOLFSSL_CTX* ctx, const byte* der, { int res; + /* Sessions made from this context hold the chain by pointer. */ + if (CheckCtxCertLoad(ctx) != 0) { + return 0; + } + /* Add chain to DER buffer. */ res = wolfssl_add_to_chain(&ctx->certChain, 1, der, (word32)derSz, ctx->heap); @@ -5299,6 +5317,10 @@ int wolfSSL_CTX_use_certificate(WOLFSSL_CTX *ctx, WOLFSSL_X509 *x) res = 0; } + if ((res == 1) && (CheckCtxCertLoad(ctx) != 0)) { + res = 0; + } + if (res == 1) { /* Replace certificate buffer with one holding the new certificate. */ FreeDer(&ctx->certificate); @@ -5386,6 +5408,11 @@ int wolfSSL_CTX_add1_chain_cert(WOLFSSL_CTX* ctx, WOLFSSL_X509* x509) ret = 0; } + /* Sessions made from this context hold the chain by pointer. */ + if ((ret == 1) && (CheckCtxCertLoad(ctx) != 0)) { + ret = 0; + } + /* Check if we already have set a certificate. */ if ((ret == 1) && (ctx->certificate == NULL)) { /* Use the certificate. */ diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index c9c0c8a44b6..026443cf954 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -2426,6 +2426,383 @@ int test_tls13_session_resumption_sni_mismatch(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + (!defined(WOLFSSL_NO_TLS12) || defined(WOLFSSL_TLS13)) && \ + (defined(HAVE_SNI) || defined(WOLFSSL_CERT_SETUP_CB)) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) + +/* What the callback under test does before it returns. The CB_CERT_CTX_ ones + * each reach a different context buffer. */ +enum { + CB_CERT_NO_CHANGE = 0, /* leaves everything as it found it */ + CB_CERT_ON_SSL, /* sets a certificate on this session alone */ + CB_CERT_CTX_CERT, /* loads a certificate on the context in use */ + CB_CERT_CTX_KEY, /* loads a key on the context in use */ +#ifdef OPENSSL_EXTRA + CB_CERT_CTX_CHAIN, /* adds to the chain on the context in use */ + CB_CERT_CTX_X509, /* sets the certificate from an X509 object */ +#endif +#ifdef WOLF_PRIVATE_KEY_ID + CB_CERT_CTX_KEYID, /* points the key at an id held elsewhere */ + CB_CERT_CTX_KEYLABEL, /* points the key at a label held elsewhere */ +#endif + CB_CERT_MODE_CNT +}; + +/* Everything a case needs, handed to the callback through its user argument + * so that cases share no state. */ +typedef struct CbCertCase { + WOLFSSL_CTX* ctx; /* context the handshake is running against */ + int mode; /* one of the CB_CERT_* values */ + int called; /* how many times the callback ran */ + int loadRet; /* what the load call returned */ + const void* ctxCertBefore; + const void* ctxCertAfter; +} CbCertCase; + +/* Whether the case has the callback reach for the context. */ +static int cb_cert_touches_ctx(int mode) +{ + return (mode != CB_CERT_NO_CHANGE) && (mode != CB_CERT_ON_SSL); +} + +/* Carry out what the case asks for, recording what the load returned. + * + * @param [in] ssl SSL object the callback was called on. + * @param [in, out] test Case being run. + */ +static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) +{ + test->called++; + test->ctxCertBefore = (const void*)test->ctx->certificate; + + switch (test->mode) { + case CB_CERT_ON_SSL: + test->loadRet = wolfSSL_use_certificate_file(ssl, svrCertFile, + CERT_FILETYPE); + break; + case CB_CERT_CTX_CERT: + test->loadRet = wolfSSL_CTX_use_certificate_file(test->ctx, + svrCertFile, CERT_FILETYPE); + break; + case CB_CERT_CTX_KEY: + test->loadRet = wolfSSL_CTX_use_PrivateKey_file(test->ctx, + svrKeyFile, CERT_FILETYPE); + break; + #ifdef WOLF_PRIVATE_KEY_ID + case CB_CERT_CTX_KEYID: { + static const byte keyId[] = { 0x01, 0x02, 0x03, 0x04 }; + + test->loadRet = wolfSSL_CTX_use_PrivateKey_Id(test->ctx, keyId, + (long)sizeof(keyId), INVALID_DEVID); + break; + } + case CB_CERT_CTX_KEYLABEL: + test->loadRet = wolfSSL_CTX_use_PrivateKey_Label(test->ctx, + "a-label", INVALID_DEVID); + break; + #endif + #ifdef OPENSSL_EXTRA + case CB_CERT_CTX_X509: { + WOLFSSL_X509* x509 = wolfSSL_X509_load_certificate_file(svrCertFile, + WOLFSSL_FILETYPE_PEM); + + if (x509 != NULL) { + test->loadRet = wolfSSL_CTX_use_certificate(test->ctx, x509); + wolfSSL_X509_free(x509); + } + break; + } + case CB_CERT_CTX_CHAIN: { + WOLFSSL_X509* x509 = wolfSSL_X509_load_certificate_file(svrCertFile, + WOLFSSL_FILETYPE_PEM); + + if (x509 != NULL) { + test->loadRet = wolfSSL_CTX_add1_chain_cert(test->ctx, x509); + wolfSSL_X509_free(x509); + } + break; + } + #endif + default: + test->loadRet = WOLFSSL_SUCCESS; + break; + } + + test->ctxCertAfter = (const void*)test->ctx->certificate; +} + +#ifdef HAVE_SNI +static int sni_cb_cert_swap(WOLFSSL* ssl, int* ad, void* arg) +{ + (void)ad; + cb_cert_action(ssl, (CbCertCase*)arg); + return 0; +} +#endif + +#ifdef WOLFSSL_CERT_SETUP_CB +/* This one reports success with 1. */ +static int cert_setup_cb_cert_swap(WOLFSSL* ssl, void* arg) +{ + cb_cert_action(ssl, (CbCertCase*)arg); + return 1; +} +#endif + +/* Drive one handshake whose callback does what mode asks for. + * + * @param [in] method_c Client method to handshake with. + * @param [in] method_s Server method to handshake with. + * @param [in] useCertCb Use the certificate setup callback rather than the + * sni callback. + * @param [in] mode One of the CB_CERT_* values. + * @return TEST_SUCCESS on success. + */ +static int test_cb_cert_swap(method_provider method_c, + method_provider method_s, int useCertCb, int mode) +{ + EXPECT_DECLS; + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + CbCertCase test; +#ifdef HAVE_SNI + const char* sni = "example.com"; +#endif + + XMEMSET(&test, 0, sizeof(test)); + test.mode = mode; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + /* The server object is built here rather than by test_memio_setup so the + * chain is on the context before the session takes a pointer to it. */ + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, NULL, + method_c, method_s), 0); + ExpectIntEQ(wolfSSL_CTX_use_certificate_chain_file(ctx_s, svrCertFile), + WOLFSSL_SUCCESS); + ExpectNotNull(ssl_s = wolfSSL_new(ctx_s)); + wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx); + wolfSSL_SetIOReadCtx(ssl_s, &test_ctx); + test.ctx = ctx_s; + + if (useCertCb) { + #ifdef WOLFSSL_CERT_SETUP_CB + wolfSSL_CTX_set_cert_cb(ctx_s, cert_setup_cb_cert_swap, &test); + #endif + } + else { + #ifdef HAVE_SNI + wolfSSL_CTX_set_servername_callback(ctx_s, sni_cb_cert_swap); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, &test), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, sni, + (word16)XSTRLEN(sni)), WOLFSSL_SUCCESS); + #endif + } + + /* Refusing the load leaves the handshake with everything it needs, so it + * completes either way. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* The whole test rests on the callback having run. */ + ExpectIntGT(test.called, 0); + + if (cb_cert_touches_ctx(mode)) { + /* The load was turned away and the context kept what it had, so no + * session was left pointing at a freed buffer. */ + ExpectIntNE(test.loadRet, WOLFSSL_SUCCESS); + ExpectPtrEq(test.ctxCertAfter, test.ctxCertBefore); + } + else { + ExpectIntEQ(test.loadRet, WOLFSSL_SUCCESS); + } + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + + return EXPECT_RESULT(); +} + +/* Run every mode against whichever protocol versions are built in. */ +static int test_cb_cert_swap_modes(int useCertCb) +{ + EXPECT_DECLS; + int mode; + + for (mode = 0; mode < CB_CERT_MODE_CNT; mode++) { + #ifndef WOLFSSL_NO_TLS12 + ExpectIntEQ(test_cb_cert_swap(wolfTLSv1_2_client_method, + wolfTLSv1_2_server_method, useCertCb, mode), TEST_SUCCESS); + #endif + #ifdef WOLFSSL_TLS13 + ExpectIntEQ(test_cb_cert_swap(wolfTLSv1_3_client_method, + wolfTLSv1_3_server_method, useCertCb, mode), TEST_SUCCESS); + #endif + } + + return EXPECT_RESULT(); +} +#endif + +/* A callback cannot replace the certificate or key on the context its own + * handshake is running against. Sessions hold those buffers by pointer, so + * replacing one frees what handshakes in flight are reading. The load is + * refused, which leaves every session untouched. Setting a certificate on the + * session alone still works, as that is the session's own. + * + * This is the sni callback; the certificate setup callback is checked by + * test_cert_setup_cb_ctx_cert_swap_refused. */ +int test_sni_cb_ctx_cert_swap_refused(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + (!defined(WOLFSSL_NO_TLS12) || defined(WOLFSSL_TLS13)) && \ + defined(HAVE_SNI) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) + ExpectIntEQ(test_cb_cert_swap_modes(0), TEST_SUCCESS); +#endif + return EXPECT_RESULT(); +} + +/* The same for the certificate setup callback, the other place a handshake + * hands control to the application while holding the context's certificate. */ +int test_cert_setup_cb_ctx_cert_swap_refused(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + (!defined(WOLFSSL_NO_TLS12) || defined(WOLFSSL_TLS13)) && \ + defined(WOLFSSL_CERT_SETUP_CB) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) + ExpectIntEQ(test_cb_cert_swap_modes(1), TEST_SUCCESS); +#endif + return EXPECT_RESULT(); +} + +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \ + (defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) +/* Hands the session a different context, which is what this callback is for. */ +static int sni_cb_switch_ctx(WOLFSSL* ssl, int* ad, void* arg) +{ + (void)ad; + return (wolfSSL_set_SSL_CTX(ssl, (WOLFSSL_CTX*)arg) != NULL) ? + 0 : fatal_return; +} +#endif + +/* Switching contexts from the callback must leave neither context marked as + * being in one: the note is per thread and is put back as it was, so both + * contexts can still be loaded once the handshake is over. */ +int test_sni_cb_switch_ctx_unblocked(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \ + (defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL, *ctx_s2 = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + const char* sni = "example.com"; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + /* The context the callback hands over to, set up the same way. */ + ExpectNotNull(ctx_s2 = wolfSSL_CTX_new(wolfTLSv1_2_server_method())); + ExpectIntEQ(wolfSSL_CTX_use_certificate_file(ctx_s2, svrCertFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_use_PrivateKey_file(ctx_s2, svrKeyFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + wolfSSL_CTX_set_servername_callback(ctx_s, sni_cb_switch_ctx); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, ctx_s2), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, sni, + (word16)XSTRLEN(sni)), WOLFSSL_SUCCESS); + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* Both contexts take a load afterwards. The one the session started on + * would stay refused if the note had been left behind on it. */ + ExpectIntEQ(wolfSSL_CTX_use_certificate_file(ctx_s, svrCertFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_use_certificate_file(ctx_s2, svrCertFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + wolfSSL_CTX_free(ctx_s2); +#endif + return EXPECT_RESULT(); +} + +/* A second session on the same context must be unharmed by a callback that + * reached for the context's certificate. Detecting the swap after the fact + * only ever protected the session whose callback made the call; refusing the + * load protects the rest of them too. */ +int test_cb_ctx_cert_swap_other_session(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) + struct test_memio_ctx test_ctx1; + struct test_memio_ctx test_ctx2; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c1 = NULL, *ssl_s1 = NULL; + WOLFSSL *ssl_c2 = NULL, *ssl_s2 = NULL; + CbCertCase test; + const char* sni = "example.com"; + + XMEMSET(&test, 0, sizeof(test)); + test.mode = CB_CERT_CTX_CERT; + XMEMSET(&test_ctx1, 0, sizeof(test_ctx1)); + XMEMSET(&test_ctx2, 0, sizeof(test_ctx2)); + + ExpectIntEQ(test_memio_setup(&test_ctx1, &ctx_c, &ctx_s, &ssl_c1, &ssl_s1, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + test.ctx = ctx_s; + + /* Both sessions exist before anything is loaded, so both are holding the + * context's certificate. */ + ExpectNotNull(ssl_c2 = wolfSSL_new(ctx_c)); + wolfSSL_SetIOWriteCtx(ssl_c2, &test_ctx2); + wolfSSL_SetIOReadCtx(ssl_c2, &test_ctx2); + ExpectNotNull(ssl_s2 = wolfSSL_new(ctx_s)); + wolfSSL_SetIOWriteCtx(ssl_s2, &test_ctx2); + wolfSSL_SetIOReadCtx(ssl_s2, &test_ctx2); + + wolfSSL_CTX_set_servername_callback(ctx_s, sni_cb_cert_swap); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, &test), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c1, WOLFSSL_SNI_HOST_NAME, sni, + (word16)XSTRLEN(sni)), WOLFSSL_SUCCESS); + + ExpectIntEQ(test_memio_do_handshake(ssl_c1, ssl_s1, 10, NULL), 0); + ExpectIntGT(test.called, 0); + ExpectIntNE(test.loadRet, WOLFSSL_SUCCESS); + + /* The second session runs no callback of its own and must still find its + * certificate where it left it. */ + wolfSSL_CTX_set_servername_callback(ctx_s, NULL); + ExpectIntEQ(test_memio_do_handshake(ssl_c2, ssl_s2, 10, NULL), 0); + + wolfSSL_free(ssl_c1); + wolfSSL_free(ssl_s1); + wolfSSL_free(ssl_c2); + wolfSSL_free(ssl_s2); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + /* Regression test for the post-ALPN_Select PSK-head check. * When ALPN_Select runs before CheckPreSharedKeys (so the per-PSK * binding check has the negotiated ALPN available), TLSX_SetALPN diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 6c3fda89962..3d3fe5eec3e 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -52,6 +52,10 @@ int test_tls12_resume_ticket_decline_fallback(void); int test_tls_set_session_min_downgrade(void); int test_tls12_session_id_resumption_sni_mismatch(void); int test_tls13_session_resumption_sni_mismatch(void); +int test_sni_cb_ctx_cert_swap_refused(void); +int test_cert_setup_cb_ctx_cert_swap_refused(void); +int test_cb_ctx_cert_swap_other_session(void); +int test_sni_cb_switch_ctx_unblocked(void); int test_tls13_resumption_with_alpn(void); int test_tls12_session_id_resumption_alpn_mismatch(void); int test_tls13_session_resumption_alpn_mismatch(void); @@ -99,6 +103,10 @@ int test_wolfSSL_get_shared_ciphers(void); TEST_DECL_GROUP("tls", test_tls_set_session_min_downgrade), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \ TEST_DECL_GROUP("tls", test_tls13_session_resumption_sni_mismatch), \ + TEST_DECL_GROUP("tls", test_sni_cb_ctx_cert_swap_refused), \ + TEST_DECL_GROUP("tls", test_cert_setup_cb_ctx_cert_swap_refused), \ + TEST_DECL_GROUP("tls", test_cb_ctx_cert_swap_other_session), \ + TEST_DECL_GROUP("tls", test_sni_cb_switch_ctx_unblocked), \ TEST_DECL_GROUP("tls", test_tls13_resumption_with_alpn), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_alpn_mismatch),\ TEST_DECL_GROUP("tls", test_tls13_session_resumption_alpn_mismatch), \ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 18ab83ca268..61f74dfc440 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2355,6 +2355,17 @@ WOLFSSL_LOCAL int HashOutput(WOLFSSL* ssl, const byte* output, int sz, int ivSz); WOLFSSL_LOCAL int HashInput(WOLFSSL* ssl, const byte* input, int sz); +/* Bracket a call out to an application callback, so that a certificate or key + * load reaching back into the same context can be refused. */ +WOLFSSL_LOCAL WOLFSSL_CTX* CtxCallbackEnter(WOLFSSL_CTX* ctx); +WOLFSSL_LOCAL void CtxCallbackExit(WOLFSSL_CTX* prev); + +#ifndef NO_CERTS +/* Call before replacing a certificate or key on a context, to refuse the + * change while this thread is inside a callback on it. */ +WOLFSSL_LOCAL int CheckCtxCertLoad(WOLFSSL_CTX* ctx); +#endif + #ifdef HAVE_SNI #ifndef NO_WOLFSSL_SERVER WOLFSSL_LOCAL int SNI_Callback(WOLFSSL* ssl); From ee9acdc448446972b9ccd5c3399803160d79398c Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Tue, 8 Sep 2026 11:12:31 -0700 Subject: [PATCH 2/4] PR feedback: add mutex/thread-safety, address altkey logic --- ChangeLog.md | 40 +++++++------- doc/dox_comments/header_files/ssl.h | 6 +- src/internal.c | 86 ++++++++++++++++++----------- src/ssl.c | 5 ++ src/ssl_api_cert.c | 12 ++-- src/ssl_load.c | 11 +++- tests/api/test_tls.c | 48 +++++++++++++++- wolfssl/internal.h | 9 +-- 8 files changed, 149 insertions(+), 68 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 623b31937bb..45dce99836d 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -2,29 +2,29 @@ ## Behavioral Changes -* **Behavioral change (loading a certificate or key on a context from inside a - callback)**: the sni callback set with `wolfSSL_CTX_set_servername_callback()` - and the certificate setup callback set with `wolfSSL_CTX_set_cert_cb()` run in - the middle of a handshake, at which point every session made from that context - is pointing at the context's certificate, chain and key. Replacing one of - those frees what those handshakes are reading. Such a load is now refused - while the calling thread is inside one of those callbacks, and the reason, - `BAD_STATE_E`, is left where `wolfSSL_get_error()` and the OpenSSL error queue - can report it. The calls affected are `wolfSSL_CTX_use_certificate()`, its - `_file` and `_buffer` forms, `wolfSSL_CTX_use_PrivateKey_file()`, `_buffer`, - `_Id` and `_Label`, the `wolfSSL_CTX_use_AltPrivateKey_*` pair, the - `wolfSSL_CTX_use_certificate_chain_*` family, and - `wolfSSL_CTX_add0_chain_cert()`, `wolfSSL_CTX_add1_chain_cert()` and - `wolfSSL_CTX_add_extra_chain_cert()`. An application that set a certificate - this way should set it on the session instead, with +* **Behavioral change (loading a certificate or key on a context while one of + its callbacks runs)**: the sni callback set with + `wolfSSL_CTX_set_servername_callback()` and the certificate setup callback set + with `wolfSSL_CTX_set_cert_cb()` run in the middle of a handshake, at which + point every session made from that context is pointing at the context's + certificate, chain and key. Replacing one of those frees what those + handshakes are reading. Such a load is now refused while one of those + callbacks is running on the context, from the callback itself or from any + other thread, and the reason, `BAD_STATE_E`, is left where + `wolfSSL_get_error()` and the OpenSSL error queue can report it. The calls + affected are `wolfSSL_CTX_use_certificate()`, its `_file` and `_buffer` forms, + `wolfSSL_CTX_use_PrivateKey_file()`, `_buffer`, `_Id` and `_Label`, the + `wolfSSL_CTX_use_AltPrivateKey_*` family, the + `wolfSSL_CTX_use_certificate_chain_*` family, + `wolfSSL_CTX_load_verify_chain_buffer_format()` with DER input, which also + replaces the chain, `wolfSSL_CTX_add0_chain_cert()`, + `wolfSSL_CTX_add1_chain_cert()`, `wolfSSL_CTX_add_extra_chain_cert()`, + `SSL_CTX_set0_chain()` and `SSL_CTX_set1_chain()`. An application that set a + certificate this way should set it on the session instead, with `wolfSSL_use_certificate_file()` and its relatives, or hand the session a different context with `wolfSSL_set_SSL_CTX()`; both are untouched, as are loads into the trust store such as `wolfSSL_CTX_load_verify_locations()` and - anything called outside a callback. The note that a callback is running is - kept per thread, so one thread's callback does not refuse another thread's - loads; on a build without thread local storage it is shared, where overlapping - callbacks can refuse a load that would have been allowed, or allow one that - would have been refused. + anything called once no callback is running. * **Behavioral change (`wc_PufReadSram` health tests the raw SRAM readout)**: the raw readout is now health tested before the context accepts it, and a diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index b5f9b6d94f4..0031fc75555 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -5983,9 +5983,9 @@ void wolfSSL_CTX_set_client_cert_cb(WOLFSSL_CTX *ctx, client_cert_cb cb); Set the certificate on the WOLFSSL object, with wolfSSL_use_certificate_file and friends, or hand it a different context with wolfSSL_set_SSL_CTX. Loading one on the WOLFSSL_CTX the handshake is running against is refused - from inside the callback and returns failure with BAD_STATE_E: sessions - already made from that context point at its certificate, and replacing it - would free what they are reading. + while the callback runs, from the callback or any other thread, and returns + failure with BAD_STATE_E: sessions already made from that context point at + its certificate, and replacing it would free what they are reading. \param ctx The WOLFSSL_CTX object. \param cb The callback function for certificate setup. diff --git a/src/internal.c b/src/internal.c index dd4d35e35b3..26d6cde0bf4 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7786,43 +7786,52 @@ int CopySSL_CTX_DhParams(WOLFSSL* ssl, WOLFSSL_CTX* ctx) } #endif /* !NO_DH */ -/* Context this thread is currently inside a callback for, if any. +/* Note that an application callback is about to run on a context. * - * Kept per thread rather than on the context: the context is shared between - * threads, so a flag on it would race with other handshakes and would be - * written to a context the callback may have swapped out from under us. Only - * ever compared, never followed, so a context freed during the callback does - * no harm. Where the build has no thread local storage this is one shared - * pointer, which costs the guard accuracy when threads overlap but still - * cannot corrupt anything. - */ -static THREAD_LS_T WOLFSSL_CTX* inCbCtx = NULL; - -/* Note that this thread is entering a callback on a context. + * The count is kept on the context, under its lock, so a callback on any + * thread is seen. The context is also held so that it outlives a callback + * that swaps it out of the session; CtxCallbackExit() lets go of it again. * - * @param [in] ctx SSL context object the callback belongs to. - * @return What was noted before, to hand back to CtxCallbackExit(). + * @param [in, out] ctx SSL context object the callback belongs to. + * @return 0 on success. + * @return BAD_MUTEX_E when the context's lock cannot be taken. */ -WOLFSSL_CTX* CtxCallbackEnter(WOLFSSL_CTX* ctx) +int CtxCallbackEnter(WOLFSSL_CTX* ctx) { - WOLFSSL_CTX* prev = inCbCtx; + int ret; - inCbCtx = ctx; + wolfSSL_RefWithMutexInc(&ctx->ref, &ret); + if (ret == 0) { + ret = wolfSSL_RefWithMutexLock(&ctx->ref); + if (ret == 0) { + ctx->callbackCnt++; + wolfSSL_RefWithMutexUnlock(&ctx->ref); + } + else { + wolfSSL_CTX_free(ctx); + } + } - return prev; + return ret; } -/* Note that this thread has left the callback. +/* Note that the callback has returned. * - * @param [in] prev What CtxCallbackEnter() handed back. + * @param [in, out] ctx SSL context object handed to CtxCallbackEnter(). */ -void CtxCallbackExit(WOLFSSL_CTX* prev) +void CtxCallbackExit(WOLFSSL_CTX* ctx) { - inCbCtx = prev; + if (wolfSSL_RefWithMutexLock(&ctx->ref) == 0) { + ctx->callbackCnt--; + wolfSSL_RefWithMutexUnlock(&ctx->ref); + } + /* Let go of the hold taken by CtxCallbackEnter(). */ + wolfSSL_CTX_free(ctx); } #ifndef NO_CERTS -/* Refuse to replace a certificate or key on a context from its own callback. +/* Refuse to replace a certificate or key on a context while a callback is + * running on it. * * Sessions made from a context point at its buffers, so replacing one frees * what handshakes already under way are reading. Setting a certificate on the @@ -7831,16 +7840,26 @@ void CtxCallbackExit(WOLFSSL_CTX* prev) * * @param [in] ctx SSL context object. May be NULL. * @return 0 when the load may go ahead. - * @return BAD_STATE_E while this thread is in a callback on the context. + * @return BAD_STATE_E while a callback is running on the context. + * @return BAD_MUTEX_E when the context's lock cannot be taken. */ int CheckCtxCertLoad(WOLFSSL_CTX* ctx) { int ret = 0; - if ((ctx != NULL) && (ctx == inCbCtx)) { - WOLFSSL_MSG("Certificate load refused: callback running on context"); - ret = BAD_STATE_E; - WOLFSSL_ERROR_VERBOSE(ret); + if (ctx != NULL) { + int running = 0; + + ret = wolfSSL_RefWithMutexLock(&ctx->ref); + if (ret == 0) { + running = ctx->callbackCnt; + wolfSSL_RefWithMutexUnlock(&ctx->ref); + } + if ((ret == 0) && (running > 0)) { + WOLFSSL_MSG("Cert load refused: callback running on context"); + ret = BAD_STATE_E; + WOLFSSL_ERROR_VERBOSE(ret); + } } return ret; @@ -45700,14 +45719,17 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ], /* Stunnel supports a custom sni callback to switch an SSL's ctx * when SNI is received. Call it now if exists */ if(ssl && ssl->ctx && ssl->ctx->sniRecvCb) { - WOLFSSL_CTX* prevCbCtx; + WOLFSSL_CTX* cbCtx = ssl->ctx; WOLFSSL_MSG("Calling custom sni callback"); - prevCbCtx = CtxCallbackEnter(ssl->ctx); + ret = CtxCallbackEnter(cbCtx); + if (ret != 0) { + return ret; + } sniRet = ssl->ctx->sniRecvCb(ssl, &ad, ssl->ctx->sniRecvCbArg); /* The callback may have switched this session to another context, - * so put back what was noted rather than reading ssl->ctx again. */ - CtxCallbackExit(prevCbCtx); + * so leave the one it was called on rather than ssl->ctx. */ + CtxCallbackExit(cbCtx); switch (sniRet) { case warning_return: WOLFSSL_MSG("Error in custom sni callback. Warning alert"); diff --git a/src/ssl.c b/src/ssl.c index fbf401d7c99..7a524bdaca3 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -8527,6 +8527,11 @@ long wolfSSL_CTX_ctrl(WOLFSSL_CTX* ctx, int cmd, long opt, void* pt) ret = WOLFSSL_FAILURE; break; } + /* Sessions made from this context hold the chain by pointer. */ + if (CheckCtxCertLoad(ctx) != 0) { + ret = WOLFSSL_FAILURE; + break; + } /* Clear certificate chain */ FreeDer(&ctx->certChain); if (sk) { diff --git a/src/ssl_api_cert.c b/src/ssl_api_cert.c index f81c405d6a2..c58b760e2fb 100644 --- a/src/ssl_api_cert.c +++ b/src/ssl_api_cert.c @@ -2821,19 +2821,23 @@ void wolfSSL_CTX_set_cert_cb(WOLFSSL_CTX* ctx, * unrecognized value. A fatal alert is sent when it failed. * @return WOLFSSL_ERROR_WANT_X509_LOOKUP when the callback returned a * negative value to ask to be called again. + * @return BAD_MUTEX_E when the context's lock cannot be taken. */ int CertSetupCbWrapper(WOLFSSL* ssl) { int ret = 0; if (ssl->ctx->certSetupCb != NULL) { - WOLFSSL_CTX* prevCbCtx; + WOLFSSL_CTX* cbCtx = ssl->ctx; WOLFSSL_MSG("Calling user cert setup callback"); - prevCbCtx = CtxCallbackEnter(ssl->ctx); + ret = CtxCallbackEnter(cbCtx); + if (ret != 0) { + return ret; + } ret = ssl->ctx->certSetupCb(ssl, ssl->ctx->certSetupCbArg); - /* The callback may have switched contexts; restore what was noted. */ - CtxCallbackExit(prevCbCtx); + /* The callback may have switched contexts; leave the one it ran on. */ + CtxCallbackExit(cbCtx); if (ret == 1) { WOLFSSL_MSG("User cert callback returned success"); ret = 0; diff --git a/src/ssl_load.c b/src/ssl_load.c index a7097374800..489890c37ef 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -2677,9 +2677,10 @@ int ProcessBuffer(WOLFSSL_CTX* ctx, const unsigned char* buff, long sz, if ((ret == 0) && (sz < 0)) { ret = BAD_FUNC_ARG; } - /* Sessions made from this context hold these by pointer. */ + /* Sessions made from this context hold these and the chain by pointer. */ if ((ret == 0) && (ssl == NULL) && ((type == CERT_TYPE) || - (type == PRIVATEKEY_TYPE) || (type == ALT_PRIVATEKEY_TYPE))) { + (type == PRIVATEKEY_TYPE) || (type == ALT_PRIVATEKEY_TYPE) || + userChain)) { ret = CheckCtxCertLoad(ctx); } @@ -4595,6 +4596,9 @@ int wolfSSL_CTX_use_AltPrivateKey_Id(WOLFSSL_CTX* ctx, const unsigned char* id, if ((ctx == NULL) || (id == NULL) || (sz < 0)) { ret = 0; } + if ((ret == 1) && (CheckCtxCertLoad(ctx) != 0)) { + ret = 0; + } if (ret == 1) { FreeDer(&ctx->altPrivateKey); @@ -4645,6 +4649,9 @@ int wolfSSL_CTX_use_AltPrivateKey_Label(WOLFSSL_CTX* ctx, const char* label, if ((ctx == NULL) || (label == NULL)) { ret = 0; } + if ((ret == 1) && (CheckCtxCertLoad(ctx) != 0)) { + ret = 0; + } if (ret == 1) { sz = (word32)XSTRLEN(label) + 1; diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 026443cf954..4a9269de6be 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -30,6 +30,7 @@ #include #include +#include #include #include @@ -2445,6 +2446,13 @@ enum { #ifdef WOLF_PRIVATE_KEY_ID CB_CERT_CTX_KEYID, /* points the key at an id held elsewhere */ CB_CERT_CTX_KEYLABEL, /* points the key at a label held elsewhere */ +#endif +#if defined(WOLF_PRIVATE_KEY_ID) && defined(WOLFSSL_DUAL_ALG_CERTS) + CB_CERT_CTX_ALTID, /* points the alternative key at an id */ + CB_CERT_CTX_ALTLABEL, /* points the alternative key at a label */ +#endif +#ifdef USE_CERT_BUFFERS_2048 + CB_CERT_CTX_CACHAIN, /* loads a CA chain that the context also sends */ #endif CB_CERT_MODE_CNT }; @@ -2455,9 +2463,12 @@ typedef struct CbCertCase { WOLFSSL_CTX* ctx; /* context the handshake is running against */ int mode; /* one of the CB_CERT_* values */ int called; /* how many times the callback ran */ + int setupFailed; /* the callback could not get what it needed */ int loadRet; /* what the load call returned */ const void* ctxCertBefore; const void* ctxCertAfter; + const void* ctxChainBefore; + const void* ctxChainAfter; } CbCertCase; /* Whether the case has the callback reach for the context. */ @@ -2475,6 +2486,7 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) { test->called++; test->ctxCertBefore = (const void*)test->ctx->certificate; + test->ctxChainBefore = (const void*)test->ctx->certChain; switch (test->mode) { case CB_CERT_ON_SSL: @@ -2502,6 +2514,26 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) "a-label", INVALID_DEVID); break; #endif + #if defined(WOLF_PRIVATE_KEY_ID) && defined(WOLFSSL_DUAL_ALG_CERTS) + case CB_CERT_CTX_ALTID: { + static const byte altKeyId[] = { 0x05, 0x06, 0x07, 0x08 }; + + test->loadRet = wolfSSL_CTX_use_AltPrivateKey_Id(test->ctx, + altKeyId, (long)sizeof(altKeyId), INVALID_DEVID); + break; + } + case CB_CERT_CTX_ALTLABEL: + test->loadRet = wolfSSL_CTX_use_AltPrivateKey_Label(test->ctx, + "an-alt-label", INVALID_DEVID); + break; + #endif + #ifdef USE_CERT_BUFFERS_2048 + case CB_CERT_CTX_CACHAIN: + test->loadRet = wolfSSL_CTX_load_verify_chain_buffer_format( + test->ctx, ca_cert_der_2048, sizeof_ca_cert_der_2048, + WOLFSSL_FILETYPE_ASN1); + break; + #endif #ifdef OPENSSL_EXTRA case CB_CERT_CTX_X509: { WOLFSSL_X509* x509 = wolfSSL_X509_load_certificate_file(svrCertFile, @@ -2511,6 +2543,9 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) test->loadRet = wolfSSL_CTX_use_certificate(test->ctx, x509); wolfSSL_X509_free(x509); } + else { + test->setupFailed = 1; + } break; } case CB_CERT_CTX_CHAIN: { @@ -2521,6 +2556,9 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) test->loadRet = wolfSSL_CTX_add1_chain_cert(test->ctx, x509); wolfSSL_X509_free(x509); } + else { + test->setupFailed = 1; + } break; } #endif @@ -2530,6 +2568,7 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) } test->ctxCertAfter = (const void*)test->ctx->certificate; + test->ctxChainAfter = (const void*)test->ctx->certChain; } #ifdef HAVE_SNI @@ -2605,14 +2644,16 @@ static int test_cb_cert_swap(method_provider method_c, * completes either way. */ ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); - /* The whole test rests on the callback having run. */ + /* The whole test rests on the callback having run and made its call. */ ExpectIntGT(test.called, 0); + ExpectIntEQ(test.setupFailed, 0); if (cb_cert_touches_ctx(mode)) { /* The load was turned away and the context kept what it had, so no * session was left pointing at a freed buffer. */ ExpectIntNE(test.loadRet, WOLFSSL_SUCCESS); ExpectPtrEq(test.ctxCertAfter, test.ctxCertBefore); + ExpectPtrEq(test.ctxChainAfter, test.ctxChainBefore); } else { ExpectIntEQ(test.loadRet, WOLFSSL_SUCCESS); @@ -2695,8 +2736,9 @@ static int sni_cb_switch_ctx(WOLFSSL* ssl, int* ad, void* arg) #endif /* Switching contexts from the callback must leave neither context marked as - * being in one: the note is per thread and is put back as it was, so both - * contexts can still be loaded once the handshake is over. */ + * being in one: the count comes off the context the callback ran on, and the + * one it switched to was never counted, so both can still be loaded once the + * handshake is over. */ int test_sni_cb_switch_ctx_unblocked(void) { EXPECT_DECLS; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 61f74dfc440..da93880b22c 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2356,13 +2356,13 @@ WOLFSSL_LOCAL int HashOutput(WOLFSSL* ssl, const byte* output, int sz, WOLFSSL_LOCAL int HashInput(WOLFSSL* ssl, const byte* input, int sz); /* Bracket a call out to an application callback, so that a certificate or key - * load reaching back into the same context can be refused. */ -WOLFSSL_LOCAL WOLFSSL_CTX* CtxCallbackEnter(WOLFSSL_CTX* ctx); -WOLFSSL_LOCAL void CtxCallbackExit(WOLFSSL_CTX* prev); + * load on the same context can be refused while it runs. */ +WOLFSSL_LOCAL int CtxCallbackEnter(WOLFSSL_CTX* ctx); +WOLFSSL_LOCAL void CtxCallbackExit(WOLFSSL_CTX* ctx); #ifndef NO_CERTS /* Call before replacing a certificate or key on a context, to refuse the - * change while this thread is inside a callback on it. */ + * change while a callback is running on it. */ WOLFSSL_LOCAL int CheckCtxCertLoad(WOLFSSL_CTX* ctx); #endif @@ -4168,6 +4168,7 @@ struct WOLFSSL_CTX { #endif wolfSSL_RefWithMutex ref; int err; /* error code in case of mutex not created */ + int callbackCnt; /* callbacks running on this context */ #ifndef NO_DH buffer serverDH_P; buffer serverDH_G; From 6deb1a39070ef1f8ac3d937f1eefc99b4824e445 Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Tue, 8 Sep 2026 12:20:05 -0700 Subject: [PATCH 3/4] More PR feedback --- ChangeLog.md | 33 +++----- doc/dox_comments/header_files/ssl.h | 9 ++- src/internal.c | 86 ++++++++++++++------- src/ssl.c | 7 ++ src/ssl_api_cert.c | 7 +- tests/api/test_tls.c | 113 +++++++++++++++++++++++++++- tests/api/test_tls.h | 2 + wolfssl/internal.h | 9 ++- 8 files changed, 200 insertions(+), 66 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 45dce99836d..05162cf29fc 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -3,28 +3,17 @@ ## Behavioral Changes * **Behavioral change (loading a certificate or key on a context while one of - its callbacks runs)**: the sni callback set with - `wolfSSL_CTX_set_servername_callback()` and the certificate setup callback set - with `wolfSSL_CTX_set_cert_cb()` run in the middle of a handshake, at which - point every session made from that context is pointing at the context's - certificate, chain and key. Replacing one of those frees what those - handshakes are reading. Such a load is now refused while one of those - callbacks is running on the context, from the callback itself or from any - other thread, and the reason, `BAD_STATE_E`, is left where - `wolfSSL_get_error()` and the OpenSSL error queue can report it. The calls - affected are `wolfSSL_CTX_use_certificate()`, its `_file` and `_buffer` forms, - `wolfSSL_CTX_use_PrivateKey_file()`, `_buffer`, `_Id` and `_Label`, the - `wolfSSL_CTX_use_AltPrivateKey_*` family, the - `wolfSSL_CTX_use_certificate_chain_*` family, - `wolfSSL_CTX_load_verify_chain_buffer_format()` with DER input, which also - replaces the chain, `wolfSSL_CTX_add0_chain_cert()`, - `wolfSSL_CTX_add1_chain_cert()`, `wolfSSL_CTX_add_extra_chain_cert()`, - `SSL_CTX_set0_chain()` and `SSL_CTX_set1_chain()`. An application that set a - certificate this way should set it on the session instead, with - `wolfSSL_use_certificate_file()` and its relatives, or hand the session a - different context with `wolfSSL_set_SSL_CTX()`; both are untouched, as are - loads into the trust store such as `wolfSSL_CTX_load_verify_locations()` and - anything called once no callback is running. + its callbacks runs)**: loading a certificate or key on a `WOLFSSL_CTX` while + its sni callback or certificate setup callback is running is now refused with + `BAD_STATE_E`. The calls affected are `wolfSSL_CTX_use_certificate()` and + its `_file` and `_buffer` forms, `wolfSSL_CTX_use_PrivateKey_file()`, + `_buffer`, `_Id` and `_Label`, the `wolfSSL_CTX_use_AltPrivateKey_*` and + `wolfSSL_CTX_use_certificate_chain_*` families, + `wolfSSL_CTX_load_verify_chain_buffer_format()` with DER input, + `wolfSSL_CTX_add0_chain_cert()`, `wolfSSL_CTX_add1_chain_cert()`, + `wolfSSL_CTX_add_extra_chain_cert()`, `SSL_CTX_set0_chain()` and + `SSL_CTX_set1_chain()`. Set the certificate on the `WOLFSSL` object instead, + or switch it to a context that already holds one with `wolfSSL_set_SSL_CTX()`. * **Behavioral change (`wc_PufReadSram` health tests the raw SRAM readout)**: the raw readout is now health tested before the context accepts it, and a diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index 0031fc75555..244f9211eec 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -5982,10 +5982,11 @@ void wolfSSL_CTX_set_client_cert_cb(WOLFSSL_CTX *ctx, client_cert_cb cb); Set the certificate on the WOLFSSL object, with wolfSSL_use_certificate_file and friends, or hand it a different context with wolfSSL_set_SSL_CTX. - Loading one on the WOLFSSL_CTX the handshake is running against is refused - while the callback runs, from the callback or any other thread, and returns - failure with BAD_STATE_E: sessions already made from that context point at - its certificate, and replacing it would free what they are reading. + Loading one on the WOLFSSL_CTX the handshake is running against, including + one handed over with wolfSSL_set_SSL_CTX, is refused from inside the + callback and fails with BAD_STATE_E, logged and put on the error queue: + sessions already made from that context point at its certificate, and + replacing it would free what they are reading. \param ctx The WOLFSSL_CTX object. \param cb The callback function for certificate setup. diff --git a/src/internal.c b/src/internal.c index 26d6cde0bf4..500201a00bb 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7786,30 +7786,41 @@ int CopySSL_CTX_DhParams(WOLFSSL* ssl, WOLFSSL_CTX* ctx) } #endif /* !NO_DH */ -/* Note that an application callback is about to run on a context. +/* Add to the count of callbacks running on a context, under its lock. + * + * @param [in, out] ctx SSL context object. + * @param [in] by Amount to add, which may be negative. + * @return 0 on success. + * @return BAD_MUTEX_E when the context's lock cannot be taken. + */ +static int CtxCallbackCount(WOLFSSL_CTX* ctx, int by) +{ + int ret = wolfSSL_RefWithMutexLock(&ctx->ref); + + if (ret == 0) { + ctx->callbackCnt += by; + wolfSSL_RefWithMutexUnlock(&ctx->ref); + } + + return ret; +} + +/* Note that an application callback is about to run on a session's context. * * The count is kept on the context, under its lock, so a callback on any - * thread is seen. The context is also held so that it outlives a callback - * that swaps it out of the session; CtxCallbackExit() lets go of it again. + * thread is seen. The session is marked as well, so that handing it another + * context during the callback carries the count over with CtxCallbackMove(). * - * @param [in, out] ctx SSL context object the callback belongs to. + * @param [in, out] ssl SSL object the callback is for. * @return 0 on success. * @return BAD_MUTEX_E when the context's lock cannot be taken. */ -int CtxCallbackEnter(WOLFSSL_CTX* ctx) +int CtxCallbackEnter(WOLFSSL* ssl) { - int ret; + int ret = CtxCallbackCount(ssl->ctx, 1); - wolfSSL_RefWithMutexInc(&ctx->ref, &ret); if (ret == 0) { - ret = wolfSSL_RefWithMutexLock(&ctx->ref); - if (ret == 0) { - ctx->callbackCnt++; - wolfSSL_RefWithMutexUnlock(&ctx->ref); - } - else { - wolfSSL_CTX_free(ctx); - } + ssl->options.inCtxCb = 1; } return ret; @@ -7817,16 +7828,37 @@ int CtxCallbackEnter(WOLFSSL_CTX* ctx) /* Note that the callback has returned. * - * @param [in, out] ctx SSL context object handed to CtxCallbackEnter(). + * @param [in, out] ssl SSL object the callback was for. */ -void CtxCallbackExit(WOLFSSL_CTX* ctx) +void CtxCallbackExit(WOLFSSL* ssl) { - if (wolfSSL_RefWithMutexLock(&ctx->ref) == 0) { - ctx->callbackCnt--; - wolfSSL_RefWithMutexUnlock(&ctx->ref); + if (ssl->options.inCtxCb) { + ssl->options.inCtxCb = 0; + (void)CtxCallbackCount(ssl->ctx, -1); + } +} + +/* Carry a running callback over to the context the session is switching to, + * so a load on that one is refused from here on just as on the first. + * + * @param [in, out] ssl SSL object inside a callback, on its old context. + * @param [in, out] ctx SSL context object the session is switching to. + * @return 0 on success. + * @return BAD_MUTEX_E when a context's lock cannot be taken. + */ +int CtxCallbackMove(WOLFSSL* ssl, WOLFSSL_CTX* ctx) +{ + int ret = CtxCallbackCount(ssl->ctx, -1); + + if (ret == 0) { + ret = CtxCallbackCount(ctx, 1); } - /* Let go of the hold taken by CtxCallbackEnter(). */ - wolfSSL_CTX_free(ctx); + if (ret != 0) { + /* Nothing is counted any more, so there is nothing to take off. */ + ssl->options.inCtxCb = 0; + } + + return ret; } #ifndef NO_CERTS @@ -7858,7 +7890,7 @@ int CheckCtxCertLoad(WOLFSSL_CTX* ctx) if ((ret == 0) && (running > 0)) { WOLFSSL_MSG("Cert load refused: callback running on context"); ret = BAD_STATE_E; - WOLFSSL_ERROR_VERBOSE(ret); + WOLFSSL_ERROR(ret); } } @@ -45719,17 +45751,13 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ], /* Stunnel supports a custom sni callback to switch an SSL's ctx * when SNI is received. Call it now if exists */ if(ssl && ssl->ctx && ssl->ctx->sniRecvCb) { - WOLFSSL_CTX* cbCtx = ssl->ctx; - WOLFSSL_MSG("Calling custom sni callback"); - ret = CtxCallbackEnter(cbCtx); + ret = CtxCallbackEnter(ssl); if (ret != 0) { return ret; } sniRet = ssl->ctx->sniRecvCb(ssl, &ad, ssl->ctx->sniRecvCbArg); - /* The callback may have switched this session to another context, - * so leave the one it was called on rather than ssl->ctx. */ - CtxCallbackExit(cbCtx); + CtxCallbackExit(ssl); switch (sniRet) { case warning_return: WOLFSSL_MSG("Error in custom sni callback. Warning alert"); diff --git a/src/ssl.c b/src/ssl.c index 7a524bdaca3..31d73d3ede9 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -8846,6 +8846,13 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) (void)ret; #endif + /* A callback handing the session over keeps its guard, on the context the + * session is about to point at. */ + if (ssl->options.inCtxCb && (CtxCallbackMove(ssl, ctx) != 0)) { + wolfSSL_CTX_free(ctx); + return NULL; + } + if (ssl->ctx != NULL) wolfSSL_CTX_free(ssl->ctx); ssl->ctx = ctx; diff --git a/src/ssl_api_cert.c b/src/ssl_api_cert.c index c58b760e2fb..bf5e1cf49c8 100644 --- a/src/ssl_api_cert.c +++ b/src/ssl_api_cert.c @@ -2828,16 +2828,13 @@ int CertSetupCbWrapper(WOLFSSL* ssl) int ret = 0; if (ssl->ctx->certSetupCb != NULL) { - WOLFSSL_CTX* cbCtx = ssl->ctx; - WOLFSSL_MSG("Calling user cert setup callback"); - ret = CtxCallbackEnter(cbCtx); + ret = CtxCallbackEnter(ssl); if (ret != 0) { return ret; } ret = ssl->ctx->certSetupCb(ssl, ssl->ctx->certSetupCbArg); - /* The callback may have switched contexts; leave the one it ran on. */ - CtxCallbackExit(cbCtx); + CtxCallbackExit(ssl); if (ret == 1) { WOLFSSL_MSG("User cert callback returned success"); ret = 0; diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 4a9269de6be..5da0066fbc7 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -2465,6 +2465,7 @@ typedef struct CbCertCase { int called; /* how many times the callback ran */ int setupFailed; /* the callback could not get what it needed */ int loadRet; /* what the load call returned */ + int queued; /* the refusal was put on the error queue */ const void* ctxCertBefore; const void* ctxCertAfter; const void* ctxChainBefore; @@ -2477,6 +2478,21 @@ static int cb_cert_touches_ctx(int mode) return (mode != CB_CERT_NO_CHANGE) && (mode != CB_CERT_ON_SSL); } +/* Whether the error queue holds the refusal, past anything else on it. */ +static int cb_cert_refusal_queued(void) +{ + int found = 0; + unsigned long err; + + while ((err = wolfSSL_ERR_get_error()) != 0) { + if (err == (unsigned long)-WC_NO_ERR_TRACE(BAD_STATE_E)) { + found = 1; + } + } + + return found; +} + /* Carry out what the case asks for, recording what the load returned. * * @param [in] ssl SSL object the callback was called on. @@ -2487,6 +2503,7 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) test->called++; test->ctxCertBefore = (const void*)test->ctx->certificate; test->ctxChainBefore = (const void*)test->ctx->certChain; + wolfSSL_ERR_clear_error(); switch (test->mode) { case CB_CERT_ON_SSL: @@ -2569,6 +2586,7 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) test->ctxCertAfter = (const void*)test->ctx->certificate; test->ctxChainAfter = (const void*)test->ctx->certChain; + test->queued = cb_cert_refusal_queued(); } #ifdef HAVE_SNI @@ -2654,9 +2672,14 @@ static int test_cb_cert_swap(method_provider method_c, ExpectIntNE(test.loadRet, WOLFSSL_SUCCESS); ExpectPtrEq(test.ctxCertAfter, test.ctxCertBefore); ExpectPtrEq(test.ctxChainAfter, test.ctxChainBefore); + #ifdef WOLFSSL_HAVE_ERROR_QUEUE + /* The reason is left where the application can read it. */ + ExpectIntEQ(test.queued, 1); + #endif } else { ExpectIntEQ(test.loadRet, WOLFSSL_SUCCESS); + ExpectIntEQ(test.queued, 0); } wolfSSL_free(ssl_c); @@ -2733,12 +2756,39 @@ static int sni_cb_switch_ctx(WOLFSSL* ssl, int* ad, void* arg) return (wolfSSL_set_SSL_CTX(ssl, (WOLFSSL_CTX*)arg) != NULL) ? 0 : fatal_return; } + +/* What the switching callback did, handed to it through its user argument. */ +typedef struct CbSwitchCase { + WOLFSSL_CTX* ctx; /* context to hand the session */ + int called; /* how many times the callback ran */ + int loadRet; /* what loading on that context returned */ + int queued; /* the refusal was put on the error queue */ +} CbSwitchCase; + +/* Hands the session a different context and then loads on that one. The + * session now points at its certificate, so the load must be refused just as + * one on the first context would be. */ +static int sni_cb_switch_ctx_then_load(WOLFSSL* ssl, int* ad, void* arg) +{ + CbSwitchCase* test = (CbSwitchCase*)arg; + + (void)ad; + test->called++; + if (wolfSSL_set_SSL_CTX(ssl, test->ctx) == NULL) { + return fatal_return; + } + wolfSSL_ERR_clear_error(); + test->loadRet = wolfSSL_CTX_use_certificate_file(test->ctx, svrCertFile, + CERT_FILETYPE); + test->queued = cb_cert_refusal_queued(); + return 0; +} #endif /* Switching contexts from the callback must leave neither context marked as - * being in one: the count comes off the context the callback ran on, and the - * one it switched to was never counted, so both can still be loaded once the - * handshake is over. */ + * being in one: the count follows the session to the context it switched to + * and comes off again when the callback returns, so both can still be loaded + * once the handshake is over. */ int test_sni_cb_switch_ctx_unblocked(void) { EXPECT_DECLS; @@ -2785,6 +2835,63 @@ int test_sni_cb_switch_ctx_unblocked(void) return EXPECT_RESULT(); } +/* The guard follows the session to a context handed over from the callback: + * the session now points at that context's certificate, so loading on it from + * the rest of the callback is refused too. Once the handshake is over, both + * contexts take a load again. */ +int test_sni_cb_switch_ctx_load_refused(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \ + (defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL, *ctx_s2 = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + CbSwitchCase test; + const char* sni = "example.com"; + + XMEMSET(&test, 0, sizeof(test)); + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + ExpectNotNull(ctx_s2 = wolfSSL_CTX_new(wolfTLSv1_2_server_method())); + ExpectIntEQ(wolfSSL_CTX_use_certificate_file(ctx_s2, svrCertFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_use_PrivateKey_file(ctx_s2, svrKeyFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + test.ctx = ctx_s2; + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + wolfSSL_CTX_set_servername_callback(ctx_s, sni_cb_switch_ctx_then_load); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, &test), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, sni, + (word16)XSTRLEN(sni)), WOLFSSL_SUCCESS); + + /* The refused load leaves the handed-over context whole, so the handshake + * completes on it. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(test.called, 1); + ExpectIntNE(test.loadRet, WOLFSSL_SUCCESS); +#ifdef WOLFSSL_HAVE_ERROR_QUEUE + ExpectIntEQ(test.queued, 1); +#endif + + ExpectIntEQ(wolfSSL_CTX_use_certificate_file(ctx_s, svrCertFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_use_certificate_file(ctx_s2, svrCertFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + wolfSSL_CTX_free(ctx_s2); +#endif + return EXPECT_RESULT(); +} + /* A second session on the same context must be unharmed by a callback that * reached for the context's certificate. Detecting the swap after the fact * only ever protected the session whose callback made the call; refusing the diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 3d3fe5eec3e..bcf6fd00dbc 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -56,6 +56,7 @@ int test_sni_cb_ctx_cert_swap_refused(void); int test_cert_setup_cb_ctx_cert_swap_refused(void); int test_cb_ctx_cert_swap_other_session(void); int test_sni_cb_switch_ctx_unblocked(void); +int test_sni_cb_switch_ctx_load_refused(void); int test_tls13_resumption_with_alpn(void); int test_tls12_session_id_resumption_alpn_mismatch(void); int test_tls13_session_resumption_alpn_mismatch(void); @@ -107,6 +108,7 @@ int test_wolfSSL_get_shared_ciphers(void); TEST_DECL_GROUP("tls", test_cert_setup_cb_ctx_cert_swap_refused), \ TEST_DECL_GROUP("tls", test_cb_ctx_cert_swap_other_session), \ TEST_DECL_GROUP("tls", test_sni_cb_switch_ctx_unblocked), \ + TEST_DECL_GROUP("tls", test_sni_cb_switch_ctx_load_refused), \ TEST_DECL_GROUP("tls", test_tls13_resumption_with_alpn), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_alpn_mismatch),\ TEST_DECL_GROUP("tls", test_tls13_session_resumption_alpn_mismatch), \ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index da93880b22c..6bcf2e70690 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2356,9 +2356,11 @@ WOLFSSL_LOCAL int HashOutput(WOLFSSL* ssl, const byte* output, int sz, WOLFSSL_LOCAL int HashInput(WOLFSSL* ssl, const byte* input, int sz); /* Bracket a call out to an application callback, so that a certificate or key - * load on the same context can be refused while it runs. */ -WOLFSSL_LOCAL int CtxCallbackEnter(WOLFSSL_CTX* ctx); -WOLFSSL_LOCAL void CtxCallbackExit(WOLFSSL_CTX* ctx); + * load on the session's context can be refused while it runs. */ +WOLFSSL_LOCAL int CtxCallbackEnter(WOLFSSL* ssl); +WOLFSSL_LOCAL void CtxCallbackExit(WOLFSSL* ssl); +/* Carry a running callback over to the context the session is switching to. */ +WOLFSSL_LOCAL int CtxCallbackMove(WOLFSSL* ssl, WOLFSSL_CTX* ctx); #ifndef NO_CERTS /* Call before replacing a certificate or key on a context, to refuse the @@ -5625,6 +5627,7 @@ struct Options { #endif word16 returnOnGoodCh:1; word16 disableRead:1; + word16 inCtxCb:1; /* inside a callback on the context */ #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WOLFSSL_ASYNC_CERT_YIELD) /* Opt-in (WOLFSSL_ASYNC_CERT_YIELD): set when we deliberately returned * WC_PENDING_E between peer certificate verifies so a cooperative scheduler From 1bb9b60616cea9b3eb49661582afe6ba23d0785c Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Tue, 8 Sep 2026 14:17:21 -0700 Subject: [PATCH 4/4] PR feedback: fix compile issue in tests --- tests/api/test_tls.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 5da0066fbc7..795eb17042b 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -2478,6 +2478,12 @@ static int cb_cert_touches_ctx(int mode) return (mode != CB_CERT_NO_CHANGE) && (mode != CB_CERT_ON_SSL); } +/* The calls that read the error queue are only built alongside it here. */ +#if defined(WOLFSSL_HAVE_ERROR_QUEUE) && defined(OPENSSL_EXTRA) + #define CB_CERT_ERR_QUEUE +#endif + +#ifdef CB_CERT_ERR_QUEUE /* Whether the error queue holds the refusal, past anything else on it. */ static int cb_cert_refusal_queued(void) { @@ -2492,6 +2498,7 @@ static int cb_cert_refusal_queued(void) return found; } +#endif /* Carry out what the case asks for, recording what the load returned. * @@ -2503,7 +2510,9 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) test->called++; test->ctxCertBefore = (const void*)test->ctx->certificate; test->ctxChainBefore = (const void*)test->ctx->certChain; +#ifdef CB_CERT_ERR_QUEUE wolfSSL_ERR_clear_error(); +#endif switch (test->mode) { case CB_CERT_ON_SSL: @@ -2586,7 +2595,9 @@ static void cb_cert_action(WOLFSSL* ssl, CbCertCase* test) test->ctxCertAfter = (const void*)test->ctx->certificate; test->ctxChainAfter = (const void*)test->ctx->certChain; +#ifdef CB_CERT_ERR_QUEUE test->queued = cb_cert_refusal_queued(); +#endif } #ifdef HAVE_SNI @@ -2672,7 +2683,7 @@ static int test_cb_cert_swap(method_provider method_c, ExpectIntNE(test.loadRet, WOLFSSL_SUCCESS); ExpectPtrEq(test.ctxCertAfter, test.ctxCertBefore); ExpectPtrEq(test.ctxChainAfter, test.ctxChainBefore); - #ifdef WOLFSSL_HAVE_ERROR_QUEUE + #ifdef CB_CERT_ERR_QUEUE /* The reason is left where the application can read it. */ ExpectIntEQ(test.queued, 1); #endif @@ -2777,10 +2788,14 @@ static int sni_cb_switch_ctx_then_load(WOLFSSL* ssl, int* ad, void* arg) if (wolfSSL_set_SSL_CTX(ssl, test->ctx) == NULL) { return fatal_return; } +#ifdef CB_CERT_ERR_QUEUE wolfSSL_ERR_clear_error(); +#endif test->loadRet = wolfSSL_CTX_use_certificate_file(test->ctx, svrCertFile, CERT_FILETYPE); +#ifdef CB_CERT_ERR_QUEUE test->queued = cb_cert_refusal_queued(); +#endif return 0; } #endif @@ -2874,7 +2889,7 @@ int test_sni_cb_switch_ctx_load_refused(void) ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); ExpectIntEQ(test.called, 1); ExpectIntNE(test.loadRet, WOLFSSL_SUCCESS); -#ifdef WOLFSSL_HAVE_ERROR_QUEUE +#ifdef CB_CERT_ERR_QUEUE ExpectIntEQ(test.queued, 1); #endif