From 22985984d1f29cc9c049c6f7ae37402ed02bdea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 27 Jul 2026 19:40:02 +0200 Subject: [PATCH 01/12] Clamp CBC pad length before deriving the MAC input length TimingPadVerify passes (pLen - macSz - padLen - 1) to ssl->hmac and relies on the callee recovering the record length by modular addition. TLS_hmac now does that addition with overflow checking and returns BUFFER_E before hashing anything, so a record whose padding length byte exceeds pLen - macSz - 1 is rejected without a MAC being computed at all, while a smaller padding byte gets the full constant time HMAC. The padding length byte is taken straight from the decrypted record, so this hands an attacker a Lucky13 style timing oracle worth an entire HMAC. Clamp the padding length in constant time before it is used, so the length handed to ssl->hmac never wraps and every value of the padding length byte results in the same amount of hashing. The rejection decision is unchanged, since MaskPadding already flags an out of range padding length. The overflow check in TLS_hmac stays as a backstop for genuinely bogus sizes. Add a regression test that drives TimingPadVerify over every padding length byte with a recording MAC callback and asserts the callback is always invoked with a length that does not wrap. Fixes F-7240. --- src/internal.c | 12 +++--- tests/api/test_hmac.c | 85 +++++++++++++++++++++++++++++++++++++++++++ tests/api/test_hmac.h | 2 + 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 838fb57cd21..592cdd0bc53 100644 --- a/src/internal.c +++ b/src/internal.c @@ -22730,11 +22730,13 @@ int TimingPadVerify(WOLFSSL* ssl, const byte* input, int padLen, int macSz, XMEMSET(verify, 0, WC_MAX_DIGEST_SIZE); good = MaskPadding(input, pLen, macSz); - /* 4th argument has potential to underflow, ssl->hmac function should - * either increment the size by (macSz + padLen + 1) before use or check on - * the size to make sure is valid. */ - ret = ssl->hmac(ssl, verify, input, (word32)(pLen - macSz - padLen - 1), padLen, - content, 1, PEER_ORDER); + /* An out of range padding length byte is already recorded in good, but the + * length handed to ssl->hmac must not underflow. Clamp it in constant time + * so that the same amount of hashing is done for every value of the + * padding length byte. */ + padLen &= ctMaskIntGTE(pLen - macSz - 1, padLen); + ret = ssl->hmac(ssl, verify, input, (word32)(pLen - macSz - padLen - 1), + padLen, content, 1, PEER_ORDER); good |= MaskMac(input, pLen, ssl->specs.hash_size, verify); /* Non-zero on failure. */ diff --git a/tests/api/test_hmac.c b/tests/api/test_hmac.c index 7c80f5e0158..393bb2771f5 100644 --- a/tests/api/test_hmac.c +++ b/tests/api/test_hmac.c @@ -784,6 +784,91 @@ int test_tls_hmac_size_overflow(void) return EXPECT_RESULT(); } /* END test_tls_hmac_size_overflow */ +/* TimingPadVerify is internal to the library, so this only links in a static + * build. */ +#if defined(WOLFSSL_TEST_STATIC_BUILD) && !defined(NO_HMAC) && \ + !defined(WOLFSSL_AEAD_ONLY) && !defined(NO_TLS) && \ + !defined(WOLFSSL_NO_TLS12) && !defined(WOLFSSL_OLD_TIMINGPADVERIFY) && \ + !defined(NO_SHA256) && !defined(NO_WOLFSSL_CLIENT) + +static int tpvHmacCalls; +static word32 tpvHmacSz; +static int tpvHmacPadSz; + +/* Record the length arguments TimingPadVerify hands to the MAC callback. */ +static int TpvRecordHmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, + int padSz, int content, int verify, int epochOrder) +{ + (void)ssl; + (void)digest; + (void)in; + (void)content; + (void)verify; + (void)epochOrder; + + tpvHmacCalls++; + tpvHmacSz = sz; + tpvHmacPadSz = padSz; + + return 0; +} +#endif + +/* The constant time CBC verify path must do the same amount of MAC work for + * every value of the attacker controlled padding length byte. TimingPadVerify + * passes (pLen - macSz - padLen - 1) to the MAC callback, so the padding length + * has to be clamped before that subtraction wraps around. A wrapped length + * makes TLS_hmac reject the record before hashing anything, which is a Lucky13 + * style oracle. */ +int test_tls_timing_pad_verify_hmac_len(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_TEST_STATIC_BUILD) && !defined(NO_HMAC) && \ + !defined(WOLFSSL_AEAD_ONLY) && !defined(NO_TLS) && \ + !defined(WOLFSSL_NO_TLS12) && !defined(WOLFSSL_OLD_TIMINGPADVERIFY) && \ + !defined(NO_SHA256) && !defined(NO_WOLFSSL_CLIENT) + WOLFSSL_CTX* ctx = NULL; + WOLFSSL* ssl = NULL; + byte record[48]; + int macSz = WC_SHA256_DIGEST_SIZE; + int pLen = (int)sizeof(record); + int pad; + + XMEMSET(record, 0, sizeof(record)); + + ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()); + ExpectNotNull(ctx); + ssl = wolfSSL_new(ctx); + ExpectNotNull(ssl); + + if (EXPECT_SUCCESS()) { + ssl->specs.hash_size = WC_SHA256_DIGEST_SIZE; + ssl->hmac = TpvRecordHmac; + + for (pad = 0; (pad < 256) && EXPECT_SUCCESS(); pad++) { + record[pLen - 1] = (byte)pad; + tpvHmacCalls = 0; + tpvHmacSz = 0; + tpvHmacPadSz = 0; + + (void)TimingPadVerify(ssl, record, pad, macSz, pLen, + application_data); + + /* The MAC is always computed, over the whole record. */ + ExpectIntEQ(tpvHmacCalls, 1); + ExpectTrue(tpvHmacSz <= (word32)pLen); + ExpectIntEQ((int)tpvHmacSz + macSz + tpvHmacPadSz + 1, pLen); + } + } + + wolfSSL_free(ssl); + wolfSSL_CTX_free(ctx); +#endif /* WOLFSSL_TEST_STATIC_BUILD && !NO_HMAC && !WOLFSSL_AEAD_ONLY && + * !NO_TLS && !WOLFSSL_NO_TLS12 && !WOLFSSL_OLD_TIMINGPADVERIFY && + * !NO_SHA256 && !NO_WOLFSSL_CLIENT */ + return EXPECT_RESULT(); +} /* END test_tls_timing_pad_verify_hmac_len */ + /* * MC/DC: wc_HmacSizeByType() has its own physical copy of the "which hash * type" compound guard (a second, separately-tracked copy of the same- diff --git a/tests/api/test_hmac.h b/tests/api/test_hmac.h index b5d8ddfc959..f6383d477f2 100644 --- a/tests/api/test_hmac.h +++ b/tests/api/test_hmac.h @@ -40,6 +40,7 @@ int test_wc_Sha384HmacSetKey(void); int test_wc_Sha384HmacUpdate(void); int test_wc_Sha384HmacFinal(void); int test_tls_hmac_size_overflow(void); +int test_tls_timing_pad_verify_hmac_len(void); int test_wc_HmacSizeByType(void); int test_wc_HmacCopy(void); int test_wc_HmacInit_Id(void); @@ -64,6 +65,7 @@ int test_wc_HKDF_NullKeyEdgeCases(void); TEST_DECL_GROUP("hmac", test_wc_Sha384HmacUpdate), \ TEST_DECL_GROUP("hmac", test_wc_Sha384HmacFinal), \ TEST_DECL_GROUP("hmac", test_tls_hmac_size_overflow), \ + TEST_DECL_GROUP("hmac", test_tls_timing_pad_verify_hmac_len), \ TEST_DECL_GROUP("hmac", test_wc_HmacSizeByType), \ TEST_DECL_GROUP("hmac", test_wc_HmacCopy), \ TEST_DECL_GROUP("hmac", test_wc_HmacInit_Id), \ From 6e4a2429e6a024c6fb9756adc6abba776e2dc32a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 13:09:07 +0200 Subject: [PATCH 02/12] Take the write lock when setting the pending DTLS peer wolfSSL_dtls_set_pending_peer acquired the DTLS peer lock for reading and then mutated the shared state under it. It frees and clears pendingPeer, calls SockAddrSet which frees and reallocates the address buffer, and writes processingPendingRecord. A read lock allows several holders at once, so two threads in this function can both free pendingPeer.sa, and a reader in wolfSSL_dtls_get_peer or the wolfio send path can observe a dangling pointer. Acquire the lock for writing instead, matching wolfSSL_dtls_set_peer, which already does so for the same class of mutation. Only affects builds that define WOLFSSL_RW_THREADED, since the locking is compiled out otherwise. Fixes F-7222. --- src/dtls.c | 20 ++++++++++++++++ src/internal.c | 59 ++++++++++++++++++++++++++++++++++++++++++---- src/ssl_api_dtls.c | 21 +++++++++++------ wolfssl/internal.h | 4 ++++ 4 files changed, 92 insertions(+), 12 deletions(-) diff --git a/src/dtls.c b/src/dtls.c index eb5308103b9..e86793dba97 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -66,6 +66,11 @@ void DtlsResetState(WOLFSSL* ssl) { +#if defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_CID) && \ + defined(WOLFSSL_RW_THREADED) + int locked; +#endif + /* Reset the state so that we can statelessly await the * ClientHello that contains the cookie. Don't gate on IsAtLeastTLSv1_3 * to handle the edge case when the peer wants a lower version. */ @@ -98,6 +103,17 @@ void DtlsResetState(WOLFSSL* ssl) ssl->options.tls1_1 = 0; ssl->options.tls1_3 = 0; #if defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_CID) +#ifdef WOLFSSL_RW_THREADED + /* wolfSSL_dtls_set_pending_peer() may run on another thread, so take the + * lock the record layer uses for these two fields before dropping what + * that call left behind. Callers must not already hold peerLock: + * re-acquiring a write lock is undefined and deadlocks rather than fails, + * so the only failure this can report is a broken or uninitialised lock. + * Clear the fields anyway in that case, since resetting the state is the + * whole point of this call and leaving a pending peer behind is worse than + * the race. dtlsProcessPendingPeer() and ProcessReplyEx() do the same. */ + locked = (wc_LockRwLock_Wr(&ssl->buffers.dtlsCtx.peerLock) == 0); +#endif ssl->buffers.dtlsCtx.processingPendingRecord = 0; /* Clear the pending peer in case user set */ XFREE(ssl->buffers.dtlsCtx.pendingPeer.sa, ssl->heap, @@ -105,6 +121,10 @@ void DtlsResetState(WOLFSSL* ssl) ssl->buffers.dtlsCtx.pendingPeer.sa = NULL; ssl->buffers.dtlsCtx.pendingPeer.sz = 0; ssl->buffers.dtlsCtx.pendingPeer.bufSz = 0; +#ifdef WOLFSSL_RW_THREADED + if (locked) + (void)wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock); +#endif #endif } diff --git a/src/internal.c b/src/internal.c index 592cdd0bc53..9708c7f7db6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -23737,6 +23737,16 @@ static int dtlsRecordIsNewest(WOLFSSL* ssl) */ static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected, int isNewest) { +#ifdef WOLFSSL_RW_THREADED + int locked; + + /* A failure here means the lock itself is broken, not that another holder + * is in the way, so carry on regardless: this bookkeeping is the caller's + * own and leaving it stale is worse than the race. Only the promotion into + * dtlsCtx.peer below is skipped, since EmbedSendTo reads that buffer + * without the lock. DtlsResetState() and ProcessReplyEx() do the same. */ + locked = (wc_LockRwLock_Wr(&ssl->buffers.dtlsCtx.peerLock) == 0); +#endif if (ssl->buffers.dtlsCtx.pendingPeer.sa != NULL) { if (!deprotected) { /* Here we have just read an entire record from the network. It is @@ -23752,11 +23762,30 @@ static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected, int isNewest) !ssl->buffers.dtlsCtx.processingPendingRecord; } else { - /* Pending peer present and record deprotected. Update the peer. */ - if (isNewest) { - (void)wolfSSL_dtls_set_peer(ssl, - ssl->buffers.dtlsCtx.pendingPeer.sa, - ssl->buffers.dtlsCtx.pendingPeer.sz); + /* Pending peer present and record deprotected. Promote it here + * rather than through wolfSSL_dtls_set_peer, which would take this + * same lock again. */ + if (isNewest + #ifdef WOLFSSL_RW_THREADED + /* Only this touches the buffer the send path reads + * without the lock, so it is the one thing a failed + * acquisition has to skip. */ + && locked + #endif + ) { + WOLFSSL_SOCKADDR* from = &ssl->buffers.dtlsCtx.pendingPeer; + + if (wolfssl_local_SockAddrSet(&ssl->buffers.dtlsCtx.peer, + from->sa, from->sz, ssl->heap) == WOLFSSL_SUCCESS) { + ssl->buffers.dtlsCtx.userSet = 1; + } + else { + /* wolfSSL_dtls_set_peer clears this when it fails to store + * a peer, and the receive path only checks the sender + * while peer.sz is non zero, so leaving it set would drop + * the check rather than fail safe. */ + ssl->buffers.dtlsCtx.userSet = 0; + } } ssl->buffers.dtlsCtx.processingPendingRecord = 0; dtlsClearPeer(&ssl->buffers.dtlsCtx.pendingPeer); @@ -23765,6 +23794,10 @@ static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected, int isNewest) else { ssl->buffers.dtlsCtx.processingPendingRecord = 0; } +#ifdef WOLFSSL_RW_THREADED + if (locked) + (void)wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock); +#endif } #endif static int DoDecrypt(WOLFSSL *ssl) @@ -25104,6 +25137,10 @@ int ProcessReply(WOLFSSL* ssl) int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) { int ret; +#if defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_CID) && \ + defined(WOLFSSL_RW_THREADED) + int locked; +#endif ret = DoProcessReplyEx(ssl, allowSocketErr); @@ -25116,8 +25153,20 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) && ret != WC_NO_ERR_TRACE(WC_PENDING_E) #endif ) { + #ifdef WOLFSSL_RW_THREADED + /* Drop the pending peer even when the lock cannot be taken, as + * DtlsResetState() and dtlsProcessPendingPeer() do. A failure here + * means the lock is broken rather than held, and nothing below + * touches dtlsCtx.peer, which is the buffer the send path reads + * without this lock. */ + locked = (wc_LockRwLock_Wr(&ssl->buffers.dtlsCtx.peerLock) == 0); + #endif dtlsClearPeer(&ssl->buffers.dtlsCtx.pendingPeer); ssl->buffers.dtlsCtx.processingPendingRecord = 0; + #ifdef WOLFSSL_RW_THREADED + if (locked) + (void)wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock); + #endif } } #endif diff --git a/src/ssl_api_dtls.c b/src/ssl_api_dtls.c index b8e506b5ab2..6cc9151d943 100644 --- a/src/ssl_api_dtls.c +++ b/src/ssl_api_dtls.c @@ -115,7 +115,13 @@ int wolfSSL_dtls_free_peer(void* addr) #ifdef WOLFSSL_DTLS /* Store a socket address into a socket address holder, resizing as needed. * - * A NULL or zero-length peer frees the holder's buffer. + * A NULL or zero-length peer frees the holder's buffer. An address that fits + * the buffer already there is copied over it rather than reallocated: + * EmbedSendTo reads peer.sa without taking peerLock, so freeing it on every + * update would widen that race rather than leave it as it is. + * + * The record layer promotes a pending peer through this while already holding + * peerLock, so it takes no lock of its own. * * @param [in, out] sockAddr Socket address holder. * @param [in] peer Socket address data, may be NULL to free. @@ -124,8 +130,8 @@ int wolfSSL_dtls_free_peer(void* addr) * @return WOLFSSL_SUCCESS on success. * @return WOLFSSL_FAILURE on allocation error. */ -static int SockAddrSet(WOLFSSL_SOCKADDR* sockAddr, void* peer, - unsigned int peerSz, void* heap) +int wolfssl_local_SockAddrSet(WOLFSSL_SOCKADDR* sockAddr, void* peer, + unsigned int peerSz, void* heap) { if (peer == NULL || peerSz == 0) { if (sockAddr->sa != NULL) @@ -174,7 +180,8 @@ int wolfSSL_dtls_set_peer(WOLFSSL* ssl, void* peer, unsigned int peerSz) if (wc_LockRwLock_Wr(&ssl->buffers.dtlsCtx.peerLock) != 0) return WOLFSSL_FAILURE; #endif - ret = SockAddrSet(&ssl->buffers.dtlsCtx.peer, peer, peerSz, ssl->heap); + ret = wolfssl_local_SockAddrSet(&ssl->buffers.dtlsCtx.peer, peer, peerSz, + ssl->heap); if (ret == WOLFSSL_SUCCESS && !(peer == NULL || peerSz == 0)) ssl->buffers.dtlsCtx.userSet = 1; else @@ -212,7 +219,7 @@ int wolfSSL_dtls_set_pending_peer(WOLFSSL* ssl, void* peer, unsigned int peerSz) if (ssl == NULL) return WOLFSSL_FAILURE; #ifdef WOLFSSL_RW_THREADED - if (wc_LockRwLock_Rd(&ssl->buffers.dtlsCtx.peerLock) != 0) + if (wc_LockRwLock_Wr(&ssl->buffers.dtlsCtx.peerLock) != 0) return WOLFSSL_FAILURE; #endif if (ssl->buffers.dtlsCtx.peer.sa != NULL && @@ -232,8 +239,8 @@ int wolfSSL_dtls_set_pending_peer(WOLFSSL* ssl, void* peer, unsigned int peerSz) ret = WOLFSSL_SUCCESS; } else { - ret = SockAddrSet(&ssl->buffers.dtlsCtx.pendingPeer, peer, peerSz, - ssl->heap); + ret = wolfssl_local_SockAddrSet(&ssl->buffers.dtlsCtx.pendingPeer, + peer, peerSz, ssl->heap); } if (ret == WOLFSSL_SUCCESS) ssl->buffers.dtlsCtx.processingPendingRecord = 0; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index da08ac117a7..f34e6ab3d07 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -7285,6 +7285,10 @@ WOLFSSL_LOCAL word32 MacSize(const WOLFSSL* ssl); word32 fragOffset); WOLFSSL_LOCAL int VerifyForTxDtlsMsgDelete(WOLFSSL* ssl, DtlsMsg* item); WOLFSSL_LOCAL void DtlsMsgPoolReset(WOLFSSL* ssl); + WOLFSSL_LOCAL int wolfssl_local_SockAddrSet(WOLFSSL_SOCKADDR* sockAddr, + void* peer, + unsigned int peerSz, + void* heap); WOLFSSL_LOCAL int DtlsMsgPoolSend(WOLFSSL* ssl, int sendOnlyFirstPacket); WOLFSSL_LOCAL void DtlsMsgDestroyFragBucket(DtlsFragBucket* fragBucket, void* heap); WOLFSSL_LOCAL int GetDtlsHandShakeHeader(WOLFSSL *ssl, const byte *input, From 2788d9dac1ed07f77142a0126ac08af62584db14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 13:45:59 +0200 Subject: [PATCH 03/12] Use the stored entry length when printing a directoryName X509PrintDirType derived the length of the alt name payload with XSTRLEN. A directoryName entry holds raw DER, which routinely contains zero bytes, and under WC_ASN_NO_HEAP the buffer is not NUL terminated at all, so the computed length was wrong in general and reading it was already out of bounds in the no-heap case. The length then fed an unsigned loop bound of src_len - 5, so any entry that XSTRLEN measured as shorter than five bytes wrapped the bound to nearly UINT32_MAX and the tag scan read far past the end of the entry. A certificate with a short or empty directoryName alt name reaches this from the ordinary certificate printing path. Take the length from entry->len, which the parser already fills in, and return early when the entry is too short to hold an OID, a tag and a length. The scan is unchanged for entries of five bytes or more, and the ASN parsing helpers now receive the true buffer length as their bound. Add a regression test that prints a certificate carrying a directoryName with an embedded zero byte before the common name, and one carrying a directoryName too short for the tag scan. Fixes F-7223. --- src/x509.c | 22 ++++++++++++++--- tests/api.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/src/x509.c b/src/x509.c index 3842b2feb60..b98909bd87f 100644 --- a/src/x509.c +++ b/src/x509.c @@ -6859,7 +6859,7 @@ static int X509PrintDirType(char * dst, int max_len, const DNS_entry * entry) word32 k = 0; word32 i = 0; const char * src = entry->name; - word32 src_len = (word32)XSTRLEN(src); + word32 src_len = 0; int total_len = 0; int bytes_left = max_len; int fld_len = 0; @@ -6867,14 +6867,24 @@ static int X509PrintDirType(char * dst, int max_len, const DNS_entry * entry) XMEMSET(dst, 0, max_len); + /* The entry holds raw DER which may contain zero bytes, and under + * WC_ASN_NO_HEAP it is not NUL terminated, so use the stored length. */ + if (entry->len > 0) { + src_len = (word32)entry->len; + } + /* loop over printable DIR tags. */ for (k = 0; k < ACERT_NUM_DIR_TAGS; ++k) { const char * pfx = acert_dir_print[k].pfx; const byte * tag = acert_dir_print[k].tag; byte asn_tag; - /* walk through entry looking for matches. */ - for (i = 0; i < src_len - 5; ++i) { + /* Walk through entry looking for matches. A match needs three bytes + * of name OID plus a tag and a length, so the last offset worth + * testing is the one with exactly five bytes left. Written as an + * addition so a short entry simply skips the loop rather than + * underflowing the bound. */ + for (i = 0; i + 5 <= src_len; ++i) { if (XMEMCMP(tag, &src[i], 3) == 0) { if (bytes_left < 5) { /* Not enough space left for name oid + tag + len. */ @@ -6994,6 +7004,12 @@ static int X509_print_name_entry(WOLFSSL_BIO* bio, } else if (entry->type == ASN_DIR_TYPE) { len = X509PrintDirType(scratch, MAX_WIDTH, entry); + if (len == 0) { + /* Nothing in the encoding was printable. Emit a placeholder, + * as the other unsupported entry types do, rather than + * failing the print of the whole certificate. */ + len = XSNPRINTF(scratch, MAX_WIDTH, "DirName:"); + } } else if (entry->type == ASN_URI_TYPE) { len = XSNPRINTF(scratch, MAX_WIDTH, "URI:%s", diff --git a/tests/api.c b/tests/api.c index 95551a6c2b8..a49a726c855 100644 --- a/tests/api.c +++ b/tests/api.c @@ -28256,6 +28256,73 @@ static int test_wolfSSL_X509_print_ext_key_usage(void) return EXPECT_RESULT(); } +static int test_wolfSSL_X509_print_dir_altname(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_FILESYSTEM) && \ + !defined(NO_RSA) && defined(XSNPRINTF) && \ + !defined(WC_DISABLE_RADIX_ZERO_PAD) && !defined(IGNORE_NAME_CONSTRAINTS) + /* A directoryName alt name holds raw DER, which routinely contains zero + * bytes. The print path must use the stored entry length rather than + * treating the DER as a NUL terminated string. */ + static const char dirName[] = { + /* countryName with an empty PrintableString, contributing a zero + * byte early in the encoding. */ + 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x00, + /* commonName "Test", which sits after that zero byte. */ + 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x04, 'T', 'e', 's', 't' + }; + /* Shorter than the five bytes the tag scan needs. */ + static const char shortDirName[] = { 0x30, 0x00 }; + X509* x509 = NULL; + BIO* bio = NULL; + char* data = NULL; + int len = 0; + char buf[8192]; + + ExpectNotNull(x509 = X509_load_certificate_file(svrCertFile, + WOLFSSL_FILETYPE_PEM)); + ExpectIntEQ(wolfSSL_X509_add_altname_ex(x509, dirName, (word32)sizeof( + dirName), ASN_DIR_TYPE), WOLFSSL_SUCCESS); + + ExpectNotNull(bio = BIO_new(BIO_s_mem())); + ExpectIntEQ(X509_print(bio, x509), SSL_SUCCESS); + /* Memory BIO data is not NUL-terminated; copy into a bounded buffer. */ + ExpectIntGT((len = BIO_get_mem_data(bio, &data)), 0); + ExpectIntLT(len, (int)sizeof(buf)); + if ((data != NULL) && (len > 0) && (len < (int)sizeof(buf))) { + XMEMCPY(buf, data, (size_t)len); + buf[len] = '\0'; + ExpectNotNull(XSTRSTR(buf, "CN=Test")); + } + BIO_free(bio); + bio = NULL; + X509_free(x509); + x509 = NULL; + + /* A directoryName too short to hold a tag must not be scanned past its + * end, and having nothing printable in it must not fail the print of the + * whole certificate. */ + ExpectNotNull(x509 = X509_load_certificate_file(svrCertFile, + WOLFSSL_FILETYPE_PEM)); + ExpectIntEQ(wolfSSL_X509_add_altname_ex(x509, shortDirName, (word32)sizeof( + shortDirName), ASN_DIR_TYPE), WOLFSSL_SUCCESS); + ExpectNotNull(bio = BIO_new(BIO_s_mem())); + ExpectIntEQ(X509_print(bio, x509), SSL_SUCCESS); + ExpectIntGT((len = BIO_get_mem_data(bio, &data)), 0); + ExpectIntLT(len, (int)sizeof(buf)); + if ((data != NULL) && (len > 0) && (len < (int)sizeof(buf))) { + XMEMCPY(buf, data, (size_t)len); + buf[len] = '\0'; + ExpectNotNull(XSTRSTR(buf, "DirName:")); + } + + BIO_free(bio); + X509_free(x509); +#endif + return EXPECT_RESULT(); +} + static int test_wolfSSL_X509_CRL_print(void) { EXPECT_DECLS; @@ -38291,6 +38358,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_X509_print), TEST_DECL(test_wolfSSL_X509_print_basic_constraints), TEST_DECL(test_wolfSSL_X509_print_ext_key_usage), + TEST_DECL(test_wolfSSL_X509_print_dir_altname), TEST_DECL(test_wolfSSL_X509_CRL_print), #endif From 96fb284398dfde9fa349e486aeac3c93e23f514f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 14:22:45 +0200 Subject: [PATCH 04/12] Honour chklen when matching an IP in X509_check_host wolfSSL_X509_check_host takes an explicit length and its own validation accepts a buffer with no NUL terminator, since only an embedded NUL is rejected and a trailing one is merely stripped when present. The iPAddress check then called CheckIPAddr, which drops the length and measures the buffer with XSTRLEN, reading past the end of a caller supplied buffer that is length delimited rather than terminated. This ran on every call, not only when checking an IP address, and is compiled in whenever WOLFSSL_IP_ALT_NAME is defined, which OPENSSL_ALL and WOLFSSL_QT enable. Call CheckHostName directly with the caller's length and the IP flag set. That is what CheckIPAddr does internally, minus the length being recomputed. Behaviour is unchanged for NUL terminated input, because the normalization above already leaves chklen equal to the string length in that case. It also fixes a matching bug, since a length delimited IP address sitting in a longer buffer no longer fails to match an iPAddress entry. Add a regression test covering an interior slice of a longer buffer and a buffer sized exactly to the name with no terminator. Fixes F-7248. --- src/x509.c | 5 ++++- tests/api/test_ossl_x509.c | 34 ++++++++++++++++++++++++++++++++++ tests/api/test_ossl_x509.h | 2 ++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/x509.c b/src/x509.c index b98909bd87f..839f4317e8d 100644 --- a/src/x509.c +++ b/src/x509.c @@ -15737,7 +15737,10 @@ int wolfSSL_X509_check_host(WOLFSSL_X509 *x, const char *chk, size_t chklen, } #ifdef WOLFSSL_IP_ALT_NAME - ret = CheckIPAddr(dCert, (char *)chk); + /* chk is length delimited and may not be NUL terminated, so check it + * against the iPAddress entries directly rather than through the + * NUL terminated CheckIPAddr helper. */ + ret = CheckHostName(dCert, (char *)chk, chklen, 0, 1); if (ret == 0) { goto out; } diff --git a/tests/api/test_ossl_x509.c b/tests/api/test_ossl_x509.c index d0d31b7ab07..b262b721422 100644 --- a/tests/api/test_ossl_x509.c +++ b/tests/api/test_ossl_x509.c @@ -435,6 +435,40 @@ int test_wolfSSL_X509_check_host(void) return EXPECT_RESULT(); } +int test_wolfSSL_X509_check_host_len(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && !defined(NO_FILESYSTEM) \ + && !defined(NO_SHA) && !defined(NO_RSA) && defined(WOLFSSL_IP_ALT_NAME) + /* chk is length delimited and need not be NUL terminated, so nothing + * past chk[chklen - 1] may be read. */ + X509* x509 = NULL; + const char sliced[] = "127.0.0.1extra"; + const size_t ipLen = 9; /* length of "127.0.0.1" */ + char* exact = NULL; + + /* cliCertFile has subjectAltName set to 'example.com', '127.0.0.1' */ + ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(cliCertFile, + SSL_FILETYPE_PEM)); + + /* An interior slice of a longer buffer must match the iPAddress SAN. */ + ExpectIntEQ(X509_check_host(x509, sliced, ipLen, 0, NULL), + WOLFSSL_SUCCESS); + + /* Same name in a buffer sized exactly to it, with no terminator. */ + ExpectNotNull(exact = (char*)XMALLOC(ipLen, NULL, DYNAMIC_TYPE_TMP_BUFFER)); + if (exact != NULL) { + XMEMCPY(exact, "127.0.0.1", ipLen); + ExpectIntEQ(X509_check_host(x509, exact, ipLen, 0, NULL), + WOLFSSL_SUCCESS); + } + XFREE(exact, NULL, DYNAMIC_TYPE_TMP_BUFFER); + + X509_free(x509); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_X509_check_email(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_x509.h b/tests/api/test_ossl_x509.h index fa6dd3a6b5f..1d3771ca5e5 100644 --- a/tests/api/test_ossl_x509.h +++ b/tests/api/test_ossl_x509.h @@ -33,6 +33,7 @@ int test_wolfSSL_i2d_X509_NAME_canon(void); int test_wolfSSL_X509_subject_name_hash(void); int test_wolfSSL_X509_issuer_name_hash(void); int test_wolfSSL_X509_check_host(void); +int test_wolfSSL_X509_check_host_len(void); int test_wolfSSL_X509_check_email(void); int test_wolfSSL_X509(void); int test_wolfSSL_X509_get0_tbs_sigalg(void); @@ -68,6 +69,7 @@ int test_wolfSSL_X509_cmp(void); TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_subject_name_hash), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_issuer_name_hash), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_check_host), \ + TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_check_host_len), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_check_email), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_get0_tbs_sigalg), \ From cb73424c31c4ea156562223a6e5a9f1ce8608e4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 15:08:52 +0200 Subject: [PATCH 05/12] Zeroize the saved HMAC pads on cleanup WOLFSSL_HMAC_CTX keeps a copy of the inner and outer pads outside the embedded wolfCrypt HMAC object so that a later init with a NULL key can restore the key. Those pads are the key combined with the fixed padding, so for any key no longer than the hash block size the key falls out of a single exclusive or. Cleanup only called wc_HmacFree on the embedded object, which zeroes what it is given but cannot reach the enclosing context, so the saved pads survived. HMAC_CTX_free then returned that heap block to the allocator with the key material still in it, where it stayed until some later allocation happened to overwrite it. Wipe both saved pads in wolfSSL_HMAC_cleanup, which HMAC_CTX_cleanup, HMAC_CTX_reset and HMAC_CTX_free all reach. Do the same on the set key failure path in the init function, since the context is reported as unkeyed there while the previous key's pads would otherwise remain. The session ticket key callback had the same leak for the same reason. It holds a WOLFSSL_HMAC_CTX on the stack, hands it to the application to be keyed with the long lived ticket HMAC key, and then only freed the embedded object, leaving the pads on the stack after every ticket encrypt and every ticket decrypt including the error paths. Have it clean up through wolfSSL_HMAC_CTX_cleanup so it picks up the wipe. Add a regression test that keys a context, checks the pads were populated, runs cleanup and requires both arrays to be zero. Fixes F-7256 and F-7257. --- src/ssl_api_ext.c | 2 +- src/ssl_crypto.c | 27 +++++++++++++++++++++ tests/api/test_ossl_mac.c | 49 +++++++++++++++++++++++++++++++++++++++ tests/api/test_ossl_mac.h | 2 ++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/ssl_api_ext.c b/src/ssl_api_ext.c index a1d98435086..a3d62a49b9f 100644 --- a/src/ssl_api_ext.c +++ b/src/ssl_api_ext.c @@ -1879,7 +1879,7 @@ static int wolfSSL_TicketKeyCb(WOLFSSL* ssl, } } - (void)wc_HmacFree(&hmacCtx.hmac); + wolfSSL_HMAC_CTX_cleanup(&hmacCtx); } (void)wolfSSL_EVP_CIPHER_CTX_cleanup(evpCtx); WC_FREE_VAR_EX(evpCtx, ssl->heap, DYNAMIC_TYPE_TMP_BUFFER); diff --git a/src/ssl_crypto.c b/src/ssl_crypto.c index 5b6b88be3d1..047110a7e8f 100644 --- a/src/ssl_crypto.c +++ b/src/ssl_crypto.c @@ -1845,6 +1845,18 @@ int wolfSSL_HMAC_Init(WOLFSSL_HMAC_CTX* ctx, const void* key, int keylen, WOLFSSL_MSG("hmac set key error"); WOLFSSL_ERROR(rc); wc_HmacFree(&ctx->hmac); + /* Context is no longer keyed, so drop any previous key's pads + * and mark it unkeyed so a later init with no key is rejected + * rather than recovering from the pads just wiped. */ + ForceZero(ctx->save_ipad, sizeof(ctx->save_ipad)); + ForceZero(ctx->save_opad, sizeof(ctx->save_opad)); + /* As in wolfSSL_HMAC_cleanup, the hash type in the wolfSSL HMAC + * object has to say unkeyed as well. wc_HmacFree only zeroes it, + * which does not read as none in every hash type enum layout. + * That alone gates the recover-from-pads path, so ctx->type keeps + * the digest the caller chose and a retry with a longer key, which + * is what the FIPS failure above asks for, still works. */ + ctx->hmac.macType = WC_HASH_TYPE_NONE; ret = 0; } if (ret == 1) { @@ -1984,6 +1996,21 @@ int wolfSSL_HMAC_cleanup(WOLFSSL_HMAC_CTX* ctx) if (ctx != NULL) { /* Free the dynamic data in the wolfSSL HMAC object. */ wc_HmacFree(&ctx->hmac); + /* The pads saved for re-init are derived from the key and live + * outside the wolfSSL HMAC object, so wipe them here. */ + ForceZero(ctx->save_ipad, sizeof(ctx->save_ipad)); + ForceZero(ctx->save_opad, sizeof(ctx->save_opad)); + /* Mark the context unkeyed so a later init with no key is rejected + * rather than recovering from the pads just wiped. Relying on the + * hash type in the wolfSSL HMAC object is not enough: a zeroed + * macType does not read as none in every hash type enum layout. */ + ctx->type = WC_HASH_TYPE_NONE; + /* The wolfCrypt object's type has to say unkeyed as well. wc_HmacFree + * zeroes it, which only reads as none where that enum value is 0; in + * the FIPS and selftest layout zero is MD5, so an init that supplies a + * digest but no key would take the recover path and MAC with the pads + * just wiped. */ + ctx->hmac.macType = WC_HASH_TYPE_NONE; } return 1; diff --git a/tests/api/test_ossl_mac.c b/tests/api/test_ossl_mac.c index d3f3bdf31a2..d1bd1df8961 100644 --- a/tests/api/test_ossl_mac.c +++ b/tests/api/test_ossl_mac.c @@ -157,6 +157,55 @@ static int test_HMAC_CTX_helper(const EVP_MD* type, unsigned char* digest, } #endif /* defined(OPENSSL_EXTRA) && !defined(NO_HMAC) */ +#if defined(OPENSSL_EXTRA) && !defined(NO_HMAC) && !defined(NO_SHA256) +/* Returns 1 when every byte of the buffer is zero. */ +static int test_mac_all_zero(const void* buf, size_t len) +{ + const byte* p = (const byte*)buf; + size_t i; + + for (i = 0; i < len; i++) { + if (p[i] != 0) { + return 0; + } + } + + return 1; +} +#endif + +int test_wolfSSL_HMAC_CTX_cleanup_zeroize(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_HMAC) && !defined(NO_SHA256) + /* The saved pads are the key combined with the fixed inner and outer + * padding, so cleanup has to wipe them along with the HMAC object. */ + WOLFSSL_HMAC_CTX ctx; + unsigned char key[] = "\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b" + "\x0b\x0b\x0b\x0b\x0b\x0b\x0b"; + + ExpectIntEQ(HMAC_CTX_init(&ctx), 1); + ExpectIntEQ(HMAC_Init_ex(&ctx, key, (int)sizeof(key) - 1, EVP_sha256(), + NULL), 1); + + /* Keying the context must have filled the saved pads. */ + ExpectIntEQ(test_mac_all_zero(ctx.save_ipad, sizeof(ctx.save_ipad)), 0); + ExpectIntEQ(test_mac_all_zero(ctx.save_opad, sizeof(ctx.save_opad)), 0); + + HMAC_CTX_cleanup(&ctx); + + ExpectIntEQ(test_mac_all_zero(ctx.save_ipad, sizeof(ctx.save_ipad)), 1); + ExpectIntEQ(test_mac_all_zero(ctx.save_opad, sizeof(ctx.save_opad)), 1); + + /* Re-initializing without a key must now be rejected. Recovering from + * the wiped pads would silently MAC with an all zero key. */ + ExpectIntEQ(HMAC_Init_ex(&ctx, NULL, 0, NULL, NULL), 0); + + HMAC_CTX_cleanup(&ctx); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_HMAC_CTX(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_mac.h b/tests/api/test_ossl_mac.h index ea422af245e..4441bef65b8 100644 --- a/tests/api/test_ossl_mac.h +++ b/tests/api/test_ossl_mac.h @@ -25,11 +25,13 @@ #include int test_wolfSSL_HMAC_CTX(void); +int test_wolfSSL_HMAC_CTX_cleanup_zeroize(void); int test_wolfSSL_HMAC(void); int test_wolfSSL_CMAC(void); #define TEST_OSSL_MAC_DECLS \ TEST_DECL_GROUP("ossl_mac", test_wolfSSL_HMAC_CTX), \ + TEST_DECL_GROUP("ossl_mac", test_wolfSSL_HMAC_CTX_cleanup_zeroize), \ TEST_DECL_GROUP("ossl_mac", test_wolfSSL_HMAC), \ TEST_DECL_GROUP("ossl_mac", test_wolfSSL_CMAC) From e2cce035eb5fe42a6e29fa4636d108195dd49157 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 15:28:12 +0200 Subject: [PATCH 06/12] Zeroize key material when clearing a WOLFSSL for reuse SSL_clear recycles a WOLFSSL object for a new connection, which is the usual pattern in connection pooling servers, and wolfSSL_shutdown calls it on success as well. It reset the option and state fields but left every piece of key material from the previous connection in place. The teardown path in SSL_ResourceFree is careful here and force zeroes the keys struct and the TLS 1.3 traffic secrets, so a reused object ended up holding material that a freed one would not. The keys struct keeps the write keys, MAC secrets and IVs, clientSecret and serverSecret keep the TLS 1.3 traffic secrets, the DTLS 1.3 epoch table keeps traffic keys, IVs and sequence number keys for every epoch, and the handshake arrays keep the master secret, the pre master secret, the PSK key and the TLS 1.3 key schedule secret. The tls-unique fields keep the Finished values of the connection that just ended, so the next caller could bind to the wrong session. The buffers are sized for the largest supported algorithm, so a later handshake that negotiates something smaller only overwrites a prefix and the tail survives. Force zero all of it. A freshly created object has these zeroed already, with two exceptions that are put back after the wipe: the multicast peer identifier sentinel, and the unprotected DTLS 1.3 epoch 0 together with the epoch pointers aimed at it, which only InitSSL sets up and without which the next handshake has no valid epoch. Wipe the handshake arrays in place rather than releasing them. They have to stay allocated because wolfSSL_set_secret, the exporter and the accessors that run after a connection all read from them on an object that is being recycled rather than freed, and because the key agreement routines take preMasterSz as the size of the buffer they may write, so that is restored to what a freshly allocated Arrays carries. An application that asked to keep the arrays still gets back everything the API can hand it, so the master secret and the exporter secret only go when it did not ask, while the pre master secret, the PSK key and the key schedule secret always do because nothing reads those back. wolfSSL_set_secret and wolfSSL_make_eap_keys both reached into the arrays without checking that they are there, which the ordinary handshake teardown can already leave them not to be, so both now report a bad argument instead. Add a regression test that runs a handshake, clears the object with the arrays kept, and requires the write keys, both traffic secrets and the pre master secret to be gone while the master secret, the exporter secret and the client random survive. It then takes that request back, clears again, and requires the master and exporter secrets to be gone with the arrays themselves still present. Fixes F-7258. --- src/ssl.c | 89 +++++++++++++++++++++++++- src/ssl_api_dtls.c | 7 ++ src/tls.c | 5 ++ tests/api.c | 137 ++++++++++++++++++++++++++++++++++++++++ tests/api/test_dtls13.c | 73 +++++++++++++++++++++ tests/api/test_dtls13.h | 4 +- 6 files changed, 313 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 72c486760b3..a77767fe8af 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -5679,7 +5679,94 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, ssl->buffers.inputBuffer.bufferSize); #endif } - ssl->keys.encryptionOn = 0; + /* Recycling the object for a new connection must not carry the + * previous connection's key material along with it. A freshly + * created object has all of this zeroed. */ + ForceZero(&ssl->keys, sizeof(Keys)); + #ifdef WOLFSSL_MULTICAST + if (ssl->options.haveMcast) { + int i; + + for (i = 0; i < WOLFSSL_DTLS_PEERSEQ_SZ; i++) + ssl->keys.peerSeq[i].peerId = INVALID_PEER_ID; + } + #endif + #ifdef WOLFSSL_TLS13 + ForceZero(ssl->clientSecret, sizeof(ssl->clientSecret)); + ForceZero(ssl->serverSecret, sizeof(ssl->serverSecret)); + /* A key update the previous connection asked for has nothing to say + * about the next one, and the keys it would rotate are gone. */ + ssl->options.sendKeyUpdate = 0; + #endif + #ifdef WOLFSSL_HAVE_TLS_UNIQUE + /* The channel binding of the connection that just ended. Leaving it + * in place would let the next caller bind to the wrong session. */ + ForceZero(ssl->clientFinished, TLS_FINISHED_SZ_MAX); + ForceZero(ssl->serverFinished, TLS_FINISHED_SZ_MAX); + ssl->clientFinished_len = 0; + ssl->serverFinished_len = 0; + #endif + #ifdef WOLFSSL_DTLS13 + /* Per-epoch traffic keys, IVs and sequence number keys. */ + ForceZero(ssl->dtls13Epochs, sizeof(ssl->dtls13Epochs)); + /* Only InitSSL() sets up the unprotected epoch 0 and aims the epoch + * pointers at it, and this object is being reused rather than freed, + * so put it back or the next handshake has no valid epoch. */ + ssl->dtls13Epochs[0].isValid = 1; + ssl->dtls13Epochs[0].side = ENCRYPT_AND_DECRYPT_SIDE; + ssl->dtls13EncryptEpoch = &ssl->dtls13Epochs[0]; + ssl->dtls13DecryptEpoch = &ssl->dtls13Epochs[0]; + /* The numbers that say which epoch to use sit outside the table and + * only ever move up, so wiping the table has to be matched here by + * hand. InitSSL() gets this for free from clearing the whole object. + * Left behind, they ask the next handshake to send under an epoch the + * table no longer holds, and Dtls13SetEpochKeys() fails the connection + * with BAD_STATE_E. */ + w64Zero(&ssl->dtls13Epoch); + w64Zero(&ssl->dtls13PeerEpoch); + w64Zero(&ssl->dtls13InvalidateBefore); + ssl->dtls13WaitKeyUpdateAck = 0; + ssl->dtls13DoKeyUpdate = 0; + ssl->dtls13SendingAckOrRtx = 0; + /* Anything still queued for retransmission or acknowledgement is + * tagged with an epoch that no longer exists, so it can never be sent + * and would only be released when the object is freed. */ + Dtls13FreeFsmResources(ssl); + ssl->dtls13Rtx.sendAcks = 0; + ssl->dtls13Rtx.retransmit = 0; + #endif + /* The handshake arrays hold the master and pre-master secrets. Wipe + * those in place rather than releasing the arrays, so that the + * allocation stays valid for wolfSSL_set_secret(), the exporter and + * the other accessors that read from it once a connection has ended. + * An application that asked to keep the arrays still gets back + * everything the API can hand it, so only the rest goes. */ + if (ssl->arrays != NULL) { + if (ssl->arrays->preMasterSecret != NULL) { + ForceZero(ssl->arrays->preMasterSecret, ENCRYPT_LEN); + /* The key agreement routines take this as the size of the + * buffer they may write, so put back what a freshly + * allocated Arrays would carry. */ + ssl->arrays->preMasterSz = ENCRYPT_LEN; + } + #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) + ForceZero(ssl->arrays->psk_key, MAX_PSK_KEY_LEN); + ssl->arrays->psk_keySz = 0; + #endif + #ifdef WOLFSSL_TLS13 + /* The key schedule secret. No API hands this one back. */ + ForceZero(ssl->arrays->secret, SECRET_LEN); + #endif + if (!ssl->options.saveArrays) { + ForceZero(ssl->arrays->masterSecret, SECRET_LEN); + #ifdef HAVE_KEYING_MATERIAL + /* Tls13_Exporter() reads this one, and exporting keying + * material requires the arrays to be kept, so it only goes + * when the application did not ask for that. */ + ForceZero(ssl->arrays->exporterSecret, WC_MAX_DIGEST_SIZE); + #endif + } + } XMEMSET(&ssl->msgsReceived, 0, sizeof(ssl->msgsReceived)); /* Discard any partial handshake-message reassembly on reuse. */ diff --git a/src/ssl_api_dtls.c b/src/ssl_api_dtls.c index 6cc9151d943..ef10a0da3ec 100644 --- a/src/ssl_api_dtls.c +++ b/src/ssl_api_dtls.c @@ -778,6 +778,13 @@ int wolfSSL_set_secret(WOLFSSL* ssl, word16 epoch, ret = BAD_FUNC_ARG; } + /* The handshake arrays are released once the handshake resources are + * freed, so a reused object may not have them any more. */ + if (ret == 0 && ssl->arrays == NULL) { + WOLFSSL_MSG("Handshake arrays not available"); + ret = BAD_FUNC_ARG; + } + if (ret == 0 && ssl->arrays->preMasterSecret == NULL) { ssl->arrays->preMasterSz = ENCRYPT_LEN; ssl->arrays->preMasterSecret = (byte*)XMALLOC(ENCRYPT_LEN, ssl->heap, diff --git a/src/tls.c b/src/tls.c index 6647207776a..07a48902ee0 100644 --- a/src/tls.c +++ b/src/tls.c @@ -804,6 +804,11 @@ int wolfSSL_make_eap_keys(WOLFSSL* ssl, void* key, unsigned int len, int ret; WC_DECLARE_VAR(seed, byte, SEED_LEN, 0); + /* The randoms and the master secret live in the handshake arrays, which + * are gone once the handshake resources have been released. */ + if (ssl == NULL || ssl->arrays == NULL) + return BAD_FUNC_ARG; + WC_ALLOC_VAR_EX(seed, byte, SEED_LEN, ssl->heap, DYNAMIC_TYPE_SEED, return MEMORY_E); diff --git a/tests/api.c b/tests/api.c index a49a726c855..95e7f98b7c4 100644 --- a/tests/api.c +++ b/tests/api.c @@ -9855,6 +9855,142 @@ static int test_wolfSSL_clear_secure_renegotiation(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) +/* Returns 1 when every byte of the buffer is zero. */ +static int test_clear_all_zero(const void* buf, size_t len) +{ + const byte* p = (const byte*)buf; + size_t i; + + for (i = 0; i < len; i++) { + if (p[i] != 0) { + return 0; + } + } + + return 1; +} +#endif + +/* wolfSSL_clear recycles the object for a new connection, so it must not carry + * the previous connection's key material into the reused object. */ +static int test_wolfSSL_clear_zeroizes_secrets(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) + struct test_memio_ctx test_ctx; + WOLFSSL_CTX* ctx_c = NULL; + WOLFSSL_CTX* ctx_s = NULL; + WOLFSSL* ssl_c = NULL; + WOLFSSL* ssl_s = NULL; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + /* Ask to hold on to the handshake arrays, both so the master secret is + * still resident when the object is recycled and so the clear is required + * to honour that request. */ + if (ssl_s != NULL) { + wolfSSL_KeepArrays(ssl_s); + } + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* The completed connection left key material behind. */ + if (EXPECT_SUCCESS() && (ssl_s != NULL)) { + ExpectIntEQ(test_clear_all_zero(ssl_s->keys.client_write_key, + sizeof(ssl_s->keys.client_write_key)), 0); + ExpectIntEQ(test_clear_all_zero(ssl_s->keys.server_write_key, + sizeof(ssl_s->keys.server_write_key)), 0); + ExpectIntEQ(test_clear_all_zero(ssl_s->clientSecret, + sizeof(ssl_s->clientSecret)), 0); + ExpectIntEQ(test_clear_all_zero(ssl_s->serverSecret, + sizeof(ssl_s->serverSecret)), 0); + ExpectNotNull(ssl_s->arrays); + } + if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL)) { + ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->masterSecret, + SECRET_LEN), 0); + } + + ExpectIntEQ(wolfSSL_clear(ssl_s), WOLFSSL_SUCCESS); + + if (EXPECT_SUCCESS() && (ssl_s != NULL)) { + ExpectIntEQ(test_clear_all_zero(ssl_s->keys.client_write_key, + sizeof(ssl_s->keys.client_write_key)), 1); + ExpectIntEQ(test_clear_all_zero(ssl_s->keys.server_write_key, + sizeof(ssl_s->keys.server_write_key)), 1); + ExpectIntEQ(test_clear_all_zero(ssl_s->clientSecret, + sizeof(ssl_s->clientSecret)), 1); + ExpectIntEQ(test_clear_all_zero(ssl_s->serverSecret, + sizeof(ssl_s->serverSecret)), 1); + /* The application asked to keep the handshake arrays, so they must + * survive along with the master secret it wants to read back. */ + ExpectNotNull(ssl_s->arrays); + } + if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL)) { + ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->masterSecret, + SECRET_LEN), 0); + /* The pre-master secret is not part of that contract. */ + ExpectNotNull(ssl_s->arrays->preMasterSecret); + } + if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL) && + (ssl_s->arrays->preMasterSecret != NULL)) { + ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->preMasterSecret, + ENCRYPT_LEN), 1); + } + if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL)) { + /* The key schedule secret is not part of the contract either. */ + ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->secret, SECRET_LEN), 1); + #ifdef HAVE_KEYING_MATERIAL + /* The exporter secret is. Exporting keying material needs the arrays + * kept, and Tls13_Exporter() reads this one to do it. */ + ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->exporterSecret, + WC_MAX_DIGEST_SIZE), 0); + #endif + } +#if (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) || \ + defined(HAVE_SECRET_CALLBACK)) && !defined(NO_WOLFSSL_CLIENT) + /* The documented reason to keep the arrays is to read this back. */ + if (EXPECT_SUCCESS()) { + byte cr[RAN_LEN]; + + ExpectIntEQ(wolfSSL_get_client_random(ssl_s, cr, sizeof(cr)), RAN_LEN); + } +#endif + + /* Take the request back and clear again. Now the master secret has to go, + * but the arrays themselves still must not: the object is being recycled + * rather than freed, and wolfSSL_set_secret() along with the accessors + * that run after a connection all write into them. Some configurations, + * OpenVPN support among them, keep the arrays for every object, so set + * this directly rather than relying on the default. */ + if (EXPECT_SUCCESS() && (ssl_s != NULL)) { + ssl_s->options.saveArrays = 0; + ExpectIntEQ(wolfSSL_clear(ssl_s), WOLFSSL_SUCCESS); + ExpectNotNull(ssl_s->arrays); + } + if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL)) { + ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->masterSecret, + SECRET_LEN), 1); + #ifdef HAVE_KEYING_MATERIAL + ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->exporterSecret, + WC_MAX_DIGEST_SIZE), 1); + #endif + ExpectNotNull(ssl_s->arrays->preMasterSecret); + /* The key agreement routines read this as the room they have. */ + ExpectIntEQ((int)ssl_s->arrays->preMasterSz, ENCRYPT_LEN); + } + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + /* Test reconnecting with a different ciphersuite after a renegotiation. */ static int test_wolfSSL_SCR_Reconnect(void) { @@ -38752,6 +38888,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_custom_ext_add_null), TEST_DECL(test_wolfSSL_wolfSSL_UseSecureRenegotiation), TEST_DECL(test_wolfSSL_clear_secure_renegotiation), + TEST_DECL(test_wolfSSL_clear_zeroizes_secrets), TEST_DECL(test_wolfSSL_SCR_Reconnect), TEST_DECL(test_wolfSSL_SCR_check_enabled), TEST_DECL(test_wolfSSL_ticket_keycb_bad_hmac), diff --git a/tests/api/test_dtls13.c b/tests/api/test_dtls13.c index 9b770afa4b2..a02ae2da7ff 100644 --- a/tests/api/test_dtls13.c +++ b/tests/api/test_dtls13.c @@ -2033,3 +2033,76 @@ int test_dtls13_5_9_0_compat_empty_echo(void) #endif return EXPECT_RESULT(); } + +/* wolfSSL_clear() wipes the DTLS 1.3 epoch table so a reused object does not + * carry the previous connection's traffic keys. Everything that says which + * epoch to use lives outside that table and only ever moves up, so it has to + * be brought back with it. Run a second handshake over the same objects to + * prove they really are reusable: with the table wiped and the numbers left + * behind, Dtls13SetEpochKeys() fails the ClientHello with BAD_STATE_E. */ +int test_dtls13_reuse_after_clear(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) \ + && (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + char readBuf[16]; + int i; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* The handshake has to have moved off epoch 0, or the reset under test + * has nothing to undo. */ + ExpectIntEQ(w64IsZero(ssl_c->dtls13Epoch), 0); + ExpectIntEQ(w64IsZero(ssl_s->dtls13Epoch), 0); + + ExpectIntEQ(wolfSSL_write(ssl_c, "first", 5), 5); + XMEMSET(readBuf, 0, sizeof(readBuf)); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), 5); + ExpectStrEQ(readBuf, "first"); + + ExpectIntEQ(wolfSSL_clear(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_clear(ssl_s), WOLFSSL_SUCCESS); + + /* Only the unprotected epoch 0 survives, and the numbers agree with it. */ + for (i = 1; i < DTLS13_EPOCH_SIZE; i++) { + ExpectIntEQ(ssl_c->dtls13Epochs[i].isValid, 0); + ExpectIntEQ(ssl_s->dtls13Epochs[i].isValid, 0); + } + ExpectIntEQ(ssl_c->dtls13Epochs[0].isValid, 1); + ExpectIntEQ(ssl_s->dtls13Epochs[0].isValid, 1); + ExpectPtrEq(ssl_c->dtls13EncryptEpoch, &ssl_c->dtls13Epochs[0]); + ExpectPtrEq(ssl_c->dtls13DecryptEpoch, &ssl_c->dtls13Epochs[0]); + ExpectPtrEq(ssl_s->dtls13EncryptEpoch, &ssl_s->dtls13Epochs[0]); + ExpectPtrEq(ssl_s->dtls13DecryptEpoch, &ssl_s->dtls13Epochs[0]); + ExpectIntEQ(w64IsZero(ssl_c->dtls13Epoch), 1); + ExpectIntEQ(w64IsZero(ssl_c->dtls13PeerEpoch), 1); + ExpectIntEQ(w64IsZero(ssl_c->dtls13InvalidateBefore), 1); + ExpectIntEQ(w64IsZero(ssl_s->dtls13Epoch), 1); + ExpectIntEQ(w64IsZero(ssl_s->dtls13PeerEpoch), 1); + ExpectIntEQ(w64IsZero(ssl_s->dtls13InvalidateBefore), 1); + + /* Whatever the first connection left in flight belongs to epochs that no + * longer exist, so start the transport over as a new association would. */ + test_memio_clear_buffer(&test_ctx, 0); + test_memio_clear_buffer(&test_ctx, 1); + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + ExpectIntEQ(wolfSSL_write(ssl_c, "second", 6), 6); + XMEMSET(readBuf, 0, sizeof(readBuf)); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), 6); + ExpectStrEQ(readBuf, "second"); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_dtls13.h b/tests/api/test_dtls13.h index 2d2872339fa..2cf31bec279 100644 --- a/tests/api/test_dtls13.h +++ b/tests/api/test_dtls13.h @@ -55,6 +55,7 @@ int test_dtls13_no_session_id_echo(void); int test_dtls13_5_9_0_compat(void); int test_dtls13_5_9_0_compat_bad_echo(void); int test_dtls13_5_9_0_compat_empty_echo(void); +int test_dtls13_reuse_after_clear(void); #define TEST_DTLS13_DECLS \ TEST_DECL_GROUP("dtls13", test_dtls13_bad_epoch_ch), \ @@ -80,6 +81,7 @@ int test_dtls13_5_9_0_compat_empty_echo(void); TEST_DECL_GROUP("dtls13", test_dtls13_no_session_id_echo), \ TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat), \ TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat_bad_echo), \ - TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat_empty_echo) + TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat_empty_echo), \ + TEST_DECL_GROUP("dtls13", test_dtls13_reuse_after_clear) #endif /* TESTS_API_DTLS13_H */ From f4fbf8d9e5dbe7d1c72be9fbe6f081a2cbb2f1db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 16:39:04 +0200 Subject: [PATCH 07/12] Keep the ECIES context heap hint across a reset wc_ecc_ctx_new_ex records the caller's heap hint in the context and then calls wc_ecc_ctx_reset, which goes through ecc_ctx_init. That function opens by clearing the whole context and only restores the algorithm choices, the protocol role and the RNG, so the heap hint was lost on every context the _ex variant produced, and on every later call to the public reset. The context was then freed with a null hint, so it went to the default allocator rather than the heap it came from, and the temporary buffers that wc_ecc_encrypt_ex and wc_ecc_decrypt take from the same hint went to the default allocator too. A default build hides this because XMALLOC discards the hint, but with static memory the block belongs to the caller's pool and handing it to the system allocator is a free of memory that was never allocated there. Save the hint before ecc_ctx_init and restore it afterwards. Doing it in the reset covers both the constructor and the public reset. The other callers of ecc_ctx_init pass an uninitialized context on the stack, so the hint must not be read there. Add a regression test that creates a context from a static heap and requires the heap to be whole again once the context is freed. Fixes F-7082. --- tests/api.c | 49 +++++++++++++++++++++++++++++++++++++++++++++ wolfcrypt/src/ecc.c | 7 +++++++ 2 files changed, 56 insertions(+) diff --git a/tests/api.c b/tests/api.c index 95e7f98b7c4..072d17dc1e5 100644 --- a/tests/api.c +++ b/tests/api.c @@ -608,6 +608,54 @@ static int test_wc_LoadStaticMemory_CTX(void) } +/* An ECIES context allocated from a heap hint must be returned to that same + * heap, so the hint has to survive the reset that sets the context defaults. */ +static int test_wc_ecc_ctx_new_ex_heap(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_STATIC_MEMORY) && !defined(WOLFSSL_STATIC_MEMORY_LEAN) && \ + defined(HAVE_ECC) && defined(HAVE_ECC_ENCRYPT) && !defined(WC_NO_RNG) + byte staticMemory[TEST_LSM_STATIC_SIZE]; + word32 sizeList[TEST_LSM_DEF_BUCKETS] = { TEST_LSM_BUCKETS }; + word32 distList[TEST_LSM_DEF_BUCKETS] = { TEST_LSM_DIST }; + WOLFSSL_HEAP_HINT* heap = NULL; + WOLFSSL_MEM_STATS stats; + WC_RNG rng; + ecEncCtx* ctx = NULL; + int rngInit = 0; + + XMEMSET(&stats, 0, sizeof(stats)); + ExpectIntEQ(wc_LoadStaticMemory_ex(&heap, + WOLFMEM_DEF_BUCKETS, sizeList, distList, + staticMemory, (word32)sizeof(staticMemory), + 0, 1), + 0); + ExpectIntEQ(wc_InitRng(&rng), 0); + if (EXPECT_SUCCESS()) { + rngInit = 1; + } + + ExpectNotNull(ctx = wc_ecc_ctx_new_ex(REQ_RESP_CLIENT, &rng, heap)); + wc_ecc_ctx_free(ctx); + ctx = NULL; + + /* The context came out of the static heap, so freeing it must put it + * back rather than hand it to the default allocator. */ + if (EXPECT_SUCCESS() && (heap != NULL)) { + ExpectIntEQ(wolfSSL_GetMemStats(heap->memory, &stats), 1); + ExpectIntEQ(stats.curAlloc, 0); + ExpectIntEQ(stats.totalAlloc, stats.totalFr); + } + + if (rngInit) { + wc_FreeRng(&rng); + } + wc_UnloadStaticMemory(heap); +#endif + return EXPECT_RESULT(); +} + + /*----------------------------------------------------------------------------* | Platform dependent function test *----------------------------------------------------------------------------*/ @@ -38165,6 +38213,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wc_LoadStaticMemory_ex), TEST_DECL(test_wc_LoadStaticMemory_CTX), + TEST_DECL(test_wc_ecc_ctx_new_ex_heap), TEST_DECL(test_wc_FreeCertList), /* Locking with Compat Mutex */ diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 34acd96ca87..3baaa85a68b 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -15131,10 +15131,17 @@ static void ecc_ctx_init(ecEncCtx* ctx, int flags, WC_RNG* rng) WOLFSSL_ABI int wc_ecc_ctx_reset(ecEncCtx* ctx, WC_RNG* rng) { + void* heap; + if (ctx == NULL || rng == NULL) return BAD_FUNC_ARG; + /* ecc_ctx_init clears the whole context, so carry the heap hint over it. + * The context has to be freed to the heap it was allocated from. */ + heap = ctx->heap; ecc_ctx_init(ctx, ctx->protocol, rng); + ctx->heap = heap; + return ecc_ctx_set_salt(ctx, ctx->protocol); } From 428bfe9f587e0e5a02b5d3f3ecf800a348b7d739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 16:59:53 +0200 Subject: [PATCH 08/12] Restore the caller's key RNG after an HPKE operation The encap and decap paths create a temporary RNG, install it into a key that belongs to the caller so the shared secret computation can blind, and then free it without taking it back out. Both wc_ecc_set_rng and wc_curve25519_set_rng only record the pointer, and nothing else ever writes that field, so the caller's key was left pointing at freed memory. Encap does this to the ephemeral key and decap to the receiver key, including the curve25519 branch. Repeated HPKE calls hide it because each one installs a fresh RNG first, but any other use of the key that consults it, such as an ECDH or a signature under timing resistance, reads the freed object. ECH holds exactly such a long lived key. Save whatever RNG the key already had and put it back before freeing the temporary one, so the key is handed back to the caller unchanged. Clearing the field instead would silently drop an RNG the caller had installed for blinding of their own. Extend the HPKE round trip test to give both keys an RNG of its own and to require them to still have it once the seal and open have finished. Fixes F-7083. --- src/ssl_ech.c | 9 ++ wolfcrypt/src/hpke.c | 195 ++++++++++++++++++++++++++++++++++++++++-- wolfcrypt/test/test.c | 78 +++++++++++++++++ 3 files changed, 273 insertions(+), 9 deletions(-) diff --git a/src/ssl_ech.c b/src/ssl_ech.c index ebc407c2af9..cd48588802c 100644 --- a/src/ssl_ech.c +++ b/src/ssl_ech.c @@ -123,6 +123,15 @@ int wolfSSL_CTX_GenerateEchConfigEx(WOLFSSL_CTX* ctx, const char* publicName, if (ret == 0) ret = wc_HpkeGenerateKeyPair(hpke, &newConfig->receiverPrivkey, rng); + /* The key outlives this RNG, and key generation may have stored a + * pointer to it in the key, so take it back out before freeing it. */ +#if defined(HAVE_CURVE25519) && defined(WOLFSSL_CURVE25519_BLINDING) + if ((ret == 0) && (kemId == DHKEM_X25519_HKDF_SHA256)) { + (void)wc_curve25519_set_rng( + (curve25519_key*)newConfig->receiverPrivkey, NULL); + } +#endif + /* done with RNG */ wc_FreeRng(rng); diff --git a/wolfcrypt/src/hpke.c b/wolfcrypt/src/hpke.c index f27de7c6797..472cb035171 100644 --- a/wolfcrypt/src/hpke.c +++ b/wolfcrypt/src/hpke.c @@ -797,6 +797,7 @@ static int wc_HpkeEncap(Hpke* hpke, void* ephemeralKey, void* receiverKey, int ret; #if defined(ECC_TIMING_RESISTANT) && defined(HAVE_ECC) WC_RNG* rng; + WC_RNG* prevRng; #endif word32 dh_len; word16 receiverPubKeySz; @@ -850,13 +851,17 @@ static int wc_HpkeEncap(Hpke* hpke, void* ephemeralKey, void* receiverKey, break; } - wc_ecc_set_rng((ecc_key*)ephemeralKey, rng); + prevRng = ((ecc_key*)ephemeralKey)->rng; + (void)wc_ecc_set_rng((ecc_key*)ephemeralKey, rng); #endif ret = wc_ecc_shared_secret((ecc_key*)ephemeralKey, (ecc_key*)receiverKey, dh, &dh_len); #ifdef ECC_TIMING_RESISTANT + /* The key belongs to the caller, so put back whatever RNG it had + * before this RNG is freed. */ + (void)wc_ecc_set_rng((ecc_key*)ephemeralKey, prevRng); wc_rng_free(rng); #endif break; @@ -1053,13 +1058,151 @@ int wc_HpkeSealBase(Hpke* hpke, void* ephemeralKey, void* receiverKey, return ret; } +#if (defined(HAVE_ECC) && defined(ECC_TIMING_RESISTANT)) || \ + (defined(HAVE_CURVE25519) && !defined(NO_SHA256) && \ + defined(WOLFSSL_CURVE25519_BLINDING)) +/* Try to make a private-only copy of a receiver key. + * + * The shared secret computation needs an RNG and the only way to hand it one + * is the key's own rng field, so without a copy it would have to write to an + * object it does not own. The ECH server passes the key its WOLFSSL_CTX shares + * between every connection made from it, where that write races the other + * connections: each saves what it finds and restores it afterwards, so one of + * them ends up holding an RNG another has already freed. + * + * Only a plain software key is copied. A key carrying a device id has to keep + * reaching that device, and both a copy without the id and an import that + * provisions the device would be worse than the write this avoids. Such a key + * computes its shared secret on the device, which never reads key->rng, so + * leaving it alone costs nothing. + * + * @param [in] hpke HPKE object, for the KEM in use and the heap hint. + * @param [in] key Receiver key to copy. + * @param [out] copy The copy, to be released with wc_HpkeFreeKey(). + * @return 1 when copy holds a usable copy of the private key. + * @return 0 when no copy could be made and the original has to be used. + */ +static int wc_HpkeCopyPrivateKey(Hpke* hpke, void* key, void** copy) +{ + int ret = WC_NO_ERR_TRACE(NOT_COMPILED_IN); +#if defined(HAVE_ECC) && defined(ECC_TIMING_RESISTANT) + byte eccPriv[ECC_MAXSIZE]; + word32 eccPrivSz = (word32)sizeof(eccPriv); +#endif +#if defined(HAVE_CURVE25519) && !defined(NO_SHA256) && \ + defined(WOLFSSL_CURVE25519_BLINDING) + byte x25519Priv[CURVE25519_KEYSIZE]; + word32 x25519PrivSz = (word32)sizeof(x25519Priv); +#endif + + *copy = NULL; + + switch (hpke->kem) + { +#if defined(HAVE_ECC) && defined(ECC_TIMING_RESISTANT) + case DHKEM_P256_HKDF_SHA256: + case DHKEM_P384_HKDF_SHA384: + case DHKEM_P521_HKDF_SHA512: + if (((ecc_key*)key)->dp == NULL) + break; + /* A key only carries a device id where there is a device to dispatch + * to. Everywhere else wc_ecc_shared_secret() is software only, so + * there is nothing a copy could take away. */ + #if defined(PLUTON_CRYPTO_ECC) || defined(WOLF_CRYPTO_CB) + if (((ecc_key*)key)->devId != INVALID_DEVID) + break; + #endif + + ret = wc_ecc_export_private_only((ecc_key*)key, eccPriv, + &eccPrivSz); + if (ret == 0) { + *copy = wc_ecc_key_new(hpke->heap); + if (*copy == NULL) + ret = MEMORY_E; + } + #if defined(PLUTON_CRYPTO_ECC) || defined(WOLF_CRYPTO_CB) + /* wc_ecc_key_new() adopts a platform default device id on some + * ports. The key being copied has none, so the copy must not gain + * one, or the shared secret would move onto a device the caller + * never asked for. */ + if (ret == 0) + ((ecc_key*)*copy)->devId = INVALID_DEVID; + #endif + if (ret == 0) { + /* The public part is not needed: the shared secret takes its + * point from the ephemeral key. */ + ret = wc_ecc_import_private_key_ex(eccPriv, eccPrivSz, NULL, 0, + (ecc_key*)*copy, ((ecc_key*)key)->dp->id); + } + ForceZero(eccPriv, sizeof(eccPriv)); + break; +#endif +#if defined(HAVE_CURVE25519) && !defined(NO_SHA256) && \ + defined(WOLFSSL_CURVE25519_BLINDING) + case DHKEM_X25519_HKDF_SHA256: + /* As above, the field only exists where a device can be dispatched + * to. */ + #ifdef WOLF_CRYPTO_CB + if (((curve25519_key*)key)->devId != INVALID_DEVID) + break; + #endif + + ret = wc_curve25519_export_private_raw_ex((curve25519_key*)key, + x25519Priv, &x25519PrivSz, EC25519_LITTLE_ENDIAN); + if (ret == 0) { + *copy = XMALLOC(sizeof(curve25519_key), hpke->heap, + DYNAMIC_TYPE_CURVE25519); + if (*copy == NULL) { + ret = MEMORY_E; + } + else { + ret = wc_curve25519_init_ex((curve25519_key*)*copy, + hpke->heap, INVALID_DEVID); + if (ret != 0) { + /* Never initialized, so it must not be freed as a + * key. */ + XFREE(*copy, hpke->heap, DYNAMIC_TYPE_CURVE25519); + *copy = NULL; + } + } + } + if (ret == 0) { + ret = wc_curve25519_import_private_ex(x25519Priv, x25519PrivSz, + (curve25519_key*)*copy, EC25519_LITTLE_ENDIAN); + } + ForceZero(x25519Priv, sizeof(x25519Priv)); + break; +#endif + default: + break; + } + + if (ret != 0 && *copy != NULL) { + wc_HpkeFreeKey(hpke, hpke->kem, *copy, hpke->heap); + *copy = NULL; + } + + return ret == 0; +} +#endif + /* compute the shared secret from the ephemeral and receiver kem keys */ static int wc_HpkeDecap(Hpke* hpke, void* receiverKey, const byte* pubKey, word16 pubKeySz, byte* sharedSecret) { int ret; -#if defined(ECC_TIMING_RESISTANT) || defined(WOLFSSL_CURVE25519_BLINDING) +#ifdef HAVE_ECC + ecc_key* eccPriv; +#endif +#if defined(HAVE_CURVE25519) && !defined(NO_SHA256) + curve25519_key* x25519Priv; +#endif +#if (defined(HAVE_ECC) && defined(ECC_TIMING_RESISTANT)) || \ + (defined(HAVE_CURVE25519) && !defined(NO_SHA256) && \ + defined(WOLFSSL_CURVE25519_BLINDING)) WC_RNG* rng; + WC_RNG* prevRng = NULL; + void* privCopy = NULL; #endif word32 dh_len; word16 receiverPubKeySz; @@ -1107,6 +1250,7 @@ static int wc_HpkeDecap(Hpke* hpke, void* receiverKey, const byte* pubKey, case DHKEM_P256_HKDF_SHA256: case DHKEM_P384_HKDF_SHA384: case DHKEM_P521_HKDF_SHA512: + eccPriv = (ecc_key*)receiverKey; #ifdef ECC_TIMING_RESISTANT rng = wc_rng_new(NULL, 0, hpke->heap); @@ -1115,19 +1259,38 @@ static int wc_HpkeDecap(Hpke* hpke, void* receiverKey, const byte* pubKey, break; } - wc_ecc_set_rng((ecc_key*)receiverKey, rng); + /* Work on a copy so that installing the RNG does not write to + * the caller's key. A key that cannot be copied has its + * private part in a device, and that device computes the + * shared secret without ever reading key->rng, so using it as + * it is costs nothing. */ + if (wc_HpkeCopyPrivateKey(hpke, receiverKey, &privCopy)) + eccPriv = (ecc_key*)privCopy; + else + prevRng = eccPriv->rng; + (void)wc_ecc_set_rng(eccPriv, rng); #endif - ret = wc_ecc_shared_secret((ecc_key*)receiverKey, - (ecc_key*)ephemeralKey, dh, &dh_len); + ret = wc_ecc_shared_secret(eccPriv, (ecc_key*)ephemeralKey, dh, + &dh_len); #ifdef ECC_TIMING_RESISTANT + if (privCopy != NULL) { + wc_HpkeFreeKey(hpke, hpke->kem, privCopy, hpke->heap); + privCopy = NULL; + } + else { + /* The key belongs to the caller, so put back whatever RNG + * it had before this RNG is freed. */ + (void)wc_ecc_set_rng(eccPriv, prevRng); + } wc_rng_free(rng); #endif break; #endif #if defined(HAVE_CURVE25519) && !defined(NO_SHA256) case DHKEM_X25519_HKDF_SHA256: + x25519Priv = (curve25519_key*)receiverKey; #ifdef WOLFSSL_CURVE25519_BLINDING rng = wc_rng_new(NULL, 0, hpke->heap); @@ -1136,12 +1299,26 @@ static int wc_HpkeDecap(Hpke* hpke, void* receiverKey, const byte* pubKey, break; } - wc_curve25519_set_rng((curve25519_key*)receiverKey, rng); + /* As above: prefer a copy so the caller's key is left alone. */ + if (wc_HpkeCopyPrivateKey(hpke, receiverKey, &privCopy)) + x25519Priv = (curve25519_key*)privCopy; + else + prevRng = x25519Priv->rng; + (void)wc_curve25519_set_rng(x25519Priv, rng); #endif - ret = wc_curve25519_shared_secret_ex( - (curve25519_key*)receiverKey, (curve25519_key*)ephemeralKey, - dh, &dh_len, EC25519_LITTLE_ENDIAN); + ret = wc_curve25519_shared_secret_ex(x25519Priv, + (curve25519_key*)ephemeralKey, dh, &dh_len, + EC25519_LITTLE_ENDIAN); #ifdef WOLFSSL_CURVE25519_BLINDING + if (privCopy != NULL) { + wc_HpkeFreeKey(hpke, hpke->kem, privCopy, hpke->heap); + privCopy = NULL; + } + else { + /* The key belongs to the caller, so put back whatever RNG + * it had before this RNG is freed. */ + (void)wc_curve25519_set_rng(x25519Priv, prevRng); + } wc_rng_free(rng); #endif break; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 5f7047ac792..0e3679615b4 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -37594,6 +37594,55 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t x963kdf_test(void) defined(HAVE_CURVE448)) && \ defined(HAVE_AESGCM) +/* Install rng on an HPKE kem key, for the key types that keep one. */ +static void hpke_test_set_key_rng(int kem, void* key, WC_RNG* rng) +{ + switch (kem) { +#if defined(HAVE_ECC) && defined(ECC_TIMING_RESISTANT) + case DHKEM_P256_HKDF_SHA256: + case DHKEM_P384_HKDF_SHA384: + case DHKEM_P521_HKDF_SHA512: + (void)wc_ecc_set_rng((ecc_key*)key, rng); + break; +#endif +#if defined(HAVE_CURVE25519) && defined(WOLFSSL_CURVE25519_BLINDING) + case DHKEM_X25519_HKDF_SHA256: + (void)wc_curve25519_set_rng((curve25519_key*)key, rng); + break; +#endif + default: + break; + } + + (void)key; + (void)rng; +} + +/* Returns 1 when the key still holds rng, or when this key type does not keep + * an RNG in this build. */ +static int hpke_test_key_rng_kept(int kem, void* key, WC_RNG* rng) +{ + switch (kem) { +#if defined(HAVE_ECC) && defined(ECC_TIMING_RESISTANT) + case DHKEM_P256_HKDF_SHA256: + case DHKEM_P384_HKDF_SHA384: + case DHKEM_P521_HKDF_SHA512: + return ((ecc_key*)key)->rng == rng; +#endif +#if defined(HAVE_CURVE25519) && defined(WOLFSSL_CURVE25519_BLINDING) + case DHKEM_X25519_HKDF_SHA256: + return ((curve25519_key*)key)->rng == rng; +#endif + default: + break; + } + + (void)key; + (void)rng; + + return 1; +} + /* test null/bad arguments for wc_HpkeInit, a one-shot seal/open round-trip * with wc_HpkeSealBase/wc_HpkeOpenBase, and auth failure cases (wrong info, * wrong AAD, tampered ciphertext, wrong receiver key) */ @@ -37689,6 +37738,14 @@ static wc_test_ret_t hpke_test_single(Hpke* hpke, int kem, int kdf, int aead) ret = WC_TEST_RET_ENC_EC(ret); } + /* The keys belong to the caller, so give them an RNG of our own. HPKE + * borrows the key to compute the shared secret and must hand it back + * unchanged. */ + if (ret == 0) { + hpke_test_set_key_rng(kem, ephemeralKey, rng); + hpke_test_set_key_rng(kem, receiverKey, rng); + } + /* Negative test case with NULL argument */ if (ret == 0) { ret = wc_HpkeSealBase(NULL, ephemeralKey, receiverKey, @@ -37976,6 +38033,13 @@ static wc_test_ret_t hpke_test_single(Hpke* hpke, int kem, int kdf, int aead) ret = WC_TEST_RET_ENC_NC; } + /* Seal and open install a temporary RNG on the caller's key and then free + * it. Neither key may be left pointing at that freed RNG. */ + if (ret == 0 && !hpke_test_key_rng_kept(kem, ephemeralKey, rng)) + ret = WC_TEST_RET_ENC_NC; + if (ret == 0 && !hpke_test_key_rng_kept(kem, receiverKey, rng)) + ret = WC_TEST_RET_ENC_NC; + if (ephemeralKey != NULL) wc_HpkeFreeKey(hpke, hpke->kem, ephemeralKey, hpke->heap); if (receiverKey != NULL) @@ -38037,6 +38101,13 @@ static wc_test_ret_t hpke_test_multi(Hpke* hpke) if (ret == 0) ret = wc_HpkeGenerateKeyPair(hpke, &receiverKey, rng); + /* The context API reaches the same encap and decap that the one-shot API + * does, so give the keys an RNG to watch here too. */ + if (ret == 0) { + hpke_test_set_key_rng(hpke->kem, ephemeralKey, rng); + hpke_test_set_key_rng(hpke->kem, receiverKey, rng); + } + /* Negative test case with NULL argument */ if (ret == 0) { ret = wc_HpkeInitSealContext(NULL, context, ephemeralKey, receiverKey, @@ -38395,6 +38466,13 @@ static wc_test_ret_t hpke_test_multi(Hpke* hpke) ret = 0; } + /* Seal and open install a temporary RNG on the caller's key and then free + * it. Neither key may be left pointing at that freed RNG. */ + if (ret == 0 && !hpke_test_key_rng_kept(hpke->kem, ephemeralKey, rng)) + ret = WC_TEST_RET_ENC_NC; + if (ret == 0 && !hpke_test_key_rng_kept(hpke->kem, receiverKey, rng)) + ret = WC_TEST_RET_ENC_NC; + if (ephemeralKey != NULL) wc_HpkeFreeKey(hpke, hpke->kem, ephemeralKey, hpke->heap); if (receiverKey != NULL) From 622d0bc15d6111223fa4dcb01ff26f14df24b653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 17:39:01 +0200 Subject: [PATCH 09/12] Use the instance address as the RNG bank DRBG nonce wc_rng_bank_init derives each instance's personalization string from &ctx->rngs[i], which is already the address of the instance, so the DRBG read the leading bytes of the instance struct rather than the address itself. Those bytes had just been cleared by the memset over the whole array and nothing writes to instance i before it is initialized, so every DRBG in the bank was instantiated with the same all zero personalization string. wc_rng_bank_inst_reinit does this correctly by taking the address of a local pointer variable, which yields the pointer value and so a distinct nonce per instance. Take the instance pointer into a local here as well and pass its address, so both paths agree and each instance gets its own value. Fixes F-7085. --- wolfcrypt/src/rng_bank.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/src/rng_bank.c b/wolfcrypt/src/rng_bank.c index 24d7cdccd38..c3b88e28a2c 100644 --- a/wolfcrypt/src/rng_bank.c +++ b/wolfcrypt/src/rng_bank.c @@ -115,6 +115,9 @@ WOLFSSL_API int wc_rng_bank_init( ctx->n_rngs = n_rngs; for (i = 0; i < n_rngs; ++i) { + /* The nonce is the address of the instance, so it has to be taken + * from a pointer to it, not from the instance itself. */ + struct wc_rng_bank_inst *rng_inst = ctx->rngs + i; #ifdef WC_VERBOSE_RNG int nretries = 0; #endif @@ -125,8 +128,8 @@ WOLFSSL_API int wc_rng_bank_init( if (flags & WC_RNG_BANK_FLAG_NO_VECTOR_OPS) need_reenable_vec = (DISABLE_VECTOR_REGISTERS() == 0); ret = wc_InitRngNonce_ex( - WC_RNG_BANK_INST_TO_RNG(ctx->rngs + i), - (byte *)&ctx->rngs[i], sizeof(byte *), heap, devId); + WC_RNG_BANK_INST_TO_RNG(rng_inst), + (byte *)&rng_inst, sizeof(byte *), heap, devId); if (need_reenable_vec) REENABLE_VECTOR_REGISTERS(); From 8e61a110caf1a2a0c45217e2165ecd20dc85e113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 28 Jul 2026 18:23:11 +0200 Subject: [PATCH 10/12] Only zeroize the SRP temporaries once they are initialized In a small stack build wc_SrpComputeKey allocates six objects up front and checks them together afterwards, so a failure of any one of them jumps to the cleanup with the others allocated but not yet passed through mp_init_multi. The cleanup decided whether to zeroize the four temporaries by testing the return code against MP_INIT_E, which does not hold on the allocation failure path, so it called mp_forcezero on uninitialized memory. That takes its length from the size field of the object being zeroized, so an unset field turns into a write of arbitrary length past the end of the allocation. Track whether mp_init_multi succeeded and gate the zeroize on that instead. The flag is only set once the objects really are initialized, so it covers the init failure case the return code test was aiming at as well. Add a regression test that fails the last of the six allocations through a custom allocator and requires the call to report a memory error without touching the objects it never initialized. Fixes F-7084. --- tests/api.c | 146 ++++++++++++++++++++++++++++++++++++++++++++ wolfcrypt/src/srp.c | 12 ++-- 2 files changed, 153 insertions(+), 5 deletions(-) diff --git a/tests/api.c b/tests/api.c index 072d17dc1e5..80e08452e49 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35934,6 +35934,151 @@ static int test_write_dup_oom(void) return EXPECT_RESULT(); } +#if defined(OPENSSL_EXTRA) && defined(WOLFCRYPT_HAVE_SRP) && \ + defined(WOLFSSL_SMALL_STACK) && \ + defined(USE_WOLFSSL_MEMORY) && !defined(WOLFSSL_NO_MALLOC) && \ + !defined(WOLFSSL_STATIC_MEMORY) && !defined(WOLFSSL_KERNEL_MODE) && \ + !defined(NO_SHA256) +/* Allocator that fails the Nth allocation once armed, counting every + * allocation rather than matching on size. Blocks handed out before the + * failure are filled with a non-zero pattern, since the point of the test is + * what the cleanup path does with allocated but not yet initialized memory. */ +static int srp_oom_count = 0; +static int srp_oom_fail_at = 0; +static int srp_oom_failed = 0; + +#ifdef WOLFSSL_DEBUG_MEMORY +static void* srp_oom_malloc_cb(size_t size, const char* func, unsigned int line) +{ + void* p; + + (void)func; + (void)line; +#else +static void* srp_oom_malloc_cb(size_t size) +{ + void* p; +#endif + + if (srp_oom_fail_at != 0) { + srp_oom_count++; + if (srp_oom_count == srp_oom_fail_at) { + srp_oom_failed = 1; + return NULL; + } + } + + p = malloc(size); + if ((p != NULL) && (srp_oom_fail_at != 0)) { + XMEMSET(p, 0xA5, size); + } + + return p; +} + +#ifdef WOLFSSL_DEBUG_MEMORY +static void srp_oom_free_cb(void* ptr, const char* func, unsigned int line) +{ + (void)func; + (void)line; +#else +static void srp_oom_free_cb(void* ptr) +{ +#endif + free(ptr); +} + +#ifdef WOLFSSL_DEBUG_MEMORY +static void* srp_oom_realloc_cb(void* ptr, size_t size, const char* func, + unsigned int line) +{ + (void)func; + (void)line; +#else +static void* srp_oom_realloc_cb(void* ptr, size_t size) +{ +#endif + return realloc(ptr, size); +} +#endif + +/* An allocation failure part way through the small stack allocations in + * wc_SrpComputeKey must not leave the cleanup path operating on mp_ints that + * were allocated but never initialized. */ +static int test_wc_SrpComputeKey_oom(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && defined(WOLFCRYPT_HAVE_SRP) && \ + defined(WOLFSSL_SMALL_STACK) && \ + defined(USE_WOLFSSL_MEMORY) && !defined(WOLFSSL_NO_MALLOC) && \ + !defined(WOLFSSL_STATIC_MEMORY) && !defined(WOLFSSL_KERNEL_MODE) && \ + !defined(NO_SHA256) + Srp srp; + byte pubKey[8]; + wolfSSL_Malloc_cb prev_mc = NULL; + wolfSSL_Free_cb prev_fc = NULL; + wolfSSL_Realloc_cb prev_rc = NULL; + int allocators_set = 0; + int ret; + int i; + int fired = 0; + + XMEMSET(pubKey, 1, sizeof(pubKey)); + + ExpectIntEQ(wolfSSL_GetAllocators(&prev_mc, &prev_fc, &prev_rc), 0); + ExpectIntEQ(wolfSSL_SetAllocators(srp_oom_malloc_cb, srp_oom_free_cb, + srp_oom_realloc_cb), 0); + if (EXPECT_SUCCESS()) { + allocators_set = 1; + } + + /* wc_SrpComputeKey allocates a hash, a digest and four mp_ints before it + * initializes any of them, so failing part way through leaves allocated + * but uninitialized mp_ints for the cleanup path to deal with. Which + * ordinal lands there moves with the math backend and the small stack + * settings, so sweep past the six it is known to make rather than pin + * one: every failure point has to come back as an error, and none of them + * may crash. */ + for (i = 1; i <= 8 && EXPECT_SUCCESS(); i++) { + /* Arm only around the call under test, so that the allocations + * wc_SrpInit and wc_SrpTerm make are neither counted nor failed. */ + ExpectIntEQ(wc_SrpInit(&srp, SRP_TYPE_SHA256, SRP_CLIENT_SIDE), 0); + if (!EXPECT_SUCCESS()) { + break; + } + + srp_oom_count = 0; + srp_oom_failed = 0; + srp_oom_fail_at = i; + + ret = wc_SrpComputeKey(&srp, pubKey, (word32)sizeof(pubKey), + pubKey, (word32)sizeof(pubKey)); + + srp_oom_fail_at = 0; + + /* Past the last allocation the call makes there is nothing to + * exercise, so only the ordinals that actually landed are judged. */ + if (srp_oom_failed) { + fired++; + ExpectIntLT(ret, 0); + } + + wc_SrpTerm(&srp); + } + + /* A sweep that never injected anything would pass without testing + * anything. */ + ExpectIntGT(fired, 0); + + srp_oom_fail_at = 0; + + if (allocators_set) { + (void)wolfSSL_SetAllocators(prev_mc, prev_fc, prev_rc); + } +#endif + return EXPECT_RESULT(); +} + static int test_read_write_hs(void) { @@ -39090,6 +39235,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_write_dup_want_write), TEST_DECL(test_write_dup_want_write_simul), TEST_DECL(test_write_dup_oom), + TEST_DECL(test_wc_SrpComputeKey_oom), TEST_DECL(test_read_write_hs), TEST_DECL(test_get_signature_nid), #ifndef WOLFSSL_TEST_APPLE_NATIVE_CERT_VALIDATION diff --git a/wolfcrypt/src/srp.c b/wolfcrypt/src/srp.c index 0267952640b..3409dbec434 100644 --- a/wolfcrypt/src/srp.c +++ b/wolfcrypt/src/srp.c @@ -745,6 +745,7 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz, byte pad = 0; int r; int hashInited = 0; + int mpInited = 0; /* validating params */ @@ -776,6 +777,7 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz, r = MP_INIT_E; goto out; } + mpInited = 1; if (mp_iszero(&srp->priv) == MP_YES) { r = SRP_CALL_ORDER_E; @@ -943,27 +945,27 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz, XFREE(hash, srp->heap, DYNAMIC_TYPE_SRP); XFREE(digest, srp->heap, DYNAMIC_TYPE_SRP); if (u) { - if (r != WC_NO_ERR_TRACE(MP_INIT_E)) + if (mpInited) mp_forcezero(u); XFREE(u, srp->heap, DYNAMIC_TYPE_SRP); } if (s) { - if (r != WC_NO_ERR_TRACE(MP_INIT_E)) + if (mpInited) mp_forcezero(s); XFREE(s, srp->heap, DYNAMIC_TYPE_SRP); } if (temp1) { - if (r != WC_NO_ERR_TRACE(MP_INIT_E)) + if (mpInited) mp_forcezero(temp1); XFREE(temp1, srp->heap, DYNAMIC_TYPE_SRP); } if (temp2) { - if (r != WC_NO_ERR_TRACE(MP_INIT_E)) + if (mpInited) mp_forcezero(temp2); XFREE(temp2, srp->heap, DYNAMIC_TYPE_SRP); } #else - if (r != WC_NO_ERR_TRACE(MP_INIT_E)) { + if (mpInited) { mp_forcezero(u); mp_forcezero(s); mp_forcezero(temp1); From 18414d7b4b77322f21fa03f4b6f10b63baf4269c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 29 Jul 2026 18:08:06 +0200 Subject: [PATCH 11/12] Let a DTLS 1.3 reader send the work it schedules With WOLFSSL_RW_THREADED the read path performs no scheduled work, because transmitting from the reader would race the write thread over the output buffer and the sending key schedule, neither of which is covered by a lock. Post-handshake the only remaining consumers are on the write path, and wolfSSL_dtls_retransmit() only helps while the handshake is unfinished. An application that reads without writing therefore never acknowledges a NewSessionTicket, KeyUpdate or connection ID message, and the peer keeps retransmitting what it is waiting to have acknowledged. RFC 9147 relies on those ACKs, so this is a protocol level break rather than a missed optimisation. Add wolfSSL_dtls13_do_scheduled_work() so such an application can send that work from its write thread, and wolfSSL_dtls13_pending_work() so it can tell when there is any. Both entry points ask the same helpers rather than each testing conditions of their own, so they cannot drift into the predicate promising work the pump then declines or silently discards, which would leave a drain loop spinning or mislead the caller about what happened. That covers key updates in particular: none is sent while one of ours is unacknowledged, since DTLS must not have two in flight and Tls13UpdateKeys() drops a locally scheduled one in that state, and a peer request is kept rather than dropped until it can be answered. The predicate also errs towards reporting work when it cannot tell, so a loop surfaces the error rather than stopping silently. Refusing an object is treated as a usage error and leaves ssl->error alone. That field is sticky, since SendData() only clears it for WANT_WRITE, pending async work and the DTLS MAC and decrypt cases, and wolfSSL_write() skips its write-dup drain while it is set, so recording one would disable the very drain a write-dup application depends on. Write-dup pairs are out of scope on both sides. They park the read side's work in the shared WriteDup struct, which only wolfSSL_write() reconciles, so they already have a drain. Completing a key update we started ourselves is out of scope too: that needs the peer's acknowledgement processed, which rotates the sending keys and creates an epoch, and the epoch table has no locking while the read thread mutates it as well. The declaration is gated to match where the definitions live, so a lean build is not promised a symbol it does not get. --- doc/dox_comments/header_files/ssl.h | 81 ++++++++++ src/ssl_api_dtls.c | 221 ++++++++++++++++++++++++++++ wolfssl/ssl.h | 7 + 3 files changed, 309 insertions(+) diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index a6bf5c3fe8c..20c57722160 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -3951,6 +3951,87 @@ int wolfSSL_dtls_got_timeout(WOLFSSL* ssl); */ int wolfSSL_dtls_retransmit(WOLFSSL* ssl); +/*! + \ingroup Setup + + \brief Sends the DTLS 1.3 work that was scheduled while reading. With + WOLFSSL_RW_THREADED the read path never transmits, because that would race + the write thread over the output buffer and the sending key schedule, so + ACKs, retransmissions and a KeyUpdate the peer asked for are only sent from + the write side. An application that reads without writing must call this, + from the same thread it uses for writing, or those messages are never sent + and the peer keeps retransmitting what it is waiting to have acknowledged. + Not for write-dup applications, which drain through wolfSSL_write(). + + A KeyUpdate is only sent while none of ours is still unacknowledged, since + DTLS must not have two in flight. Completing one we started needs the + peer's acknowledgement processed, which rotates the sending keys and so + does not happen here, so in a WOLFSSL_RW_THREADED build a peer request + arriving after that point is held rather than answered. + + A send that could only write part of a record returns WOLFSSL_FATAL_ERROR + with wolfSSL_get_error() reporting SSL_ERROR_WANT_WRITE. That is not a + failure of the connection: the record is held and the next call sends the + rest, so wolfSSL_dtls13_pending_work() keeps reporting work until it is + out. Treat it as a retry rather than as a reason to stop draining. + + \return WOLFSSL_SUCCESS on success, including when there was nothing to do. + \return WOLFSSL_FATAL_ERROR if ssl is NULL, is not a DTLS 1.3 object, is + part of a write-dup pair, or the send failed. Call wolfSSL_get_error() to + tell a retryable SSL_ERROR_WANT_WRITE from a real failure. + + \param ssl a pointer to a WOLFSSL structure, created using wolfSSL_new(). + + _Example_ + \code + WOLFSSL* ssl; + ... + while (wolfSSL_dtls13_pending_work(ssl)) { + if (wolfSSL_dtls13_do_scheduled_work(ssl) != WOLFSSL_SUCCESS) { + if (wolfSSL_get_error(ssl, 0) == SSL_ERROR_WANT_WRITE) { + // the socket is full, wait for it and call again + break; + } + // a real error + break; + } + } + \endcode + + \sa wolfSSL_dtls13_pending_work + \sa wolfSSL_dtls_retransmit +*/ +int wolfSSL_dtls13_do_scheduled_work(WOLFSSL* ssl); + +/*! + \ingroup Setup + + \brief Reports whether the object has DTLS 1.3 work waiting to be sent by + wolfSSL_dtls13_do_scheduled_work(). Only meaningful with + WOLFSSL_RW_THREADED. The answer is advisory and can change as soon as it is + returned. Only work the pump can actually carry out is reported, so a drain + loop over the pair terminates; waiting for the peer to acknowledge a key + update we sent is not reported, as there is nothing to send for it. A + record the pump could only write in part is reported, so that the retry it + owes is not lost. + + \return 1 if there is work to send, or if it could not be determined. + \return 0 if there is nothing to do, or ssl is not supported here. + + \param ssl a pointer to a WOLFSSL structure, created using wolfSSL_new(). + + _Example_ + \code + WOLFSSL* ssl; + ... + if (wolfSSL_dtls13_pending_work(ssl)) + wolfSSL_dtls13_do_scheduled_work(ssl); + \endcode + + \sa wolfSSL_dtls13_do_scheduled_work +*/ +int wolfSSL_dtls13_pending_work(WOLFSSL* ssl); + /*! \brief This function is used to determine if the SSL session has been configured to use DTLS. diff --git a/src/ssl_api_dtls.c b/src/ssl_api_dtls.c index ef10a0da3ec..d7262a6a7e2 100644 --- a/src/ssl_api_dtls.c +++ b/src/ssl_api_dtls.c @@ -1369,6 +1369,227 @@ int wolfSSL_dtls_retransmit(WOLFSSL* ssl) return WOLFSSL_SUCCESS; } +#ifdef WOLFSSL_DTLS13 +/* Is this object the kind the scheduled-work API operates on? + * + * A write-dup pair parks the read side's scheduled work in the shared WriteDup + * struct, and only wolfSSL_write() reconciles it back onto the write side. + * Such applications already have a working drain and are deliberately out of + * scope here, on either side of the pair. + */ +static int Dtls13ScheduledWorkObject(WOLFSSL* ssl) +{ + if (!ssl->options.dtls || !IsAtLeastTLSv1_3(ssl->version)) + return 0; +#ifdef HAVE_WRITE_DUP + if (ssl->dupWrite != NULL) + return 0; +#endif + + return 1; +} + +/* Is there any point running the scheduled work now? + * + * While the handshake runs, wolfSSL_connect()/wolfSSL_accept() and + * wolfSSL_dtls_retransmit() already drive it. Kept separate from the object + * check above so the pump can tell a caller error, which it reports, from + * simply having nothing to do yet, which it does not. + */ +static int Dtls13ScheduledWorkReady(WOLFSSL* ssl) +{ + return Dtls13ScheduledWorkObject(ssl) && ssl->options.handShakeDone; +} + +/* Can a key update be sent right now? + * + * DTLS must not have two in flight, so not while one of ours is still + * unacknowledged. This governs both an update we scheduled ourselves and + * answering one the peer asked for, because Tls13UpdateKeys() silently drops + * the former in that state. Both entry points ask this same question: the + * predicate must not report work the pump then declines or discards, or a + * drain loop over the pair never terminates and the caller is misled about + * what was done. + */ +static int Dtls13CanSendKeyUpdate(WOLFSSL* ssl) +{ + return !ssl->dtls13WaitKeyUpdateAck; +} + +/* Check whether the object has DTLS 1.3 work waiting to be sent. + * + * Only meaningful with WOLFSSL_RW_THREADED, where the read path does not + * transmit. The answer is advisory: it can change as soon as it is returned, + * and wolfSSL_dtls13_do_scheduled_work() is safe to call regardless. + * + * Reports only work that wolfSSL_dtls13_do_scheduled_work() can carry out, so + * that a drain loop over the pair terminates. + * + * @param [in] ssl SSL/TLS object. + * @return 1 when there is work to send, or when it could not be determined. + * @return 0 when there is nothing to do, or ssl is not supported here. + */ +int wolfSSL_dtls13_pending_work(WOLFSSL* ssl) +{ + int pending = 0; + + WOLFSSL_ENTER("wolfSSL_dtls13_pending_work"); + + if (ssl == NULL || !Dtls13ScheduledWorkReady(ssl)) + return 0; + + /* A record this API built but could only write in part. The pump retries + * it, so a drain loop has to be told the retry is still owed. */ + if (ssl->buffers.outputBuffer.length > 0 && ssl->dtls13SendingAckOrRtx) + pending = 1; + + /* dtls13WaitKeyUpdateAck deliberately does not count as work: it says we + * are waiting on the peer, not that we have anything to send. */ + if (!pending && (ssl->dtls13DoKeyUpdate || ssl->options.sendKeyUpdate) && + Dtls13CanSendKeyUpdate(ssl)) { + pending = 1; + } + + if (!pending) { + #ifdef WOLFSSL_RW_THREADED + if (wc_LockMutex(&ssl->dtls13Rtx.mutex) != 0) { + /* Report work rather than nothing, so a drain loop calls the pump + * and surfaces the error instead of stopping silently. */ + return 1; + } + #endif + pending = ssl->dtls13Rtx.sendAcks || ssl->dtls13Rtx.retransmit; + #ifdef WOLFSSL_RW_THREADED + (void)wc_UnLockMutex(&ssl->dtls13Rtx.mutex); + #endif + } + + WOLFSSL_LEAVE("wolfSSL_dtls13_pending_work", pending); + + return pending; +} + +/* Send any DTLS 1.3 work that was scheduled while reading. + * + * Covers pending ACKs, retransmissions and key updates, including the + * KeyUpdate response a peer asked for. With WOLFSSL_RW_THREADED the read path + * never transmits, because doing so would race the write thread over the + * output buffer and the sending key schedule, so this work is only performed + * from the write side. An application that reads without writing has to call + * this or those messages are never sent, and the peer keeps retransmitting + * what it is waiting to have acknowledged. + * + * Call it from the same thread used for writing. Calling it concurrently with + * a write on another thread has the same effect as two concurrent writes. + * + * Not for write-dup applications: those park the read side's work in the + * shared WriteDup struct, which only wolfSSL_write() reconciles, so they + * already have a drain and get WOLFSSL_FATAL_ERROR here rather than a call + * that quietly does nothing. + * + * Note that a key update started from our own side still needs the peer's ACK + * to be processed before it completes, and that processing rotates the sending + * keys, so it cannot run here. + * + * A send that only wrote part of a record reports WANT_WRITE through + * wolfSSL_get_error(). The record is held and the next call sends the rest, + * so that is a retry rather than a reason to stop draining. + * + * @param [in] ssl SSL/TLS object. + * @return WOLFSSL_SUCCESS on success, including when there was nothing to do. + * @return WOLFSSL_FATAL_ERROR when ssl is NULL, unsupported, or on error. + */ +int wolfSSL_dtls13_do_scheduled_work(WOLFSSL* ssl) +{ + int ret; + + WOLFSSL_ENTER("wolfSSL_dtls13_do_scheduled_work"); + + if (ssl == NULL) + return WOLFSSL_FATAL_ERROR; + + /* Rejecting the object is a usage error, not something that happened to + * the connection, so leave ssl->error alone. It is sticky: SendData() + * only clears it for WANT_WRITE, WC_PENDING_E and the DTLS MAC/decrypt + * cases, and wolfSSL_write() skips the write-dup drain entirely while it + * is set, so recording one here would disable the very drain a write-dup + * application relies on. */ +#ifdef HAVE_WRITE_DUP + if (ssl->dupWrite != NULL) { + WOLFSSL_MSG("Write dup objects drain through wolfSSL_write"); + WOLFSSL_LEAVE("wolfSSL_dtls13_do_scheduled_work", WOLFSSL_FATAL_ERROR); + return WOLFSSL_FATAL_ERROR; + } +#endif + + if (!Dtls13ScheduledWorkObject(ssl)) { + WOLFSSL_MSG("Not a DTLS 1.3 object this API handles"); + WOLFSSL_LEAVE("wolfSSL_dtls13_do_scheduled_work", WOLFSSL_FATAL_ERROR); + return WOLFSSL_FATAL_ERROR; + } + + if (!Dtls13ScheduledWorkReady(ssl)) + return WOLFSSL_SUCCESS; + + /* An earlier call may have left a record only partly written. Retry it + * first: the request that produced it has already been consumed, so + * nothing else would send it, and every other caller of + * Dtls13DoScheduledWork() pairs it with this same flush. Only a record + * this API built is retried here, which is what dtls13SendingAckOrRtx + * marks, so a partly written application record is left to the + * wolfSSL_write() the caller has to repeat anyway. */ + if (ssl->buffers.outputBuffer.length > 0 && ssl->dtls13SendingAckOrRtx) { + ret = SendBuffered(ssl); + if (ret != 0) { + ssl->error = ret; + WOLFSSL_ERROR(ret); + return WOLFSSL_FATAL_ERROR; + } + ssl->dtls13SendingAckOrRtx = 0; + } + + ret = Dtls13DoScheduledWork(ssl); + if (ret < 0) { + ssl->error = ret; + WOLFSSL_ERROR(ret); + return WOLFSSL_FATAL_ERROR; + } + + /* Answer a KeyUpdate the peer requested. The read path defers this the + * same way, and SendData() is otherwise the only thing that clears it. + * Skip it while one of ours is still unacknowledged: DTLS must not have + * two KeyUpdates in flight, and Dtls13DoScheduledWork() above may have + * just started one. */ + if (ssl->options.sendKeyUpdate && Dtls13CanSendKeyUpdate(ssl)) { + /* Drop the request before sending, as SendData() does. DTLS commits + * the record to the retransmit queue and consumes the handshake + * number on WANT_WRITE as well, and sets dtls13WaitKeyUpdateAck + * unconditionally, so the update is under way from that point on. + * Leaving the request set would send a second one once the peer + * acknowledges the first. */ + ssl->options.sendKeyUpdate = 0; + /* Mark the record as ours so a short write is retried by the flush + * above rather than waiting for a retransmission timer. */ + ssl->dtls13SendingAckOrRtx = 1; + ret = SendTls13KeyUpdate(ssl); + if (ret != 0) { + /* Keep the mark only while there is something left to send, so a + * failure that wrote nothing does not leave it standing. */ + ssl->dtls13SendingAckOrRtx = + (ssl->buffers.outputBuffer.length > 0); + ssl->error = ret; + WOLFSSL_ERROR(ret); + return WOLFSSL_FATAL_ERROR; + } + ssl->dtls13SendingAckOrRtx = 0; + } + + WOLFSSL_LEAVE("wolfSSL_dtls13_do_scheduled_work", WOLFSSL_SUCCESS); + + return WOLFSSL_SUCCESS; +} +#endif /* WOLFSSL_DTLS13 */ + #endif /* DTLS */ #endif /* LEANPSK */ diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index b28ebeaae0b..9464f23b512 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -1847,6 +1847,13 @@ WOLFSSL_API int wolfSSL_dtls_set_timeout_init(WOLFSSL* ssl, int timeout); WOLFSSL_API int wolfSSL_dtls_set_timeout_max(WOLFSSL* ssl, int timeout); WOLFSSL_API int wolfSSL_dtls_got_timeout(WOLFSSL* ssl); WOLFSSL_API int wolfSSL_dtls_retransmit(WOLFSSL* ssl); +/* Defined alongside the other DTLS calls, inside the !WOLFSSL_LEANPSK block of + * ssl_api_dtls.c, so gate the declaration the same way rather than promising a + * symbol that build does not provide. */ +#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_LEANPSK) +WOLFSSL_API int wolfSSL_dtls13_pending_work(WOLFSSL* ssl); +WOLFSSL_API int wolfSSL_dtls13_do_scheduled_work(WOLFSSL* ssl); +#endif WOLFSSL_API int wolfSSL_dtls(WOLFSSL* ssl); WOLFSSL_API void* wolfSSL_dtls_create_peer(int port, char* ip); From 9b4f00df6b39e9f8c27221697a9b2963ef470a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 29 Jul 2026 18:08:06 +0200 Subject: [PATCH 12/12] Cover the DTLS 1.3 scheduled work API in the tests Several DTLS tests drive a connection with reads alone and then assert that something was sent, an ACK in most cases. With WOLFSSL_RW_THREADED that only happens once the application asks for it, so stand in for such an application and pump where the send is expected. The helper is a no-op elsewhere, so builds whose read path sends for itself are unchanged. test_dtls13_ack_overflow needs the same treatment in its setup, where the ACK the first reads scheduled would otherwise be left in the seen-record list and counted by the assertions that follow. It sits in the dtls13 group rather than dtls, so a run of the dtls group alone does not cover it. Add a test for the new API that runs in every build rather than only the threaded one. It schedules a key update the way the AEAD failure limit does and requires the predicate to report it, the pump to perform it and put a record on the wire, and the wait for the peer's acknowledgement not to be reported as work. It then drives the state that would wedge a drain loop, a peer requesting a KeyUpdate while ours is unacknowledged, and requires pump and predicate to agree that nothing can be sent and the request to be kept until it can. It also covers the bad argument cases, a DTLS 1.2 object being refused rather than quietly succeeding, and that refusing an object records no error against the connection and leaves it usable. The AEAD limit test excludes its second key update and its hard limit check from threaded builds. Both need the acknowledgement processing that stays off the write path: without it the decrypting epoch stops matching the one the drop counter is placed on, so the read never reaches the limit and the test hangs rather than failing. --- tests/api/test_dtls.c | 158 ++++++++++++++++++++++++++++++++++++++++ tests/api/test_dtls.h | 2 + tests/api/test_dtls13.c | 4 + tests/utils.h | 18 +++++ 4 files changed, 182 insertions(+) diff --git a/tests/api/test_dtls.c b/tests/api/test_dtls.c index 0f22fc606ed..95decd2b11a 100644 --- a/tests/api/test_dtls.c +++ b/tests/api/test_dtls.c @@ -852,6 +852,7 @@ int test_dtls13_new_connection_id(void) recSz), 0); ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1); ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + TEST_DTLS13_PUMP(ssl_s); /* the server ACKed the message */ ExpectIntGT(test_ctx.c_len, 0); ExpectIntEQ(wolfSSL_dtls_cid_get_tx_size(ssl_s, &cidSz), 1); @@ -902,6 +903,7 @@ int test_dtls13_new_connection_id(void) recSz), 0); ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1); ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + TEST_DTLS13_PUMP(ssl_s); cidSz = 0; ExpectIntEQ(wolfSSL_dtls_cid_get_tx_size(ssl_s, &cidSz), 1); ExpectIntEQ(cidSz, sizeof(newCid)); @@ -970,6 +972,7 @@ int test_dtls13_request_connection_id(void) recSz), 0); ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1); ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + TEST_DTLS13_PUMP(ssl_s); ExpectIntGT(test_ctx.c_len, 0); /* nothing but the ACK reaches the client */ ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1); @@ -2151,6 +2154,7 @@ int test_dtls_drop_client_ack(void) /* this should re-send the ack immediately */ ExpectIntEQ(wolfSSL_read(ssl_s, data, 32), -1); ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + TEST_DTLS13_PUMP(ssl_s); ExpectIntEQ(test_ctx.c_msg_count, 1); /* This should advance the connection on the client */ @@ -4012,6 +4016,22 @@ static void test_AEAD_get_limits(WOLFSSL* ssl, w64wrapper* hardLimit, } } +/* A DTLS 1.3 key update is scheduled while reading but is only transmitted + * from the write path. With WOLFSSL_RW_THREADED the read path does no + * scheduled work at all, because the reader must not transmit while a writer + * thread may be doing so, so an application that reads without writing has to + * pump the deferred work itself. Do what such an application would do. */ +static void test_AEAD_drain_scheduled_work(WOLFSSL* ssl) +{ + /* Match where wolfSSL_dtls13_do_scheduled_work() is declared, in + * wolfssl/ssl.h, or a WOLFSSL_LEANPSK build has no declaration for it. */ +#if defined(WOLFSSL_RW_THREADED) && !defined(WOLFSSL_LEANPSK) + AssertIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl), WOLFSSL_SUCCESS); +#else + (void)ssl; +#endif +} + static void test_AEAD_limit_client(WOLFSSL* ssl) { int ret; @@ -4049,6 +4069,7 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) /* Key update should be sent and negotiated */ ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf)); AssertIntGT(ret, 0); + test_AEAD_drain_scheduled_work(ssl); /* Epoch after one key update is 4 */ if (w64Equal(ssl->dtls13PeerEpoch, w64From32(0, 4)) && w64Equal(Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch)->dropCount, counter)) { @@ -4058,6 +4079,17 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) } AssertTrue(didReKey); + /* Everything below needs the ACK of that first key update to be + * processed, which rotates our own sending keys and so creates an epoch. + * The epoch table has no locking, and with WOLFSSL_RW_THREADED the read + * thread mutates it too, so that processing deliberately stays off the + * write path and a second key update cannot complete in such a build. + * Don't assert behaviour that is knowingly not provided. The hard limit + * check below is inside this too, and deliberately: without that ACK the + * decrypting epoch stops matching the one the drop counter is placed on, + * so the read never reaches the limit and loops until the test times out. + */ +#ifndef WOLFSSL_RW_THREADED if (!w64IsZero(sendLimit)) { /* Test the sending limit for AEAD ciphers */ #ifdef WOLFSSL_MUTEX_INITIALIZER @@ -4095,7 +4127,13 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf)); AssertIntEQ(ret, WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)); AssertIntEQ(wolfSSL_get_error(ssl, ret), WC_NO_ERR_TRACE(DECRYPT_ERROR)); +#else + (void)sendLimit; + (void)hardLimit; +#endif + /* Always signal completion, including on the paths skipped above, so the + * peer thread does not spin waiting for a flag that never gets set. */ #ifdef WOLFSSL_ATOMIC_INITIALIZER WOLFSSL_ATOMIC_STORE(test_AEAD_done, 1); #else @@ -6250,6 +6288,126 @@ int test_wolfSSL_dtls_create_free_peer(void) return EXPECT_RESULT(); } +/* wolfSSL_dtls13_do_scheduled_work() and wolfSSL_dtls13_pending_work() exist + * so an application whose read thread cannot transmit can send it itself. + * Cover the contract in every build, not just the threaded one: the pump is + * always safe to call, and the predicate must not claim work it cannot do. */ +int test_wolfSSL_dtls_scheduled_work(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_LEANPSK) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) + struct test_memio_ctx test_ctx; + WOLFSSL_CTX* ctx_c = NULL; + WOLFSSL_CTX* ctx_s = NULL; + WOLFSSL* ssl_c = NULL; + WOLFSSL* ssl_s = NULL; + int prevLen = 0; + + /* Bad arguments. */ + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(NULL), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_dtls13_pending_work(NULL), 0); + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* Idle connection: nothing to do, and pumping is still a success. */ + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_dtls13_pending_work(ssl_s), 0); + + /* Pumping must be idempotent, and must not invent traffic when there is + * nothing scheduled. Compare against what is already queued rather than + * clearing it, which would drop records the peer still needs. */ + prevLen = test_ctx.c_len; + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(test_ctx.c_len, prevLen); + + /* The connection still works in both directions after being pumped. */ + ExpectIntEQ(test_dtls_communication(ssl_s, ssl_c), TEST_SUCCESS); + + /* Positive path. Schedule a key update the way Dtls13CheckAEADFailLimit() + * does when the AEAD failure limit is reached, then require the predicate + * to report it, the pump to perform it, and both to settle afterwards. */ + if (ssl_s != NULL) { + ssl_s->dtls13DoKeyUpdate = 1; + } + ExpectIntEQ(wolfSSL_dtls13_pending_work(ssl_s), 1); + prevLen = test_ctx.c_len; + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl_s), WOLFSSL_SUCCESS); + /* The update was performed, and it put a record on the wire. */ + if (ssl_s != NULL) { + ExpectIntEQ(ssl_s->dtls13DoKeyUpdate, 0); + } + ExpectIntGT(test_ctx.c_len, prevLen); + /* Waiting for the peer's ACK is not work we can do, so the predicate must + * report nothing rather than making a drain loop spin. */ + if (ssl_s != NULL) { + ExpectIntEQ(ssl_s->dtls13WaitKeyUpdateAck, 1); + } + ExpectIntEQ(wolfSSL_dtls13_pending_work(ssl_s), 0); + + /* The peer asking for a KeyUpdate while one of ours is unacknowledged is + * the state that wedges a drain loop: DTLS must not put two in flight, so + * the pump declines, and the predicate must decline to report it too. + * Pump and predicate have to agree, or the documented loop never ends. */ + if (ssl_s != NULL) { + ssl_s->options.sendKeyUpdate = 1; + } + ExpectIntEQ(wolfSSL_dtls13_pending_work(ssl_s), 0); + prevLen = test_ctx.c_len; + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(test_ctx.c_len, prevLen); + /* The request is kept, not silently dropped, for when it can be sent. */ + if (ssl_s != NULL) { + ExpectIntEQ(ssl_s->options.sendKeyUpdate, 1); + /* Once nothing is outstanding the pair agrees it is sendable. */ + ssl_s->dtls13WaitKeyUpdateAck = 0; + } + ExpectIntEQ(wolfSSL_dtls13_pending_work(ssl_s), 1); + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl_s), WOLFSSL_SUCCESS); + if (ssl_s != NULL) { + ExpectIntEQ(ssl_s->options.sendKeyUpdate, 0); + } + ExpectIntGT(test_ctx.c_len, prevLen); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + ssl_c = NULL; + ssl_s = NULL; + ctx_c = NULL; + ctx_s = NULL; + + /* A DTLS 1.2 object is out of scope: the pump must say so rather than + * quietly succeed, and the predicate must not claim work. */ +#ifndef WOLFSSL_NO_TLS12 + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_2_client_method, wolfDTLSv1_2_server_method), 0); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl_s), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_dtls13_pending_work(ssl_s), 0); + /* Refusing the object must not record an error against the connection. + * ssl->error is sticky, and wolfSSL_write() skips its write-dup drain + * while it is set, so a rejected call has to leave it alone. */ + if (ssl_s != NULL) { + ExpectIntEQ(ssl_s->error, 0); + } + /* And the connection must still be usable afterwards. */ + ExpectIntEQ(test_dtls_communication(ssl_s, ssl_c), TEST_SUCCESS); +#endif + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_dtls_get0_peer(void) { EXPECT_DECLS; diff --git a/tests/api/test_dtls.h b/tests/api/test_dtls.h index 28b84547b7a..37d8a8a2a47 100644 --- a/tests/api/test_dtls.h +++ b/tests/api/test_dtls.h @@ -53,6 +53,7 @@ int test_dtls_mtu_split_messages(void); int test_dtls_set_session_min_downgrade(void); int test_dtls12_export_import_etm(void); int test_wolfSSL_dtls_create_free_peer(void); +int test_wolfSSL_dtls_scheduled_work(void); int test_wolfSSL_dtls_get0_peer(void); int test_wolfSSL_dtls_set_timeout_init(void); int test_wolfSSL_dtls_retransmit(void); @@ -177,6 +178,7 @@ int test_WOLFSSL_dtls_version_alert(void); TEST_DECL_GROUP("dtls", test_dtls13_no_session_id_echo), \ TEST_DECL_GROUP("dtls", test_dtls_set_session_min_downgrade), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_create_free_peer), \ + TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_scheduled_work), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_get0_peer), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_set_timeout_init), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_retransmit), \ diff --git a/tests/api/test_dtls13.c b/tests/api/test_dtls13.c index a02ae2da7ff..c8fdcd65dfd 100644 --- a/tests/api/test_dtls13.c +++ b/tests/api/test_dtls13.c @@ -1226,6 +1226,10 @@ int test_dtls13_ack_overflow(void) ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1); ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + /* Flush the ACK those reads scheduled, so the seen-record list starts + * empty and the counts below are exact. */ + TEST_DTLS13_PUMP(ssl_c); + TEST_DTLS13_PUMP(ssl_s); /* Edge case 1: one below limit - all inserts must succeed */ for (i = 0; i < DTLS13_ACK_MAX_RECORDS - 1; i++) { diff --git a/tests/utils.h b/tests/utils.h index e201ebdec16..a12c027ca6c 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -51,6 +51,24 @@ extern const char* currentTestName; #define HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_BUILD #endif +/* With WOLFSSL_RW_THREADED the read path never transmits, so anything a read + * schedules to be sent, an ACK in particular, is only sent from the write + * side. Stand in for the application, which is required to pump that work + * from its write thread. A no-op in builds where reads send for themselves. */ +#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_RW_THREADED) && \ + !defined(WOLFSSL_LEANPSK) + #define TEST_DTLS13_PUMP(ssl) \ + do { \ + ExpectIntEQ(wolfSSL_dtls13_do_scheduled_work(ssl), \ + WOLFSSL_SUCCESS); \ + } while (0) +#else + #define TEST_DTLS13_PUMP(ssl) \ + do { \ + (void)(ssl); \ + } while (0) +#endif + #ifdef HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_BUILD #define TEST_MEMIO_BUF_SZ (64 * 1024) #define TEST_MEMIO_MAX_MSGS 32