From 713ce1f88981ab7e2fdbaef5f700d79f217673f0 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 17 Nov 2023 15:32:12 +0800 Subject: [PATCH] various improvement - improve change log entry - improve comments - remove unnecessary statement - change type of client_age Signed-off-by: Jerry Yu --- ChangeLog.d/gnutls_anti_replay_fail.txt | 7 ++++--- library/ssl_tls13_server.c | 8 ++++---- programs/ssl/ssl_server2.c | 4 ++-- tests/suites/test_suite_ssl.function | 3 --- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/ChangeLog.d/gnutls_anti_replay_fail.txt b/ChangeLog.d/gnutls_anti_replay_fail.txt index cb65b3ba65..cb35284e1c 100644 --- a/ChangeLog.d/gnutls_anti_replay_fail.txt +++ b/ChangeLog.d/gnutls_anti_replay_fail.txt @@ -1,4 +1,5 @@ Bugfix - * Fixes #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 bigger than GnuTLS server. + * Switch to milliseconds as the unit for ticket creation and reception time + instead of seconds. That avoids rounding errors when computing the age of + tickets compared to peer using a millisecond clock (observed with GnuTLS). + Fixes #6623. diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 63929d8310..d8ce375089 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -113,7 +113,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( #if defined(MBEDTLS_HAVE_TIME) mbedtls_ms_time_t now; mbedtls_ms_time_t server_age; - mbedtls_ms_time_t client_age; + uint32_t client_age; mbedtls_ms_time_t age_diff; #endif @@ -195,8 +195,8 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( if (now < session->ticket_creation_time) { MBEDTLS_SSL_DEBUG_MSG( - 3, ("Invalid ticket start time ( now = %" MBEDTLS_PRINTF_MS_TIME - ", start = %" MBEDTLS_PRINTF_MS_TIME " )", + 3, ("Invalid ticket creation time ( now = %" MBEDTLS_PRINTF_MS_TIME + ", creation_time = %" MBEDTLS_PRINTF_MS_TIME " )", now, session->ticket_creation_time)); goto exit; } @@ -233,7 +233,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * sync up their system time every 6000/360/2~=8 hours. */ client_age = obfuscated_ticket_age - session->ticket_age_add; - age_diff = server_age - client_age; + age_diff = server_age - (mbedtls_ms_time_t)client_age; if (age_diff < -MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE || age_diff > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE) { MBEDTLS_SSL_DEBUG_MSG( diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index f85bc1af84..c96128b94c 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1430,14 +1430,14 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, (7 * 24 * 3600 * 1000 + 1000); break; case 5: - /* Ticket is valid, but client age is below the upper bound of tolerance window. */ + /* Ticket is valid, but client age is below the lower bound of the tolerance window. */ session->ticket_age_add += MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE + 4 * 1000; /* Make sure the execution time does not affect the result */ session->ticket_creation_time = mbedtls_ms_time(); break; case 6: - /* Ticket is valid, but client age is beyond the lower bound of tolerance window. */ + /* Ticket is valid, but client age is beyond the upper bound of the tolerance window. */ session->ticket_age_add -= MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE + 4 * 1000; /* Make sure the execution time does not affect the result */ session->ticket_creation_time = mbedtls_ms_time(); diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index fc9b75ce4d..444c32a37a 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2189,7 +2189,6 @@ void ssl_serialize_session_save_buf_size(int ticket_len, char *crt_file, /* Prepare dummy session and get serialized size */ ((void) endpoint_type); - ((void) tls_version); ((void) ticket_len); ((void) crt_file); @@ -2250,7 +2249,6 @@ void ssl_serialize_session_load_buf_size(int ticket_len, char *crt_file, /* Prepare serialized session data */ ((void) endpoint_type); - ((void) tls_version); ((void) ticket_len); ((void) crt_file); @@ -2323,7 +2321,6 @@ void ssl_session_serialize_version_check(int corrupt_major, mbedtls_ssl_session_init(&session); USE_PSA_INIT(); ((void) endpoint_type); - ((void) tls_version); switch (tls_version) { #if defined(MBEDTLS_SSL_PROTO_TLS1_3)