From d1100b0b45170261b8e37933d1b89bcda3cbd9f4 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 21 Nov 2023 13:02:39 +0100 Subject: [PATCH 01/16] Disable ticket module when useless Disable ticket module if either the TLS server or the support for session tickets is not enabled at build time as in that case the ticket module is not used by the TLS library. Signed-off-by: Ronald Cron --- include/mbedtls/config_adjust_ssl.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/config_adjust_ssl.h b/include/mbedtls/config_adjust_ssl.h index 5dd331c76..39c7b3b11 100644 --- a/include/mbedtls/config_adjust_ssl.h +++ b/include/mbedtls/config_adjust_ssl.h @@ -34,6 +34,10 @@ #undef MBEDTLS_SSL_PROTO_DTLS #endif +#if !(defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_SESSION_TICKETS)) +#undef MBEDTLS_SSL_TICKET_C +#endif + #if !defined(MBEDTLS_SSL_PROTO_DTLS) #undef MBEDTLS_SSL_DTLS_ANTI_REPLAY #undef MBEDTLS_SSL_DTLS_CONNECTION_ID From ce72763f78891e8a6e46bdabaf792a97dc6358e1 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Nov 2023 08:06:41 +0100 Subject: [PATCH 02/16] ssl_ticket.c: Remove client code ssl_ticket.c is a server module. Signed-off-by: Ronald Cron --- library/ssl_ticket.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index cd730fb31..d0bd495e6 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -504,11 +504,6 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, 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 = (mbedtls_ms_time_t) ctx->ticket_lifetime * 1000; From 3c3e2e62f699c285931f80229307db4c6753adcb Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Nov 2023 08:10:34 +0100 Subject: [PATCH 03/16] ssl_ticket.c: Remove TLS server guard The ticket module is removed from the build if the TLS server is not in the build now thus no need for the guard. Signed-off-by: Ronald Cron --- library/ssl_ticket.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index d0bd495e6..b47af8613 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -499,12 +499,9 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { /* Check for expiration */ 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 - mbedtls_ms_time_t ticket_lifetime = (mbedtls_ms_time_t) ctx->ticket_lifetime * 1000; From e34f124ff107783c31c18308af8fd9febc2b74eb Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Nov 2023 08:14:07 +0100 Subject: [PATCH 04/16] ssl_ticket.c: Remove pedantic server endpoint check When calculating the ticket age, remove the check that the endpoint is a server. The module is supposed to be used only server side. Furthermore, if such check was necessary, it should be at the beginning of all ssl_ticket.c APIs. As there is no such protection in any API, just remove the check. Signed-off-by: Ronald Cron --- library/ssl_ticket.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index b47af8613..e47cf42ec 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -498,10 +498,8 @@ 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 = -1; - if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { - ticket_age = mbedtls_ms_time() - session->ticket_creation_time; - } + mbedtls_ms_time_t ticket_age = mbedtls_ms_time() - + session->ticket_creation_time; mbedtls_ms_time_t ticket_lifetime = (mbedtls_ms_time_t) ctx->ticket_lifetime * 1000; From ba5165e09a4f38db594384a446566257cad1fcde Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 21 Nov 2023 13:53:18 +0100 Subject: [PATCH 05/16] ssl_ticket.c: Fix ticket lifetime enforcement Take into account that the lifetime of tickets can be changed through the mbedtls_ssl_ticket_rotate() API. Signed-off-by: Ronald Cron --- include/mbedtls/ssl_ticket.h | 4 ++++ library/ssl_ticket.c | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/ssl_ticket.h b/include/mbedtls/ssl_ticket.h index 6d59c12da..58420495a 100644 --- a/include/mbedtls/ssl_ticket.h +++ b/include/mbedtls/ssl_ticket.h @@ -50,6 +50,10 @@ typedef struct mbedtls_ssl_ticket_key { #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t MBEDTLS_PRIVATE(generation_time); /*!< key generation timestamp (seconds) */ #endif + /*! Lifetime of the key in seconds. This is also the lifetime of the + * tickets created under that key. + */ + uint32_t MBEDTLS_PRIVATE(lifetime); #if !defined(MBEDTLS_USE_PSA_CRYPTO) mbedtls_cipher_context_t MBEDTLS_PRIVATE(ctx); /*!< context for auth enc/decryption */ #else diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index e47cf42ec..1ea7a50c4 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -75,6 +75,10 @@ static int ssl_ticket_gen_key(mbedtls_ssl_ticket_context *ctx, #if defined(MBEDTLS_HAVE_TIME) key->generation_time = mbedtls_time(NULL); #endif + /* The lifetime of a key is the configured lifetime of the tickets when + * the key is created. + */ + key->lifetime = ctx->ticket_lifetime; if ((ret = ctx->f_rng(ctx->p_rng, key->name, sizeof(key->name))) != 0) { return ret; @@ -116,16 +120,17 @@ static int ssl_ticket_update_keys(mbedtls_ssl_ticket_context *ctx) #if !defined(MBEDTLS_HAVE_TIME) ((void) ctx); #else - if (ctx->ticket_lifetime != 0) { + mbedtls_ssl_ticket_key * const key = ctx->keys + ctx->active; + if (key->lifetime != 0) { mbedtls_time_t current_time = mbedtls_time(NULL); - mbedtls_time_t key_time = ctx->keys[ctx->active].generation_time; + mbedtls_time_t key_time = key->generation_time; #if defined(MBEDTLS_USE_PSA_CRYPTO) psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; #endif if (current_time >= key_time && - (uint64_t) (current_time - key_time) < ctx->ticket_lifetime) { + (uint64_t) (current_time - key_time) < key->lifetime) { return 0; } @@ -198,6 +203,8 @@ int mbedtls_ssl_ticket_rotate(mbedtls_ssl_ticket_context *ctx, #if defined(MBEDTLS_HAVE_TIME) key->generation_time = mbedtls_time(NULL); #endif + key->lifetime = lifetime; + return 0; } @@ -331,7 +338,7 @@ int mbedtls_ssl_ticket_write(void *p_ticket, key = &ctx->keys[ctx->active]; - *ticket_lifetime = ctx->ticket_lifetime; + *ticket_lifetime = key->lifetime; memcpy(key_name, key->name, TICKET_KEY_NAME_BYTES); @@ -515,7 +522,7 @@ int mbedtls_ssl_ticket_parse(void *p_ticket, mbedtls_time_t current_time = mbedtls_time(NULL); if (current_time < session->start || - (uint32_t) (current_time - session->start) > ctx->ticket_lifetime) { + (uint32_t) (current_time - session->start) > key->lifetime) { ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; goto cleanup; } From 17ef8dfddb998a7eab9de8cdfd0ff5c3b6e069fa Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Nov 2023 10:29:42 +0100 Subject: [PATCH 06/16] ssl_session: Define unconditionally the endpoint field The endpoint field is needed to serialize/deserialize a session in TLS 1.2 the same way it is needed in the TLS 1.3 case: client specific fields that should not be in the serialized version on server side if both TLS client and server are enabled in the TLS library. Signed-off-by: Ronald Cron --- include/mbedtls/ssl.h | 2 +- library/ssl_tls12_client.c | 1 + library/ssl_tls12_server.c | 1 + library/ssl_tls13_client.c | 4 +--- library/ssl_tls13_server.c | 4 ---- 5 files changed, 4 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 36295269a..0be81afe1 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1199,6 +1199,7 @@ struct mbedtls_ssl_session { * or resuming a session instead of the configured minor TLS version. */ mbedtls_ssl_protocol_version MBEDTLS_PRIVATE(tls_version); + uint8_t MBEDTLS_PRIVATE(endpoint); /*!< 0: client, 1: server */ #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t MBEDTLS_PRIVATE(start); /*!< start time of current session */ @@ -1228,7 +1229,6 @@ struct mbedtls_ssl_session { #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) - uint8_t MBEDTLS_PRIVATE(endpoint); /*!< 0: client, 1: server */ 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 */ diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 0c5af87f4..5469850b1 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -1268,6 +1268,7 @@ static int ssl_parse_server_hello(mbedtls_ssl_context *ssl) ssl->tls_version = (mbedtls_ssl_protocol_version) mbedtls_ssl_read_version(buf, ssl->conf->transport); ssl->session_negotiate->tls_version = ssl->tls_version; + ssl->session_negotiate->endpoint = ssl->conf->endpoint; if (ssl->tls_version < ssl->conf->min_tls_version || ssl->tls_version > ssl->conf->max_tls_version) { diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 5a9f6ca4e..e433627a0 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1161,6 +1161,7 @@ read_record_header: ssl->tls_version = (mbedtls_ssl_protocol_version) mbedtls_ssl_read_version(buf, ssl->conf->transport); ssl->session_negotiate->tls_version = ssl->tls_version; + ssl->session_negotiate->endpoint = ssl->conf->endpoint; if (ssl->tls_version != MBEDTLS_SSL_VERSION_TLS1_2) { MBEDTLS_SSL_DEBUG_MSG(1, ("server only supports TLS 1.2")); diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 5c668bdf2..1bfac586a 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1476,10 +1476,8 @@ static int ssl_tls13_preprocess_server_hello(mbedtls_ssl_context *ssl, return SSL_SERVER_HELLO_TLS1_2; } -#if defined(MBEDTLS_SSL_SESSION_TICKETS) - ssl->session_negotiate->endpoint = ssl->conf->endpoint; ssl->session_negotiate->tls_version = ssl->tls_version; -#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + ssl->session_negotiate->endpoint = ssl->conf->endpoint; handshake->received_extensions = MBEDTLS_SSL_EXT_MASK_NONE; diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 6e2866a11..fa1f4786d 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1437,12 +1437,8 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, * We negotiate TLS 1.3. */ ssl->tls_version = MBEDTLS_SSL_VERSION_TLS1_3; - -#if defined(MBEDTLS_SSL_SESSION_TICKETS) - /* Store minor version for later use with ticket serialization. */ ssl->session_negotiate->tls_version = MBEDTLS_SSL_VERSION_TLS1_3; ssl->session_negotiate->endpoint = ssl->conf->endpoint; -#endif /* * We are negotiating the version 1.3 of the protocol. Do what we have From 7b1921ac571ad3eb2f2b552b8d07896c8e6e2a7d Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 23 Nov 2023 12:31:56 +0100 Subject: [PATCH 07/16] Add endpoint in TLS 1.2 session serialization data Signed-off-by: Ronald Cron --- library/ssl_tls.c | 8 ++++++-- tests/include/test/ssl_helpers.h | 1 + tests/src/test_helpers/ssl_helpers.c | 3 +++ tests/suites/test_suite_ssl.function | 17 ++++++----------- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 8c1e37251..9fabda442 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8942,6 +8942,7 @@ unsigned int mbedtls_ssl_tls12_get_preferred_hash_for_sig_alg( * * struct { * uint64 start_time; + * uint8 endpoint; * uint8 ciphersuite[2]; // defined by the standard * uint8 session_id_len; // at most 32 * opaque session_id[32]; @@ -8988,13 +8989,15 @@ static size_t ssl_tls12_session_save(const mbedtls_ssl_session *session, /* * Basic mandatory fields */ - used += 2 /* ciphersuite */ + used += 1 /* endpoint */ + + 2 /* ciphersuite */ + 1 /* id_len */ + sizeof(session->id) + sizeof(session->master) + 4; /* verify_result */ if (used <= buf_len) { + *p++ = session->endpoint; MBEDTLS_PUT_UINT16_BE(session->ciphersuite, p, 0); p += 2; @@ -9129,10 +9132,11 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, /* * Basic mandatory fields */ - if (2 + 1 + 32 + 48 + 4 > (size_t) (end - p)) { + if (1 + 2 + 1 + 32 + 48 + 4 > (size_t) (end - p)) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + session->endpoint = *p++; session->ciphersuite = MBEDTLS_GET_UINT16_BE(p, 0); p += 2; diff --git a/tests/include/test/ssl_helpers.h b/tests/include/test/ssl_helpers.h index d03c62414..ec00fd54d 100644 --- a/tests/include/test/ssl_helpers.h +++ b/tests/include/test/ssl_helpers.h @@ -531,6 +531,7 @@ int mbedtls_test_ssl_prepare_record_mac(mbedtls_record *record, */ int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, int ticket_len, + int endpoint_type, const char *crt_file); #if defined(MBEDTLS_SSL_PROTO_TLS1_3) diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 3d8937da6..8f20fa6d4 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1636,12 +1636,15 @@ exit: #if defined(MBEDTLS_SSL_PROTO_TLS1_2) int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, int ticket_len, + int endpoint_type, const char *crt_file) { #if defined(MBEDTLS_HAVE_TIME) session->start = mbedtls_time(NULL) - 42; #endif session->tls_version = MBEDTLS_SSL_VERSION_TLS1_2; + session->endpoint = endpoint_type == MBEDTLS_SSL_IS_CLIENT ? + MBEDTLS_SSL_IS_CLIENT : MBEDTLS_SSL_IS_SERVER; session->ciphersuite = 0xabcd; session->id_len = sizeof(session->id); memset(session->id, 66, session->id_len); diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 8a03d1b97..b116705b1 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1941,7 +1941,6 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, USE_PSA_INIT(); /* Prepare a dummy session to work on */ - ((void) endpoint_type); ((void) tls_version); ((void) ticket_len); ((void) crt_file); @@ -1955,7 +1954,7 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #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); + &original, ticket_len, endpoint_type, crt_file) == 0); } #endif @@ -1995,6 +1994,7 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #endif TEST_ASSERT(original.tls_version == restored.tls_version); + TEST_ASSERT(original.endpoint == restored.endpoint); TEST_ASSERT(original.ciphersuite == restored.ciphersuite); #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { @@ -2053,7 +2053,6 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if (tls_version == MBEDTLS_SSL_VERSION_TLS1_3) { - TEST_ASSERT(original.endpoint == restored.endpoint); TEST_ASSERT(original.ciphersuite == restored.ciphersuite); TEST_ASSERT(original.ticket_age_add == restored.ticket_age_add); TEST_ASSERT(original.ticket_flags == restored.ticket_flags); @@ -2123,7 +2122,6 @@ void ssl_serialize_session_load_save(int ticket_len, char *crt_file, USE_PSA_INIT(); /* Prepare a dummy session to work on */ - ((void) endpoint_type); ((void) ticket_len); ((void) crt_file); @@ -2138,7 +2136,7 @@ void ssl_serialize_session_load_save(int ticket_len, char *crt_file, #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); + &session, ticket_len, endpoint_type, crt_file) == 0); break; #endif default: @@ -2197,7 +2195,6 @@ void ssl_serialize_session_save_buf_size(int ticket_len, char *crt_file, USE_PSA_INIT(); /* Prepare dummy session and get serialized size */ - ((void) endpoint_type); ((void) ticket_len); ((void) crt_file); @@ -2211,7 +2208,7 @@ void ssl_serialize_session_save_buf_size(int ticket_len, char *crt_file, #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); + &session, ticket_len, endpoint_type, crt_file) == 0); break; #endif default: @@ -2257,7 +2254,6 @@ void ssl_serialize_session_load_buf_size(int ticket_len, char *crt_file, USE_PSA_INIT(); /* Prepare serialized session data */ - ((void) endpoint_type); ((void) ticket_len); ((void) crt_file); @@ -2272,7 +2268,7 @@ void ssl_serialize_session_load_buf_size(int ticket_len, char *crt_file, #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); + &session, ticket_len, endpoint_type, crt_file) == 0); break; #endif @@ -2329,7 +2325,6 @@ void ssl_session_serialize_version_check(int corrupt_major, mbedtls_ssl_session_init(&session); USE_PSA_INIT(); - ((void) endpoint_type); switch (tls_version) { #if defined(MBEDTLS_SSL_PROTO_TLS1_3) @@ -2341,7 +2336,7 @@ void ssl_session_serialize_version_check(int corrupt_major, #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); + &session, 0, endpoint_type, NULL) == 0); break; #endif From feb577a949a95d7e55b18898a52a45edd4645ff3 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 23 Nov 2023 12:34:43 +0100 Subject: [PATCH 08/16] Fix TLS 1.2 session serialization on server side Signed-off-by: Ronald Cron --- library/ssl_tls.c | 66 +++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9fabda442..7d371bf1a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -9059,20 +9059,22 @@ static size_t ssl_tls12_session_save(const mbedtls_ssl_session *session, * Session ticket if any, plus associated data */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) - used += 3 + session->ticket_len + 4; /* len + ticket + lifetime */ + if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { + used += 3 + session->ticket_len + 4; /* len + ticket + lifetime */ - if (used <= buf_len) { - *p++ = MBEDTLS_BYTE_2(session->ticket_len); - *p++ = MBEDTLS_BYTE_1(session->ticket_len); - *p++ = MBEDTLS_BYTE_0(session->ticket_len); + if (used <= buf_len) { + *p++ = MBEDTLS_BYTE_2(session->ticket_len); + *p++ = MBEDTLS_BYTE_1(session->ticket_len); + *p++ = MBEDTLS_BYTE_0(session->ticket_len); - if (session->ticket != NULL) { - memcpy(p, session->ticket, session->ticket_len); - p += session->ticket_len; + if (session->ticket != NULL) { + memcpy(p, session->ticket, session->ticket_len); + p += session->ticket_len; + } + + MBEDTLS_PUT_UINT32_BE(session->ticket_lifetime, p, 0); + p += 4; } - - MBEDTLS_PUT_UINT32_BE(session->ticket_lifetime, p, 0); - p += 4; } #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ @@ -9241,33 +9243,35 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, * Session ticket and associated data */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) - if (3 > (size_t) (end - p)) { - return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; - } - - session->ticket_len = MBEDTLS_GET_UINT24_BE(p, 0); - p += 3; - - if (session->ticket_len != 0) { - if (session->ticket_len > (size_t) (end - p)) { + if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { + if (3 > (size_t) (end - p)) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - session->ticket = mbedtls_calloc(1, session->ticket_len); - if (session->ticket == NULL) { - return MBEDTLS_ERR_SSL_ALLOC_FAILED; + session->ticket_len = MBEDTLS_GET_UINT24_BE(p, 0); + p += 3; + + if (session->ticket_len != 0) { + if (session->ticket_len > (size_t) (end - p)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + + session->ticket = mbedtls_calloc(1, session->ticket_len); + if (session->ticket == NULL) { + return MBEDTLS_ERR_SSL_ALLOC_FAILED; + } + + memcpy(session->ticket, p, session->ticket_len); + p += session->ticket_len; } - memcpy(session->ticket, p, session->ticket_len); - p += session->ticket_len; - } + if (4 > (size_t) (end - p)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } - if (4 > (size_t) (end - p)) { - return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + session->ticket_lifetime = MBEDTLS_GET_UINT32_BE(p, 0); + p += 4; } - - session->ticket_lifetime = MBEDTLS_GET_UINT32_BE(p, 0); - p += 4; #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ /* From d1c106c787eacbf42563e4c2360d59c1bd861a37 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Nov 2023 09:14:02 +0100 Subject: [PATCH 09/16] Define ticket creation time in TLS 1.2 case as well The purpose of this change is to eventually base the calculation in ssl_ticket.c of the ticket age when parsing a ticket on the ticket creation time both in TLS 1.2 and TLS 1.3 case. Signed-off-by: Ronald Cron --- include/mbedtls/ssl.h | 17 +++++++++-------- programs/ssl/ssl_server2.c | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 0be81afe1..542340b9b 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1228,6 +1228,12 @@ struct mbedtls_ssl_session { uint32_t MBEDTLS_PRIVATE(ticket_lifetime); /*!< ticket lifetime hint */ #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_SRV_C) && \ + defined(MBEDTLS_HAVE_TIME) + /*! Time in milliseconds when the ticket was created. */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); +#endif + #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) 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 */ @@ -1238,15 +1244,10 @@ struct mbedtls_ssl_session { char *MBEDTLS_PRIVATE(hostname); /*!< host name binded with tickets */ #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_CLI_C */ -#if defined(MBEDTLS_HAVE_TIME) -#if defined(MBEDTLS_SSL_CLI_C) - mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_reception_time); /*!< time when ticket was received. */ +#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) + /*! Time in milliseconds when the last ticket was received. */ + mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_reception_time); #endif -#if defined(MBEDTLS_SSL_SRV_C) - mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); /*!< time when ticket was created. */ -#endif -#endif /* MBEDTLS_HAVE_TIME */ - #endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_EARLY_DATA) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 598d38cac..1216ba183 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1420,7 +1420,6 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, return MBEDTLS_ERR_SSL_INVALID_MAC; case 2: return MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 3: /* Creation time in the future. */ session->ticket_creation_time = mbedtls_ms_time() + 1000; @@ -1430,6 +1429,7 @@ int dummy_ticket_parse(void *p_ticket, mbedtls_ssl_session *session, session->ticket_creation_time = mbedtls_ms_time() - (7 * 24 * 3600 * 1000 + 1000); break; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) case 5: /* 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; From c57f86e1322bc18e5a817ea0b087d82e273af8fb Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Nov 2023 09:50:01 +0100 Subject: [PATCH 10/16] Add ticket creation time to TLS 1.2 session serialization Signed-off-by: Ronald Cron --- library/ssl_tls.c | 32 ++++++++++++++++++++++++---- tests/src/test_helpers/ssl_helpers.c | 14 +++++++++--- tests/suites/test_suite_ssl.function | 28 +++++------------------- 3 files changed, 44 insertions(+), 30 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 7d371bf1a..8fc4d4da1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8951,6 +8951,7 @@ unsigned int mbedtls_ssl_tls12_get_preferred_hash_for_sig_alg( * opaque peer_cert<0..2^24-1>; // length 0 means no peer cert * opaque ticket<0..2^24-1>; // length 0 means no ticket * uint32 ticket_lifetime; + * uint64 ticket_creation_time; * uint8 mfl_code; // up to 255 according to standard * uint8 encrypt_then_mac; // 0 or 1 * } serialized_session_tls12; @@ -9058,7 +9059,8 @@ static size_t ssl_tls12_session_save(const mbedtls_ssl_session *session, /* * Session ticket if any, plus associated data */ -#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) +#if defined(MBEDTLS_SSL_SESSION_TICKETS) +#if defined(MBEDTLS_SSL_CLI_C) if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { used += 3 + session->ticket_len + 4; /* len + ticket + lifetime */ @@ -9076,7 +9078,18 @@ static size_t ssl_tls12_session_save(const mbedtls_ssl_session *session, p += 4; } } -#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ +#endif /* MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) + if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { + used += 8; + + if (used <= buf_len) { + MBEDTLS_PUT_UINT64_BE((uint64_t) session->ticket_creation_time, p, 0); + p += 8; + } + } +#endif /* MBEDTLS_HAVE_TIME && MBEDTLS_SSL_SRV_C */ +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ /* * Misc extension-related info @@ -9242,7 +9255,8 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, /* * Session ticket and associated data */ -#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) +#if defined(MBEDTLS_SSL_SESSION_TICKETS) +#if defined(MBEDTLS_SSL_CLI_C) if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { if (3 > (size_t) (end - p)) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; @@ -9272,7 +9286,17 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, session->ticket_lifetime = MBEDTLS_GET_UINT32_BE(p, 0); p += 4; } -#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ +#endif /* MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) + if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { + if (8 > (size_t) (end - p)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + session->ticket_creation_time = MBEDTLS_GET_UINT64_BE(p, 0); + p += 8; + } +#endif /* MBEDTLS_HAVE_TIME && MBEDTLS_SSL_SRV_C */ +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ /* * Misc extension-related info diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 8f20fa6d4..c0c5ca4bb 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1639,6 +1639,8 @@ int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, int endpoint_type, const char *crt_file) { + (void) ticket_len; + #if defined(MBEDTLS_HAVE_TIME) session->start = mbedtls_time(NULL) - 42; #endif @@ -1710,7 +1712,8 @@ int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, #endif /* MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED && MBEDTLS_FS_IO */ session->verify_result = 0xdeadbeef; -#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) +#if defined(MBEDTLS_SSL_SESSION_TICKETS) +#if defined(MBEDTLS_SSL_CLI_C) if (ticket_len != 0) { session->ticket = mbedtls_calloc(1, ticket_len); if (session->ticket == NULL) { @@ -1720,9 +1723,14 @@ int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, } session->ticket_len = ticket_len; session->ticket_lifetime = 86401; -#else - (void) ticket_len; +#endif /* MBEDTLS_SSL_CLI_C */ + +#if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_HAVE_TIME) + if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { + session->ticket_creation_time = mbedtls_ms_time() - 42; + } #endif +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) session->mfl_code = 1; diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index b116705b1..bcddba2a0 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1972,26 +1972,13 @@ 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) - 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 - - default: - /* should never happen */ - TEST_ASSERT(0); - break; + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2) { + TEST_ASSERT(original.start == restored.start); } - - +#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_SRV_C) + TEST_ASSERT(original.ticket_creation_time == restored.ticket_creation_time); #endif +#endif /* MBEDTLS_HAVE_TIME */ TEST_ASSERT(original.tls_version == restored.tls_version); TEST_ASSERT(original.endpoint == restored.endpoint); @@ -2070,11 +2057,6 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, original.max_early_data_size == restored.max_early_data_size); #endif -#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) - if (endpoint_type == MBEDTLS_SSL_IS_SERVER) { - TEST_ASSERT(original.ticket_creation_time == restored.ticket_creation_time); - } -#endif #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) if (endpoint_type == MBEDTLS_SSL_IS_CLIENT) { #if defined(MBEDTLS_HAVE_TIME) From 3c0072b58e1a6ff4678ef79ff367e6dae848278b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Nov 2023 10:00:14 +0100 Subject: [PATCH 11/16] ssl_ticket.c: Base ticket age check on the ticket creation time Signed-off-by: Ronald Cron --- include/mbedtls/ssl.h | 45 +++++++++++++++++++++++++++++++++++++- library/ssl_ticket.c | 35 ++++++++++------------------- library/ssl_tls12_server.c | 3 +++ library/ssl_tls13_server.c | 7 +++--- 4 files changed, 62 insertions(+), 28 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 542340b9b..f4b96d80f 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1230,7 +1230,22 @@ struct mbedtls_ssl_session { #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_SRV_C) && \ defined(MBEDTLS_HAVE_TIME) - /*! Time in milliseconds when the ticket was created. */ + /*! When a ticket is created by a TLS server as part of an established TLS + * session, the ticket creation time may need to be saved for the ticket + * module to be able to check the ticket age when the ticket is used. + * That's the purpose of this field. + * Before creating a new ticket, an Mbed TLS server set this field with + * its current time in milliseconds. This time may then be saved in the + * session ticket data by the session ticket writing function and + * recovered by the ticket parsing function later when the ticket is used. + * The ticket module may then use this time to compute the ticket age and + * determine if it has expired or not. + * The Mbed TLS implementations of the session ticket writing and parsing + * functions save and retrieve the ticket creation time as part of the + * session ticket data. The session ticket parsing function relies on + * the mbedtls_ssl_session_get_ticket_creation_time() API to get the + * ticket creation time from the session ticket data. + */ mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); #endif @@ -2573,6 +2588,34 @@ void mbedtls_ssl_conf_session_tickets_cb(mbedtls_ssl_config *conf, mbedtls_ssl_ticket_write_t *f_ticket_write, mbedtls_ssl_ticket_parse_t *f_ticket_parse, void *p_ticket); + +#if defined(MBEDTLS_HAVE_TIME) +/** + * \brief Get the creation time of a session ticket. + * + * \note See the documentation of \c ticket_creation_time for information about + * the intended usage of this function. + * + * \param session SSL session + * \param ticket_creation_time On exit, holds the ticket creation time in + * milliseconds. + * + * \return 0 on success, + * MBEDTLS_ERR_SSL_BAD_INPUT_DATA if an input is not valid. + */ +static inline int mbedtls_ssl_session_get_ticket_creation_time( + mbedtls_ssl_session *session, mbedtls_ms_time_t *ticket_creation_time) +{ + if (session == NULL || ticket_creation_time == NULL || + session->MBEDTLS_PRIVATE(endpoint) != MBEDTLS_SSL_IS_SERVER) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + + *ticket_creation_time = session->MBEDTLS_PRIVATE(ticket_creation_time); + + return 0; +} +#endif /* MBEDTLS_HAVE_TIME */ #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_SRV_C */ /** diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 1ea7a50c4..5da3887b8 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -502,33 +502,22 @@ 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->ticket_creation_time; - mbedtls_ms_time_t ticket_lifetime = - (mbedtls_ms_time_t) ctx->ticket_lifetime * 1000; + mbedtls_ms_time_t ticket_creation_time, ticket_age; + mbedtls_ms_time_t ticket_lifetime = + (mbedtls_ms_time_t) ctx->ticket_lifetime * 1000; - if (ticket_age < 0 || ticket_age > ticket_lifetime) { - ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; - goto cleanup; - } + ret = mbedtls_ssl_session_get_ticket_creation_time(session, + &ticket_creation_time); + if (ret != 0) { + goto cleanup; } -#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); - if (current_time < session->start || - (uint32_t) (current_time - session->start) > key->lifetime) { - ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; - goto cleanup; - } + ticket_age = mbedtls_ms_time() - ticket_creation_time; + if (ticket_age < 0 || ticket_age > ticket_lifetime) { + ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; + goto cleanup; } -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ -#endif /* MBEDTLS_HAVE_TIME */ +#endif cleanup: #if defined(MBEDTLS_THREADING_C) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index e433627a0..38f322c93 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -4282,6 +4282,9 @@ static int ssl_write_new_session_ticket(mbedtls_ssl_context *ssl) * 10 . 9+n ticket content */ +#if defined(MBEDTLS_HAVE_TIME) + ssl->session_negotiate->ticket_creation_time = mbedtls_ms_time(); +#endif if ((ret = ssl->conf->f_ticket_write(ssl->conf->p_ticket, ssl->session_negotiate, ssl->out_msg + 10, diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index fa1f4786d..ceb07ba22 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -3140,10 +3140,6 @@ 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_time = mbedtls_ms_time(); -#endif - /* Set ticket_flags depends on the advertised psk key exchange mode */ mbedtls_ssl_tls13_session_clear_ticket_flags( session, MBEDTLS_SSL_TLS1_3_TICKET_FLAGS_MASK); @@ -3278,6 +3274,9 @@ static int ssl_tls13_write_new_session_ticket_body(mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_PTR(p, end, 4 + 4 + 1 + ticket_nonce_size + 2); /* Generate ticket and ticket_lifetime */ +#if defined(MBEDTLS_HAVE_TIME) + session->ticket_creation_time = mbedtls_ms_time(); +#endif ret = ssl->conf->f_ticket_write(ssl->conf->p_ticket, session, p + 9 + ticket_nonce_size + 2, From 40a4ab0e0c7ff17f19f8783b9d4ad3ae5b96fa42 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 15 Jan 2024 10:21:30 +0100 Subject: [PATCH 12/16] ssl_tls.c: Factorize save/load of endpoint and ciphersuite Move the save/load of session endpoint and ciphersuite that are common to TLS 1.2 and TLS 1.3 serialized data from the specialized ssl_{tls12,tls13}_session_{save,load} functions to ssl__session_{save,load}. Signed-off-by: Ronald Cron --- library/ssl_tls.c | 64 +++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 8fc4d4da1..806cc030e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2449,8 +2449,6 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite( * } ClientOnlyData; * * struct { - * uint8 endpoint; - * uint8 ciphersuite[2]; * uint32 ticket_age_add; * uint8 ticket_flags; * opaque resumption_key<0..255>; @@ -2476,11 +2474,9 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, size_t hostname_len = (session->hostname == NULL) ? 0 : strlen(session->hostname) + 1; #endif - size_t needed = 1 /* endpoint */ - + 2 /* ciphersuite */ - + 4 /* ticket_age_add */ - + 1 /* ticket_flags */ - + 1; /* resumption_key length */ + size_t needed = 4 /* ticket_age_add */ + + 1 /* ticket_flags */ + + 1; /* resumption_key length */ *olen = 0; if (session->resumption_key_len > MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN) { @@ -2523,14 +2519,12 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, return MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL; } - p[0] = session->endpoint; - MBEDTLS_PUT_UINT16_BE(session->ciphersuite, p, 1); - MBEDTLS_PUT_UINT32_BE(session->ticket_age_add, p, 3); - p[7] = session->ticket_flags; + MBEDTLS_PUT_UINT32_BE(session->ticket_age_add, p, 0); + p[4] = session->ticket_flags; /* save resumption_key */ - p[8] = session->resumption_key_len; - p += 9; + p[5] = session->resumption_key_len; + p += 6; memcpy(p, session->resumption_key, session->resumption_key_len); p += session->resumption_key_len; @@ -2589,17 +2583,15 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, const unsigned char *p = buf; const unsigned char *end = buf + len; - if (end - p < 9) { + if (end - p < 6) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - session->endpoint = p[0]; - session->ciphersuite = MBEDTLS_GET_UINT16_BE(p, 1); - session->ticket_age_add = MBEDTLS_GET_UINT32_BE(p, 3); - session->ticket_flags = p[7]; + session->ticket_age_add = MBEDTLS_GET_UINT32_BE(p, 0); + session->ticket_flags = p[4]; /* load resumption_key */ - session->resumption_key_len = p[8]; - p += 9; + session->resumption_key_len = p[5]; + p += 6; if (end - p < session->resumption_key_len) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; @@ -3777,11 +3769,16 @@ static int ssl_session_save(const mbedtls_ssl_session *session, } /* - * TLS version identifier + * TLS version identifier, endpoint, ciphersuite */ - used += 1; + used += 1 /* TLS version */ + + 1 /* endpoint */ + + 2; /* ciphersuite */ if (used <= buf_len) { *p++ = MBEDTLS_BYTE_0(session->tls_version); + *p++ = session->endpoint; + MBEDTLS_PUT_UINT16_BE(session->ciphersuite, p, 0); + p += 2; } /* Forward to version-specific serialization routine. */ @@ -3864,12 +3861,15 @@ static int ssl_session_load(mbedtls_ssl_session *session, } /* - * TLS version identifier + * TLS version identifier, endpoint, ciphersuite */ - if (1 > (size_t) (end - p)) { + if (4 > (size_t) (end - p)) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } session->tls_version = (mbedtls_ssl_protocol_version) (0x0300 | *p++); + session->endpoint = *p++; + session->ciphersuite = MBEDTLS_GET_UINT16_BE(p, 0); + p += 2; /* Dispatch according to TLS version. */ remaining_len = (size_t) (end - p); @@ -8942,8 +8942,6 @@ unsigned int mbedtls_ssl_tls12_get_preferred_hash_for_sig_alg( * * struct { * uint64 start_time; - * uint8 endpoint; - * uint8 ciphersuite[2]; // defined by the standard * uint8 session_id_len; // at most 32 * opaque session_id[32]; * opaque master[48]; // fixed length in the standard @@ -8990,18 +8988,12 @@ static size_t ssl_tls12_session_save(const mbedtls_ssl_session *session, /* * Basic mandatory fields */ - used += 1 /* endpoint */ - + 2 /* ciphersuite */ - + 1 /* id_len */ + used += 1 /* id_len */ + sizeof(session->id) + sizeof(session->master) + 4; /* verify_result */ if (used <= buf_len) { - *p++ = session->endpoint; - MBEDTLS_PUT_UINT16_BE(session->ciphersuite, p, 0); - p += 2; - *p++ = MBEDTLS_BYTE_0(session->id_len); memcpy(p, session->id, 32); p += 32; @@ -9147,14 +9139,10 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, /* * Basic mandatory fields */ - if (1 + 2 + 1 + 32 + 48 + 4 > (size_t) (end - p)) { + if (1 + 32 + 48 + 4 > (size_t) (end - p)) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - session->endpoint = *p++; - session->ciphersuite = MBEDTLS_GET_UINT16_BE(p, 0); - p += 2; - session->id_len = *p++; memcpy(session->id, p, 32); p += 32; From 7fdee8b7102d5525ef3d331b76b6c9b86cd4d8d9 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 9 Feb 2024 09:26:12 +0100 Subject: [PATCH 13/16] ssl_session: Reorder some fields to reduce padding Signed-off-by: Ronald Cron --- include/mbedtls/ssl.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index f4b96d80f..d8cc76ff4 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1194,12 +1194,12 @@ struct mbedtls_ssl_session { #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */ unsigned char MBEDTLS_PRIVATE(exported); + uint8_t MBEDTLS_PRIVATE(endpoint); /*!< 0: client, 1: server */ /** TLS version negotiated in the session. Used if and when renegotiating * or resuming a session instead of the configured minor TLS version. */ mbedtls_ssl_protocol_version MBEDTLS_PRIVATE(tls_version); - uint8_t MBEDTLS_PRIVATE(endpoint); /*!< 0: client, 1: server */ #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t MBEDTLS_PRIVATE(start); /*!< start time of current session */ @@ -1250,9 +1250,9 @@ struct mbedtls_ssl_session { #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) - 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 */ + uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */ + uint8_t MBEDTLS_PRIVATE(ticket_flags); /*!< Ticket flags */ + uint8_t MBEDTLS_PRIVATE(resumption_key_len); /*!< resumption_key length */ 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 c7fa82ee3318941eddf73ee757464c0ca29e315c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 9 Feb 2024 09:33:09 +0100 Subject: [PATCH 14/16] tests: ssl: Improve test parameter sanity check Signed-off-by: Ronald Cron --- tests/src/test_helpers/ssl_helpers.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index c0c5ca4bb..1b6f060c7 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1645,8 +1645,11 @@ int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, session->start = mbedtls_time(NULL) - 42; #endif session->tls_version = MBEDTLS_SSL_VERSION_TLS1_2; - session->endpoint = endpoint_type == MBEDTLS_SSL_IS_CLIENT ? - MBEDTLS_SSL_IS_CLIENT : MBEDTLS_SSL_IS_SERVER; + + TEST_ASSERT(endpoint_type == MBEDTLS_SSL_IS_CLIENT || + endpoint_type == MBEDTLS_SSL_IS_SERVER); + + session->endpoint = endpoint_type; session->ciphersuite = 0xabcd; session->id_len = sizeof(session->id); memset(session->id, 66, session->id_len); @@ -1739,6 +1742,7 @@ int mbedtls_test_ssl_tls12_populate_session(mbedtls_ssl_session *session, session->encrypt_then_mac = 1; #endif +exit: return 0; } #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ From 7b0ac0b98087c209cae540576d4d4a8a9e0f8743 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 9 Feb 2024 09:50:18 +0100 Subject: [PATCH 15/16] Add change log for mbedtls_ssl_session_get_ticket_creation_time() Signed-off-by: Ronald Cron --- ChangeLog.d/get_ticket_creation_time.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/get_ticket_creation_time.txt diff --git a/ChangeLog.d/get_ticket_creation_time.txt b/ChangeLog.d/get_ticket_creation_time.txt new file mode 100644 index 000000000..7b5166c8e --- /dev/null +++ b/ChangeLog.d/get_ticket_creation_time.txt @@ -0,0 +1,3 @@ +Features + * Add getter (mbedtls_ssl_session_get_ticket_creation_time()) to access + `mbedtls_ssl_session.ticket_creation_time`. From a93e25e7499eb672a71f409ab124d753a24b3a32 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 9 Feb 2024 10:01:30 +0100 Subject: [PATCH 16/16] tls12: Fix documentation of TLS 1.2 session serialized data Signed-off-by: Ronald Cron --- library/ssl_tls.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 806cc030e..e57bee2d6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8941,19 +8941,24 @@ unsigned int mbedtls_ssl_tls12_get_preferred_hash_for_sig_alg( /* Serialization of TLS 1.2 sessions: * * struct { - * uint64 start_time; - * uint8 session_id_len; // at most 32 - * opaque session_id[32]; - * opaque master[48]; // fixed length in the standard - * uint32 verify_result; - * opaque peer_cert<0..2^24-1>; // length 0 means no peer cert - * opaque ticket<0..2^24-1>; // length 0 means no ticket - * uint32 ticket_lifetime; - * uint64 ticket_creation_time; - * uint8 mfl_code; // up to 255 according to standard - * uint8 encrypt_then_mac; // 0 or 1 - * } serialized_session_tls12; + * opaque ticket<0..2^24-1>; // length 0 means no ticket + * uint32 ticket_lifetime; + * } ClientOnlyData; * + * struct { + * uint64 start_time; + * uint8 session_id_len; // at most 32 + * opaque session_id[32]; + * opaque master[48]; // fixed length in the standard + * uint32 verify_result; + * opaque peer_cert<0..2^24-1>; // length 0 means no peer cert + * select (endpoint) { + * case client: ClientOnlyData; + * case server: uint64 ticket_creation_time; + * }; + * uint8 mfl_code; // up to 255 according to standard + * uint8 encrypt_then_mac; // 0 or 1 + * } serialized_session_tls12; */ static size_t ssl_tls12_session_save(const mbedtls_ssl_session *session, unsigned char *buf,