crypt/wolfssl
1
0
Fork 0
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-10-24 04:09:07 +03:00
Code Activity

DTLS 1.3: only ack handshake records that were processed or buffered

Browse Source 
F-8926

RFC 9147 Sec 7 forbids acking a record whose handshake message was neither
processed nor buffered: the peer stops retransmitting it and the handshake
deadlocks. Dtls13RecordRecvd() added the ack at the record layer, before
_Dtls13HandshakeRecv() had a chance to drop the message (oversized plaintext
message, non-ClientHello before the ClientHello is verified, fragmented
plaintext message we don't reassemble, a buffering failure, or a handler
returning WANT_WRITE).

DtlsMsgStore() now reports whether the fragment was stored so the DTLS 1.3
caller can tell "buffered" from "dropped", and the three DtlsMsgSet() paths
that dropped a fragment on an allocation failure while returning success now
return MEMORY_ERROR. The DTLS 1.2 callers keep ignoring the value, matching
their current behaviour.

A retransmission of an already processed message is still acked. That message
was processed, and the ack is what stops a peer that retransmitted because our
previous ack was lost. The post-handshake CertificateRequest stays implicitly
acked by the flight it triggers.

Acks are keyed by record, so a record packing a dropped message behind a kept
one is still acked. Closing that needs record-scoped state in DoProcessReplyEx
and is not worth it for a peer that only hurts itself.
This commit is contained in:
Juliusz Sosinowicz
2026-08-14 18:18:08 +00:00
parent 65559843b5
commit eee0c8d7c2
3 changed files with 60 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
src/dtls13.c
View File

@@ -803,6 +803,18 @@ int Dtls13RtxAddAck(WOLFSSL* ssl, w64wrapper epoch, w64wrapper seq)
return 0;
}
/* RFC 9147 Sec 7: only ack a record whose message we processed or buffered.
* Acking a dropped message stops the peer retransmitting it and deadlocks. */
static void Dtls13RtxAddAckForCurRecord(WOLFSSL* ssl)
{
/* Stateless processing must not touch the ssl object. */
if (!ssl->options.dtlsStateful)
return;
if (Dtls13RtxAddAck(ssl, ssl->keys.curEpoch64, ssl->keys.curSeq) != 0)
WOLFSSL_MSG("can't save ack fragment");
}
static void Dtls13RtxFlushAcks(WOLFSSL* ssl)
{
Dtls13RecordNumber *list, *rn;
@@ -934,8 +946,9 @@ static void Dtls13SaveOrFlushClientHello(WOLFSSL* ssl)
}
}
/* implicitAck is set when the flight we send already acks this record. */
static int Dtls13RtxMsgRecvd(WOLFSSL* ssl, enum HandShakeType hs,
word32 fragOffset)
word32 fragOffset, byte* implicitAck)
{
WOLFSSL_ENTER("Dtls13RtxMsgRecvd");
@@ -965,6 +978,10 @@ static int Dtls13RtxMsgRecvd(WOLFSSL* ssl, enum HandShakeType hs,
/* retransmission detected. */
ssl->dtls13Rtx.retransmit = 1;
/* Already processed, so acking is allowed. It stops a peer that
* retransmitted because our earlier ack was lost. */
Dtls13RtxAddAckForCurRecord(ssl);
/* the other peer may have retransmitted because an ACK for a flight
that needs explicit ACK was lost.*/
if (ssl->dtls13Rtx.seenRecords != NULL)
@@ -985,6 +1002,7 @@ static int Dtls13RtxMsgRecvd(WOLFSSL* ssl, enum HandShakeType hs,
should be rare and simplifies the code. Otherwise, it would be
necessary to track which record number contained a CertificateRequest
with a particular context id */
*implicitAck = 1;
Dtls13RtxRemoveCurAck(ssl);
}
@@ -1616,19 +1634,14 @@ int Dtls13ParseUnifiedRecordLayer(WOLFSSL* ssl, const byte* input,
int Dtls13RecordRecvd(WOLFSSL* ssl)
{
int ret;
if (ssl->curRL.type != handshake)
return 0;
if (!ssl->options.dtls13SendMoreAcks)
ssl->dtls13FastTimeout = 1;
ret = Dtls13RtxAddAck(ssl, ssl->keys.curEpoch64, ssl->keys.curSeq);
if (ret != 0)
WOLFSSL_MSG("can't save ack fragment");
return ret;
/* Acking happens in Dtls13RtxAddAckForCurRecord(). */
return 0;
}
static void Dtls13RtxMoveToEndOfList(WOLFSSL* ssl, Dtls13RtxRecord** prevNext,
@@ -1874,10 +1887,12 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size,
byte usingAsyncCrypto;
word32 messageLength;
byte handshakeType;
byte implicitAck;
word32 idx;
int ret;
idx = 0;
implicitAck = 0;
ret = GetDtlsHandShakeHeader(ssl, input, &idx, &handshakeType,
&messageLength, &fragOff, &fragLength, size);
if (ret != 0)
@@ -1937,7 +1952,8 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size,
if (fragOff + fragLength > messageLength)
return BUFFER_ERROR;
ret = Dtls13RtxMsgRecvd(ssl, (enum HandShakeType)handshakeType, fragOff);
ret = Dtls13RtxMsgRecvd(ssl, (enum HandShakeType)handshakeType, fragOff,
&implicitAck);
if (ret != 0)
return ret;
@@ -2001,10 +2017,13 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size,
ssl->keys.dtls_expected_peer_handshake_number ||
usingAsyncCrypto) {
if (ssl->dtls_rx_msg_list_sz < DTLS_POOL_SZ) {
DtlsMsgStore(ssl, (word16)w64GetLow32(ssl->keys.curEpoch64),
ssl->keys.dtls_peer_handshake_number,
input + DTLS_HANDSHAKE_HEADER_SZ, messageLength, handshakeType,
fragOff, fragLength, ssl->heap);
/* Only ack a fragment we really buffered. */
if (DtlsMsgStore(ssl, (word16)w64GetLow32(ssl->keys.curEpoch64),
ssl->keys.dtls_peer_handshake_number,
input + DTLS_HANDSHAKE_HEADER_SZ, messageLength,
handshakeType, fragOff, fragLength, ssl->heap) == 0) {
Dtls13RtxAddAckForCurRecord(ssl);
}
}
else {
/* DTLS_POOL_SZ outstanding messages is way more than enough for any
@@ -2025,6 +2044,9 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size,
if (ret != 0)
return ret;
if (!implicitAck)
Dtls13RtxAddAckForCurRecord(ssl);
Dtls13MsgWasProcessed(ssl, (enum HandShakeType)handshakeType);
/* check if we have buffered some message */

37
src/internal.c
View File

@@ -10717,10 +10717,10 @@ int DtlsMsgSet(DtlsMsg* msg, word32 seq, word16 epoch, const byte* data, byte ty
}
prev->m.m.next =
DtlsMsgCreateFragBucket(fragOffset, data, fragSz, heap);
if (prev->m.m.next != NULL) {
msg->bytesReceived += fragSz;
msg->fragBucketListCount++;
}
if (prev->m.m.next == NULL)
return MEMORY_ERROR;
msg->bytesReceived += fragSz;
msg->fragBucketListCount++;
}
else if (fragOffsetEnd < cur->m.m.offset) {
/* Fragment is entirely before cur with a gap */
@@ -10741,6 +10741,7 @@ int DtlsMsgSet(DtlsMsg* msg, word32 seq, word16 epoch, const byte* data, byte ty
else {
/* reset on error */
*prev_next = cur;
return MEMORY_ERROR;
}
}
else {
@@ -10754,8 +10755,11 @@ int DtlsMsgSet(DtlsMsg* msg, word32 seq, word16 epoch, const byte* data, byte ty
/* We can combine the buckets */
*prev_next = DtlsMsgCombineFragBuckets(msg, cur, next,
fragOffset, data, fragSz, heap);
if (*prev_next == NULL) /* reset on error */
if (*prev_next == NULL) {
/* reset on error */
*prev_next = cur;
return MEMORY_ERROR;
}
}
}
}
@@ -10777,7 +10781,8 @@ DtlsMsg* DtlsMsgFind(DtlsMsg* head, word16 epoch, word32 seq)
}
void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
/* Returns 0 when the fragment was stored, negative when it was dropped. */
int DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
word32 dataSz, byte type, word32 fragOffset, word32 fragSz, void* heap)
{
/* See if seq exists in the list. If it isn't in the list, make
@@ -10799,6 +10804,7 @@ void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
DtlsMsg* head = ssl->dtls_rx_msg_list;
byte encrypted = ssl->keys.decryptedCur == 1;
int ret = 0;
WOLFSSL_ENTER("DtlsMsgStore");
if (head != NULL) {
@@ -10807,11 +10813,13 @@ void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
cur = DtlsMsgNew(dataSz, 0, heap);
if (cur == NULL) {
WOLFSSL_MSG("DtlsMsgNew allocation failed");
ssl->error = MEMORY_E;
ret = MEMORY_E;
ssl->error = ret;
}
else {
if (DtlsMsgSet(cur, seq, epoch, data, type,
fragOffset, fragSz, heap, dataSz, encrypted) < 0) {
ret = DtlsMsgSet(cur, seq, epoch, data, type,
fragOffset, fragSz, heap, dataSz, encrypted);
if (ret < 0) {
DtlsMsgDelete(cur, heap);
}
else {
@@ -10822,7 +10830,7 @@ void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
}
else {
/* If this fails, the data is just dropped. */
DtlsMsgSet(cur, seq, epoch, data, type, fragOffset,
ret = DtlsMsgSet(cur, seq, epoch, data, type, fragOffset,
fragSz, heap, dataSz, encrypted);
}
}
@@ -10830,10 +10838,11 @@ void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
head = DtlsMsgNew(dataSz, 0, heap);
if (head == NULL) {
WOLFSSL_MSG("DtlsMsgNew allocation failed");
ssl->error = MEMORY_E;
ret = MEMORY_E;
ssl->error = ret;
}
else if (DtlsMsgSet(head, seq, epoch, data, type, fragOffset,
fragSz, heap, dataSz, encrypted) < 0) {
else if ((ret = DtlsMsgSet(head, seq, epoch, data, type, fragOffset,
fragSz, heap, dataSz, encrypted)) < 0) {
DtlsMsgDelete(head, heap);
head = NULL;
}
@@ -10843,6 +10852,8 @@ void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
}
ssl->dtls_rx_msg_list = head;
return ret;
}

2
wolfssl/internal.h
View File

@@ -7431,7 +7431,7 @@ WOLFSSL_LOCAL word32 MacSize(const WOLFSSL* ssl);
word32 totalLen, byte encrypted);
WOLFSSL_TEST_VIS DtlsMsg* DtlsMsgFind(DtlsMsg* head, word16 epoch, word32 seq);
WOLFSSL_TEST_VIS void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq,
WOLFSSL_TEST_VIS int DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq,
const byte* data, word32 dataSz, byte type,
word32 fragOffset, word32 fragSz,
void* heap);