From 4b7e8cc40206d88b6c8e4920c671b81e03de0eeb Mon Sep 17 00:00:00 2001 From: Mattia Moffa Date: Fri, 5 Jun 2026 20:01:18 +0200 Subject: [PATCH] DTLS: check CID is newest before promoting new peer address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows RFC 9146 ยง 6. Fixes #10609 --- src/internal.c | 55 +++++++++++++++++--- tests/api/test_dtls.c | 118 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_dtls.h | 2 + 3 files changed, 169 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index a8d26cdc42..501fc95492 100644 --- a/src/internal.c +++ b/src/internal.c @@ -22907,14 +22907,46 @@ static void dtlsClearPeer(WOLFSSL_SOCKADDR* peer) peer->bufSz = 0; } +static int dtlsRecordIsNewest(WOLFSSL* ssl) +{ + WOLFSSL_DTLS_PEERSEQ* peerSeq; + +#ifdef WOLFSSL_DTLS13 + if (IsAtLeastTLSv1_3(ssl->version)) { + Dtls13Epoch* e = ssl->dtls13DecryptEpoch; + + if (!w64Equal(ssl->keys.curEpoch64, ssl->dtls13PeerEpoch)) + return w64GT(ssl->keys.curEpoch64, ssl->dtls13PeerEpoch); + if (e == NULL || !w64Equal(ssl->keys.curEpoch64, e->epochNumber)) + e = Dtls13GetEpoch(ssl, ssl->keys.curEpoch64); + if (e == NULL) + return 0; + return w64GTE(ssl->keys.curSeq, e->nextPeerSeqNumber); + } +#endif + + peerSeq = ssl->keys.peerSeq; +#ifdef WOLFSSL_MULTICAST + if (ssl->options.haveMcast) + return 1; +#endif + + if (ssl->keys.curEpoch != peerSeq->nextEpoch) + return ssl->keys.curEpoch > peerSeq->nextEpoch; + if (ssl->keys.curSeq_hi != peerSeq->nextSeq_hi) + return ssl->keys.curSeq_hi > peerSeq->nextSeq_hi; + return ssl->keys.curSeq_lo >= peerSeq->nextSeq_lo; +} + /** * @brief Handle pending peer during record processing. * @param ssl WOLFSSL object. * @param deprotected 0 when we have not decrypted the record yet * 1 when we have decrypted and verified the record + * @param isNewest 1 when the record is newer than any previously received */ -static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected) +static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected, int isNewest) { if (ssl->buffers.dtlsCtx.pendingPeer.sa != NULL) { if (!deprotected) { @@ -22932,9 +22964,11 @@ static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected) } else { /* Pending peer present and record deprotected. Update the peer. */ - (void)wolfSSL_dtls_set_peer(ssl, - ssl->buffers.dtlsCtx.pendingPeer.sa, - ssl->buffers.dtlsCtx.pendingPeer.sz); + if (isNewest) { + (void)wolfSSL_dtls_set_peer(ssl, + ssl->buffers.dtlsCtx.pendingPeer.sa, + ssl->buffers.dtlsCtx.pendingPeer.sz); + } ssl->buffers.dtlsCtx.processingPendingRecord = 0; dtlsClearPeer(&ssl->buffers.dtlsCtx.pendingPeer); } @@ -23276,7 +23310,7 @@ static int DoProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) #ifdef WOLFSSL_DTLS #ifdef WOLFSSL_DTLS_CID if (ssl->options.dtls) - dtlsProcessPendingPeer(ssl, 0); + dtlsProcessPendingPeer(ssl, 0, 0); #endif if (ssl->options.dtls && DtlsShouldDrop(ssl, ret)) { DropAndRestartProcessReply(ssl); @@ -23595,6 +23629,9 @@ static int DoProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) case runProcessingOneRecord: #ifdef WOLFSSL_DTLS if (ssl->options.dtls) { +#ifdef WOLFSSL_DTLS_CID + int dtlsPeerNewer = 1; +#endif #ifdef WOLFSSL_DTLS13 if (IsAtLeastTLSv1_3(ssl->version)) { if (!Dtls13CheckWindow(ssl)) { @@ -23613,6 +23650,9 @@ static int DoProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) /* Only update the window once we enter stateful parsing */ if (ssl->options.dtlsStateful) { +#ifdef WOLFSSL_DTLS_CID + dtlsPeerNewer = dtlsRecordIsNewest(ssl); +#endif ret = Dtls13UpdateWindowRecordRecvd(ssl); if (ret != 0) { WOLFSSL_ERROR(ret); @@ -23623,12 +23663,15 @@ static int DoProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) else #endif /* WOLFSSL_DTLS13 */ if (IsDtlsNotSctpMode(ssl)) { +#ifdef WOLFSSL_DTLS_CID + dtlsPeerNewer = dtlsRecordIsNewest(ssl); +#endif DtlsUpdateWindow(ssl); } #ifdef WOLFSSL_DTLS_CID /* Update the peer if we were able to de-protect the message */ if (IsEncryptionOn(ssl, 0)) - dtlsProcessPendingPeer(ssl, 1); + dtlsProcessPendingPeer(ssl, 1, dtlsPeerNewer); #endif } #endif /* WOLFSSL_DTLS */ diff --git a/tests/api/test_dtls.c b/tests/api/test_dtls.c index 572dda6c94..1406ac0f1f 100644 --- a/tests/api/test_dtls.c +++ b/tests/api/test_dtls.c @@ -518,6 +518,124 @@ int test_wolfSSL_dtls_set_pending_peer(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_CID) +/* Capture the single client->server record currently buffered, then reset the + * server's incoming buffer so the next record can be captured independently. */ +static int test_dtls_capture_record(struct test_memio_ctx* test_ctx, + byte* out, int* outLen) +{ + EXPECT_DECLS; + ExpectIntEQ(test_ctx->s_msg_count, 1); + if (EXPECT_SUCCESS()) { + *outLen = test_ctx->s_len; + XMEMCPY(out, test_ctx->s_buff, (size_t)test_ctx->s_len); + test_ctx->s_len = 0; + test_ctx->s_msg_count = 0; + test_ctx->s_msg_pos = 0; + } + return EXPECT_RESULT(); +} + +/* Deliver a previously captured record to the server's incoming buffer. */ +static void test_dtls_inject_record(struct test_memio_ctx* test_ctx, + const byte* rec, int recLen) +{ + XMEMCPY(test_ctx->s_buff, rec, (size_t)recLen); + test_ctx->s_len = recLen; + test_ctx->s_msg_sizes[0] = recLen; + test_ctx->s_msg_count = 1; + test_ctx->s_msg_pos = 0; +} + +static int test_dtls_pending_peer_not_newest(method_provider method_c, + method_provider method_s) +{ + EXPECT_DECLS; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + unsigned char peer[10]; + unsigned int peerSz; + unsigned char readBuf[10]; + unsigned char client_cid[] = { 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 }; + unsigned char server_cid[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; + byte recA[512], recB[512], recC[512]; + int lenA = 0, lenB = 0, lenC = 0; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + method_c, method_s), 0); + + ExpectIntEQ(wolfSSL_dtls_cid_use(ssl_c), 1); + ExpectIntEQ(wolfSSL_dtls_cid_set(ssl_c, server_cid, sizeof(server_cid)), 1); + ExpectIntEQ(wolfSSL_dtls_cid_use(ssl_s), 1); + ExpectIntEQ(wolfSSL_dtls_cid_set(ssl_s, client_cid, sizeof(client_cid)), 1); + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* Capture three consecutive client records with increasing seq numbers. */ + ExpectIntEQ(wolfSSL_write(ssl_c, "msgA", 5), 5); + ExpectIntEQ(test_dtls_capture_record(&test_ctx, recA, &lenA), TEST_SUCCESS); + ExpectIntEQ(wolfSSL_write(ssl_c, "msgB", 5), 5); + ExpectIntEQ(test_dtls_capture_record(&test_ctx, recB, &lenB), TEST_SUCCESS); + ExpectIntEQ(wolfSSL_write(ssl_c, "msgC", 5), 5); + ExpectIntEQ(test_dtls_capture_record(&test_ctx, recC, &lenC), TEST_SUCCESS); + + /* Deliver the newer record (B) first to advance the receive window. */ + test_dtls_inject_record(&test_ctx, recB, lenB); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), 5); + ExpectStrEQ(readBuf, "msgB"); + peerSz = sizeof(peer); + ExpectIntEQ(wolfSSL_dtls_get_peer(ssl_s, peer, &peerSz), 0); + + /* Nominate a pending peer, then deliver the older record (A). It is a valid + * CID record but not newer than B, so the peer must NOT be updated. */ + ExpectIntEQ(wolfSSL_dtls_set_pending_peer(ssl_s, (void*)"123", 4), 1); + test_dtls_inject_record(&test_ctx, recA, lenA); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), 5); + ExpectStrEQ(readBuf, "msgA"); + peerSz = sizeof(peer); + ExpectIntEQ(wolfSSL_dtls_get_peer(ssl_s, peer, &peerSz), 0); + + /* Nominate again, then deliver the newest record (C). Now the peer must be + * updated. */ + ExpectIntEQ(wolfSSL_dtls_set_pending_peer(ssl_s, (void*)"456", 4), 1); + test_dtls_inject_record(&test_ctx, recC, lenC); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), 5); + ExpectStrEQ(readBuf, "msgC"); + peerSz = sizeof(peer); + ExpectIntEQ(wolfSSL_dtls_get_peer(ssl_s, peer, &peerSz), 1); + ExpectIntEQ(peerSz, 4); + ExpectStrEQ(peer, "456"); + + wolfSSL_free(ssl_s); + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_s); + wolfSSL_CTX_free(ctx_c); + + return EXPECT_RESULT(); +} +#endif + +int test_wolfSSL_dtls_set_pending_peer_not_newest(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_CID) +#ifndef WOLFSSL_NO_TLS12 + ExpectIntEQ(test_dtls_pending_peer_not_newest(wolfDTLSv1_2_client_method, + wolfDTLSv1_2_server_method), TEST_SUCCESS); +#endif +#ifdef WOLFSSL_DTLS13 + ExpectIntEQ(test_dtls_pending_peer_not_newest(wolfDTLSv1_3_client_method, + wolfDTLSv1_3_server_method), TEST_SUCCESS); +#endif +#endif + return EXPECT_RESULT(); +} + diff --git a/tests/api/test_dtls.h b/tests/api/test_dtls.h index fd22d125bf..7679a6646d 100644 --- a/tests/api/test_dtls.h +++ b/tests/api/test_dtls.h @@ -25,6 +25,7 @@ int test_dtls12_basic_connection_id(void); int test_wolfSSL_dtls_cid_parse(void); int test_wolfSSL_dtls_set_pending_peer(void); +int test_wolfSSL_dtls_set_pending_peer_not_newest(void); int test_dtls_version_checking(void); int test_dtls_short_ciphertext(void); int test_dtls12_record_length_mismatch(void); @@ -84,6 +85,7 @@ int test_WOLFSSL_dtls_version_alert(void); TEST_DECL_GROUP("dtls", test_dtls12_basic_connection_id), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_cid_parse), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_set_pending_peer), \ + TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_set_pending_peer_not_newest),\ TEST_DECL_GROUP("dtls", test_dtls_version_checking), \ TEST_DECL_GROUP("dtls", test_dtls_short_ciphertext), \ TEST_DECL_GROUP("dtls", test_dtls12_record_length_mismatch), \