Merge pull request #1529 from ronald-cron-arm/dtls

Fixes relative to DTLS invalid/unexpected first record
This commit is contained in:
minosgalanakis
2026-03-25 22:31:24 +00:00
committed by GitHub
7 changed files with 138 additions and 80 deletions
+71 -22
View File
@@ -275,6 +275,7 @@ exit:
/* Forward declarations for functions related to message buffering. */
static void ssl_buffering_free_slot(mbedtls_ssl_context *ssl,
uint8_t slot);
static void ssl_buffering_shift_slots(mbedtls_ssl_context *ssl, unsigned shift);
static void ssl_free_buffered_record(mbedtls_ssl_context *ssl);
MBEDTLS_CHECK_RETURN_CRITICAL
static int ssl_load_buffered_message(mbedtls_ssl_context *ssl);
@@ -2985,8 +2986,12 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl)
* expected `message_seq` for the incoming and outgoing
* handshake messages.
*/
ssl->handshake->in_msg_seq = recv_msg_seq;
ssl->handshake->out_msg_seq = recv_msg_seq;
if ((ssl->handshake->in_msg_seq == 0) && (recv_msg_seq > 0)) {
MBEDTLS_SSL_DEBUG_MSG(3, ("shift slots by %u", recv_msg_seq));
ssl_buffering_shift_slots(ssl, recv_msg_seq);
ssl->handshake->in_msg_seq = recv_msg_seq;
ssl->handshake->out_msg_seq = recv_msg_seq;
}
/* Epoch should be 0 for initial handshakes */
if (ssl->in_ctr[0] != 0 || ssl->in_ctr[1] != 0) {
@@ -2996,6 +3001,7 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl)
memcpy(&ssl->cur_out_ctr[2], ssl->in_ctr + 2,
sizeof(ssl->cur_out_ctr) - 2);
} else if (mbedtls_ssl_is_handshake_over(ssl) == 1) {
/* In case of a post-handshake ClientHello that initiates a
* renegotiation check that the handshake message sequence
@@ -3180,28 +3186,10 @@ int mbedtls_ssl_update_handshake_status(mbedtls_ssl_context *ssl)
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM &&
ssl->handshake != NULL) {
unsigned offset;
mbedtls_ssl_hs_buffer *hs_buf;
/* Increment handshake sequence number */
hs->in_msg_seq++;
/*
* Clear up handshake buffering and reassembly structure.
*/
/* Free first entry */
ssl_buffering_free_slot(ssl, 0);
/* Shift all other entries */
for (offset = 0, hs_buf = &hs->buffering.hs[0];
offset + 1 < MBEDTLS_SSL_MAX_BUFFERED_HS;
offset++, hs_buf++) {
*hs_buf = *(hs_buf + 1);
}
/* Create a fresh last entry */
memset(hs_buf, 0, sizeof(mbedtls_ssl_hs_buffer));
ssl_buffering_shift_slots(ssl, 1);
}
#endif
return 0;
@@ -3618,7 +3606,7 @@ static int ssl_parse_record_header(mbedtls_ssl_context const *ssl,
(
"datagram of length %u too small to hold DTLS record header of length %u",
(unsigned) len,
(unsigned) (rec_hdr_len_len + rec_hdr_len_len)));
(unsigned) (rec_hdr_len_offset + rec_hdr_len_len)));
return MBEDTLS_ERR_SSL_INVALID_RECORD;
}
@@ -4750,6 +4738,31 @@ static int ssl_get_next_record(mbedtls_ssl_context *ssl)
ret = MBEDTLS_ERR_SSL_UNEXPECTED_RECORD;
}
#if defined(MBEDTLS_SSL_SRV_C)
/*
* In DTLS, invalid records are usually ignored because it is easy
* for an attacker to inject UDP datagrams, and we do not want such
* packets to disrupt the entire connection.
*
* However, when expecting the ClientHello, we reject invalid or
* unexpected records. This avoids waiting for further records
* before receiving at least one valid message. Such records could
* be leftover messages from a previous connection, accidental
* input, or part of a DoS attempt.
*
* Since no valid message has been received yet, immediately
* closing the connection does not result in any loss.
*/
if ((ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER) &&
(ssl->state == MBEDTLS_SSL_CLIENT_HELLO)
#if defined(MBEDTLS_SSL_RENEGOTIATION)
&& (ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE)
#endif
) {
return ret;
}
#endif /* MBEDTLS_SSL_SRV_C */
if (ret == MBEDTLS_ERR_SSL_UNEXPECTED_RECORD) {
#if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C)
/* Reset in pointers to default state for TLS/DTLS records,
@@ -6134,6 +6147,42 @@ static void ssl_buffering_free_slot(mbedtls_ssl_context *ssl,
}
}
/*
* Shift the buffering slots to the left by `shift` positions.
* After the operation, slot i contains the previous slot i + shift.
*/
static void ssl_buffering_shift_slots(mbedtls_ssl_context *ssl,
unsigned shift)
{
mbedtls_ssl_handshake_params * const hs = ssl->handshake;
unsigned offset;
if (shift == 0) {
return;
}
if (shift >= MBEDTLS_SSL_MAX_BUFFERED_HS) {
shift = MBEDTLS_SSL_MAX_BUFFERED_HS;
}
/* Free discarded entries */
for (offset = 0; offset < shift; offset++) {
ssl_buffering_free_slot(ssl, offset);
}
/* Shift remaining entries left */
for (offset = 0; offset + shift < MBEDTLS_SSL_MAX_BUFFERED_HS; offset++) {
hs->buffering.hs[offset] = hs->buffering.hs[offset + shift];
}
/* Reset the remaining entries at the end. Some may already have been
* cleared by the loop freeing the discarded entries, but resetting all
* of them is simpler and avoids tracking which ones were already handled.
*/
for (; offset < MBEDTLS_SSL_MAX_BUFFERED_HS; offset++) {
memset(&hs->buffering.hs[offset], 0, sizeof(hs->buffering.hs[offset]));
}
}
#endif /* MBEDTLS_SSL_PROTO_DTLS */
/*
+33
View File
@@ -879,6 +879,39 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl)
*/
if ((ret = mbedtls_ssl_read_record(ssl, 0)) != 0) {
MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_read_record ", ret);
#if defined(MBEDTLS_SSL_PROTO_DTLS)
/*
* In the case of an alert message corresponding to the termination of
* a previous connection, `ssl_parse_record_header()` and then
* `mbedtls_ssl_read_record()` may return
* MBEDTLS_ERR_SSL_UNEXPECTED_RECORD because of a non zero epoch.
*
* Historically, the library has returned
* MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE in this situation.
* The sample program dtls_server.c relies on this behavior
* (see
* https://github.com/Mbed-TLS/mbedtls/blob/d5e35a376bee23fad0b17f2e3e94a32ce4017c64/programs/ssl/dtls_server.c#L295),
* and user applications may rely on it as well.
*
* For compatibility, map MBEDTLS_ERR_SSL_UNEXPECTED_RECORD
* to MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE here.
*
* MBEDTLS_ERR_SSL_UNEXPECTED_RECORD does not appear to be
* used to detect a specific error condition, so this mapping
* should not remove any meaningful distinction.
*/
if ((ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM)
#if defined(MBEDTLS_SSL_RENEGOTIATION)
&& (ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE)
#endif
) {
if (ret == MBEDTLS_ERR_SSL_UNEXPECTED_RECORD) {
ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE;
}
}
#endif /* MBEDTLS_SSL_PROTO_DTLS */
return ret;
}
+1 -49
View File
@@ -4131,55 +4131,7 @@ close_notify:
} while (ret == MBEDTLS_ERR_SSL_WANT_WRITE);
ret = 0;
/*
* In the DTLS case, attempt to read a possible response to the close
* notification. This avoids reconnecting to the same client when we
* reset and later receive its close-notification response during
* step 3 (waiting for a client to connect).
*
* Stop waiting for the response if the connection has already ended.
*
* The waiting loop below relies on mbedtls_ssl_read() returning regularly
* in order to keep the total waiting time approximately bounded to 1s. If
* no read timeout is configured (see the read_timeout option), or if the
* configured timeout is close to or larger than 1s, the total waiting time
* may exceed 1s by a significant margin.
*/
#if defined(MBEDTLS_SSL_PROTO_DTLS) && defined(MBEDTLS_HAVE_TIME)
if (opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
mbedtls_ms_time_t start = mbedtls_ms_time();
for (;;) {
ret = mbedtls_ssl_read(&ssl, buf, opt.buffer_size);
/*
* mbedtls_ssl_read() returned some data or timed out, loop if we
* have not spent already too much time, quite arbitrarily 1s.
*/
if ((ret > 0) || (ret == MBEDTLS_ERR_SSL_TIMEOUT)) {
if ((mbedtls_ms_time() - start) < 1000) {
continue;
}
}
if (ret == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY) {
mbedtls_printf(" done, received client close notification.\n");
} else {
/* ret = 0, silent transport EOF or ret < 0 except
* MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY. Note that we do not
* handle specifically the non-fatal error codes like
* MBEDTLS_ERR_SSL_WANT_READ as we do not really expect them
* here.
*/
mbedtls_printf(" done\n");
}
break;
}
ret = 0;
} else
#endif /* MBEDTLS_SSL_PROTO_DTLS && MBEDTLS_HAVE_TIME */
{
mbedtls_printf(" done\n");
}
fflush(stdout);
mbedtls_printf(" done\n");
#if defined(MBEDTLS_SSL_CACHE_C)
if (opt.cache_remove > 0) {
-1
View File
@@ -557,7 +557,6 @@ setup_arguments()
# with OpenSSL 1.0.1h, -www, -WWW and -HTTP break DTLS handshakes
if is_dtls "$MODE"; then
O_SERVER_ARGS="$O_SERVER_ARGS"
M_SERVER_ARGS="$M_SERVER_ARGS read_timeout=1000"
else
O_SERVER_ARGS="$O_SERVER_ARGS -www"
fi
+9
View File
@@ -50,6 +50,15 @@ class CoverageTask(outcome_analysis.CoverageTask):
# TLS doesn't use restartable ECDH yet.
# https://github.com/Mbed-TLS/mbedtls/issues/7294
re.compile(r'EC restart:.*no USE_PSA.*'),
# The following test fails intermittently on the CI with a frequency
# that significantly impacts CI throughput. They are thus disabled
# for the time being. See
# https://github.com/Mbed-TLS/mbedtls/issues/10652 for more
# information.
'DTLS proxy: 3d, openssl client, fragmentation',
'DTLS proxy: 3d, openssl client, fragmentation, nbio',
'DTLS proxy: 3d, gnutls client, fragmentation',
'DTLS proxy: 3d, gnutls client, fragmentation, nbio=2',
],
'test_suite_config.mbedtls_boolean': [
# Missing coverage of test configurations.
@@ -165,7 +165,6 @@ component_test_tls1_2_ccm_psk_dtls () {
msg "build: configs/config-ccm-psk-dtls1_2.h"
MBEDTLS_CONFIG="configs/config-ccm-psk-dtls1_2.h"
CRYPTO_CONFIG="configs/crypto-config-ccm-psk-tls1_2.h"
tf-psa-crypto/scripts/config.py -f "$CRYPTO_CONFIG" set MBEDTLS_HAVE_TIME
CC=$ASAN_CC cmake -DMBEDTLS_CONFIG_FILE="$MBEDTLS_CONFIG" -DTF_PSA_CRYPTO_CONFIG_FILE="$CRYPTO_CONFIG" -D CMAKE_BUILD_TYPE:String=Asan .
make
+24 -7
View File
@@ -9978,7 +9978,7 @@ run_test "DTLS reassembly: no fragmentation (gnutls client)" \
"$G_NEXT_CLI -u --mtu 2048 --insecure 127.0.0.1" \
0 \
-S "found fragmented DTLS handshake message" \
-S "error"
-s "HTTP/1.0 200 OK"
requires_gnutls
requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2
@@ -9988,7 +9988,7 @@ run_test "DTLS reassembly: some fragmentation (gnutls client)" \
0 \
-s "found fragmented DTLS handshake message" \
-s "Certificate handshake message has been buffered and reassembled" \
-S "error"
-s "HTTP/1.0 200 OK"
# Set the MTU to 128 bytes. The minimum size of a DTLS 1.2 record
# containing a ClientHello handshake message is 69 bytes, without any cookie,
@@ -10003,7 +10003,7 @@ run_test "DTLS reassembly: more fragmentation (gnutls client)" \
"$G_NEXT_CLI -u --mtu 103 --insecure 127.0.0.1" \
0 \
-s "ClientHello handshake message has been buffered and reassembled" \
-S "error"
-s "HTTP/1.0 200 OK"
requires_gnutls
requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2
@@ -10012,7 +10012,7 @@ run_test "DTLS reassembly: more fragmentation, nbio (gnutls client)" \
"$G_NEXT_CLI -u --mtu 103 --insecure 127.0.0.1" \
0 \
-s "ClientHello handshake message has been buffered and reassembled" \
-S "error"
-s "HTTP/1.0 200 OK"
# No fragmentation and renegotiation tests with GnuTLS client as the feature
# does not work properly.
@@ -10053,7 +10053,7 @@ run_test "DTLS reassembly: no fragmentation (openssl client)" \
"$O_NEXT_CLI -dtls -mtu 2048 -cert $DATA_FILES_PATH/server5.crt -key $DATA_FILES_PATH/server5.key" \
0 \
-S "found fragmented DTLS handshake message" \
-S "error"
-s "HTTP/1.0 200 OK"
# Minimum possible MTU for OpenSSL server: 256 bytes.
# We expect the client Certificate handshake message to be fragmented and
@@ -10068,7 +10068,7 @@ run_test "DTLS reassembly: some fragmentation (openssl client)" \
0 \
-s "found fragmented DTLS handshake message" \
-s "Certificate handshake message has been buffered and reassembled" \
-S "error"
-s "HTTP/1.0 200 OK"
requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2
run_test "DTLS reassembly: fragmentation, nbio (openssl client)" \
@@ -10077,7 +10077,7 @@ run_test "DTLS reassembly: fragmentation, nbio (openssl client)" \
0 \
-s "found fragmented DTLS handshake message" \
-s "Certificate handshake message has been buffered and reassembled" \
-S "error"
-s "HTTP/1.0 200 OK"
# Tests for sending fragmented handshake messages with DTLS
#
@@ -12169,6 +12169,10 @@ run_test "DTLS proxy: 3d, openssl client" \
0 \
-s "HTTP/1.0 200 OK"
# The following test fails intermittently on the CI with a frequency that
# significantly impacts CI throughput. Disable it for the time being.
# See https://github.com/Mbed-TLS/mbedtls/issues/10652 for more information.
skip_next_test
requires_openssl_next
client_needs_more_time 8
not_with_valgrind # risk of non-mbedtls peer timing out
@@ -12182,6 +12186,10 @@ run_test "DTLS proxy: 3d, openssl client, fragmentation" \
-s "found fragmented DTLS handshake message" \
-s "Certificate handshake message has been buffered and reassembled"
# The following test fails intermittently on the CI with a frequency that
# significantly impacts CI throughput. Disable it for the time being.
# See https://github.com/Mbed-TLS/mbedtls/issues/10652 for more information.
skip_next_test
requires_openssl_next
client_needs_more_time 8
not_with_valgrind # risk of non-mbedtls peer timing out
@@ -12250,6 +12258,11 @@ run_test "DTLS proxy: 3d, gnutls client" \
# fragmentation to remain the case across GnuTLS version updates. Avoid using a
# smaller MTU, as the smaller the MTU, the more likely the handshake is to fail
# in this very unreliable connection emulation.
# The following test fails intermittently on the CI with a frequency that
# significantly impacts CI throughput. Disable it for the time being.
# See https://github.com/Mbed-TLS/mbedtls/issues/10652 for more information.
skip_next_test
requires_gnutls
client_needs_more_time 8
not_with_valgrind # risk of non-mbedtls peer timing out
@@ -12262,6 +12275,10 @@ run_test "DTLS proxy: 3d, gnutls client, fragmentation" \
-s "HTTP/1.0 200 OK" \
-s "ClientHello handshake message has been buffered and reassembled"
# The following test fails intermittently on the CI with a frequency that
# significantly impacts CI throughput. Disable it for the time being.
# See https://github.com/Mbed-TLS/mbedtls/issues/10652 for more information.
skip_next_test
requires_gnutls
client_needs_more_time 8
not_with_valgrind # risk of non-mbedtls peer timing out