From f05b6eed0cffaef2b543e4e580bba7ee0d370698 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 15 Dec 2022 10:17:41 +0800 Subject: [PATCH 1/5] Revert "Skip early data basic check temp" This reverts commit 4e83173bb7b6770d99022d79ac2d3624c80b8c37. Signed-off-by: Jerry Yu --- tests/opt-testcases/tls13-misc.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/opt-testcases/tls13-misc.sh b/tests/opt-testcases/tls13-misc.sh index 710fb3433..4b0a833ea 100755 --- a/tests/opt-testcases/tls13-misc.sh +++ b/tests/opt-testcases/tls13-misc.sh @@ -264,9 +264,6 @@ run_test "TLS 1.3: G->m: PSK: configured ephemeral only, good." \ 0 \ -s "key exchange mode: ephemeral$" -# skip the basic check now cause it will randomly trigger the anti-replay protection in gnutls_server -# Add it back once we fix the issue -skip_next_test requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C From a15af378671718fa4292495aa27495d686b02064 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 5 Dec 2022 15:55:24 +0800 Subject: [PATCH 2/5] Change time resolution of reco_delay from second to millionseconds Per gnutls anti replay issue, it needs millionsecond time delay for improve the fail rate. From test result of #6712, this can improve the fail rate from 4% to 92%. Signed-off-by: Jerry Yu --- programs/ssl/ssl_client2.c | 4 ++-- tests/opt-testcases/tls13-misc.sh | 4 ++-- tests/ssl-opt.sh | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 02ee7cf69..304d10c0c 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -424,7 +424,7 @@ int main( void ) " reconnect=%%d number of reconnections using session resumption\n" \ " default: 0 (disabled)\n" \ " reco_server_name=%%s default: NULL\n" \ - " reco_delay=%%d default: 0 seconds\n" \ + " reco_delay=%%d default: 0 millionseconds\n" \ " reco_mode=%%d 0: copy session, 1: serialize session\n" \ " default: 1\n" \ " reconnect_hard=%%d default: 0 (disabled)\n" \ @@ -3184,7 +3184,7 @@ reconnect: #if defined(MBEDTLS_TIMING_C) if( opt.reco_delay > 0 ) - mbedtls_net_usleep( 1000000 * opt.reco_delay ); + mbedtls_net_usleep( 1000 * opt.reco_delay ); #endif mbedtls_printf( " . Reconnecting with saved session..." ); diff --git a/tests/opt-testcases/tls13-misc.sh b/tests/opt-testcases/tls13-misc.sh index 4b0a833ea..3aaf3f330 100755 --- a/tests/opt-testcases/tls13-misc.sh +++ b/tests/opt-testcases/tls13-misc.sh @@ -274,7 +274,7 @@ requires_any_configs_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED run_test "TLS 1.3 m->G: EarlyData: basic check, good" \ "$G_NEXT_SRV -d 10 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:+ECDHE-PSK:+PSK --earlydata --disable-client-cert" \ - "$P_CLI debug_level=4 early_data=1 reco_mode=1 reconnect=1 reco_delay=2" \ + "$P_CLI debug_level=4 early_data=1 reco_mode=1 reconnect=1 reco_delay=900" \ 1 \ -c "Reconnecting with saved session" \ -c "NewSessionTicket: early_data(42) extension received." \ @@ -295,7 +295,7 @@ requires_any_configs_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED run_test "TLS 1.3 m->G: EarlyData: no early_data in NewSessionTicket, good" \ "$G_NEXT_SRV -d 10 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:+ECDHE-PSK:+PSK --disable-client-cert" \ - "$P_CLI debug_level=4 early_data=1 reco_mode=1 reconnect=1 reco_delay=2" \ + "$P_CLI debug_level=4 early_data=1 reco_mode=1 reconnect=1" \ 0 \ -c "Reconnecting with saved session" \ -C "NewSessionTicket: early_data(42) extension received." \ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index df78c8fe1..1a5c9a307 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -3632,7 +3632,7 @@ run_test "Session resume using tickets: cache disabled" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "Session resume using tickets: timeout" \ "$P_SRV debug_level=3 tickets=1 cache_max=0 ticket_timeout=1" \ - "$P_CLI debug_level=3 tickets=1 reconnect=1 reco_delay=2" \ + "$P_CLI debug_level=3 tickets=1 reconnect=1 reco_delay=2000" \ 0 \ -c "client hello, adding session ticket extension" \ -s "found session ticket extension" \ @@ -3942,7 +3942,7 @@ run_test "Session resume using tickets, DTLS: cache disabled" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "Session resume using tickets, DTLS: timeout" \ "$P_SRV debug_level=3 dtls=1 tickets=1 cache_max=0 ticket_timeout=1" \ - "$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1 skip_close_notify=1 reco_delay=2" \ + "$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1 skip_close_notify=1 reco_delay=2000" \ 0 \ -c "client hello, adding session ticket extension" \ -s "found session ticket extension" \ @@ -4066,7 +4066,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_SSL_CACHE_C run_test "Session resume using cache: timeout < delay" \ "$P_SRV debug_level=3 tickets=0 cache_timeout=1" \ - "$P_CLI debug_level=3 tickets=0 reconnect=1 reco_delay=2" \ + "$P_CLI debug_level=3 tickets=0 reconnect=1 reco_delay=2000" \ 0 \ -S "session successfully restored from cache" \ -S "session successfully restored from ticket" \ @@ -4077,7 +4077,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_SSL_CACHE_C run_test "Session resume using cache: no timeout" \ "$P_SRV debug_level=3 tickets=0 cache_timeout=0" \ - "$P_CLI debug_level=3 tickets=0 reconnect=1 reco_delay=2" \ + "$P_CLI debug_level=3 tickets=0 reconnect=1 reco_delay=2000" \ 0 \ -s "session successfully restored from cache" \ -S "session successfully restored from ticket" \ @@ -4213,7 +4213,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_SSL_CACHE_C run_test "Session resume using cache, DTLS: timeout < delay" \ "$P_SRV dtls=1 debug_level=3 tickets=0 cache_timeout=1" \ - "$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1 reco_delay=2" \ + "$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1 reco_delay=2000" \ 0 \ -S "session successfully restored from cache" \ -S "session successfully restored from ticket" \ @@ -4224,7 +4224,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_SSL_CACHE_C run_test "Session resume using cache, DTLS: no timeout" \ "$P_SRV dtls=1 debug_level=3 tickets=0 cache_timeout=0" \ - "$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1 reco_delay=2" \ + "$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1 reco_delay=2000" \ 0 \ -s "session successfully restored from cache" \ -S "session successfully restored from ticket" \ @@ -9880,7 +9880,7 @@ run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ key_file=data_files/server8.key \ hs_timeout=10000-60000 \ force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ - mtu=1450 reconnect=1 skip_close_notify=1 reco_delay=1" \ + mtu=1450 reconnect=1 skip_close_notify=1 reco_delay=1000" \ 0 \ -S "autoreduction" \ -s "found fragmented DTLS handshake message" \ From bdb936b7a5da71a6bcaab605852cfb433536a366 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sat, 7 Jan 2023 16:07:46 +0800 Subject: [PATCH 3/5] Workaround anti replay fail of GnuTLS Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 08d492487..b8ca482b8 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -947,6 +947,16 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( uint32_t obfuscated_ticket_age = (uint32_t)( now - session->ticket_received ); + /* Workaround for anti replay fail of GnuTLS server. + * + * The time unit of ticket age is milliseconds, but current unit is + * seconds. If the ticket was received at the end of first second and + * sent in next second, GnuTLS think it is replay attack. + * + */ + if( obfuscated_ticket_age > 0 ) + obfuscated_ticket_age -= 1; + obfuscated_ticket_age *= 1000; obfuscated_ticket_age += session->ticket_age_add; From 99e902f479f1a8fe104f3485b7a51fe2ed1a2318 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sat, 7 Jan 2023 16:11:09 +0800 Subject: [PATCH 4/5] Add changlog entry. Signed-off-by: Jerry Yu --- ChangeLog.d/workaround_gnutls_anti_replay_fail.txt | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 ChangeLog.d/workaround_gnutls_anti_replay_fail.txt diff --git a/ChangeLog.d/workaround_gnutls_anti_replay_fail.txt b/ChangeLog.d/workaround_gnutls_anti_replay_fail.txt new file mode 100644 index 000000000..fba6f7840 --- /dev/null +++ b/ChangeLog.d/workaround_gnutls_anti_replay_fail.txt @@ -0,0 +1,6 @@ +Bugfix + * Workaround #6623. That is time unit issue. The unit of ticket age is + seconds in MBedTLS and milliseconds in GnuTLS. If the real age is 10ms, + it might be 1s(1000ms), as a result, the age of MBedTLS is greater than + GnuTLS server. Reduce 1 if the age is greater than 1 second to workaround + it. From 3e60cada5d4bf2980096cd4c222c6d884eae096b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 10 Jan 2023 14:58:08 +0800 Subject: [PATCH 5/5] Improve comment and changlog Signed-off-by: Jerry Yu --- .../workaround_gnutls_anti_replay_fail.txt | 11 ++++++----- library/ssl_tls13_client.c | 17 +++++++++++------ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/ChangeLog.d/workaround_gnutls_anti_replay_fail.txt b/ChangeLog.d/workaround_gnutls_anti_replay_fail.txt index fba6f7840..cebc2b7ef 100644 --- a/ChangeLog.d/workaround_gnutls_anti_replay_fail.txt +++ b/ChangeLog.d/workaround_gnutls_anti_replay_fail.txt @@ -1,6 +1,7 @@ Bugfix - * Workaround #6623. That is time unit issue. The unit of ticket age is - seconds in MBedTLS and milliseconds in GnuTLS. If the real age is 10ms, - it might be 1s(1000ms), as a result, the age of MBedTLS is greater than - GnuTLS server. Reduce 1 if the age is greater than 1 second to workaround - it. + * In TLS 1.3, when using a ticket for session resumption, tweak its age + calculation on the client side. It prevents a server with more accurate + ticket timestamps (typically timestamps in milliseconds) compared to the + Mbed TLS ticket timestamps (in seconds) to compute a ticket age smaller + than the age computed and transmitted by the client and thus potentially + reject the ticket. Fix #6623. diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index b8ca482b8..1cd2ac575 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -947,12 +947,17 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( uint32_t obfuscated_ticket_age = (uint32_t)( now - session->ticket_received ); - /* Workaround for anti replay fail of GnuTLS server. - * - * The time unit of ticket age is milliseconds, but current unit is - * seconds. If the ticket was received at the end of first second and - * sent in next second, GnuTLS think it is replay attack. - * + /* + * The ticket timestamp is in seconds but the ticket age is in + * milliseconds. If the ticket was received at the end of a second and + * re-used here just at the beginning of the next second, the computed + * age `now - session->ticket_received` is equal to 1s thus 1000 ms + * while the actual age could be just a few milliseconds or tens of + * milliseconds. If the server has more accurate ticket timestamps + * (typically timestamps in milliseconds), as part of the processing of + * the ClientHello, it may compute a ticket lifetime smaller than the + * one computed here and potentially reject the ticket. To avoid that, + * remove one second to the ticket age if possible. */ if( obfuscated_ticket_age > 0 ) obfuscated_ticket_age -= 1;