Merge pull request #8913 from ronald-cron-arm/tls13-ticket-lifetime
TLS 1.3: Enforce ticket maximum lifetime and discard tickets with 0 lifetime
diff --git a/include/mbedtls/ssl_ticket.h b/include/mbedtls/ssl_ticket.h
index 5842049..2ee1400 100644
--- a/include/mbedtls/ssl_ticket.h
+++ b/include/mbedtls/ssl_ticket.h
@@ -108,10 +108,16 @@
* 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.
*
+ * \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
*/
@@ -145,10 +151,16 @@
* \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.
*
+ * \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_ticket.c b/library/ssl_ticket.c
index 5da3887..6a31b0b 100644
--- a/library/ssl_ticket.c
+++ b/library/ssl_ticket.c
@@ -504,7 +504,7 @@
#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);
diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c
index bda77e4..7fcc394 100644
--- a/library/ssl_tls13_client.c
+++ b/library/ssl_tls13_client.c
@@ -2917,12 +2917,17 @@
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,
@@ -2934,6 +2939,10 @@
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();
@@ -2990,7 +2999,7 @@
session, ssl->conf->tls13_kex_modes);
MBEDTLS_SSL_PRINT_TICKET_FLAGS(4, session->ticket_flags);
- return 0;
+ return POSTPROCESS_NEW_SESSION_TICKET_SIGNAL;
}
/*
@@ -3011,12 +3020,37 @@
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);
@@ -3133,10 +3167,6 @@
#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/library/ssl_tls13_server.c b/library/ssl_tls13_server.c
index 887c5c6..203aa17 100644
--- a/library/ssl_tls13_server.c
+++ b/library/ssl_tls13_server.c
@@ -3266,20 +3266,21 @@
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/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c
index 332befd..43133d9 100644
--- a/programs/ssl/ssl_client2.c
+++ b/programs/ssl/ssl_client2.c
@@ -3072,16 +3072,16 @@
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) {
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index f1e4f71..0e86368 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -13532,6 +13532,61 @@
-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_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