DTLS: fix several sequence-number, epoch and Connection ID vulnerabilities

src/dtls.c

@@ -1323,6 +1323,16 @@ int TLSX_ConnectionID_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     if (cidSz + OPAQUE8_LEN > length)
         return BUFFER_ERROR;
 
+#if DTLS_CID_MAX_SIZE < 255
+    /* The peer's CID becomes our TX CID. RFC 9146 allows up to 255 bytes, our
+     * send buffers are sized for DTLS_CID_MAX_SIZE. */
+    if (cidSz > DTLS_CID_MAX_SIZE) {
+        WOLFSSL_MSG("Peer CID larger than DTLS_CID_MAX_SIZE");
+        WOLFSSL_ERROR_VERBOSE(DTLS_CID_ERROR);
+        return DTLS_CID_ERROR;
+    }
+#endif
+
     info = DtlsCidGetInfo(ssl);
     if (info == NULL)
         return BAD_STATE_E;

src/dtls13.c

@@ -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 */

src/internal.c

@@ -10306,6 +10306,30 @@ static WC_INLINE void DtlsSEQIncrement(WOLFSSL* ssl, int order)
         }
     }
 }
+
+#ifndef WOLFSSL_NO_TLS12
+/* Is the send sequence number at its last legal value? DtlsSEQIncrement()
+ * would wrap the word16 high half to 0 and reuse sequence numbers. */
+static WC_INLINE int DtlsSEQAtMax(WOLFSSL* ssl, int order)
+{
+#ifdef HAVE_SECURE_RENEGOTIATION
+    order = DtlsCheckOrder(ssl, order);
+#endif
+
+    if (order == PREV_ORDER) {
+        return ssl->keys.dtls_prev_sequence_number_hi == 0xFFFF &&
+               ssl->keys.dtls_prev_sequence_number_lo == 0xFFFFFFFFU;
+    }
+    else if (order == PEER_ORDER) {
+        /* the peer's sequence number is taken from the record */
+        return 0;
+    }
+    else {
+        return ssl->keys.dtls_sequence_number_hi == 0xFFFF &&
+               ssl->keys.dtls_sequence_number_lo == 0xFFFFFFFFU;
+    }
+}
+#endif /* !WOLFSSL_NO_TLS12 */
 #endif /* WOLFSSL_DTLS */
 
 #if defined(WOLFSSL_DTLS) || !defined(WOLFSSL_NO_TLS12)
@@ -10717,10 +10741,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 +10765,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 +10779,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 +10805,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 +10828,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 +10837,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,18 +10854,19 @@ 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);
         }
     }
     else {
         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 +10876,8 @@ void DtlsMsgStore(WOLFSSL* ssl, word16 epoch, word32 seq, const byte* data,
     }
 
     ssl->dtls_rx_msg_list = head;
+
+    return ret;
 }
 
 
@@ -13085,7 +13120,14 @@ static int GetDtlsRecordHeader(WOLFSSL* ssl, word32* inOutIdx,
     }
 
 #ifdef WOLFSSL_DTLS_CID
-    if (rh->type == dtls12_cid && (cidSz = DtlsGetCidRxSize(ssl)) == 0)
+    if (rh->type == dtls12_cid) {
+        if ((cidSz = DtlsGetCidRxSize(ssl)) == 0)
+            return DTLS_CID_ERROR;
+    }
+    /* RFC 9146 Sec 4: with a receive CID every protected record must use the
+     * dtls12_cid type. The MAC covers the inner content type, not the wire
+     * type, so a record re-framed as application_data still authenticates. */
+    else if (ssl->keys.curEpoch != 0 && DtlsGetCidRxSize(ssl) != 0)
         return DTLS_CID_ERROR;
 #endif
 
@@ -20075,6 +20117,17 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
             *inOutIdx = expectedIdx;
             return SendAlert(ssl, alert_warning, no_renegotiation);
         }
+#ifdef WOLFSSL_DTLS
+        /* RFC 6347 Sec 4.1: the epoch must not wrap. Both directions are
+         * checked because a second ClientHello can arrive between the peer's
+         * CCS and our own. */
+        if (ssl->options.dtls && (ssl->keys.dtls_epoch == 0xFFFF ||
+                ssl->keys.peerSeq[0].nextEpoch == 0xFFFF)) {
+            WOLFSSL_MSG("Refusing renegotiation. Epoch would wrap");
+            *inOutIdx = expectedIdx;
+            return SendAlert(ssl, alert_warning, no_renegotiation);
+        }
+#endif
         ret = ResetHandshakeStateForReneg(ssl);
         if (ret != 0)
             return ret;
@@ -26327,7 +26380,7 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
      * increments, so refuse at hi == lo == 0xFFFFFFFF (2^64-1): that last legal
      * value is deliberately sacrificed to avoid wrapping to 0 and reusing
      * sequence number 0. The caller must renegotiate or close. DTLS sequence
-     * numbers are epoch-scoped and handled elsewhere. */
+     * numbers are epoch-scoped and checked just below. */
     if (!sizeOnly && !ssl->options.dtls &&
             ssl->keys.sequence_number_hi == 0xFFFFFFFFU &&
             ssl->keys.sequence_number_lo == 0xFFFFFFFFU) {
@@ -26336,6 +26389,17 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
         return SEQUENCE_NUMBER_E;
     }
 
+#ifdef WOLFSSL_DTLS
+    /* RFC 6347 Sec 4.1: don't wrap the sequence number. Only protected records
+     * reach here, so the epoch 0 counter SendHelloVerifyRequest() copies from
+     * the peer is unaffected. */
+    if (!sizeOnly && ssl->options.dtls && DtlsSEQAtMax(ssl, epochOrder)) {
+        WOLFSSL_MSG("DTLS write sequence number would wrap");
+        WOLFSSL_ERROR_VERBOSE(SEQUENCE_NUMBER_E);
+        return SEQUENCE_NUMBER_E;
+    }
+#endif
+
 #ifdef WOLFSSL_ASYNC_CRYPT
     ret = WC_NO_PENDING_E;
     if (asyncOkay) {

src/ssl_api_ext.c

@@ -1008,7 +1008,8 @@ static int _Rehandshake(WOLFSSL* ssl)
         ret = SECURE_RENEGOTIATION_E;
     }
     #ifdef WOLFSSL_DTLS
-    else if ((ssl->options.dtls) && (ssl->keys.dtls_epoch == 0xFFFF)) {
+    else if ((ssl->options.dtls) && ((ssl->keys.dtls_epoch == 0xFFFF) ||
+            (ssl->keys.peerSeq[0].nextEpoch == 0xFFFF))) {
         WOLFSSL_MSG("Secure Renegotiation not allowed. Epoch would wrap");
         ret = SECURE_RENEGOTIATION_E;
     }

src/tls13.c

@@ -12900,6 +12900,12 @@ static int SendTls13Finished(WOLFSSL* ssl)
 #endif /* WOLFSSL_DTLS13 */
 
     outputSz = WC_MAX_DIGEST_SIZE + DTLS_HANDSHAKE_HEADER_SZ + MAX_MSG_EXTRA;
+#ifdef WOLFSSL_DTLS13
+    /* MAX_MSG_EXTRA only budgets RECORD_HEADER_SZ. The DTLS 1.3 unified header
+     * is longer and grows with the TX CID. */
+    if (isDtls)
+        outputSz += Dtls13GetRlHeaderLength(ssl, 1);
+#endif /* WOLFSSL_DTLS13 */
     /* Check buffers are big enough and grow if needed. */
     if ((ret = CheckAvailableSize(ssl, outputSz)) != 0)
         return ret;
@@ -13167,7 +13173,9 @@ int SendTls13KeyUpdate(WOLFSSL* ssl)
         }
     }
 
-    outputSz = OPAQUE8_LEN + MAX_MSG_EXTRA;
+    /* i already carries the real record and handshake header lengths.
+     * MAX_MSG_EXTRA only budgets RECORD_HEADER_SZ. */
+    outputSz = (int)i + OPAQUE8_LEN + MAX_MSG_EXTRA;
     /* Check buffers are big enough and grow if needed. */
     if ((ret = CheckAvailableSize(ssl, outputSz)) != 0)
         return ret;