DTLS 1.3: do not send ACKs until we negotiate 1.3 (through SH)

src/dtls13.c

@@ -362,8 +362,16 @@ int Dtls13ProcessBufferedMessages(WOLFSSL* ssl)
         if (!msg->ready)
             break;
 
-        ret = DoTls13HandShakeMsgType(ssl, msg->fullMsg, &idx, msg->type,
+        /* We may have DTLS <=1.2 msgs stored from before we knew which version
-                msg->sz, msg->sz);
+         * we were going to use. Interpret correctly. */
+        if (IsAtLeastTLSv1_3(ssl->version)) {
+            ret = DoTls13HandShakeMsgType(ssl, msg->fullMsg, &idx, msg->type,
+                    msg->sz, msg->sz);
+        }
+        else {
+            ret = DoHandShakeMsgType(ssl, msg->fullMsg, &idx, msg->type,
+                    msg->sz, msg->sz);
+        }
 
         /* processing certificate_request triggers a connect. The error came
          * from there, the message can be considered processed successfully.
@@ -371,7 +379,8 @@ int Dtls13ProcessBufferedMessages(WOLFSSL* ssl)
          * waiting to flush the output buffer. */
         if ((ret == 0 || ret == WANT_WRITE) || (msg->type == certificate_request &&
                          ssl->options.handShakeDone && ret == WC_PENDING_E)) {
-            Dtls13MsgWasProcessed(ssl, (enum HandShakeType)msg->type);
+            if (IsAtLeastTLSv1_3(ssl->version))
+                Dtls13MsgWasProcessed(ssl, (enum HandShakeType)msg->type);
 
             ssl->dtls_rx_msg_list = msg->next;
             DtlsMsgDelete(msg, ssl->heap);
@@ -625,7 +634,7 @@ static void Dtls13RtxRecordUnlink(WOLFSSL* ssl, Dtls13RtxRecord** prevNext,
     *prevNext = r->next;
 }
 
-static void Dtls13RtxFlushBuffered(WOLFSSL* ssl, byte keepNewSessionTicket)
+void Dtls13RtxFlushBuffered(WOLFSSL* ssl, byte keepNewSessionTicket)
 {
     Dtls13RtxRecord *r, **prevNext;
 
@@ -809,7 +818,14 @@ static int Dtls13RtxMsgRecvd(WOLFSSL* ssl, enum HandShakeType hs,
            sent flight. The only exception is, on the server side, receiving the
            last client flight does not ACK any sent new_session_ticket
            messages. */
-        Dtls13RtxFlushBuffered(ssl, 1);
+        /* We don't want to clear the buffer until we have done version
+         * negotiation in the SH or have received a unified header in the
+         * DTLS record (ssl->dtls13Rtx.sendAcks should only be set when that
+         * is true). */
+        if (ssl->options.serverState >= SERVER_HELLO_COMPLETE ||
+                    ssl->dtls13Rtx.sendAcks)
+            /* Use 1.2 API to clear 1.2 buffers too */
+            DtlsMsgPoolReset(ssl);
     }
 
     if (ssl->keys.dtls_peer_handshake_number <
@@ -853,6 +869,8 @@ static int Dtls13RtxMsgRecvd(WOLFSSL* ssl, enum HandShakeType hs,
 void Dtls13FreeFsmResources(WOLFSSL* ssl)
 {
     Dtls13RtxFlushAcks(ssl);
+    /* Use 1.2 API to clear 1.2 buffers too */
+    DtlsMsgPoolReset(ssl);
     Dtls13RtxFlushBuffered(ssl, 0);
 }
 
@@ -2471,7 +2489,13 @@ int Dtls13RtxTimeout(WOLFSSL* ssl)
 {
     int ret = 0;
 
-    if (ssl->dtls13Rtx.seenRecords != NULL) {
+    /* We don't want to send acks until we have done version
+     * negotiation in the SH or have received a unified header in the
+     * DTLS record (ssl->dtls13Rtx.sendAcks should only be set when that
+     * is true). */
+    if (ssl->dtls13Rtx.seenRecords != NULL &&
+            (ssl->options.serverState >= SERVER_HELLO_COMPLETE ||
+                    ssl->dtls13Rtx.sendAcks)) {
         ssl->dtls13Rtx.sendAcks = 0;
         /* reset fast timeout as we are sending ACKs */
         ssl->dtls13FastTimeout = 0;

src/internal.c

@@ -9185,6 +9185,10 @@ void DtlsMsgPoolReset(WOLFSSL* ssl)
         ssl->dtls_tx_msg = NULL;
         ssl->dtls_tx_msg_list_sz = 0;
     }
+#ifdef WOLFSSL_DTLS13
+    /* Clear DTLS 1.3 buffer too */
+    Dtls13RtxFlushBuffered(ssl, 1);
+#endif
 }
 
 
@@ -16050,7 +16054,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
 }
 
 
-static int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
+int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                           byte type, word32 size, word32 totalSz)
 {
     int ret = 0;

src/ssl.c

@@ -12447,7 +12447,7 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl,
                     return WOLFSSL_FATAL_ERROR;
                 }
                 /* if resumption failed, reset needed state */
-                else if (neededState == SERVER_FINISHED_COMPLETE)
+                else if (neededState == SERVER_FINISHED_COMPLETE) {
                     if (!ssl->options.resuming) {
                     #ifdef WOLFSSL_DTLS
                         if (IsDtlsNotSctpMode(ssl))
@@ -12456,8 +12456,9 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl,
                     #endif
                             neededState = SERVER_HELLODONE_COMPLETE;
                     }
-#ifdef WOLFSSL_DTLS13
+                }
 
+#ifdef WOLFSSL_DTLS13
                 if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version)
                     && ssl->dtls13Rtx.sendAcks == 1) {
                     ssl->dtls13Rtx.sendAcks = 0;
@@ -12474,8 +12475,6 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl,
                         return WOLFSSL_FATAL_ERROR;
                     }
                 }
-
-
 #endif /* WOLFSSL_DTLS13 */
             }
 

tests/api.c

@@ -5597,7 +5597,7 @@ static WC_INLINE int test_ssl_memio_write_cb(WOLFSSL *ssl, char *data, int sz,
     }
 
     if ((unsigned)(*len + sz) > TEST_SSL_MEMIO_BUF_SZ)
-            return WOLFSSL_CBIO_ERR_WANT_READ;
+        return WOLFSSL_CBIO_ERR_WANT_WRITE;
 
     XMEMCPY(buf + *len, data, sz);
     *len += sz;
@@ -64476,6 +64476,237 @@ static int test_dtls_downgrade_scr(void)
 }
 #endif
 
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
+static int test_dtls_client_hello_timeout_downgrade_read_cb(WOLFSSL *ssl,
+        char *data, int sz, void *ctx)
+{
+    static int call_counter = 0;
+    call_counter++;
+    (void)ssl;
+    (void)data;
+    (void)sz;
+    (void)ctx;
+    switch (call_counter) {
+        case 1:
+        case 2:
+            return WOLFSSL_CBIO_ERR_TIMEOUT;
+        case 3:
+            return WOLFSSL_CBIO_ERR_WANT_READ;
+        default:
+            AssertIntLE(call_counter, 3);
+            return -1;
+    }
+}
+#endif
+
+/* Make sure we don't send acks before getting a server hello */
+static int test_dtls_client_hello_timeout_downgrade(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
+    WOLFSSL_CTX *ctx_c = NULL;
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_c = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+    DtlsRecordLayerHeader* dtlsRH;
+    size_t len;
+    byte sequence_number[8];
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+
+        ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+            wolfDTLS_client_method, wolfDTLSv1_2_server_method), 0);
+
+        if (i == 0) {
+            /* First time simulate timeout in IO layer */
+            /* CH1 */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            /* HVR */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
+            /* CH2 */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            /* SH flight */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
+            /* Drop the SH */
+            dtlsRH = (DtlsRecordLayerHeader*)(test_ctx.c_buff);
+            len = (size_t)((dtlsRH->length[0] << 8) | dtlsRH->length[1]);
+            XMEMMOVE(test_ctx.c_buff, test_ctx.c_buff +
+                    sizeof(DtlsRecordLayerHeader) + len, test_ctx.c_len -
+                   (sizeof(DtlsRecordLayerHeader) + len));
+            test_ctx.c_len -= sizeof(DtlsRecordLayerHeader) + len;
+            /* Read the remainder of the flight */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            wolfSSL_SSLSetIORecv(ssl_c,
+                    test_dtls_client_hello_timeout_downgrade_read_cb);
+            /* CH3 */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            wolfSSL_SSLSetIORecv(ssl_c, test_memio_read_cb);
+        }
+        else {
+            /* Second time call wolfSSL_dtls_got_timeout */
+            /* CH1 */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            /* HVR */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
+            /* CH2 */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            /* SH flight */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
+            /* Drop the SH */
+            dtlsRH = (DtlsRecordLayerHeader*)(test_ctx.c_buff);
+            len = (size_t)((dtlsRH->length[0] << 8) | dtlsRH->length[1]);
+            XMEMMOVE(test_ctx.c_buff, test_ctx.c_buff +
+                    sizeof(DtlsRecordLayerHeader) + len, test_ctx.c_len -
+                   (sizeof(DtlsRecordLayerHeader) + len));
+            test_ctx.c_len -= sizeof(DtlsRecordLayerHeader) + len;
+            /* Read the remainder of the flight */
+            ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            /* Quick timeout should be set as we received at least one msg */
+            ExpectIntEQ(wolfSSL_dtls13_use_quick_timeout(ssl_c), 1);
+            ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_c), WOLFSSL_SUCCESS);
+            /* Quick timeout should be cleared after a quick timeout */
+            /* CH3 */
+            ExpectIntEQ(wolfSSL_dtls13_use_quick_timeout(ssl_c), 0);
+            ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_c), WOLFSSL_SUCCESS);
+        }
+
+        /* Parse out to make sure we got exactly one ClientHello message */
+        XMEMSET(&sequence_number, 0, sizeof(sequence_number));
+        /* Second ClientHello after HVR */
+        sequence_number[7] = 2;
+        dtlsRH = (DtlsRecordLayerHeader*)test_ctx.s_buff;
+        ExpectIntEQ(dtlsRH->type, handshake);
+        ExpectIntEQ(dtlsRH->pvMajor, DTLS_MAJOR);
+        ExpectIntEQ(dtlsRH->pvMinor, DTLSv1_2_MINOR);
+        ExpectIntEQ(XMEMCMP(sequence_number, dtlsRH->sequence_number,
+                sizeof(sequence_number)), 0);
+        len = (size_t)((dtlsRH->length[0] << 8) | dtlsRH->length[1]);
+        ExpectIntEQ(sizeof(DtlsRecordLayerHeader) + len, test_ctx.s_len);
+
+        /* Connection should be able to continue */
+        ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+
+        wolfSSL_free(ssl_c);
+        wolfSSL_free(ssl_s);
+        wolfSSL_CTX_free(ctx_c);
+        wolfSSL_CTX_free(ctx_s);
+        ssl_c = NULL;
+        ssl_s = NULL;
+        ctx_c = NULL;
+        ctx_s = NULL;
+        if (!EXPECT_SUCCESS())
+            break;
+    }
+
+#endif
+    return EXPECT_RESULT();
+}
+
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
+static int test_dtls_client_hello_timeout_read_cb(WOLFSSL *ssl, char *data,
+        int sz, void *ctx)
+{
+    static int call_counter = 0;
+    call_counter++;
+    (void)ssl;
+    (void)data;
+    (void)sz;
+    (void)ctx;
+    switch (call_counter) {
+        case 1:
+            return WOLFSSL_CBIO_ERR_TIMEOUT;
+        case 2:
+            return WOLFSSL_CBIO_ERR_WANT_READ;
+        default:
+            AssertIntLE(call_counter, 2);
+            return -1;
+    }
+}
+#endif
+
+/* Make sure we don't send acks before getting a server hello */
+static int test_dtls_client_hello_timeout(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
+    WOLFSSL *ssl_c = NULL;
+    WOLFSSL_CTX *ctx_c = NULL;
+    struct test_memio_ctx test_ctx;
+    DtlsRecordLayerHeader* dtlsRH;
+    size_t idx;
+    size_t len;
+    byte sequence_number[8];
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+
+        ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL,
+            wolfDTLSv1_3_client_method, NULL), 0);
+
+        if (i == 0) {
+            /* First time simulate timeout in IO layer */
+            wolfSSL_SSLSetIORecv(ssl_c, test_dtls_client_hello_timeout_read_cb);
+            ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+        }
+        else {
+            /* Second time call wolfSSL_dtls_got_timeout */
+            ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
+            ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+            ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_c), WOLFSSL_SUCCESS);
+        }
+
+        /* Parse out to make sure we got exactly two ClientHello messages */
+        idx = 0;
+        XMEMSET(&sequence_number, 0, sizeof(sequence_number));
+        /* First ClientHello */
+        dtlsRH = (DtlsRecordLayerHeader*)(test_ctx.s_buff + idx);
+        ExpectIntEQ(dtlsRH->type, handshake);
+        ExpectIntEQ(dtlsRH->pvMajor, DTLS_MAJOR);
+        ExpectIntEQ(dtlsRH->pvMinor, DTLSv1_2_MINOR);
+        ExpectIntEQ(XMEMCMP(sequence_number, dtlsRH->sequence_number,
+                sizeof(sequence_number)), 0);
+        len = (size_t)((dtlsRH->length[0] << 8) | dtlsRH->length[1]);
+        ExpectIntLT(idx + sizeof(DtlsRecordLayerHeader) + len, test_ctx.s_len);
+        idx += sizeof(DtlsRecordLayerHeader) + len;
+        /* Second ClientHello */
+        sequence_number[7] = 1;
+        dtlsRH = (DtlsRecordLayerHeader*)(test_ctx.s_buff + idx);
+        ExpectIntEQ(dtlsRH->type, handshake);
+        ExpectIntEQ(dtlsRH->pvMajor, DTLS_MAJOR);
+        ExpectIntEQ(dtlsRH->pvMinor, DTLSv1_2_MINOR);
+        ExpectIntEQ(XMEMCMP(sequence_number, dtlsRH->sequence_number,
+                sizeof(sequence_number)), 0);
+        len = (size_t)((dtlsRH->length[0] << 8) | dtlsRH->length[1]);
+        ExpectIntEQ(idx + sizeof(DtlsRecordLayerHeader) + len, test_ctx.s_len);
+
+        wolfSSL_free(ssl_c);
+        wolfSSL_CTX_free(ctx_c);
+        ssl_c = NULL;
+        ctx_c = NULL;
+        if (!EXPECT_SUCCESS())
+            break;
+    }
+
+#endif
+    return EXPECT_RESULT();
+}
+
 /*----------------------------------------------------------------------------*
  | Main
  *----------------------------------------------------------------------------*/
@@ -65735,6 +65966,8 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_session_ticket_no_id),
     TEST_DECL(test_dtls_downgrade_scr_server),
     TEST_DECL(test_dtls_downgrade_scr),
+    TEST_DECL(test_dtls_client_hello_timeout_downgrade),
+    TEST_DECL(test_dtls_client_hello_timeout),
     /* This test needs to stay at the end to clean up any caches allocated. */
     TEST_DECL(test_wolfSSL_Cleanup)
 };

tests/utils.h

@@ -119,6 +119,17 @@ cleanup:
 /* This set of memio functions allows for more fine tuned control of the TLS
  * connection operations. For new tests, try to use ssl_memio first. */
 
+/* To dump the memory in gdb use
+ *   dump memory client.bin test_ctx.c_buff test_ctx.c_buff+test_ctx.c_len
+ *   dump memory server.bin test_ctx.s_buff test_ctx.s_buff+test_ctx.s_len
+ * This can be imported into Wireshark by transforming the file with
+ *   od -Ax -tx1 -v client.bin > client.bin.hex
+ *   od -Ax -tx1 -v server.bin > server.bin.hex
+ * And then loading test_output.dump.hex into Wireshark using the
+ * "Import from Hex Dump..." option ion and selecting the TCP
+ * encapsulation option.
+ */
+
 #define HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES
 
 #define TEST_MEMIO_BUF_SZ (64 * 1024)
@@ -157,7 +168,7 @@ static WC_INLINE int test_memio_write_cb(WOLFSSL *ssl, char *data, int sz,
     }
 
     if ((unsigned)(*len + sz) > TEST_MEMIO_BUF_SZ)
-            return WOLFSSL_CBIO_ERR_WANT_READ;
+        return WOLFSSL_CBIO_ERR_WANT_WRITE;
 
     XMEMCPY(buf + *len, data, sz);
     *len += sz;

wolfssl/internal.h

@@ -2135,6 +2135,9 @@ WOLFSSL_LOCAL void InitSSL_CTX_Suites(WOLFSSL_CTX* ctx);
 WOLFSSL_LOCAL int InitSSL_Suites(WOLFSSL* ssl);
 WOLFSSL_LOCAL int InitSSL_Side(WOLFSSL* ssl, word16 side);
 
+
+WOLFSSL_LOCAL int DoHandShakeMsgType(WOLFSSL* ssl, byte* input,
+        word32* inOutIdx, byte type, word32 size, word32 totalSz);
 /* for sniffer */
 WOLFSSL_LOCAL int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
                             word32 size, word32 totalSz, int sniff);
@@ -6531,6 +6534,8 @@ WOLFSSL_LOCAL int Dtls13HashHandshake(WOLFSSL* ssl, const byte* input,
 WOLFSSL_LOCAL int Dtls13HashClientHello(const WOLFSSL* ssl, byte* hash,
         int* hashSz, const byte* body, word32 length, CipherSpecs* specs);
 WOLFSSL_LOCAL void Dtls13FreeFsmResources(WOLFSSL* ssl);
+WOLFSSL_LOCAL void Dtls13RtxFlushBuffered(WOLFSSL* ssl,
+        byte keepNewSessionTicket);
 WOLFSSL_LOCAL int Dtls13RtxTimeout(WOLFSSL* ssl);
 WOLFSSL_LOCAL int Dtls13ProcessBufferedMessages(WOLFSSL* ssl);
 WOLFSSL_LOCAL int Dtls13CheckAEADFailLimit(WOLFSSL* ssl);