Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/dtls13.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ typedef struct Dtls13RecordPlaintextHeader {
supported. */
#define DTLS13_UNIFIED_HEADER_SIZE 5
#define DTLS13_MIN_CIPHERTEXT 16
#define DTLS13_MIN_RTX_INTERVAL 1
#ifndef DTLS13_MIN_RTX_INTERVAL
#define DTLS13_MIN_RTX_INTERVAL 1000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think of using DTLS_TIMEOUT_INIT as default here?

#endif

#ifndef NO_WOLFSSL_CLIENT
WOLFSSL_METHOD* wolfDTLSv1_3_client_method_ex(void* heap)
Expand Down Expand Up @@ -1570,21 +1572,24 @@ static int Dtls13RtxSendBuffered(WOLFSSL* ssl)
int isLast;
int sendSz;
#ifndef NO_ASN_TIME
#ifdef WOLFSSL_32BIT_MILLI_TIME
word32 now;
#else
sword64 now;
#endif
#endif
int ret;

WOLFSSL_ENTER("Dtls13RtxSendBuffered");

#ifndef NO_ASN_TIME
now = LowResTimer();
now = TimeNowInMilliseconds();
if (now - ssl->dtls13Rtx.lastRtx < DTLS13_MIN_RTX_INTERVAL) {
#ifdef WOLFSSL_DEBUG_TLS
WOLFSSL_MSG("Avoid too fast retransmission");
#endif /* WOLFSSL_DEBUG_TLS */
return 0;
}

ssl->dtls13Rtx.lastRtx = now;
#endif

Expand Down
4 changes: 2 additions & 2 deletions src/tls13.c
Original file line number Diff line number Diff line change
Expand Up @@ -1665,7 +1665,7 @@ int DeriveTls13Keys(WOLFSSL* ssl, int secret, int side, int store)
return ret;
}

#if (defined(HAVE_SESSION_TICKET) || !defined(NO_PSK))
#if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) || defined(WOLFSSL_DTLS13)
#ifdef WOLFSSL_32BIT_MILLI_TIME
#ifndef NO_ASN_TIME
#if defined(USER_TICKS)
Expand Down Expand Up @@ -2264,7 +2264,7 @@ int DeriveTls13Keys(WOLFSSL* ssl, int secret, int side, int store)
*/
#endif /* !NO_ASN_TIME */
#endif /* WOLFSSL_32BIT_MILLI_TIME */
#endif /* HAVE_SESSION_TICKET || !NO_PSK */
#endif /* HAVE_SESSION_TICKET || !NO_PSK || WOLFSSL_DTLS13 */

/* Add record layer header to message.
*
Expand Down
75 changes: 75 additions & 0 deletions tests/api/test_dtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -2489,3 +2489,78 @@ int test_dtls_mtu_split_messages(void)
return TEST_SKIPPED;
#endif
}

/* Test DTLS 1.3 minimum retransmission interval. This test calls
* wolfSSL_dtls_got_timeout() to simulate timeouts and verify that
* retransmissions are spaced at least DTLS13_MIN_RTX_INTERVAL apart.
*/
int test_dtls13_min_rtx_interval(void)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test relies on timing and may introduce flakiness. I don't see an easy way to test without timing dependency. If we want to keep it as it is, we need at least a clear comment about the risk imo.

{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
defined(WOLFSSL_DTLS13) && !defined(DTLS13_MIN_RTX_INTERVAL) && \
!defined(NO_ASN_TIME)
/* We don't want to test when DTLS13_MIN_RTX_INTERVAL is defined because
* it may be too low to trigger reliably in a test. The default value is
* 1 second which is sufficient for testing here. */
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
struct test_memio_ctx test_ctx;
int c_msg_count = 0;

XMEMSET(&test_ctx, 0, sizeof(test_ctx));

/* Setup DTLS 1.3 contexts */
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);

/* CH0 */
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), SSL_ERROR_WANT_READ);

/* HRR */
ExpectIntEQ(wolfSSL_accept(ssl_s), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), SSL_ERROR_WANT_READ);

/* CH1 */
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), SSL_ERROR_WANT_READ);

/* SH ... FINISHED */
ExpectIntEQ(wolfSSL_accept(ssl_s), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), SSL_ERROR_WANT_READ);

/* We should have SH ... FINISHED messages in the buffer */
ExpectIntGE(test_ctx.c_msg_count, 2);

/* Drop everything */
test_memio_clear_buffer(&test_ctx, 1);

/* First timeout. This one should trigger a retransmission */
if (wolfSSL_dtls13_use_quick_timeout(ssl_s))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrap the result of the wolfSSL_dtls13_use_quick_timeout in an Expect assert

ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_s), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_s), WOLFSSL_SUCCESS);
/* Save the message count to make sure no new messages are sent */
ExpectIntGE(c_msg_count = test_ctx.c_msg_count, 2);
Comment on lines +2543 to +2544
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: two instruction to make it more readable


/* Second timeout. This one should not trigger a retransmission */
if (wolfSSL_dtls13_use_quick_timeout(ssl_s))
ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_s), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_s), WOLFSSL_SUCCESS);
/* This is the critical check. The message count should not increase
* after the second timeout. DTLS13_MIN_RTX_INTERVAL should have blocked
* retransmission here. */
ExpectIntEQ(c_msg_count, test_ctx.c_msg_count);

/* Now complete the handshake. We didn't clear the first retransmission
* so the handshake should proceed without issues. */
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

/* Cleanup */
wolfSSL_free(ssl_c);
wolfSSL_CTX_free(ctx_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}
4 changes: 3 additions & 1 deletion tests/api/test_dtls.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ int test_dtls_memio_wolfio(void);
int test_dtls_memio_wolfio_stateless(void);
int test_dtls_mtu_fragment_headroom(void);
int test_dtls_mtu_split_messages(void);
int test_dtls13_min_rtx_interval(void);

#define TEST_DTLS_DECLS \
TEST_DECL_GROUP("dtls", test_dtls12_basic_connection_id), \
Expand Down Expand Up @@ -77,5 +78,6 @@ int test_dtls_mtu_split_messages(void);
TEST_DECL_GROUP("dtls", test_dtls_memio_wolfio), \
TEST_DECL_GROUP("dtls", test_dtls_mtu_fragment_headroom), \
TEST_DECL_GROUP("dtls", test_dtls_mtu_split_messages), \
TEST_DECL_GROUP("dtls", test_dtls_memio_wolfio_stateless)
TEST_DECL_GROUP("dtls", test_dtls_memio_wolfio_stateless), \
TEST_DECL_GROUP("dtls", test_dtls13_min_rtx_interval)
#endif /* TESTS_API_DTLS_H */
7 changes: 6 additions & 1 deletion wolfssl/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -5773,7 +5773,11 @@ typedef struct Dtls13Rtx {
Dtls13RtxRecord *rtxRecords;
Dtls13RtxRecord **rtxRecordTailPtr;
Dtls13RecordNumber *seenRecords;
#ifdef WOLFSSL_32BIT_MILLI_TIME
word32 lastRtx;
#else
sword64 lastRtx;
#endif
byte triggeredRtxs; /* Unused? */
byte sendAcks;
byte retransmit;
Expand Down Expand Up @@ -6825,7 +6829,8 @@ WOLFSSL_LOCAL word32 MacSize(const WOLFSSL* ssl);

WOLFSSL_LOCAL void WriteSEQ(WOLFSSL* ssl, int verifyOrder, byte* out);

#if defined(WOLFSSL_TLS13) && (defined(HAVE_SESSION_TICKET) || !defined(NO_PSK))
#if defined(WOLFSSL_TLS13) && (defined(HAVE_SESSION_TICKET) || \
!defined(NO_PSK) || defined(WOLFSSL_DTLS13))
#ifdef WOLFSSL_32BIT_MILLI_TIME
WOLFSSL_LOCAL word32 TimeNowInMilliseconds(void);
#else
Expand Down