From cebffc3446fb5d9865e2189450b7aa8ddc8cbd07 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 15 Dec 2022 18:00:05 +0800 Subject: [PATCH 01/26] change time unit of ticket to milliseconds Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 4 ++-- library/ssl_client.c | 7 +++--- library/ssl_ticket.c | 14 +++++++++++- library/ssl_tls13_client.c | 22 ++----------------- library/ssl_tls13_server.c | 45 +++++++++++++++++++------------------- programs/ssl/ssl_server2.c | 8 +++---- 6 files changed, 47 insertions(+), 53 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 7294bb144..cf1c19a08 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1217,7 +1217,7 @@ struct mbedtls_ssl_session { mbedtls_ssl_protocol_version MBEDTLS_PRIVATE(tls_version); #if defined(MBEDTLS_HAVE_TIME) - mbedtls_time_t MBEDTLS_PRIVATE(start); /*!< starting time */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(start); /*!< starting time */ #endif int MBEDTLS_PRIVATE(ciphersuite); /*!< chosen ciphersuite */ size_t MBEDTLS_PRIVATE(id_len); /*!< session id length */ @@ -1255,7 +1255,7 @@ struct mbedtls_ssl_session { #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_CLI_C */ #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) - mbedtls_time_t MBEDTLS_PRIVATE(ticket_received); /*!< time ticket was received */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_received); /*!< time ticket was received */ #endif /* MBEDTLS_HAVE_TIME && MBEDTLS_SSL_CLI_C */ #endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS */ diff --git a/library/ssl_client.c b/library/ssl_client.c index 7a7840662..21114910e 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -756,10 +756,9 @@ static int ssl_prepare_client_hello(mbedtls_ssl_context *ssl) if (ssl->handshake->resume != 0 && session_negotiate->tls_version == MBEDTLS_SSL_VERSION_TLS1_3 && session_negotiate->ticket != NULL) { - mbedtls_time_t now = mbedtls_time(NULL); - uint64_t age = (uint64_t) (now - session_negotiate->ticket_received); - if (session_negotiate->ticket_received > now || - age > session_negotiate->ticket_lifetime) { + mbedtls_ms_time_t now = mbedtls_ms_time(); + mbedtls_ms_time_t age = now - session_negotiate->ticket_received; + if (age < 0 || age > session_negotiate->ticket_lifetime * 1000) { /* Without valid ticket, disable session resumption.*/ MBEDTLS_SSL_DEBUG_MSG( 3, ("Ticket expired, disable session resumption")); diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 875abcbb3..c89a5cdb0 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -495,6 +495,18 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, } #if defined(MBEDTLS_HAVE_TIME) +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { + /* Check for expiration */ + mbedtls_ms_time_t ticket_age = mbedtls_ms_time() - session->start; + mbedtls_ms_time_t ticket_lifetime = ctx->ticket_lifetime * 1000; + + if (ticket_age < 0 || ticket_age > ticket_lifetime) { + ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; + goto cleanup; + } + } else +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ { /* Check for expiration */ mbedtls_time_t current_time = mbedtls_time(NULL); @@ -505,7 +517,7 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, goto cleanup; } } -#endif +#endif /* MBEDTLS_HAVE_TIME */ cleanup: #if defined(MBEDTLS_THREADING_C) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index eac632600..32ad7aaed 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -931,28 +931,10 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( if (ssl_tls13_ticket_get_identity( ssl, &hash_alg, &identity, &identity_len) == 0) { #if defined(MBEDTLS_HAVE_TIME) - mbedtls_time_t now = mbedtls_time(NULL); + mbedtls_ms_time_t now = mbedtls_ms_time(); mbedtls_ssl_session *session = ssl->session_negotiate; uint32_t obfuscated_ticket_age = (uint32_t) (now - session->ticket_received); - - /* - * 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; - } - - obfuscated_ticket_age *= 1000; obfuscated_ticket_age += session->ticket_age_add; ret = ssl_tls13_write_identity(ssl, p, end, @@ -2837,7 +2819,7 @@ static int ssl_tls13_postprocess_new_session_ticket(mbedtls_ssl_context *ssl, #if defined(MBEDTLS_HAVE_TIME) /* Store ticket creation time */ - session->ticket_received = mbedtls_time(NULL); + session->ticket_received = mbedtls_ms_time(); #endif ciphersuite_info = mbedtls_ssl_ciphersuite_from_id(session->ciphersuite); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b418ee635..8bcb0e407 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -111,9 +111,10 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( unsigned char *ticket_buffer; unsigned int key_exchanges; #if defined(MBEDTLS_HAVE_TIME) - mbedtls_time_t now; - uint64_t age_in_s; - int64_t age_diff_in_ms; + mbedtls_ms_time_t now; + mbedtls_ms_time_t server_age; + mbedtls_ms_time_t client_age; + mbedtls_ms_time_t age_diff; #endif ((void) obfuscated_ticket_age); @@ -190,17 +191,16 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; #if defined(MBEDTLS_HAVE_TIME) - now = mbedtls_time(NULL); + now = mbedtls_ms_time(); if (now < session->start) { MBEDTLS_SSL_DEBUG_MSG( - 3, ("Invalid ticket start time ( now=%" MBEDTLS_PRINTF_LONGLONG - ", start=%" MBEDTLS_PRINTF_LONGLONG " )", - (long long) now, (long long) session->start)); + 3, ("Invalid ticket start time ( now=%" MBEDTLS_PRINTF_MS_TIME + ", start=%" MBEDTLS_PRINTF_MS_TIME " )", now, session->start)); goto exit; } - age_in_s = (uint64_t) (now - session->start); + server_age = now - session->start; /* RFC 8446 section 4.6.1 * @@ -213,10 +213,10 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * * For time being, the age MUST be less than 604800 seconds (7 days). */ - if (age_in_s > 604800) { + if (server_age > 604800*1000) { MBEDTLS_SSL_DEBUG_MSG( - 3, ("Ticket age exceeds limitation ticket_age=%lu", - (long unsigned int) age_in_s)); + 3, ("Ticket age exceeds limitation ticket_age=%" MBEDTLS_PRINTF_MS_TIME, + server_age)); goto exit; } @@ -227,18 +227,19 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * ticket_age_add from PskIdentity.obfuscated_ticket_age modulo 2^32) is * within a small tolerance of the time since the ticket was issued. * - * NOTE: When `now == session->start`, `age_diff_in_ms` may be negative - * as the age units are different on the server (s) and in the - * client (ms) side. Add a -1000 ms tolerance window to take this - * into account. + * NOTE: Typical crystal RTC accuracy specifications are from ±100 to ±20 + * parts per million (360 to 72 million seconds per hour). Defualt + * tolerance windows is 6000 millionsections, that means client host + * MUST sync up system time every 16 hours. Otherwise, the ticket will + * be invalid. */ - age_diff_in_ms = age_in_s * 1000; - age_diff_in_ms -= (obfuscated_ticket_age - session->ticket_age_add); - if (age_diff_in_ms <= -1000 || - age_diff_in_ms > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE) { + client_age = obfuscated_ticket_age - session->ticket_age_add; + age_diff = server_age - client_age; + if (age_diff < -1000 || + age_diff > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE) { MBEDTLS_SSL_DEBUG_MSG( - 3, ("Ticket age outside tolerance window ( diff=%d )", - (int) age_diff_in_ms)); + 3, ("Ticket age outside tolerance window ( diff=%" MBEDTLS_PRINTF_MS_TIME ")", + age_diff)); goto exit; } @@ -2877,7 +2878,7 @@ static int ssl_tls13_prepare_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("=> prepare NewSessionTicket msg")); #if defined(MBEDTLS_HAVE_TIME) - session->start = mbedtls_time(NULL); + session->start = mbedtls_ms_time(); #endif /* Set ticket_flags depends on the advertised psk key exchange mode */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 3e2360ed6..ce53f9274 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1420,16 +1420,16 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, case 2: return MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; case 3: - session->start = mbedtls_time(NULL) + 10; + session->start = mbedtls_ms_time() + 10 * 1000; break; case 4: - session->start = mbedtls_time(NULL) - 10 - 7 * 24 * 3600; + session->start = mbedtls_ms_time() - 10 * 1000 - 7 * 24 * 3600 * 1000; break; case 5: - session->start = mbedtls_time(NULL) - 10; + session->start = mbedtls_ms_time() - 10 * 1000; break; case 6: - session->start = mbedtls_time(NULL); + session->start = mbedtls_ms_time(); #if defined(MBEDTLS_SSL_PROTO_TLS1_3) session->ticket_age_add -= 1000; #endif From fe38e948b83da423752deb6f0567f8a194f88628 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 15 Dec 2022 11:16:15 +0800 Subject: [PATCH 02/26] Add changelog entry for anti_replay_fail Signed-off-by: Jerry Yu --- ChangeLog.d/gnutls_anti_replay_fail.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/gnutls_anti_replay_fail.txt diff --git a/ChangeLog.d/gnutls_anti_replay_fail.txt b/ChangeLog.d/gnutls_anti_replay_fail.txt new file mode 100644 index 000000000..cb65b3ba6 --- /dev/null +++ b/ChangeLog.d/gnutls_anti_replay_fail.txt @@ -0,0 +1,4 @@ +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. From 03511b00aab27008bb4b900ecd5f433323cc8c03 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 27 Feb 2023 10:48:41 +0800 Subject: [PATCH 03/26] Replace c99 fmt macro For c99 compatible compilers, we use PRI64d and others use official fix. Signed-off-by: Jerry Yu --- include/mbedtls/debug.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/mbedtls/debug.h b/include/mbedtls/debug.h index 0aef2ed65..9a17488d2 100644 --- a/include/mbedtls/debug.h +++ b/include/mbedtls/debug.h @@ -120,7 +120,12 @@ /* (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1800) */ #if !defined(MBEDTLS_PRINTF_MS_TIME) +#include +#if !defined(PRId64) +#define MBEDTLS_PRINTF_MS_TIME MBEDTLS_PRINTF_LONGLONG +#else #define MBEDTLS_PRINTF_MS_TIME PRId64 +#endif #endif /* MBEDTLS_PRINTF_MS_TIME */ #ifdef __cplusplus From f16efbc78d917c46d18ea82ae1ed8e975c519261 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 30 Oct 2023 11:06:24 +0800 Subject: [PATCH 04/26] fix various issues - Add comments for ticket test hooks - improve code style. Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 10 ++++++---- programs/ssl/ssl_server2.c | 14 +++++++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8bcb0e407..7b8cc6e74 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -195,8 +195,9 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( if (now < session->start) { MBEDTLS_SSL_DEBUG_MSG( - 3, ("Invalid ticket start time ( now=%" MBEDTLS_PRINTF_MS_TIME - ", start=%" MBEDTLS_PRINTF_MS_TIME " )", now, session->start)); + 3, ("Invalid ticket start time ( now = %" MBEDTLS_PRINTF_MS_TIME + ", start = %" MBEDTLS_PRINTF_MS_TIME " )", + now, session->start)); goto exit; } @@ -213,7 +214,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * * For time being, the age MUST be less than 604800 seconds (7 days). */ - if (server_age > 604800*1000) { + if (server_age > 604800 * 1000) { MBEDTLS_SSL_DEBUG_MSG( 3, ("Ticket age exceeds limitation ticket_age=%" MBEDTLS_PRINTF_MS_TIME, server_age)); @@ -238,7 +239,8 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( if (age_diff < -1000 || age_diff > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE) { MBEDTLS_SSL_DEBUG_MSG( - 3, ("Ticket age outside tolerance window ( diff=%" MBEDTLS_PRINTF_MS_TIME ")", + 3, ("Ticket age outside tolerance window ( diff = %" + MBEDTLS_PRINTF_MS_TIME ")", age_diff)); goto exit; } diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index ce53f9274..7df30f017 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1416,35 +1416,43 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, switch (opt.dummy_ticket % 11) { case 1: + /* Callback function return INVALID_MAC */ return MBEDTLS_ERR_SSL_INVALID_MAC; case 2: + /* Callback function return ticket expired */ return MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; case 3: + /* Built-in check, the start time is in future. */ session->start = mbedtls_ms_time() + 10 * 1000; break; case 4: + /* Built-in check, ticket expired due to too old. */ session->start = mbedtls_ms_time() - 10 * 1000 - 7 * 24 * 3600 * 1000; break; case 5: + /* Built-in check, age outside tolerance window, too young. */ session->start = mbedtls_ms_time() - 10 * 1000; break; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 6: + /* Built-in check, age outside tolerance window, too old. */ session->start = mbedtls_ms_time(); -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) session->ticket_age_add -= 1000; -#endif break; -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 7: + /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_NONE; break; case 8: + /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK; break; case 9: + /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; break; case 10: + /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ALL; break; #endif From 702fc590ed660467fd6a4733fd6367b3181ec510 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 31 Oct 2023 14:22:04 +0800 Subject: [PATCH 05/26] Add ticket_creation field Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index cf1c19a08..a152a30f7 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1217,7 +1217,7 @@ struct mbedtls_ssl_session { mbedtls_ssl_protocol_version MBEDTLS_PRIVATE(tls_version); #if defined(MBEDTLS_HAVE_TIME) - mbedtls_ms_time_t MBEDTLS_PRIVATE(start); /*!< starting time */ + mbedtls_time_t MBEDTLS_PRIVATE(start); /*!< start time of current session */ #endif int MBEDTLS_PRIVATE(ciphersuite); /*!< chosen ciphersuite */ size_t MBEDTLS_PRIVATE(id_len); /*!< session id length */ @@ -1248,6 +1248,7 @@ struct mbedtls_ssl_session { uint8_t MBEDTLS_PRIVATE(ticket_flags); /*!< Ticket flags */ uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */ uint8_t MBEDTLS_PRIVATE(resumption_key_len); /*!< resumption_key length */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation); /*!< create time of ticket */ unsigned char MBEDTLS_PRIVATE(resumption_key)[MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN]; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && defined(MBEDTLS_SSL_CLI_C) From ec6d07870d5ede5eebc7b5be0540e41145977482 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 31 Oct 2023 14:42:20 +0800 Subject: [PATCH 06/26] Replace `start` with `ticket_creation` Signed-off-by: Jerry Yu --- library/ssl_ticket.c | 8 +++++--- library/ssl_tls.c | 4 ++-- library/ssl_tls13_server.c | 8 ++++---- programs/ssl/ssl_server2.c | 11 ++++++----- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index c89a5cdb0..05249ea07 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -498,16 +498,17 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { /* Check for expiration */ - mbedtls_ms_time_t ticket_age = mbedtls_ms_time() - session->start; + mbedtls_ms_time_t ticket_age = mbedtls_ms_time() - session->ticket_creation; mbedtls_ms_time_t ticket_lifetime = ctx->ticket_lifetime * 1000; if (ticket_age < 0 || ticket_age > ticket_lifetime) { ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; goto cleanup; } - } else + } #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ - { +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { /* Check for expiration */ mbedtls_time_t current_time = mbedtls_time(NULL); @@ -517,6 +518,7 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, goto cleanup; } } +#endif #endif /* MBEDTLS_HAVE_TIME */ cleanup: diff --git a/library/ssl_tls.c b/library/ssl_tls.c index f8555767e..d7276362f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2537,7 +2537,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { - MBEDTLS_PUT_UINT64_BE((uint64_t) session->start, p, 0); + MBEDTLS_PUT_UINT64_BE((uint64_t) session->ticket_creation, p, 0); p += 8; } #endif /* MBEDTLS_HAVE_TIME */ @@ -2616,7 +2616,7 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, if (end - p < 8) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - session->start = MBEDTLS_GET_UINT64_BE(p, 0); + session->ticket_creation = MBEDTLS_GET_UINT64_BE(p, 0); p += 8; } #endif /* MBEDTLS_HAVE_TIME */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 7b8cc6e74..744e984ac 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -193,15 +193,15 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( #if defined(MBEDTLS_HAVE_TIME) now = mbedtls_ms_time(); - if (now < session->start) { + if (now < session->ticket_creation) { MBEDTLS_SSL_DEBUG_MSG( 3, ("Invalid ticket start time ( now = %" MBEDTLS_PRINTF_MS_TIME ", start = %" MBEDTLS_PRINTF_MS_TIME " )", - now, session->start)); + now, session->ticket_creation)); goto exit; } - server_age = now - session->start; + server_age = now - session->ticket_creation; /* RFC 8446 section 4.6.1 * @@ -2880,7 +2880,7 @@ static int ssl_tls13_prepare_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("=> prepare NewSessionTicket msg")); #if defined(MBEDTLS_HAVE_TIME) - session->start = mbedtls_ms_time(); + session->ticket_creation = mbedtls_ms_time(); #endif /* Set ticket_flags depends on the advertised psk key exchange mode */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 7df30f017..a8f47149f 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1421,22 +1421,23 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, case 2: /* Callback function return ticket expired */ return MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 3: /* Built-in check, the start time is in future. */ - session->start = mbedtls_ms_time() + 10 * 1000; + session->ticket_creation = mbedtls_ms_time() + 10 * 1000; break; case 4: /* Built-in check, ticket expired due to too old. */ - session->start = mbedtls_ms_time() - 10 * 1000 - 7 * 24 * 3600 * 1000; + session->ticket_creation = mbedtls_ms_time() - 10 * 1000 - 7 * 24 * 3600 * 1000; break; case 5: /* Built-in check, age outside tolerance window, too young. */ - session->start = mbedtls_ms_time() - 10 * 1000; + session->ticket_creation = mbedtls_ms_time() - 10 * 1000; break; -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + case 6: /* Built-in check, age outside tolerance window, too old. */ - session->start = mbedtls_ms_time(); + session->ticket_creation = mbedtls_ms_time(); session->ticket_age_add -= 1000; break; case 7: From 28547c49ed07957d7da06c981642e5ffe5879094 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 31 Oct 2023 14:42:50 +0800 Subject: [PATCH 07/26] update tests Signed-off-by: Jerry Yu --- tests/src/test_helpers/ssl_helpers.c | 6 ++- tests/suites/test_suite_ssl.function | 65 ++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 54b57be81..f7cd1030f 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1633,6 +1633,7 @@ exit: } #endif /* MBEDTLS_SSL_SOME_SUITES_USE_MAC */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, int ticket_len, const char *crt_file) @@ -1729,6 +1730,7 @@ int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, return 0; } +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, @@ -1752,14 +1754,14 @@ int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, #if defined(MBEDTLS_HAVE_TIME) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { - session->start = mbedtls_time(NULL) - 42; + session->ticket_creation = mbedtls_ms_time() - 42; } #endif #if defined(MBEDTLS_SSL_CLI_C) if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { #if defined(MBEDTLS_HAVE_TIME) - session->ticket_received = mbedtls_time(NULL) - 40; + session->ticket_received = mbedtls_ms_time() - 40; #endif session->ticket_lifetime = 0xfedcba98; diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 7cdf17eb6..4f9ec1a06 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1943,16 +1943,21 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, /* Prepare a dummy session to work on */ ((void) endpoint_type); ((void) tls_version); + ((void) ticket_len); + ((void) crt_file); #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( &original, 0, endpoint_type) == 0); - } else + } #endif - { + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( &original, ticket_len, crt_file) == 0); } +#endif /* Serialize it */ TEST_ASSERT(mbedtls_ssl_session_save(&original, NULL, 0, &len) @@ -1968,8 +1973,20 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, * Make sure both session structures are identical */ #if defined(MBEDTLS_HAVE_TIME) - TEST_ASSERT(original.start == restored.start); +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { + TEST_ASSERT(original.ticket_creation == restored.ticket_creation); + } #endif + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { + TEST_ASSERT(original.start == restored.start); + } +#endif + +#endif + TEST_ASSERT(original.tls_version == restored.tls_version); TEST_ASSERT(original.ciphersuite == restored.ciphersuite); #if defined(MBEDTLS_SSL_PROTO_TLS1_2) @@ -2049,7 +2066,7 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if (endpoint_type == MBEDTLS_SSL_IS_SERVER) { - TEST_ASSERT(original.start == restored.start); + TEST_ASSERT(original.ticket_creation == restored.ticket_creation); } #endif #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) @@ -2097,16 +2114,22 @@ void ssl_serialize_session_load_save(int ticket_len, char *crt_file, /* Prepare a dummy session to work on */ ((void) endpoint_type); ((void) tls_version); + ((void) ticket_len); + ((void) crt_file); #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( &session, 0, endpoint_type) == 0); - } else + } #endif - { + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { + TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( &session, ticket_len, crt_file) == 0); } +#endif /* Get desired buffer size for serializing */ TEST_ASSERT(mbedtls_ssl_session_save(&session, NULL, 0, &len0) @@ -2160,16 +2183,22 @@ 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); #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( &session, 0, endpoint_type) == 0); - } else + } #endif - { + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( &session, ticket_len, crt_file) == 0); } +#endif + TEST_ASSERT(mbedtls_ssl_session_save(&session, NULL, 0, &good_len) == MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); @@ -2209,16 +2238,22 @@ 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); #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( &session, 0, endpoint_type) == 0); - } else + } #endif - { + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( &session, ticket_len, crt_file) == 0); } +#endif + TEST_ASSERT(mbedtls_ssl_session_save(&session, NULL, 0, &good_len) == MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); TEST_CALLOC(good_buf, good_len); @@ -2272,11 +2307,15 @@ void ssl_session_serialize_version_check(int corrupt_major, if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( &session, 0, endpoint_type) == 0); - } else + } #endif - TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( - &session, 0, NULL) == 0); +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { + TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( + &session, 0, NULL) == 0); + } +#endif /* Infer length of serialized session. */ TEST_ASSERT(mbedtls_ssl_session_save(&session, From 8cf44953b23e498ac4aa046ce1e9d8862bad5f6c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 31 Oct 2023 16:48:45 +0800 Subject: [PATCH 08/26] guards ticket creation field Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index a152a30f7..9365c6241 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1248,7 +1248,9 @@ struct mbedtls_ssl_session { uint8_t MBEDTLS_PRIVATE(ticket_flags); /*!< Ticket flags */ uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */ uint8_t MBEDTLS_PRIVATE(resumption_key_len); /*!< resumption_key length */ +#if defined(MBEDTLS_HAVE_TIME) mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation); /*!< create time of ticket */ +#endif unsigned char MBEDTLS_PRIVATE(resumption_key)[MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN]; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && defined(MBEDTLS_SSL_CLI_C) From 31b601aa15e4080573ec75df00ab5be5db33a2bc Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 11:27:21 +0800 Subject: [PATCH 09/26] improve comments Signed-off-by: Jerry Yu --- library/ssl_ticket.c | 2 +- library/ssl_tls13_server.c | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 05249ea07..5fef4ebb9 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -518,7 +518,7 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, goto cleanup; } } -#endif +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ #endif /* MBEDTLS_HAVE_TIME */ cleanup: diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 744e984ac..fb579d58f 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -212,7 +212,6 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * Clients MUST NOT attempt to use tickets which have ages greater than * the "ticket_lifetime" value which was provided with the ticket. * - * For time being, the age MUST be less than 604800 seconds (7 days). */ if (server_age > 604800 * 1000) { MBEDTLS_SSL_DEBUG_MSG( @@ -228,11 +227,10 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * ticket_age_add from PskIdentity.obfuscated_ticket_age modulo 2^32) is * within a small tolerance of the time since the ticket was issued. * - * NOTE: Typical crystal RTC accuracy specifications are from ±100 to ±20 - * parts per million (360 to 72 million seconds per hour). Defualt - * tolerance windows is 6000 millionsections, that means client host - * MUST sync up system time every 16 hours. Otherwise, the ticket will - * be invalid. + * NOTE: The typical accuracy of an RTC crystal is ±100 to ±20 parts per + * million (360 to 72 milliseconds per hour). Default tolerance + * windows is 6s, thus in the worst case client and servers must + * 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; From 3ff0b1fda3faa242ac023c62441c6c578b8024c4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 11:29:12 +0800 Subject: [PATCH 10/26] Cleanup ticket negative tests. - improve comments - case 3/4 is for server age check. - case 5/6 is for client age check Signed-off-by: Jerry Yu --- programs/ssl/ssl_server2.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index a8f47149f..aa8afd9c6 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1416,44 +1416,45 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, switch (opt.dummy_ticket % 11) { case 1: - /* Callback function return INVALID_MAC */ return MBEDTLS_ERR_SSL_INVALID_MAC; case 2: - /* Callback function return ticket expired */ return MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; #if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 3: - /* Built-in check, the start time is in future. */ - session->ticket_creation = mbedtls_ms_time() + 10 * 1000; + /* Creation time in the future. */ + session->ticket_creation = mbedtls_ms_time() + + MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE + + 4 * 1000; break; case 4: - /* Built-in check, ticket expired due to too old. */ - session->ticket_creation = mbedtls_ms_time() - 10 * 1000 - 7 * 24 * 3600 * 1000; + /* Ticket reaches the end of lifetime. */ + session->ticket_creation = mbedtls_ms_time() - session->ticket_lifetime - + MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE - 4 * 1000; break; case 5: - /* Built-in check, age outside tolerance window, too young. */ - session->ticket_creation = mbedtls_ms_time() - 10 * 1000; + /* Ticket is valid, but client age is beyond the upper bound of 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 = mbedtls_ms_time(); break; case 6: - /* Built-in check, age outside tolerance window, too old. */ + /* Ticket is valid, but client age is beyond the lower bound of 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 = mbedtls_ms_time(); - session->ticket_age_add -= 1000; break; case 7: - /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_NONE; break; case 8: - /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK; break; case 9: - /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; break; case 10: - /* Built-in check, ticket permission check. */ session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ALL; break; #endif From 28e7c554f48a61374793efd25373456a8d36a3a4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 12:05:56 +0800 Subject: [PATCH 11/26] Change the bottom of tolerance window The unit of ticket time has been changed to milliseconds. And age difference might be negative Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index fb579d58f..e7800f1dd 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -234,7 +234,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( */ client_age = obfuscated_ticket_age - session->ticket_age_add; age_diff = server_age - client_age; - if (age_diff < -1000 || + if (age_diff < -MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE || age_diff > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE) { MBEDTLS_SSL_DEBUG_MSG( 3, ("Ticket age outside tolerance window ( diff = %" From 034a8b77d1f4fcf00196be559178e65e93cdaeab Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 12:20:19 +0800 Subject: [PATCH 12/26] Update document of ticket age tolerance Signed-off-by: Jerry Yu --- include/mbedtls/mbedtls_config.h | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index d137f00a2..4acac9c08 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -4099,19 +4099,21 @@ /** * \def MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE * - * Maximum time difference in milliseconds tolerated between the age of a - * ticket from the server and client point of view. - * From the client point of view, the age of a ticket is the time difference - * between the time when the client proposes to the server to use the ticket - * (time of writing of the Pre-Shared Key Extension including the ticket) and - * the time the client received the ticket from the server. - * From the server point of view, the age of a ticket is the time difference - * between the time when the server receives a proposition from the client - * to use the ticket and the time when the ticket was created by the server. - * The server age is expected to be always greater than the client one and - * MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE defines the - * maximum difference tolerated for the server to accept the ticket. - * This is not used in TLS 1.2. + * Maximum allowd ticket age difference in milliseconds tolerated between + * server and client. Default value is 6000. This is not used in TLS 1.2. + * + * - The client ticket age is the time difference between the time when the + * client proposes to the server to use the ticket and the time the client + * received the ticket from the server. + * - The server ticket age is the time difference between the time when the + * server receives a proposition from the client to use the ticket and the + * time when the ticket was created by the server. + * + * The ages might be different due to accuracy of RTC crypstal. The typical + * accuracy of an RTC crystal is ±100 to ±20 parts per million (360 to 72 + * milliseconds per hour). Default tolerance windows is 6s, thus in the worst + * case client and servers must sync up their system time every 6000/360/2~=8 + * hours. * */ //#define MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE 6000 From 46c7926f747f3980c0dbdef4a2b23b52540b6c9a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 13:58:16 +0800 Subject: [PATCH 13/26] Add maximum ticket lifetime check Also add comments for age cast Signed-off-by: Jerry Yu --- library/ssl_misc.h | 3 +++ library/ssl_tls13_client.c | 10 ++++++++++ library/ssl_tls13_server.c | 4 ++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 8482ee151..c636ad461 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2765,6 +2765,9 @@ int mbedtls_ssl_session_set_hostname(mbedtls_ssl_session *session, #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) + +#define MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME (604800) + static inline unsigned int mbedtls_ssl_session_get_ticket_flags( mbedtls_ssl_session *session, unsigned int flags) { diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 32ad7aaed..294a294cc 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -933,6 +933,10 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( #if defined(MBEDTLS_HAVE_TIME) mbedtls_ms_time_t now = mbedtls_ms_time(); mbedtls_ssl_session *session = ssl->session_negotiate; + /* The ticket age has been checked to be smaller that the + * `ticket_lifetime` in ssl_prepare_client_hello() which is smaller than + * 7 days (enforced in ssl_tls13_parse_new_session_ticket()) . Thus the + * cast to `uint32_t` of the ticket age is safe. */ uint32_t obfuscated_ticket_age = (uint32_t) (now - session->ticket_received); obfuscated_ticket_age += session->ticket_age_add; @@ -2744,6 +2748,12 @@ static int ssl_tls13_parse_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime: %u", (unsigned int) session->ticket_lifetime)); + if (session->ticket_lifetime > + MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME) { + /* TODO: Add new return value here? */ + MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime exceeds 7 days.")); + return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + } session->ticket_age_add = MBEDTLS_GET_UINT32_BE(p, 4); MBEDTLS_SSL_DEBUG_MSG(3, diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index e7800f1dd..5c606e4b2 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -3025,8 +3025,8 @@ static int ssl_tls13_write_new_session_ticket_body(mbedtls_ssl_context *ssl, * MAY treat a ticket as valid for a shorter period of time than what * is stated in the ticket_lifetime. */ - if (ticket_lifetime > 604800) { - ticket_lifetime = 604800; + if (ticket_lifetime > MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME) { + ticket_lifetime = MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME; } MBEDTLS_PUT_UINT32_BE(ticket_lifetime, p, 0); MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime: %u", From 25ba4d40ef4397c5b9ad42b79a0f0c3b3954ae4b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 14:12:20 +0800 Subject: [PATCH 14/26] rename `ticket_creation` to `ticket_creation_time` Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 14 ++++++++------ library/ssl_ticket.c | 2 +- library/ssl_tls.c | 6 +++--- library/ssl_tls13_server.c | 8 ++++---- programs/ssl/ssl_server2.c | 14 +++++++------- tests/src/test_helpers/ssl_helpers.c | 2 +- tests/suites/test_suite_ssl.function | 4 ++-- 7 files changed, 26 insertions(+), 24 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 9365c6241..d01164278 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1248,18 +1248,20 @@ struct mbedtls_ssl_session { uint8_t MBEDTLS_PRIVATE(ticket_flags); /*!< Ticket flags */ uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */ uint8_t MBEDTLS_PRIVATE(resumption_key_len); /*!< resumption_key length */ -#if defined(MBEDTLS_HAVE_TIME) - mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation); /*!< create time of ticket */ -#endif unsigned char MBEDTLS_PRIVATE(resumption_key)[MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN]; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && defined(MBEDTLS_SSL_CLI_C) char *MBEDTLS_PRIVATE(hostname); /*!< host name binded with tickets */ #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_CLI_C */ -#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) - mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_received); /*!< time ticket was received */ -#endif /* MBEDTLS_HAVE_TIME && MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_HAVE_TIME) +#if defined(MBEDTLS_SSL_CLI_C) + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_received); /*!< time that ticket was received */ +#endif +#if defined(MBEDTLS_SSL_SRV_C) + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); /*!< create time of ticket */ +#endif +#endif /* MBEDTLS_HAVE_TIME */ #endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS */ diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 5fef4ebb9..0277bfa2c 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -498,7 +498,7 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { /* Check for expiration */ - mbedtls_ms_time_t ticket_age = mbedtls_ms_time() - session->ticket_creation; + mbedtls_ms_time_t ticket_age = mbedtls_ms_time() - session->ticket_creation_time; mbedtls_ms_time_t ticket_lifetime = ctx->ticket_lifetime * 1000; if (ticket_age < 0 || ticket_age > ticket_lifetime) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d7276362f..42d1b86b1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2457,7 +2457,7 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite( * uint32 max_early_data_size; * select ( endpoint ) { * case client: ClientOnlyData; - * case server: uint64 start_time; + * case server: uint64 ticket_creation_time_time; * }; * } serialized_session_tls13; * @@ -2537,7 +2537,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { - MBEDTLS_PUT_UINT64_BE((uint64_t) session->ticket_creation, p, 0); + MBEDTLS_PUT_UINT64_BE((uint64_t) session->ticket_creation_time, p, 0); p += 8; } #endif /* MBEDTLS_HAVE_TIME */ @@ -2616,7 +2616,7 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, if (end - p < 8) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - session->ticket_creation = MBEDTLS_GET_UINT64_BE(p, 0); + session->ticket_creation_time = MBEDTLS_GET_UINT64_BE(p, 0); p += 8; } #endif /* MBEDTLS_HAVE_TIME */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 5c606e4b2..c9c0e1f08 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -193,15 +193,15 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( #if defined(MBEDTLS_HAVE_TIME) now = mbedtls_ms_time(); - if (now < session->ticket_creation) { + 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 " )", - now, session->ticket_creation)); + now, session->ticket_creation_time)); goto exit; } - server_age = now - session->ticket_creation; + server_age = now - session->ticket_creation_time; /* RFC 8446 section 4.6.1 * @@ -2878,7 +2878,7 @@ static int ssl_tls13_prepare_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("=> prepare NewSessionTicket msg")); #if defined(MBEDTLS_HAVE_TIME) - session->ticket_creation = mbedtls_ms_time(); + session->ticket_creation_time = mbedtls_ms_time(); #endif /* Set ticket_flags depends on the advertised psk key exchange mode */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index aa8afd9c6..1bfa529af 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1422,28 +1422,28 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 3: /* Creation time in the future. */ - session->ticket_creation = mbedtls_ms_time() + - MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE + - 4 * 1000; + session->ticket_creation_time = mbedtls_ms_time() + + MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE + + 4 * 1000; break; case 4: /* Ticket reaches the end of lifetime. */ - session->ticket_creation = mbedtls_ms_time() - session->ticket_lifetime - - MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE - 4 * 1000; + session->ticket_creation_time = mbedtls_ms_time() - session->ticket_lifetime - + MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE - 4 * 1000; break; case 5: /* Ticket is valid, but client age is beyond the upper bound of 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 = mbedtls_ms_time(); + session->ticket_creation_time = mbedtls_ms_time(); break; case 6: /* Ticket is valid, but client age is beyond the lower bound of 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 = mbedtls_ms_time(); + session->ticket_creation_time = mbedtls_ms_time(); break; case 7: session->ticket_flags = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_NONE; diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index f7cd1030f..9acb1997e 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1754,7 +1754,7 @@ int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, #if defined(MBEDTLS_HAVE_TIME) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { - session->ticket_creation = mbedtls_ms_time() - 42; + session->ticket_creation_time = mbedtls_ms_time() - 42; } #endif diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 4f9ec1a06..ebbbddb20 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1975,7 +1975,7 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #if defined(MBEDTLS_HAVE_TIME) #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { - TEST_ASSERT(original.ticket_creation == restored.ticket_creation); + TEST_ASSERT(original.ticket_creation_time == restored.ticket_creation_time); } #endif @@ -2066,7 +2066,7 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if (endpoint_type == MBEDTLS_SSL_IS_SERVER) { - TEST_ASSERT(original.ticket_creation == restored.ticket_creation); + TEST_ASSERT(original.ticket_creation_time == restored.ticket_creation_time); } #endif #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) From 342a555eefd27dc20c44489de597477c5839879a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 14:23:39 +0800 Subject: [PATCH 15/26] rename ticket received Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 2 +- library/ssl_client.c | 2 +- library/ssl_tls.c | 8 ++++---- library/ssl_tls13_client.c | 4 ++-- tests/src/test_helpers/ssl_helpers.c | 2 +- tests/suites/test_suite_ssl.function | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d01164278..3213a2bc8 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1256,7 +1256,7 @@ struct mbedtls_ssl_session { #if defined(MBEDTLS_HAVE_TIME) #if defined(MBEDTLS_SSL_CLI_C) - mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_received); /*!< time that ticket was received */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_reception_time); /*!< time that ticket was received */ #endif #if defined(MBEDTLS_SSL_SRV_C) mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); /*!< create time of ticket */ diff --git a/library/ssl_client.c b/library/ssl_client.c index 21114910e..0e87d8607 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -757,7 +757,7 @@ static int ssl_prepare_client_hello(mbedtls_ssl_context *ssl) session_negotiate->tls_version == MBEDTLS_SSL_VERSION_TLS1_3 && session_negotiate->ticket != NULL) { mbedtls_ms_time_t now = mbedtls_ms_time(); - mbedtls_ms_time_t age = now - session_negotiate->ticket_received; + mbedtls_ms_time_t age = now - session_negotiate->ticket_reception_time; if (age < 0 || age > session_negotiate->ticket_lifetime * 1000) { /* Without valid ticket, disable session resumption.*/ MBEDTLS_SSL_DEBUG_MSG( diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 42d1b86b1..2a52047ec 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2443,7 +2443,7 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite( * * struct { * opaque hostname<0..2^16-1>; - * uint64 ticket_received; + * uint64 ticket_reception_time; * uint32 ticket_lifetime; * opaque ticket<1..2^16-1>; * } ClientOnlyData; @@ -2492,7 +2492,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #endif #if defined(MBEDTLS_HAVE_TIME) - needed += 8; /* start_time or ticket_received */ + needed += 8; /* start_time or ticket_reception_time */ #endif #if defined(MBEDTLS_SSL_CLI_C) @@ -2555,7 +2555,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined(MBEDTLS_HAVE_TIME) - MBEDTLS_PUT_UINT64_BE((uint64_t) session->ticket_received, p, 0); + MBEDTLS_PUT_UINT64_BE((uint64_t) session->ticket_reception_time, p, 0); p += 8; #endif MBEDTLS_PUT_UINT32_BE(session->ticket_lifetime, p, 0); @@ -2651,7 +2651,7 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, if (end - p < 8) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - session->ticket_received = MBEDTLS_GET_UINT64_BE(p, 0); + session->ticket_reception_time = MBEDTLS_GET_UINT64_BE(p, 0); p += 8; #endif if (end - p < 4) { diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 294a294cc..bb688b79c 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -938,7 +938,7 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( * 7 days (enforced in ssl_tls13_parse_new_session_ticket()) . Thus the * cast to `uint32_t` of the ticket age is safe. */ uint32_t obfuscated_ticket_age = - (uint32_t) (now - session->ticket_received); + (uint32_t) (now - session->ticket_reception_time); obfuscated_ticket_age += session->ticket_age_add; ret = ssl_tls13_write_identity(ssl, p, end, @@ -2829,7 +2829,7 @@ static int ssl_tls13_postprocess_new_session_ticket(mbedtls_ssl_context *ssl, #if defined(MBEDTLS_HAVE_TIME) /* Store ticket creation time */ - session->ticket_received = mbedtls_ms_time(); + session->ticket_reception_time = mbedtls_ms_time(); #endif ciphersuite_info = mbedtls_ssl_ciphersuite_from_id(session->ciphersuite); diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 9acb1997e..dd06d176a 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1761,7 +1761,7 @@ int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_CLI_C) if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { #if defined(MBEDTLS_HAVE_TIME) - session->ticket_received = mbedtls_ms_time() - 40; + session->ticket_reception_time = mbedtls_ms_time() - 40; #endif session->ticket_lifetime = 0xfedcba98; diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index ebbbddb20..6963ce24d 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2072,7 +2072,7 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) if (endpoint_type == MBEDTLS_SSL_IS_CLIENT) { #if defined(MBEDTLS_HAVE_TIME) - TEST_ASSERT(original.ticket_received == restored.ticket_received); + TEST_ASSERT(original.ticket_reception_time == restored.ticket_reception_time); #endif TEST_ASSERT(original.ticket_lifetime == restored.ticket_lifetime); TEST_ASSERT(original.ticket_len == restored.ticket_len); From cf9135100eb57d4e0d5f2861418eafd97da3f877 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 14 Nov 2023 11:06:52 +0800 Subject: [PATCH 16/26] fix various issues - fix CI failure due to wrong usage of ticket_lifetime - Improve document and comments Signed-off-by: Jerry Yu --- include/mbedtls/mbedtls_config.h | 12 ++++++------ include/mbedtls/ssl.h | 4 ++-- library/ssl_misc.h | 2 -- library/ssl_tls.c | 4 ++-- library/ssl_tls13_client.c | 8 +++----- library/ssl_tls13_server.c | 4 ++-- programs/ssl/ssl_server2.c | 13 +++++-------- 7 files changed, 20 insertions(+), 27 deletions(-) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 4acac9c08..407b31e1c 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -4099,7 +4099,7 @@ /** * \def MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE * - * Maximum allowd ticket age difference in milliseconds tolerated between + * Maximum allowed ticket age difference in milliseconds tolerated between * server and client. Default value is 6000. This is not used in TLS 1.2. * * - The client ticket age is the time difference between the time when the @@ -4109,11 +4109,11 @@ * server receives a proposition from the client to use the ticket and the * time when the ticket was created by the server. * - * The ages might be different due to accuracy of RTC crypstal. The typical - * accuracy of an RTC crystal is ±100 to ±20 parts per million (360 to 72 - * milliseconds per hour). Default tolerance windows is 6s, thus in the worst - * case client and servers must sync up their system time every 6000/360/2~=8 - * hours. + * The ages might be different due to the client and server clocks not running + * at the same pace. The typical accuracy of an RTC crystal is ±100 to ±20 parts + * per million (360 to 72 milliseconds per hour). Default tolerance window is + * 6s, thus in the worst case clients and servers must sync up their system time + * every 6000/360/2~=8 hours. * */ //#define MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE 6000 diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 3213a2bc8..07a9e888e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1256,10 +1256,10 @@ struct mbedtls_ssl_session { #if defined(MBEDTLS_HAVE_TIME) #if defined(MBEDTLS_SSL_CLI_C) - mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_reception_time); /*!< time that ticket was received */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_reception_time); /*!< time when ticket was received. */ #endif #if defined(MBEDTLS_SSL_SRV_C) - mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); /*!< create time of ticket */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); /*!< time when ticket was created. */ #endif #endif /* MBEDTLS_HAVE_TIME */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index c636ad461..2d2504b75 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2766,8 +2766,6 @@ int mbedtls_ssl_session_set_hostname(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) -#define MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME (604800) - static inline unsigned int mbedtls_ssl_session_get_ticket_flags( mbedtls_ssl_session *session, unsigned int flags) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2a52047ec..944caa09c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2457,7 +2457,7 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite( * uint32 max_early_data_size; * select ( endpoint ) { * case client: ClientOnlyData; - * case server: uint64 ticket_creation_time_time; + * case server: uint64 ticket_creation_time; * }; * } serialized_session_tls13; * @@ -2492,7 +2492,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #endif #if defined(MBEDTLS_HAVE_TIME) - needed += 8; /* start_time or ticket_reception_time */ + needed += 8; /* ticket_creation_time or ticket_reception_time */ #endif #if defined(MBEDTLS_SSL_CLI_C) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index bb688b79c..e7a4aef79 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -933,7 +933,7 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( #if defined(MBEDTLS_HAVE_TIME) mbedtls_ms_time_t now = mbedtls_ms_time(); mbedtls_ssl_session *session = ssl->session_negotiate; - /* The ticket age has been checked to be smaller that the + /* The ticket age has been checked to be smaller than the * `ticket_lifetime` in ssl_prepare_client_hello() which is smaller than * 7 days (enforced in ssl_tls13_parse_new_session_ticket()) . Thus the * cast to `uint32_t` of the ticket age is safe. */ @@ -2748,11 +2748,9 @@ static int ssl_tls13_parse_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime: %u", (unsigned int) session->ticket_lifetime)); - if (session->ticket_lifetime > - MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME) { - /* TODO: Add new return value here? */ + if (session->ticket_lifetime > 604800) { MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime exceeds 7 days.")); - return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + return MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER; } session->ticket_age_add = MBEDTLS_GET_UINT32_BE(p, 4); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index c9c0e1f08..465bf99d6 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -3025,8 +3025,8 @@ static int ssl_tls13_write_new_session_ticket_body(mbedtls_ssl_context *ssl, * MAY treat a ticket as valid for a shorter period of time than what * is stated in the ticket_lifetime. */ - if (ticket_lifetime > MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME) { - ticket_lifetime = MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME; + if (ticket_lifetime > 604800) { + ticket_lifetime = 604800; } MBEDTLS_PUT_UINT32_BE(ticket_lifetime, p, 0); MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime: %u", diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 1bfa529af..f85bc1af8 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1422,18 +1422,15 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 3: /* Creation time in the future. */ - session->ticket_creation_time = mbedtls_ms_time() + - MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE + - 4 * 1000; + session->ticket_creation_time = mbedtls_ms_time() + 1000; break; case 4: - /* Ticket reaches the end of lifetime. */ - session->ticket_creation_time = mbedtls_ms_time() - session->ticket_lifetime - - MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE - 4 * 1000; + /* Ticket has reached the end of lifetime. */ + session->ticket_creation_time = mbedtls_ms_time() - + (7 * 24 * 3600 * 1000 + 1000); break; case 5: - /* Ticket is valid, but client age is beyond the upper bound of tolerance window. */ - + /* Ticket is valid, but client age is below the upper bound of 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(); From 472a69260bf868c329607b0e2bcf9c09d10a31e4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 14 Nov 2023 11:25:24 +0800 Subject: [PATCH 17/26] fix build failure Signed-off-by: Jerry Yu --- library/ssl_ticket.c | 13 ++++++++++++- tests/src/test_helpers/ssl_helpers.c | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 0277bfa2c..61c87bed9 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -498,7 +498,18 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { /* Check for expiration */ - mbedtls_ms_time_t ticket_age = mbedtls_ms_time() - session->ticket_creation_time; + mbedtls_ms_time_t ticket_age = -1; +#if defined(MBEDTLS_SSL_SRV_C) + if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { + ticket_age = mbedtls_ms_time() - session->ticket_creation_time; + } +#endif +#if defined(MBEDTLS_SSL_CLI_C) + if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { + ticket_age = mbedtls_ms_time() - session->ticket_reception_time; + } +#endif + mbedtls_ms_time_t ticket_lifetime = ctx->ticket_lifetime * 1000; if (ticket_age < 0 || ticket_age > ticket_lifetime) { diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index dd06d176a..d02d30539 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1752,7 +1752,7 @@ int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, session->max_early_data_size = 0x87654321; #endif -#if defined(MBEDTLS_HAVE_TIME) +#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { session->ticket_creation_time = mbedtls_ms_time() - 42; } From 8e0174ac05e19a986508c65e70cc1bd6621f6ab2 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Nov 2023 13:58:16 +0800 Subject: [PATCH 18/26] Add maximum ticket lifetime check Also add comments for age cast Signed-off-by: Jerry Yu --- library/ssl_misc.h | 2 ++ library/ssl_tls13_client.c | 3 ++- library/ssl_tls13_server.c | 6 +++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 2d2504b75..c636ad461 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2766,6 +2766,8 @@ int mbedtls_ssl_session_set_hostname(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) +#define MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME (604800) + static inline unsigned int mbedtls_ssl_session_get_ticket_flags( mbedtls_ssl_session *session, unsigned int flags) { diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index e7a4aef79..44814b99f 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2748,7 +2748,8 @@ static int ssl_tls13_parse_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime: %u", (unsigned int) session->ticket_lifetime)); - if (session->ticket_lifetime > 604800) { + if (session->ticket_lifetime > + MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME) { MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime exceeds 7 days.")); return MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER; } diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 465bf99d6..8076b1411 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -213,7 +213,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * the "ticket_lifetime" value which was provided with the ticket. * */ - if (server_age > 604800 * 1000) { + if (server_age > MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME * 1000) { MBEDTLS_SSL_DEBUG_MSG( 3, ("Ticket age exceeds limitation ticket_age=%" MBEDTLS_PRINTF_MS_TIME, server_age)); @@ -3025,8 +3025,8 @@ static int ssl_tls13_write_new_session_ticket_body(mbedtls_ssl_context *ssl, * MAY treat a ticket as valid for a shorter period of time than what * is stated in the ticket_lifetime. */ - if (ticket_lifetime > 604800) { - ticket_lifetime = 604800; + if (ticket_lifetime > MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME) { + ticket_lifetime = MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME; } MBEDTLS_PUT_UINT32_BE(ticket_lifetime, p, 0); MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime: %u", From 04fceb782ba776431134090c66f8e4e6ea636df9 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 15 Nov 2023 09:52:46 +0800 Subject: [PATCH 19/26] Add freshness check information into document Signed-off-by: Jerry Yu --- include/mbedtls/mbedtls_config.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 407b31e1c..a4e90c5af 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -4115,6 +4115,7 @@ * 6s, thus in the worst case clients and servers must sync up their system time * every 6000/360/2~=8 hours. * + * See section 8.3 of the TLS 1.3 specification(RFC 8446) for more information. */ //#define MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE 6000 From 9cb953a402da4f7bd5bdd0b3de8bb86fe0c743ef Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 15 Nov 2023 09:54:11 +0800 Subject: [PATCH 20/26] improve document Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8076b1411..374e1a2b3 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -229,7 +229,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * * NOTE: The typical accuracy of an RTC crystal is ±100 to ±20 parts per * million (360 to 72 milliseconds per hour). Default tolerance - * windows is 6s, thus in the worst case client and servers must + * window is 6s, thus in the worst case clients and servers must * sync up their system time every 6000/360/2~=8 hours. */ client_age = obfuscated_ticket_age - session->ticket_age_add; From b2455d24724c0133b5dac84c96849cb8cb097eb4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 15 Nov 2023 09:57:32 +0800 Subject: [PATCH 21/26] Guards ticket_creation_time Signed-off-by: Jerry Yu --- tests/suites/test_suite_ssl.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 6963ce24d..23f0ff9c9 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1973,7 +1973,7 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, * Make sure both session structures are identical */ #if defined(MBEDTLS_HAVE_TIME) -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SRV_C) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { TEST_ASSERT(original.ticket_creation_time == restored.ticket_creation_time); } From d84c14f80c68b22180a8259a42ee31a187469ba7 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 16 Nov 2023 13:33:57 +0800 Subject: [PATCH 22/26] improve code style Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 2 +- tests/suites/test_suite_ssl.function | 56 +++++++++++++++++----------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 374e1a2b3..63929d831 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -215,7 +215,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( */ if (server_age > MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME * 1000) { MBEDTLS_SSL_DEBUG_MSG( - 3, ("Ticket age exceeds limitation ticket_age=%" MBEDTLS_PRINTF_MS_TIME, + 3, ("Ticket age exceeds limitation ticket_age = %" MBEDTLS_PRINTF_MS_TIME, server_age)); goto exit; } diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 23f0ff9c9..fb2ae5421 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2185,19 +2185,25 @@ void ssl_serialize_session_save_buf_size(int ticket_len, char *crt_file, ((void) tls_version); ((void) ticket_len); ((void) crt_file); -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { - TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( - &session, 0, endpoint_type) == 0); - } -#endif -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { - TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( - &session, ticket_len, crt_file) == 0); - } + switch (tls_version) { +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + case MBEDTLS_SSL_VERSION_TLS1_3: + TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( + &session, 0, endpoint_type) == 0); + break; #endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + case MBEDTLS_SSL_VERSION_TLS1_2: + TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( + &session, ticket_len, crt_file) == 0); + break; +#endif + default: + /* should never happen */ + TEST_ASSERT(0); + break; + } TEST_ASSERT(mbedtls_ssl_session_save(&session, NULL, 0, &good_len) == MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); @@ -2303,19 +2309,25 @@ void ssl_session_serialize_version_check(int corrupt_major, USE_PSA_INIT(); ((void) endpoint_type); ((void) tls_version); -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { - TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( - &session, 0, endpoint_type) == 0); - } -#endif -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { - TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( - &session, 0, NULL) == 0); - } + switch (tls_version) { +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + case MBEDTLS_SSL_VERSION_TLS1_3: + TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( + &session, 0, endpoint_type) == 0); + break; #endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + case MBEDTLS_SSL_VERSION_TLS1_2: + TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( + &session, 0, NULL) == 0); + break; +#endif + default: + /* should never happen */ + TEST_ASSERT(0); + break; + } /* Infer length of serialized session. */ TEST_ASSERT(mbedtls_ssl_session_save(&session, From 4ac648ef20f3094298b374aebb0c54eddb0ff6d9 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 16 Nov 2023 13:58:38 +0800 Subject: [PATCH 23/26] improve readability Signed-off-by: Jerry Yu --- tests/suites/test_suite_ssl.function | 46 +++++++++++++++++++--------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index fb2ae5421..fc9b75ce4 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1973,17 +1973,24 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, * Make sure both session structures are identical */ #if defined(MBEDTLS_HAVE_TIME) + switch (tls_version) { #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SRV_C) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { - TEST_ASSERT(original.ticket_creation_time == restored.ticket_creation_time); - } + case MBEDTLS_SSL_VERSION_TLS1_3: + TEST_ASSERT(original.ticket_creation_time == restored.ticket_creation_time); + break; +#endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + case MBEDTLS_SSL_VERSION_TLS1_2: + TEST_ASSERT(original.start == restored.start); + break; #endif -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { - TEST_ASSERT(original.start == restored.start); + default: + /* should never happen */ + TEST_ASSERT(0); + break; } -#endif + #endif @@ -2246,20 +2253,28 @@ void ssl_serialize_session_load_buf_size(int ticket_len, char *crt_file, ((void) tls_version); ((void) ticket_len); ((void) crt_file); + + switch (tls_version) { #if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { - TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( - &session, 0, endpoint_type) == 0); - } + case MBEDTLS_SSL_VERSION_TLS1_3: + TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( + &session, 0, endpoint_type) == 0); + break; #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { - TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( - &session, ticket_len, crt_file) == 0); - } + case MBEDTLS_SSL_VERSION_TLS1_2: + TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( + &session, ticket_len, crt_file) == 0); + break; #endif + default: + /* should never happen */ + TEST_ASSERT(0); + break; + } + TEST_ASSERT(mbedtls_ssl_session_save(&session, NULL, 0, &good_len) == MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); TEST_CALLOC(good_buf, good_len); @@ -2321,6 +2336,7 @@ void ssl_session_serialize_version_check(int corrupt_major, case MBEDTLS_SSL_VERSION_TLS1_2: TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( &session, 0, NULL) == 0); + break; #endif default: From 713ce1f88981ab7e2fdbaef5f700d79f217673f0 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 17 Nov 2023 15:32:12 +0800 Subject: [PATCH 24/26] 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 cb65b3ba6..cb35284e1 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 63929d831..d8ce37508 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 f85bc1af8..c96128b94 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 fc9b75ce4..444c32a37 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) From 60e997205d618d10b77a820b3aed6cb4e1e27626 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 20 Nov 2023 09:55:24 +0800 Subject: [PATCH 25/26] replace check string The output has been changed Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 2 +- tests/opt-testcases/tls13-misc.sh | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index d8ce37508..d983a0039 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -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 - (mbedtls_ms_time_t)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/tests/opt-testcases/tls13-misc.sh b/tests/opt-testcases/tls13-misc.sh index 3816a2b45..920838449 100755 --- a/tests/opt-testcases/tls13-misc.sh +++ b/tests/opt-testcases/tls13-misc.sh @@ -86,7 +86,7 @@ run_test "TLS 1.3 m->m: Session resumption failure, ticket authentication failed -S "key exchange mode: psk$" \ -s "ticket is not authentic" \ -S "ticket is expired" \ - -S "Invalid ticket start time" \ + -S "Invalid ticket creation time" \ -S "Ticket age exceeds limitation" \ -S "Ticket age outside tolerance window" @@ -105,7 +105,7 @@ run_test "TLS 1.3 m->m: Session resumption failure, ticket expired." \ -S "key exchange mode: psk$" \ -S "ticket is not authentic" \ -s "ticket is expired" \ - -S "Invalid ticket start time" \ + -S "Invalid ticket creation time" \ -S "Ticket age exceeds limitation" \ -S "Ticket age outside tolerance window" @@ -124,7 +124,7 @@ run_test "TLS 1.3 m->m: Session resumption failure, invalid start time." \ -S "key exchange mode: psk$" \ -S "ticket is not authentic" \ -S "ticket is expired" \ - -s "Invalid ticket start time" \ + -s "Invalid ticket creation time" \ -S "Ticket age exceeds limitation" \ -S "Ticket age outside tolerance window" @@ -143,7 +143,7 @@ run_test "TLS 1.3 m->m: Session resumption failure, ticket expired. too old" \ -S "key exchange mode: psk$" \ -S "ticket is not authentic" \ -S "ticket is expired" \ - -S "Invalid ticket start time" \ + -S "Invalid ticket creation time" \ -s "Ticket age exceeds limitation" \ -S "Ticket age outside tolerance window" @@ -162,7 +162,7 @@ run_test "TLS 1.3 m->m: Session resumption failure, age outside tolerance window -S "key exchange mode: psk$" \ -S "ticket is not authentic" \ -S "ticket is expired" \ - -S "Invalid ticket start time" \ + -S "Invalid ticket creation time" \ -S "Ticket age exceeds limitation" \ -s "Ticket age outside tolerance window" @@ -181,7 +181,7 @@ run_test "TLS 1.3 m->m: Session resumption failure, age outside tolerance window -S "key exchange mode: psk$" \ -S "ticket is not authentic" \ -S "ticket is expired" \ - -S "Invalid ticket start time" \ + -S "Invalid ticket creation time" \ -S "Ticket age exceeds limitation" \ -s "Ticket age outside tolerance window" From aa5dc24df9206132118ab6d5868382069b8a9149 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 20 Nov 2023 18:07:54 +0800 Subject: [PATCH 26/26] Change if to switch case Signed-off-by: Jerry Yu --- tests/suites/test_suite_ssl.function | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 444c32a37..85776cca3 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2120,23 +2120,28 @@ void ssl_serialize_session_load_save(int ticket_len, char *crt_file, /* Prepare a dummy session to work on */ ((void) endpoint_type); - ((void) tls_version); ((void) ticket_len); ((void) crt_file); + + switch (tls_version) { #if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { - TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( - &session, 0, endpoint_type) == 0); - } + case MBEDTLS_SSL_VERSION_TLS1_3: + TEST_ASSERT(mbedtls_test_ssl_tls13_populate_session( + &session, 0, endpoint_type) == 0); + break; #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { - - TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( - &session, ticket_len, crt_file) == 0); - } + case MBEDTLS_SSL_VERSION_TLS1_2: + TEST_ASSERT(mbedtls_test_ssl_tls12_populate_session( + &session, ticket_len, crt_file) == 0); + break; #endif + default: + /* should never happen */ + TEST_ASSERT(0); + break; + } /* Get desired buffer size for serializing */ TEST_ASSERT(mbedtls_ssl_session_save(&session, NULL, 0, &len0)