From 509491f5548732126577589bf5cfbcebf7e05f72 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Mon, 16 Jun 2025 09:59:32 +0200 Subject: [PATCH 1/3] dtls13: wolfSSL_is_init_finished true after last server ACK Do not consider the handshake finished until the last server ACK. This way the application knows where to switch from wolfSSL_negotiate/wolfSSL_connect to wolfSSL_read/wolfSSL_write. --- src/ssl.c | 7 +++++++ tests/api.c | 3 +-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 80589ad5c..e4909e35a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12811,6 +12811,13 @@ cleanup: if (ssl == NULL) return 0; +#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_NO_CLIENT) + if (ssl->options.side == WOLFSSL_CLIENT_END && ssl->options.dtls + && IsAtLeastTLSv1_3(ssl->version)) { + return ssl->options.serverState == SERVER_FINISHED_ACKED; + } +#endif /* WOLFSSL_DTLS13 && !WOLFSSL_NO_CLIENT */ + /* Can't use ssl->options.connectState and ssl->options.acceptState * because they differ in meaning for TLS <=1.2 and 1.3 */ if (ssl->options.handShakeState == HANDSHAKE_DONE) diff --git a/tests/api.c b/tests/api.c index a5b7ad435..653618222 100644 --- a/tests/api.c +++ b/tests/api.c @@ -66072,8 +66072,7 @@ static int test_dtls13_missing_finished_server(void) ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); /* Let's clear the output */ test_memio_clear_buffer(&test_ctx, 0); - /* We should signal that the handshake is done */ - ExpectTrue(wolfSSL_is_init_finished(ssl_c)); + ExpectFalse(wolfSSL_is_init_finished(ssl_c)); /* Let's send some app data */ ExpectIntEQ(wolfSSL_write(ssl_c, test_str, sizeof(test_str)), sizeof(test_str)); From b1b49c9ffb04ec77da0c2c84c75a285f93b4932d Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Mon, 16 Jun 2025 09:19:59 +0200 Subject: [PATCH 2/3] dtls13: always send ACKs on detected retransmission Otherwise the connection can stall due the indefinite delay of an explicit ACK, for exapmle: -> client sends the last Finished message <- server sends the ACK, but the ACK is lost -> client rentrasmit the Finished message - server delay sending of the ACK until a fast timeout -> client rentrasmit the Finished message quicker than the server timeout - server resets the timeout, delaying sending the ACK -> client rentrasmit the Finished... --- src/dtls13.c | 2 +- tests/api.c | 1 + tests/api/test_dtls.c | 79 +++++++++++++++++++++++++++++++++++++++++++ tests/api/test_dtls.h | 1 + 4 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/dtls13.c b/src/dtls13.c index 375c00af0..a07f8d3df 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -905,7 +905,7 @@ static int Dtls13RtxMsgRecvd(WOLFSSL* ssl, enum HandShakeType hs, /* the other peer may have retransmitted because an ACK for a flight that needs explicit ACK was lost.*/ if (ssl->dtls13Rtx.seenRecords != NULL) - ssl->dtls13Rtx.sendAcks = (byte)ssl->options.dtls13SendMoreAcks; + ssl->dtls13Rtx.sendAcks = 1; } if (ssl->keys.dtls_peer_handshake_number == diff --git a/tests/api.c b/tests/api.c index 653618222..8fc5ca854 100644 --- a/tests/api.c +++ b/tests/api.c @@ -68477,6 +68477,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_dtls_cid_parse), TEST_DECL(test_dtls13_epochs), TEST_DECL(test_dtls_rtx_across_epoch_change), + TEST_DECL(test_dtls_drop_client_ack), TEST_DECL(test_dtls13_ack_order), TEST_DECL(test_dtls_version_checking), TEST_DECL(test_ocsp_status_callback), diff --git a/tests/api/test_dtls.c b/tests/api/test_dtls.c index d6a106b39..75f495533 100644 --- a/tests/api/test_dtls.c +++ b/tests/api/test_dtls.c @@ -1383,3 +1383,82 @@ int test_dtls_rtx_across_epoch_change(void) defined(WOLFSSL_DTLS13) */ return EXPECT_RESULT(); } +int test_dtls_drop_client_ack(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_DTLS13) && defined(WOLFSSL_DTLS) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + char data[32]; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + /* Setup DTLS contexts */ + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), + 0); + + /* disable new session ticket to simplify testing */ + ExpectIntEQ(wolfSSL_no_ticket_TLSv13(ssl_s), 0); + + /* CH0 */ + wolfSSL_SetLoggingPrefix("client:"); + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + + /* HRR */ + wolfSSL_SetLoggingPrefix("server:"); + ExpectIntEQ(wolfSSL_accept(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + + /* CH1 */ + wolfSSL_SetLoggingPrefix("client:"); + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + + /* SH ... FINISHED */ + wolfSSL_SetLoggingPrefix("server:"); + ExpectIntEQ(wolfSSL_accept(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + + /* ... FINISHED */ + wolfSSL_SetLoggingPrefix("client:"); + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + + /* init is finished should return false at this point */ + ExpectFalse(wolfSSL_is_init_finished(ssl_c)); + + /* ACK */ + ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_SUCCESS); + /* Drop the ack */ + test_memio_clear_buffer(&test_ctx, 1); + + /* trigger client timeout, finished should be rtx */ + ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_c), WOLFSSL_SUCCESS); + /* this should have triggered a rtx */ + ExpectIntGT(test_ctx.s_msg_count, 0); + + /* this should re-send the ack immediately */ + ExpectIntEQ(wolfSSL_read(ssl_s, data, 32), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(test_ctx.c_msg_count, 1); + + /* This should advance the connection on the client */ + ExpectIntEQ(wolfSSL_negotiate(ssl_c), WOLFSSL_SUCCESS); + + /* Test communication works correctly */ + ExpectIntEQ(test_dtls_communication(ssl_s, ssl_c), TEST_SUCCESS); + + /* Cleanup */ + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif /* defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_DTLS13) */ + return EXPECT_RESULT(); +} diff --git a/tests/api/test_dtls.h b/tests/api/test_dtls.h index 18ace27b6..e5801d084 100644 --- a/tests/api/test_dtls.h +++ b/tests/api/test_dtls.h @@ -37,4 +37,5 @@ int test_dtls13_short_read(void); int test_records_span_network_boundaries(void); int test_dtls_record_cross_boundaries(void); int test_dtls_rtx_across_epoch_change(void); +int test_dtls_drop_client_ack(void); #endif /* TESTS_API_DTLS_H */ From e82c099bec5652e92bc06bcb0935f8b3f66be5da Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Mon, 16 Jun 2025 18:42:17 +0200 Subject: [PATCH 3/3] fix indentation --- src/ssl.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index e4909e35a..b86ec13fc 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12812,10 +12812,10 @@ cleanup: return 0; #if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_NO_CLIENT) - if (ssl->options.side == WOLFSSL_CLIENT_END && ssl->options.dtls - && IsAtLeastTLSv1_3(ssl->version)) { - return ssl->options.serverState == SERVER_FINISHED_ACKED; - } + if (ssl->options.side == WOLFSSL_CLIENT_END && ssl->options.dtls + && IsAtLeastTLSv1_3(ssl->version)) { + return ssl->options.serverState == SERVER_FINISHED_ACKED; + } #endif /* WOLFSSL_DTLS13 && !WOLFSSL_NO_CLIENT */ /* Can't use ssl->options.connectState and ssl->options.acceptState @@ -26759,4 +26759,3 @@ void wolfSSL_FIPS_drbg_set_app_data(WOLFSSL_DRBG_CTX *ctx, void *app_data) #endif /* !WOLFCRYPT_ONLY */ -