From 7201bc6b05e2f8db8c4cfd6268d007059a416e02 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 8 Mar 2024 15:45:36 +0100 Subject: [PATCH 1/5] ssl_client2: Fix early data log Signed-off-by: Ronald Cron --- programs/ssl/ssl_client2.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 332befdf93..43133d901c 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -3072,16 +3072,16 @@ reconnect: frags++; written += ret; } while (written < len); - } end_of_early_data: - buf[written] = '\0'; - mbedtls_printf( - " %" MBEDTLS_PRINTF_SIZET " bytes of early data written in %" MBEDTLS_PRINTF_SIZET " fragments\n\n%s\n", - written, - frags, - (char *) buf); + buf[written] = '\0'; + mbedtls_printf( + " %" MBEDTLS_PRINTF_SIZET " bytes of early data written in %" MBEDTLS_PRINTF_SIZET " fragments\n\n%s\n", + written, + frags, + (char *) buf); + } #endif /* MBEDTLS_SSL_EARLY_DATA */ while ((ret = mbedtls_ssl_handshake(&ssl)) != 0) { From 0050dff6ab9015cd80ba12a296d965bbabe06b01 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 8 Mar 2024 16:30:22 +0100 Subject: [PATCH 2/5] ssl_ticket.h: Fix note in API documentation Since the merge of #8574 it is not the case anymore that the lifetime of keys is twice the lifetime of tickets. Signed-off-by: Ronald Cron --- include/mbedtls/ssl_ticket.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl_ticket.h b/include/mbedtls/ssl_ticket.h index 58420495a1..a1e70d4656 100644 --- a/include/mbedtls/ssl_ticket.h +++ b/include/mbedtls/ssl_ticket.h @@ -108,8 +108,7 @@ void mbedtls_ssl_ticket_init(mbedtls_ssl_ticket_context *ctx); * least as strong as the strongest ciphersuite * supported. Usually that means a 256-bit key. * - * \note The lifetime of the keys is twice the lifetime of tickets. - * It is recommended to pick a reasonable lifetime so as not + * \note It is recommended to pick a reasonable lifetime so as not * to negate the benefits of forward secrecy. * * \return 0 if successful, @@ -145,8 +144,7 @@ int mbedtls_ssl_ticket_setup(mbedtls_ssl_ticket_context *ctx, * \note \c klength must be sufficient for use by cipher specified * to \c mbedtls_ssl_ticket_setup * - * \note The lifetime of the keys is twice the lifetime of tickets. - * It is recommended to pick a reasonable lifetime so as not + * \note It is recommended to pick a reasonable lifetime so as not * to negate the benefits of forward secrecy. * * \return 0 if successful, From 97dfc726f342131ad99f8d7f5a7b03dc6789ee9c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 8 Mar 2024 16:34:59 +0100 Subject: [PATCH 3/5] ssl_ticket.c: Fix ticket lifetime when parsing This is the lifetime of the key used to decrypt the ticket that should be used when parsing a ticket, not the ticket module lifetime that may have been changed since the key was created. Signed-off-by: Ronald Cron --- library/ssl_ticket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 5da3887b81..6a31b0bee6 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -504,7 +504,7 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, #if defined(MBEDTLS_HAVE_TIME) mbedtls_ms_time_t ticket_creation_time, ticket_age; mbedtls_ms_time_t ticket_lifetime = - (mbedtls_ms_time_t) ctx->ticket_lifetime * 1000; + (mbedtls_ms_time_t) key->lifetime * 1000; ret = mbedtls_ssl_session_get_ticket_creation_time(session, &ticket_creation_time); From ce79488dd544be0081c13b301a9b0bfc946adb08 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 22 Nov 2023 15:01:18 +0800 Subject: [PATCH 4/5] tls13: srv: Fail connection if ticket lifetime exceed 7 days Signed-off-by: Jerry Yu Signed-off-by: Ronald Cron --- include/mbedtls/ssl_ticket.h | 14 ++++++++++++++ library/ssl_tls13_server.c | 21 +++++++++++---------- tests/ssl-opt.sh | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ssl_ticket.h b/include/mbedtls/ssl_ticket.h index a1e70d4656..2ee1400210 100644 --- a/include/mbedtls/ssl_ticket.h +++ b/include/mbedtls/ssl_ticket.h @@ -111,6 +111,13 @@ void mbedtls_ssl_ticket_init(mbedtls_ssl_ticket_context *ctx); * \note It is recommended to pick a reasonable lifetime so as not * to negate the benefits of forward secrecy. * + * \note The TLS 1.3 specification states that ticket lifetime must + * be smaller than seven days. If ticket lifetime has been + * set to a value greater than seven days in this module then + * if the TLS 1.3 is configured to send tickets after the + * handshake it will fail the connection when trying to send + * the first ticket. + * * \return 0 if successful, * or a specific MBEDTLS_ERR_XXX error code */ @@ -147,6 +154,13 @@ int mbedtls_ssl_ticket_setup(mbedtls_ssl_ticket_context *ctx, * \note It is recommended to pick a reasonable lifetime so as not * to negate the benefits of forward secrecy. * + * \note The TLS 1.3 specification states that ticket lifetime must + * be smaller than seven days. If ticket lifetime has been + * set to a value greater than seven days in this module then + * if the TLS 1.3 is configured to send tickets after the + * handshake it will fail the connection when trying to send + * the first ticket. + * * \return 0 if successful, * or a specific MBEDTLS_ERR_XXX error code */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 5c24657af6..13ae61df72 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -3271,20 +3271,21 @@ static int ssl_tls13_write_new_session_ticket_body(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_RET(1, "write_ticket", ret); return ret; } - /* RFC 8446 4.6.1 + + /* RFC 8446 section 4.6.1 + * * ticket_lifetime: Indicates the lifetime in seconds as a 32-bit - * unsigned integer in network byte order from the time of ticket - * issuance. Servers MUST NOT use any value greater than - * 604800 seconds (7 days). The value of zero indicates that the - * ticket should be discarded immediately. Clients MUST NOT cache - * tickets for longer than 7 days, regardless of the ticket_lifetime, - * and MAY delete tickets earlier based on local policy. A server - * MAY treat a ticket as valid for a shorter period of time than what - * is stated in the ticket_lifetime. + * unsigned integer in network byte order from the time of ticket + * issuance. Servers MUST NOT use any value greater than + * 604800 seconds (7 days) ... */ if (ticket_lifetime > MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME) { - ticket_lifetime = MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME; + MBEDTLS_SSL_DEBUG_MSG( + 1, ("Ticket lifetime (%u) is greater than 7 days.", + (unsigned int) ticket_lifetime)); + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + MBEDTLS_PUT_UINT32_BE(ticket_lifetime, p, 0); MBEDTLS_SSL_DEBUG_MSG(3, ("ticket_lifetime: %u", (unsigned int) ticket_lifetime)); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index fd2fc0a1b1..3d76501e27 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -13532,6 +13532,42 @@ run_test "TLS 1.3: NewSessionTicket: Basic check, m->m" \ -s "key exchange mode: psk_ephemeral" \ -s "found pre_shared_key extension" +requires_all_configs_enabled MBEDTLS_SSL_PROTO_TLS1_3 \ + MBEDTLS_SSL_CLI_C MBEDTLS_SSL_SRV_C \ + MBEDTLS_SSL_SESSION_TICKETS MBEDTLS_HAVE_TIME \ + MBEDTLS_DEBUG_C \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +requires_any_configs_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED +run_test "TLS 1.3 m->m: NewSessionTicket: Ticket lifetime max value (7d)" \ + "$P_SRV debug_level=1 crt_file=data_files/server5.crt key_file=data_files/server5.key ticket_timeout=604800 tickets=1" \ + "$P_CLI reco_mode=1 reconnect=1" \ + 0 \ + -c "Protocol is TLSv1.3" \ + -c "HTTP/1.0 200 OK" \ + -c "got new session ticket" \ + -c "Reconnecting with saved session... ok" \ + -s "Protocol is TLSv1.3" \ + -S "Ticket lifetime (604800) is greater than 7 days." + +requires_all_configs_enabled MBEDTLS_SSL_PROTO_TLS1_3 \ + MBEDTLS_SSL_CLI_C MBEDTLS_SSL_SRV_C \ + MBEDTLS_SSL_SESSION_TICKETS MBEDTLS_HAVE_TIME \ + MBEDTLS_DEBUG_C \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +requires_any_configs_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED +run_test "TLS 1.3 m->m: NewSessionTicket: Ticket lifetime too long (7d + 1s)" \ + "$P_SRV debug_level=1 crt_file=data_files/server5.crt key_file=data_files/server5.key ticket_timeout=604801 tickets=1" \ + "$P_CLI reco_mode=1 reconnect=1" \ + 1 \ + -c "Protocol is TLSv1.3" \ + -C "HTTP/1.0 200 OK" \ + -C "got new session ticket" \ + -C "Reconnecting with saved session... ok" \ + -S "Protocol is TLSv1.3" \ + -s "Ticket lifetime (604801) is greater than 7 days." + requires_openssl_tls1_3_with_compatible_ephemeral requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_DEBUG_C From 9422725abafebb93ccda200010c0f2ee8e7a4c4d Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 8 Mar 2024 17:51:23 +0100 Subject: [PATCH 5/5] tls13: cli: Discard ticket with zero lifetime Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 50 ++++++++++++++++++++++++++++++-------- tests/ssl-opt.sh | 19 +++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 88d6c9e019..367009c0c8 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2916,12 +2916,17 @@ static int ssl_tls13_parse_new_session_ticket(mbedtls_ssl_context *ssl, return ret; } - /* session has been updated, allow export */ - session->exported = 0; - return 0; } +/* Non negative return values for ssl_tls13_postprocess_new_session_ticket(). + * - POSTPROCESS_NEW_SESSION_TICKET_SIGNAL, all good, we have to signal the + * application that a valid ticket has been received. + * - POSTPROCESS_NEW_SESSION_TICKET_DISCARD, no fatal error, we keep the + * connection alive but we do not signal the ticket to the application. + */ +#define POSTPROCESS_NEW_SESSION_TICKET_SIGNAL 0 +#define POSTPROCESS_NEW_SESSION_TICKET_DISCARD 1 MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_postprocess_new_session_ticket(mbedtls_ssl_context *ssl, unsigned char *ticket_nonce, @@ -2933,6 +2938,10 @@ static int ssl_tls13_postprocess_new_session_ticket(mbedtls_ssl_context *ssl, psa_algorithm_t psa_hash_alg; int hash_length; + if (session->ticket_lifetime == 0) { + return POSTPROCESS_NEW_SESSION_TICKET_DISCARD; + } + #if defined(MBEDTLS_HAVE_TIME) /* Store ticket creation time */ session->ticket_reception_time = mbedtls_ms_time(); @@ -2989,7 +2998,7 @@ static int ssl_tls13_postprocess_new_session_ticket(mbedtls_ssl_context *ssl, session, ssl->conf->tls13_kex_modes); MBEDTLS_SSL_PRINT_TICKET_FLAGS(4, session->ticket_flags); - return 0; + return POSTPROCESS_NEW_SESSION_TICKET_SIGNAL; } /* @@ -3010,12 +3019,37 @@ static int ssl_tls13_process_new_session_ticket(mbedtls_ssl_context *ssl) ssl, MBEDTLS_SSL_HS_NEW_SESSION_TICKET, &buf, &buf_len)); + /* + * We are about to update (maybe only partially) ticket data thus block + * any session export for the time being. + */ + ssl->session->exported = 1; + MBEDTLS_SSL_PROC_CHK(ssl_tls13_parse_new_session_ticket( ssl, buf, buf + buf_len, &ticket_nonce, &ticket_nonce_len)); - MBEDTLS_SSL_PROC_CHK(ssl_tls13_postprocess_new_session_ticket( - ssl, ticket_nonce, ticket_nonce_len)); + MBEDTLS_SSL_PROC_CHK_NEG(ssl_tls13_postprocess_new_session_ticket( + ssl, ticket_nonce, ticket_nonce_len)); + + switch (ret) { + case POSTPROCESS_NEW_SESSION_TICKET_SIGNAL: + /* + * All good, we have received a new valid ticket, session data can + * be exported now and we signal the ticket to the application. + */ + ssl->session->exported = 0; + ret = MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET; + break; + + case POSTPROCESS_NEW_SESSION_TICKET_DISCARD: + ret = 0; + MBEDTLS_SSL_DEBUG_MSG(2, ("Discard new session ticket")); + break; + + default: + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + } mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_HANDSHAKE_OVER); @@ -3132,10 +3166,6 @@ int mbedtls_ssl_tls13_handshake_client_step(mbedtls_ssl_context *ssl) #if defined(MBEDTLS_SSL_SESSION_TICKETS) case MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET: ret = ssl_tls13_process_new_session_ticket(ssl); - if (ret != 0) { - break; - } - ret = MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET; break; #endif /* MBEDTLS_SSL_SESSION_TICKETS */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 3d76501e27..a846e6ab0b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -13568,6 +13568,25 @@ run_test "TLS 1.3 m->m: NewSessionTicket: Ticket lifetime too long (7d + 1s)" -S "Protocol is TLSv1.3" \ -s "Ticket lifetime (604801) is greater than 7 days." +requires_all_configs_enabled MBEDTLS_SSL_PROTO_TLS1_3 \ + MBEDTLS_SSL_CLI_C MBEDTLS_SSL_SRV_C \ + MBEDTLS_SSL_SESSION_TICKETS MBEDTLS_HAVE_TIME \ + MBEDTLS_DEBUG_C \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +requires_any_configs_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED +run_test "TLS 1.3 m->m: NewSessionTicket: ticket lifetime=0" \ + "$P_SRV debug_level=2 crt_file=data_files/server5.crt key_file=data_files/server5.key ticket_timeout=0 tickets=1" \ + "$P_CLI debug_level=2 reco_mode=1 reconnect=1" \ + 1 \ + -c "Protocol is TLSv1.3" \ + -c "HTTP/1.0 200 OK" \ + -c "Discard new session ticket" \ + -C "got new session ticket" \ + -c "Reconnecting with saved session... failed" \ + -s "Protocol is TLSv1.3" \ + -s "<= write new session ticket" + requires_openssl_tls1_3_with_compatible_ephemeral requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_DEBUG_C