From cf9135100eb57d4e0d5f2861418eafd97da3f877 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 14 Nov 2023 11:06:52 +0800 Subject: [PATCH] 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();