From 60b5dfc2ef28731a2c53330c8c94cc1190422c72 Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Tue, 7 Jul 2026 11:14:01 -0700 Subject: [PATCH 1/5] Add test case to swap certificates in SNI callback --- tests/api/test_tls13.c | 60 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 2 ++ 2 files changed, 62 insertions(+) diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index a691c90fc8..0f92e3cbdf 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -3627,6 +3627,66 @@ int test_tls13_pha(void) } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_SNI) && !defined(WOLFSSL_COPY_CERT) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_RSA) && !defined(NO_CERTS) +/* React to the client SNI by reloading the certificate on the existing CTX. + * Because the SSL was created without WOLFSSL_COPY_CERT, the cert pointer is + * identical between the CTX and SSL objects. This test validates the + * cert file replacement. + * Note the updated cert will not be used for this connection. */ +static int test_cert_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg) +{ + WOLFSSL_CTX* ctx = wolfSSL_get_SSL_CTX(ssl); + (void)ad; + (void)arg; + (void)wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, + WOLFSSL_FILETYPE_PEM); + /* 0 means ack the servername and continue the handshake. */ + return 0; +} +#endif + +int test_tls13_cert_alias_uaf_sni(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_SNI) && !defined(WOLFSSL_COPY_CERT) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_RSA) && !defined(NO_CERTS) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + const char* host = "example.com"; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + /* Server cert/key are loaded on ctx_s, so ssl_s->buffers.certificate + * aliases ctx_s->certificate. */ + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + + /* Server swaps its CTX certificate when the SNI arrives. */ + wolfSSL_CTX_set_servername_callback(ctx_s, test_cert_alias_sni_cb); + + /* Client offers SNI so the server callback fires while parsing the + * ClientHello. */ + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, host, + (word16)XSTRLEN(host)), WOLFSSL_SUCCESS); + + /* The callback loads a new certificate on the server CTX, which is + * aliased by the SSL. The handshake must complete without a UAF. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + + #if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) && \ !defined(WOLFSSL_MLKEM_NO_DECAPSULATE) && \ diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index 4ea8110825..876f03efa0 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -30,6 +30,7 @@ int test_tls13_bad_psk_binder(void); int test_tls13_rpk_handshake(void); int test_tls13_rpk_handshake_no_negotiation(void); int test_tls13_pha(void); +int test_tls13_cert_alias_uaf_sni(void); int test_tls13_pq_groups(void); int test_tls13_multi_pqc_key_share(void); int test_tls13_early_data(void); @@ -108,6 +109,7 @@ int test_tls13_pqc_hybrid_async_server(void); TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \ TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake_no_negotiation), \ TEST_DECL_GROUP("tls13", test_tls13_pha), \ + TEST_DECL_GROUP("tls13", test_tls13_cert_alias_uaf_sni), \ TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \ TEST_DECL_GROUP("tls13", test_tls13_multi_pqc_key_share), \ TEST_DECL_GROUP("tls13", test_tls13_early_data), \ From b493831b6c609c6895c3c6ca1a62c32ce3489f9c Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 8 Jul 2026 10:08:24 -0700 Subject: [PATCH 2/5] Use refcount on DER buffer to prevent UAF --- src/internal.c | 10 +++++++++ src/ssl.c | 21 +++++++++---------- src/ssl_load.c | 12 ++++------- tests/api.c | 17 +++++----------- wolfcrypt/src/asn.c | 37 +++++++++++++++++++++++++++------- wolfssl/wolfcrypt/asn.h | 1 + wolfssl/wolfcrypt/asn_public.h | 5 +++-- 7 files changed, 63 insertions(+), 40 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8aa3fe065e..12ede5bebb 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7174,6 +7174,10 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) /* ctx still owns certificate, certChain, key, dh, and cm */ ssl->buffers.certificate = ctx->certificate; ssl->buffers.certChain = ctx->certChain; + /* Increment refcount on cert pointers since we've aliased them. This + * prevents early freeing of the cert buffers. */ + RefDer(ssl->buffers.certificate); + RefDer(ssl->buffers.certChain); #endif ssl->buffers.certChainCnt = ctx->certChainCnt; #ifndef WOLFSSL_BLIND_PRIVATE_KEY @@ -9218,6 +9222,12 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl) #ifndef NO_CERTS ssl->keepCert = 0; /* make sure certificate is free'd */ wolfSSL_UnloadCertsKeys(ssl); + /* Release references on cert buffers aliased from the context. Owned + * buffers were already freed and cleared by wolfSSL_UnloadCertsKeys. */ + FreeDer(&ssl->buffers.certificate); + ssl->buffers.weOwnCert = 0; + FreeDer(&ssl->buffers.certChain); + ssl->buffers.weOwnCertChain = 0; #endif #ifndef NO_RSA FreeKey(ssl, DYNAMIC_TYPE_RSA, (void**)&ssl->peerRsaKey); diff --git a/src/ssl.c b/src/ssl.c index 52abaa02c9..c493bcf5d2 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12727,17 +12727,10 @@ void wolfSSL_certs_clear(WOLFSSL* ssl) if (ssl == NULL) return; - /* ctx still owns certificate, certChain, key, dh, and cm */ - if (ssl->buffers.weOwnCert) { - FreeDer(&ssl->buffers.certificate); - ssl->buffers.weOwnCert = 0; - } - ssl->buffers.certificate = NULL; - if (ssl->buffers.weOwnCertChain) { - FreeDer(&ssl->buffers.certChain); - ssl->buffers.weOwnCertChain = 0; - } - ssl->buffers.certChain = NULL; + FreeDer(&ssl->buffers.certificate); + ssl->buffers.weOwnCert = 0; + FreeDer(&ssl->buffers.certChain); + ssl->buffers.weOwnCertChain = 0; #ifdef WOLFSSL_TLS13 ssl->buffers.certChainCnt = 0; #endif @@ -13200,8 +13193,14 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) } #else /* ctx owns certificate, certChain and key */ + FreeDer(&ssl->buffers.certificate); + ssl->buffers.weOwnCert = 0; + FreeDer(&ssl->buffers.certChain); + ssl->buffers.weOwnCertChain = 0; ssl->buffers.certificate = ctx->certificate; ssl->buffers.certChain = ctx->certChain; + RefDer(ssl->buffers.certificate); + RefDer(ssl->buffers.certChain); #endif #ifdef WOLFSSL_TLS13 ssl->buffers.certChainCnt = ctx->certChainCnt; diff --git a/src/ssl_load.c b/src/ssl_load.c index b63a3b8430..5161f69725 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -242,10 +242,7 @@ static int ProcessUserChainRetain(WOLFSSL_CTX* ctx, WOLFSSL* ssl, /* Store in SSL object if available. */ if (ssl != NULL) { - /* Dispose of old chain if not reference to context's. */ - if (ssl->buffers.weOwnCertChain) { - FreeDer(&ssl->buffers.certChain); - } + FreeDer(&ssl->buffers.certChain); /* Allocate and copy the buffer into SSL object. */ ret = AllocCopyDer(&ssl->buffers.certChain, chainBuffer, len, type, heap); @@ -2224,15 +2221,14 @@ static int ProcessBufferCertHandleDer(WOLFSSL_CTX* ctx, WOLFSSL* ssl, /* Leaf certificate - our certificate. */ else if (type == CERT_TYPE) { if (ssl != NULL) { - /* Free previous certificate if we own it. */ + #ifdef KEEP_OUR_CERT if (ssl->buffers.weOwnCert) { - FreeDer(&ssl->buffers.certificate); - #ifdef KEEP_OUR_CERT /* Dispose of X509 version of certificate. */ wolfSSL_X509_free(ssl->ourCert); ssl->ourCert = NULL; - #endif } + #endif + FreeDer(&ssl->buffers.certificate); /* Store certificate as ours. */ ssl->buffers.certificate = der; #ifdef KEEP_OUR_CERT diff --git a/tests/api.c b/tests/api.c index eed600c040..874168db72 100644 --- a/tests/api.c +++ b/tests/api.c @@ -3956,25 +3956,18 @@ static int test_wolfSSL_add_to_chain_overflow(void) if (EXPECT_SUCCESS()) { /* Now ctx->certificate is set, next add goes to certChain via * wolfssl_add_to_chain. Fake a chain whose length is near UINT32_MAX - * so the size calculation (len + CERT_HEADER_SZ + certSz) overflows. */ - fakeChain = (DerBuffer*)XMALLOC(sizeof(DerBuffer) + 16, ctx->heap, - DYNAMIC_TYPE_CERT); - ExpectNotNull(fakeChain); + * so the size calculation (len + CERT_HEADER_SZ + certSz) overflows. + * Use AllocDer so the buffer refcount is initialized. */ + ExpectIntEQ(AllocDer(&fakeChain, 16, CERT_TYPE, ctx->heap), 0); } if (EXPECT_SUCCESS()) { - XMEMSET(fakeChain, 0, sizeof(DerBuffer) + 16); - fakeChain->buffer = (byte*)(fakeChain + 1); fakeChain->length = WOLFSSL_MAX_32BIT - 2; /* will overflow with any cert */ - fakeChain->type = CERT_TYPE; - fakeChain->dynType = DYNAMIC_TYPE_CERT; /* Replace the real chain with our fake one. */ - if (ctx->certChain != NULL) { - XFREE(ctx->certChain, ctx->heap, DYNAMIC_TYPE_CERT); - } + FreeDer(&ctx->certChain); ctx->certChain = fakeChain; } else { - XFREE(fakeChain, ctx ? ctx->heap : NULL, DYNAMIC_TYPE_CERT); + FreeDer(&fakeChain); } /* Try to add another cert - this MUST fail due to overflow guard. */ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 02e018f5d4..b17bf3fa56 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -24693,6 +24693,14 @@ int AllocDer(DerBuffer** pDer, word32 length, int type, void* heap) der->heap = heap; der->buffer = (byte*)der + sizeof(DerBuffer); der->length = length; + wolfSSL_RefInit(&der->ref, &ret); + #ifdef WOLFSSL_REFCNT_ERROR_RETURN + if (ret != 0) { + XFREE(der, heap, dynType); + *pDer = NULL; + return ret; + } + #endif ret = 0; /* Success */ } else { ret = BAD_FUNC_ARG; @@ -24700,6 +24708,15 @@ int AllocDer(DerBuffer** pDer, word32 length, int type, void* heap) return ret; } +void RefDer(DerBuffer* der) +{ + if (der != NULL) { + int ref_err = 0; + wolfSSL_RefInc(&der->ref, &ref_err); + (void)ref_err; + } +} + int AllocCopyDer(DerBuffer** pDer, const unsigned char* buff, word32 length, int type, void* heap) { @@ -24715,15 +24732,21 @@ void FreeDer(DerBuffer** pDer) { if (pDer && *pDer) { DerBuffer* der = (DerBuffer*)*pDer; + int isZero = 1; + int ref_err = 0; - /* ForceZero private keys */ - if (((der->type == PRIVATEKEY_TYPE) || - (der->type == ALT_PRIVATEKEY_TYPE)) && der->buffer != NULL) { - ForceZero(der->buffer, der->length); + wolfSSL_RefDec(&der->ref, &isZero, &ref_err); + (void)ref_err; + if (isZero) { + if (((der->type == PRIVATEKEY_TYPE) || + (der->type == ALT_PRIVATEKEY_TYPE)) && der->buffer != NULL) { + ForceZero(der->buffer, der->length); + } + der->buffer = NULL; + der->length = 0; + wolfSSL_RefFree(&der->ref); + XFREE(der, der->heap, der->dynType); } - der->buffer = NULL; - der->length = 0; - XFREE(der, der->heap, der->dynType); *pDer = NULL; } diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 3a4dc6ded3..91778564c1 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2811,6 +2811,7 @@ WOLFSSL_API int AllocDer(DerBuffer** der, word32 length, int type, WOLFSSL_LOCAL int AllocCopyDer(DerBuffer** der, const unsigned char* buff, word32 length, int type, void* heap); WOLFSSL_API void FreeDer(DerBuffer** der); +WOLFSSL_LOCAL void RefDer(DerBuffer* der); #ifdef WOLFSSL_ASN_PARSE_KEYUSAGE WOLFSSL_LOCAL int ParseKeyUsageStr(const char* value, word16* keyUsage, diff --git a/wolfssl/wolfcrypt/asn_public.h b/wolfssl/wolfcrypt/asn_public.h index d557ac94cd..62a77afca9 100644 --- a/wolfssl/wolfcrypt/asn_public.h +++ b/wolfssl/wolfcrypt/asn_public.h @@ -241,8 +241,9 @@ typedef struct DerBuffer { byte* buffer; void* heap; word32 length; - int type; /* enum CertType */ - int dynType; /* DYNAMIC_TYPE_* */ + int type; /* enum CertType */ + int dynType; /* DYNAMIC_TYPE_* */ + wolfSSL_Ref ref; /* refcount for aliased pointers to the der */ } DerBuffer; typedef struct WOLFSSL_ASN1_TIME { From a76d50daa29cf7249dff07650d612eff62cb3fd6 Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 8 Jul 2026 14:38:49 -0700 Subject: [PATCH 3/5] PR feedback regarding return code checking --- src/internal.c | 12 ++++++++---- src/ssl.c | 10 ++++++++-- tests/api/test_tls13.c | 9 +++++++-- wolfcrypt/src/asn.c | 17 +++++++++++++---- wolfssl/wolfcrypt/asn.h | 2 +- 5 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/internal.c b/src/internal.c index 12ede5bebb..f65681f39b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7173,11 +7173,15 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) #else /* ctx still owns certificate, certChain, key, dh, and cm */ ssl->buffers.certificate = ctx->certificate; + if (!RefDer(ssl->buffers.certificate)) { + ssl->buffers.certificate = NULL; + return BAD_MUTEX_E; + } ssl->buffers.certChain = ctx->certChain; - /* Increment refcount on cert pointers since we've aliased them. This - * prevents early freeing of the cert buffers. */ - RefDer(ssl->buffers.certificate); - RefDer(ssl->buffers.certChain); + if (!RefDer(ssl->buffers.certChain)) { + ssl->buffers.certChain = NULL; + return BAD_MUTEX_E; + } #endif ssl->buffers.certChainCnt = ctx->certChainCnt; #ifndef WOLFSSL_BLIND_PRIVATE_KEY diff --git a/src/ssl.c b/src/ssl.c index c493bcf5d2..cb23542d7d 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -13198,9 +13198,15 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) FreeDer(&ssl->buffers.certChain); ssl->buffers.weOwnCertChain = 0; ssl->buffers.certificate = ctx->certificate; + if (!RefDer(ssl->buffers.certificate)) { + ssl->buffers.certificate = NULL; + return NULL; + } ssl->buffers.certChain = ctx->certChain; - RefDer(ssl->buffers.certificate); - RefDer(ssl->buffers.certChain); + if (!RefDer(ssl->buffers.certChain)) { + ssl->buffers.certChain = NULL; + return NULL; + } #endif #ifdef WOLFSSL_TLS13 ssl->buffers.certChainCnt = ctx->certChainCnt; diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 0f92e3cbdf..fd7290145c 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -3639,9 +3639,10 @@ int test_tls13_pha(void) static int test_cert_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg) { WOLFSSL_CTX* ctx = wolfSSL_get_SSL_CTX(ssl); + int* cbRet = (int*)arg; (void)ad; - (void)arg; - (void)wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, + /* Feed the reload result back to the parent function below. */ + *cbRet = wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, WOLFSSL_FILETYPE_PEM); /* 0 means ack the servername and continue the handshake. */ return 0; @@ -3659,6 +3660,7 @@ int test_tls13_cert_alias_uaf_sni(void) WOLFSSL *ssl_c = NULL, *ssl_s = NULL; struct test_memio_ctx test_ctx; const char* host = "example.com"; + int cbRet = WOLFSSL_FATAL_ERROR; XMEMSET(&test_ctx, 0, sizeof(test_ctx)); /* Server cert/key are loaded on ctx_s, so ssl_s->buffers.certificate @@ -3668,6 +3670,7 @@ int test_tls13_cert_alias_uaf_sni(void) /* Server swaps its CTX certificate when the SNI arrives. */ wolfSSL_CTX_set_servername_callback(ctx_s, test_cert_alias_sni_cb); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, &cbRet), WOLFSSL_SUCCESS); /* Client offers SNI so the server callback fires while parsing the * ClientHello. */ @@ -3677,6 +3680,8 @@ int test_tls13_cert_alias_uaf_sni(void) /* The callback loads a new certificate on the server CTX, which is * aliased by the SSL. The handshake must complete without a UAF. */ ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* Fail the test if the callback's cert reload failed or didn't run. */ + ExpectIntEQ(cbRet, WOLFSSL_SUCCESS); wolfSSL_free(ssl_c); wolfSSL_free(ssl_s); diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index b17bf3fa56..f44fb7c975 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -24708,13 +24708,22 @@ int AllocDer(DerBuffer** pDer, word32 length, int type, void* heap) return ret; } -void RefDer(DerBuffer* der) +/* Increments the reference count on a shared DER buffer. + * + * A mutex-backed refcount can fail to lock, leaving the count un-incremented; + * return failure so callers don't proceed with an un-referenced alias. + * + * @param [in, out] der DER buffer. May be NULL. + * @return 1 on success + * @return 0 on error + */ +int RefDer(DerBuffer* der) { + int err = 0; if (der != NULL) { - int ref_err = 0; - wolfSSL_RefInc(&der->ref, &ref_err); - (void)ref_err; + wolfSSL_RefInc(&der->ref, &err); } + return !err; } int AllocCopyDer(DerBuffer** pDer, const unsigned char* buff, word32 length, diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 91778564c1..64aceaac5a 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2811,7 +2811,7 @@ WOLFSSL_API int AllocDer(DerBuffer** der, word32 length, int type, WOLFSSL_LOCAL int AllocCopyDer(DerBuffer** der, const unsigned char* buff, word32 length, int type, void* heap); WOLFSSL_API void FreeDer(DerBuffer** der); -WOLFSSL_LOCAL void RefDer(DerBuffer* der); +WOLFSSL_LOCAL int RefDer(DerBuffer* der); #ifdef WOLFSSL_ASN_PARSE_KEYUSAGE WOLFSSL_LOCAL int ParseKeyUsageStr(const char* value, word16* keyUsage, From 0b2304fedb712b3f393f1a85a2e0a6c82ecf8dbc Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 8 Jul 2026 22:17:15 -0700 Subject: [PATCH 4/5] Fix coding-no test --- tests/api/test_tls13.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index fd7290145c..393c9db425 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -3642,8 +3642,7 @@ static int test_cert_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg) int* cbRet = (int*)arg; (void)ad; /* Feed the reload result back to the parent function below. */ - *cbRet = wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, - WOLFSSL_FILETYPE_PEM); + *cbRet = wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, CERT_FILETYPE); /* 0 means ack the servername and continue the handshake. */ return 0; } From 160f72f81e94af1812d2340cf3501e32ec201753 Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 8 Jul 2026 22:17:52 -0700 Subject: [PATCH 5/5] Add refcounts on key, keyMask, altKey, altKeyMask to prevent UAF --- src/internal.c | 28 ++++++++++++++++-- src/ssl.c | 40 +++++++++++++------------ src/ssl_load.c | 35 ++++++++++------------ src/tls13.c | 25 +++++++++++----- tests/api/test_tls13.c | 67 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 2 ++ 6 files changed, 150 insertions(+), 47 deletions(-) diff --git a/src/internal.c b/src/internal.c index f65681f39b..59e3efe992 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7203,6 +7203,12 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) } #else ssl->buffers.key = ctx->privateKey; + /* Hold a reference on the shared key so reloading it on the CTX cannot + * free it while this SSL still aliases it. */ + if (!RefDer(ssl->buffers.key)) { + ssl->buffers.key = NULL; + return BAD_MUTEX_E; + } #endif #else if (ctx->privateKey != NULL) { @@ -7233,6 +7239,11 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) #ifdef WOLFSSL_DUAL_ALG_CERTS #ifndef WOLFSSL_BLIND_PRIVATE_KEY ssl->buffers.altKey = ctx->altPrivateKey; + /* Hold a reference on the shared alternative key. */ + if (!RefDer(ssl->buffers.altKey)) { + ssl->buffers.altKey = NULL; + return BAD_MUTEX_E; + } #else if (ctx->altPrivateKey != NULL) { ret = AllocCopyDer(&ssl->buffers.altKey, ctx->altPrivateKey->buffer, @@ -9226,12 +9237,25 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl) #ifndef NO_CERTS ssl->keepCert = 0; /* make sure certificate is free'd */ wolfSSL_UnloadCertsKeys(ssl); - /* Release references on cert buffers aliased from the context. Owned - * buffers were already freed and cleared by wolfSSL_UnloadCertsKeys. */ + /* Release references on cert and key buffers aliased from the context. + * Owned buffers were already freed and cleared by + * wolfSSL_UnloadCertsKeys. */ FreeDer(&ssl->buffers.certificate); ssl->buffers.weOwnCert = 0; FreeDer(&ssl->buffers.certChain); ssl->buffers.weOwnCertChain = 0; + FreeDer(&ssl->buffers.key); + ssl->buffers.weOwnKey = 0; +#ifdef WOLFSSL_BLIND_PRIVATE_KEY + FreeDer(&ssl->buffers.keyMask); +#endif +#ifdef WOLFSSL_DUAL_ALG_CERTS + FreeDer(&ssl->buffers.altKey); + ssl->buffers.weOwnAltKey = 0; +#ifdef WOLFSSL_BLIND_PRIVATE_KEY + FreeDer(&ssl->buffers.altKeyMask); +#endif +#endif /* WOLFSSL_DUAL_ALG_CERTS */ #endif #ifndef NO_RSA FreeKey(ssl, DYNAMIC_TYPE_RSA, (void**)&ssl->peerRsaKey); diff --git a/src/ssl.c b/src/ssl.c index cb23542d7d..abca84d999 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12734,16 +12734,11 @@ void wolfSSL_certs_clear(WOLFSSL* ssl) #ifdef WOLFSSL_TLS13 ssl->buffers.certChainCnt = 0; #endif - if (ssl->buffers.weOwnKey) { - FreeDer(&ssl->buffers.key); - #ifdef WOLFSSL_BLIND_PRIVATE_KEY - FreeDer(&ssl->buffers.keyMask); - #endif - ssl->buffers.weOwnKey = 0; - } - ssl->buffers.key = NULL; + /* Release our reference to the key (owned copy or CTX alias). */ + FreeDer(&ssl->buffers.key); + ssl->buffers.weOwnKey = 0; #ifdef WOLFSSL_BLIND_PRIVATE_KEY - ssl->buffers.keyMask = NULL; + FreeDer(&ssl->buffers.keyMask); #endif ssl->buffers.keyType = 0; ssl->buffers.keyId = 0; @@ -12751,16 +12746,11 @@ void wolfSSL_certs_clear(WOLFSSL* ssl) ssl->buffers.keySz = 0; ssl->buffers.keyDevId = 0; #ifdef WOLFSSL_DUAL_ALG_CERTS - if (ssl->buffers.weOwnAltKey) { - FreeDer(&ssl->buffers.altKey); - #ifdef WOLFSSL_BLIND_PRIVATE_KEY - FreeDer(&ssl->buffers.altKeyMask); - #endif - ssl->buffers.weOwnAltKey = 0; - } - ssl->buffers.altKey = NULL; + /* Release our reference to the alternative key (owned copy or CTX alias). */ + FreeDer(&ssl->buffers.altKey); + ssl->buffers.weOwnAltKey = 0; #ifdef WOLFSSL_BLIND_PRIVATE_KEY - ssl->buffers.altKeyMask = NULL; + FreeDer(&ssl->buffers.altKeyMask); #endif #endif /* WOLFSSL_DUAL_ALG_CERTS */ } @@ -13230,7 +13220,14 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) ssl->buffers.key = ctx->privateKey; } #else + /* Release our previous key reference before aliasing the new CTX's. */ + FreeDer(&ssl->buffers.key); + ssl->buffers.weOwnKey = 0; ssl->buffers.key = ctx->privateKey; + if (!RefDer(ssl->buffers.key)) { + ssl->buffers.key = NULL; + return NULL; + } #endif #else if (ctx->privateKey != NULL) { @@ -13267,7 +13264,14 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) ssl->options.haveMlDsaSig = ctx->haveMlDsaSig; #ifdef WOLFSSL_DUAL_ALG_CERTS #ifndef WOLFSSL_BLIND_PRIVATE_KEY + /* Release our previous alt key reference before aliasing the new CTX's. */ + FreeDer(&ssl->buffers.altKey); + ssl->buffers.weOwnAltKey = 0; ssl->buffers.altKey = ctx->altPrivateKey; + if (!RefDer(ssl->buffers.altKey)) { + ssl->buffers.altKey = NULL; + return NULL; + } #else if (ctx->altPrivateKey != NULL) { ret = AllocCopyDer(&ssl->buffers.altKey, ctx->altPrivateKey->buffer, diff --git a/src/ssl_load.c b/src/ssl_load.c index 5161f69725..223613b2e2 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -1317,13 +1317,12 @@ static int ProcessBufferPrivKeyHandleDer(WOLFSSL_CTX* ctx, WOLFSSL* ssl, else #endif /* WOLFSSL_DUAL_ALG_CERTS */ if (ssl != NULL) { - /* Dispose of previous key if not context's. */ - if (ssl->buffers.weOwnKey) { - FreeDer(&ssl->buffers.key); - #ifdef WOLFSSL_BLIND_PRIVATE_KEY - FreeDer(&ssl->buffers.keyMask); - #endif - } + /* Release our reference to the previous key (owned copy or CTX + * alias) before storing the new one. */ + FreeDer(&ssl->buffers.key); + #ifdef WOLFSSL_BLIND_PRIVATE_KEY + FreeDer(&ssl->buffers.keyMask); + #endif ssl->buffers.keyId = 0; ssl->buffers.keyLabel = 0; ssl->buffers.keyDevId = INVALID_DEVID; @@ -4621,12 +4620,11 @@ int wolfSSL_use_PrivateKey_Id(WOLFSSL* ssl, const unsigned char* id, } /* Dispose of old private key if owned and allocate and copy in id. */ - if (ssl->buffers.weOwnKey) { - FreeDer(&ssl->buffers.key); - #ifdef WOLFSSL_BLIND_PRIVATE_KEY - FreeDer(&ssl->buffers.keyMask); - #endif - } + /* Release our reference to the old key (owned copy or CTX alias). */ + FreeDer(&ssl->buffers.key); +#ifdef WOLFSSL_BLIND_PRIVATE_KEY + FreeDer(&ssl->buffers.keyMask); +#endif if (AllocCopyDer(&ssl->buffers.key, id, (word32)sz, PRIVATEKEY_TYPE, ssl->heap) != 0) { ret = 0; @@ -4697,12 +4695,11 @@ int wolfSSL_use_PrivateKey_Label(WOLFSSL* ssl, const char* label, int devId) sz = (word32)XSTRLEN(label) + 1; /* Dispose of old private key if owned and allocate and copy in label. */ - if (ssl->buffers.weOwnKey) { - FreeDer(&ssl->buffers.key); - #ifdef WOLFSSL_BLIND_PRIVATE_KEY - FreeDer(&ssl->buffers.keyMask); - #endif - } + /* Release our reference to the old key (owned copy or CTX alias). */ + FreeDer(&ssl->buffers.key); +#ifdef WOLFSSL_BLIND_PRIVATE_KEY + FreeDer(&ssl->buffers.keyMask); +#endif if (AllocCopyDer(&ssl->buffers.key, (const byte*)label, (word32)sz, PRIVATEKEY_TYPE, ssl->heap) != 0) { ret = 0; diff --git a/src/tls13.c b/src/tls13.c index a20b6ec9e2..a1646985c9 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -10142,20 +10142,29 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl) } ssl->buffers.keyType = ssl->buffers.altKeyType; ssl->buffers.keySz = ssl->buffers.altKeySz; - /* If we own it, free key before overriding it. */ - if (ssl->buffers.weOwnKey) { - FreeDer(&ssl->buffers.key); - #ifdef WOLFSSL_BLIND_PRIVATE_KEY - FreeDer(&ssl->buffers.keyMask); - #endif - } + /* Release our reference to the current key before + * overriding it (owned copy or CTX alias). */ + FreeDer(&ssl->buffers.key); + #ifdef WOLFSSL_BLIND_PRIVATE_KEY + FreeDer(&ssl->buffers.keyMask); + #endif - /* Swap keys */ + /* Swap keys. key and altKey now reference the same buffer, + * so take a reference for the new alias. */ ssl->buffers.key = ssl->buffers.altKey; ssl->buffers.weOwnKey = ssl->buffers.weOwnAltKey; + if (!RefDer(ssl->buffers.key)) { + ssl->buffers.key = NULL; + ssl->buffers.weOwnKey = 0; + ERROR_OUT(BAD_MUTEX_E, exit_scv); + } #ifdef WOLFSSL_BLIND_PRIVATE_KEY ssl->buffers.keyMask = ssl->buffers.altKeyMask; + if (!RefDer(ssl->buffers.keyMask)) { + ssl->buffers.keyMask = NULL; + ERROR_OUT(BAD_MUTEX_E, exit_scv); + } /* Unblind the alternative key before decoding */ wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask); #endif diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 393c9db425..030e85a14b 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -3691,6 +3691,73 @@ int test_tls13_cert_alias_uaf_sni(void) } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_SNI) && !defined(WOLFSSL_COPY_KEY) && \ + !defined(WOLFSSL_BLIND_PRIVATE_KEY) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_RSA) && !defined(NO_CERTS) +/* React to the client SNI by reloading the private key on the existing CTX. + * Without WOLFSSL_COPY_KEY the key buffer is shared between the CTX and SSL, + * so freeing it on the CTX would dangle the SSL's alias. The server signs its + * CertificateVerify with that key, so a dangling alias is a heap-use-after-free + * on the signing path; the handshake must still complete. */ +static int test_key_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg) +{ + WOLFSSL_CTX* ctx = wolfSSL_get_SSL_CTX(ssl); + int* cbRet = (int*)arg; + (void)ad; + /* Feed the reload result back to the parent function below. */ + *cbRet = wolfSSL_CTX_use_PrivateKey_file(ctx, svrKeyFile, CERT_FILETYPE); + /* 0 means ack the servername and continue the handshake. */ + return 0; +} +#endif + +int test_tls13_key_alias_uaf_sni(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_SNI) && !defined(WOLFSSL_COPY_KEY) && \ + !defined(WOLFSSL_BLIND_PRIVATE_KEY) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_RSA) && !defined(NO_CERTS) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + const char* host = "example.com"; + int cbRet = WOLFSSL_FATAL_ERROR; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + /* Server cert/key are loaded on ctx_s, so ssl_s->buffers.key aliases + * ctx_s->privateKey. */ + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + + /* Server swaps its CTX private key when the SNI arrives. */ + wolfSSL_CTX_set_servername_callback(ctx_s, test_key_alias_sni_cb); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, &cbRet), WOLFSSL_SUCCESS); + + /* Client offers SNI so the server callback fires while parsing the + * ClientHello. */ + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, host, + (word16)XSTRLEN(host)), WOLFSSL_SUCCESS); + + /* The callback frees the aliased key mid-handshake; the server then signs + * CertificateVerify with ssl->buffers.key. The handshake must complete + * without a UAF. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* Fail the test if the callback's key reload failed or didn't run. */ + ExpectIntEQ(cbRet, WOLFSSL_SUCCESS); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + + #if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) && \ !defined(WOLFSSL_MLKEM_NO_DECAPSULATE) && \ diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index 876f03efa0..563cdd8bf9 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -31,6 +31,7 @@ int test_tls13_rpk_handshake(void); int test_tls13_rpk_handshake_no_negotiation(void); int test_tls13_pha(void); int test_tls13_cert_alias_uaf_sni(void); +int test_tls13_key_alias_uaf_sni(void); int test_tls13_pq_groups(void); int test_tls13_multi_pqc_key_share(void); int test_tls13_early_data(void); @@ -110,6 +111,7 @@ int test_tls13_pqc_hybrid_async_server(void); TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake_no_negotiation), \ TEST_DECL_GROUP("tls13", test_tls13_pha), \ TEST_DECL_GROUP("tls13", test_tls13_cert_alias_uaf_sni), \ + TEST_DECL_GROUP("tls13", test_tls13_key_alias_uaf_sni), \ TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \ TEST_DECL_GROUP("tls13", test_tls13_multi_pqc_key_share), \ TEST_DECL_GROUP("tls13", test_tls13_early_data), \