From 2824a209bcb7dc06c9ee61d15c998c8a4f4be21d Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Fri, 23 Feb 2024 17:51:47 +0000 Subject: [PATCH 1/9] Add ALPN information in session tickets Signed-off-by: Waleed Elmelegy --- include/mbedtls/ssl.h | 3 + library/ssl_tls.c | 82 ++++++++++++++++++++++++++-- library/ssl_tls13_server.c | 22 +++++++- tests/src/test_helpers/ssl_helpers.c | 9 ++- tests/suites/test_suite_ssl.function | 9 +++ 5 files changed, 118 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 39bea79092..a6ee9a4487 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1312,6 +1312,9 @@ struct mbedtls_ssl_session { #if defined(MBEDTLS_SSL_EARLY_DATA) uint32_t MBEDTLS_PRIVATE(max_early_data_size); /*!< maximum amount of early data in tickets */ +#if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) + char *alpn; /*!< ALPN negotiated in the session */ +#endif #endif #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 681ccab441..d7d26ab063 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3735,7 +3735,25 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) /* Serialization of TLS 1.3 sessions: * - * For more detail, see the description of ssl_session_save(). + * struct { + * opaque hostname<0..2^16-1>; + * uint64 ticket_reception_time; + * uint32 ticket_lifetime; + * opaque ticket<1..2^16-1>; + * } ClientOnlyData; + * + * struct { + * uint32 ticket_age_add; + * uint8 ticket_flags; + * opaque resumption_key<0..255>; + * uint32 max_early_data_size; + * uint16 record_size_limit; + * select ( endpoint ) { + * case client: ClientOnlyData; + * case server: uint64 ticket_creation_time; + * }; + * } serialized_session_tls13; + * */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) MBEDTLS_CHECK_RETURN_CRITICAL @@ -3750,9 +3768,16 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, size_t hostname_len = (session->hostname == NULL) ? 0 : strlen(session->hostname) + 1; #endif + +#if defined(MBEDTLS_SSL_SRV_C) && \ + defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) + const uint8_t alpn_len = (session->alpn == NULL) ? + 0 : (uint8_t) strlen(session->alpn) + 1; +#endif 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) { @@ -3771,6 +3796,15 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, needed += 8; /* ticket_creation_time or ticket_reception_time */ #endif +#if defined(MBEDTLS_SSL_SRV_C) + if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) + needed += 1 /* alpn_len */ + + alpn_len; /* alpn */ +#endif + } +#endif /* MBEDTLS_SSL_SRV_C */ + #if defined(MBEDTLS_SSL_CLI_C) if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) @@ -3813,13 +3847,24 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, p += 2; #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */ -#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) +#if defined(MBEDTLS_SSL_SRV_C) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { +#if defined(MBEDTLS_HAVE_TIME) MBEDTLS_PUT_UINT64_BE((uint64_t) session->ticket_creation_time, p, 0); p += 8; - } #endif /* MBEDTLS_HAVE_TIME */ +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) + *p++ = alpn_len; + if (alpn_len > 0) { + /* save chosen alpn */ + memcpy(p, session->alpn, alpn_len); + p += alpn_len; + } +#endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ + } +#endif /* MBEDTLS_SSL_SRV_C */ + #if defined(MBEDTLS_SSL_CLI_C) if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) @@ -3894,16 +3939,39 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, p += 2; #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */ -#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) +#if defined(MBEDTLS_SSL_SRV_C) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { +#if defined(MBEDTLS_HAVE_TIME) if (end - p < 8) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } session->ticket_creation_time = MBEDTLS_GET_UINT64_BE(p, 0); p += 8; - } #endif /* MBEDTLS_HAVE_TIME */ +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) + uint8_t alpn_len; + + if (end - p < 1) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + alpn_len = *p++; + + if (end - p < alpn_len) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + if (alpn_len > 0) { + session->alpn = mbedtls_calloc(alpn_len, sizeof(char)); + if (session->alpn == NULL) { + return MBEDTLS_ERR_SSL_ALLOC_FAILED; + } + memcpy(session->alpn, p, alpn_len); + p += alpn_len; + } +#endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ + } +#endif /* MBEDTLS_SSL_SRV_C */ + #if defined(MBEDTLS_SSL_CLI_C) if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) { #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) @@ -4848,6 +4916,10 @@ void mbedtls_ssl_session_free(mbedtls_ssl_session *session) #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) mbedtls_free(session->hostname); +#endif +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) && \ + defined(MBEDTLS_SSL_SRV_C) + mbedtls_free(session->alpn); #endif mbedtls_free(session->ticket); #endif diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 887c5c6c8f..291d64500d 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -467,7 +467,17 @@ static int ssl_tls13_session_copy_ticket(mbedtls_ssl_session *dst, #if defined(MBEDTLS_SSL_EARLY_DATA) dst->max_early_data_size = src->max_early_data_size; -#endif + +#if defined(MBEDTLS_SSL_ALPN) + if (src->alpn != NULL) { + dst->alpn = mbedtls_calloc(strlen(src->alpn) + 1, sizeof(char)); + if (dst->alpn == NULL) { + return MBEDTLS_ERR_SSL_ALLOC_FAILED; + } + memcpy(dst->alpn, src->alpn, strlen(src->alpn) + 1); + } +#endif /* MBEDTLS_SSL_ALPN */ +#endif /* MBEDTLS_SSL_EARLY_DATA*/ return 0; } @@ -3137,6 +3147,16 @@ static int ssl_tls13_prepare_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_PRINT_TICKET_FLAGS(4, session->ticket_flags); +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) + if (ssl->alpn_chosen != NULL) { + session->alpn = mbedtls_calloc(strlen(ssl->alpn_chosen) + 1, sizeof(char)); + if (session->alpn == NULL) { + return MBEDTLS_ERR_SSL_ALLOC_FAILED; + } + memcpy(session->alpn, ssl->alpn_chosen, strlen(ssl->alpn_chosen) + 1); + } +#endif + /* Generate ticket_age_add */ if ((ret = ssl->conf->f_rng(ssl->conf->p_rng, (unsigned char *) &session->ticket_age_add, diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 56e03f1090..89c1bbf522 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1793,7 +1793,14 @@ int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_EARLY_DATA) session->max_early_data_size = 0x87654321; -#endif +#if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) + session->alpn = mbedtls_calloc(strlen("ALPNExample")+1, sizeof(char)); + if (session->alpn == NULL) { + return -1; + } + strcpy(session->alpn, "ALPNExample"); +#endif /* MBEDTLS_SSL_ALPN && MBEDTLS_SSL_SRV_C */ +#endif /* MBEDTLS_SSL_EARLY_DATA */ #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 8cf2105a52..da07f2c62f 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2104,6 +2104,15 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, #if defined(MBEDTLS_SSL_EARLY_DATA) TEST_ASSERT( original.max_early_data_size == restored.max_early_data_size); +#if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) + if (endpoint_type == MBEDTLS_SSL_IS_SERVER) { + TEST_ASSERT(original.alpn != NULL); + TEST_ASSERT(restored.alpn != NULL); + TEST_ASSERT(memcmp(original.alpn, + restored.alpn, + strlen(original.alpn)) == 0); + } +#endif #endif #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) From 883f77cb08501c5fb618eb39fe5b575d7fd58374 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 6 Mar 2024 19:09:41 +0000 Subject: [PATCH 2/9] Add mbedtls_ssl_session_set_alpn() function Signed-off-by: Waleed Elmelegy --- include/mbedtls/ssl.h | 7 ++-- library/ssl_misc.h | 7 ++++ library/ssl_tls.c | 56 +++++++++++++++++++++++----- library/ssl_tls13_server.c | 19 ++++------ tests/src/test_helpers/ssl_helpers.c | 6 +-- tests/suites/test_suite_ssl.function | 9 ++--- 6 files changed, 71 insertions(+), 33 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index a6ee9a4487..b05bfe1b72 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1304,6 +1304,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_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) + char *ticket_alpn; /*!< ALPN negotiated in the session */ +#endif + #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); @@ -1312,9 +1316,6 @@ struct mbedtls_ssl_session { #if defined(MBEDTLS_SSL_EARLY_DATA) uint32_t MBEDTLS_PRIVATE(max_early_data_size); /*!< maximum amount of early data in tickets */ -#if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) - char *alpn; /*!< ALPN negotiated in the session */ -#endif #endif #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 2ec898b453..948c802299 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2852,6 +2852,13 @@ int mbedtls_ssl_session_set_hostname(mbedtls_ssl_session *session, const char *hostname); #endif +#if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_EARLY_DATA) && \ + defined(MBEDTLS_SSL_ALPN) +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_session_set_alpn(mbedtls_ssl_session *session, + const char *alpn); +#endif + #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) #define MBEDTLS_SSL_TLS1_3_MAX_ALLOWED_TICKET_LIFETIME (604800) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d7d26ab063..f78b97d444 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2450,6 +2450,7 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite( #if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) + psa_status_t mbedtls_ssl_cipher_to_psa(mbedtls_cipher_type_t mbedtls_cipher_type, size_t taglen, psa_algorithm_t *alg, @@ -3771,8 +3772,8 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SRV_C) && \ defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - const uint8_t alpn_len = (session->alpn == NULL) ? - 0 : (uint8_t) strlen(session->alpn) + 1; + const uint8_t alpn_len = (session->ticket_alpn == NULL) ? + 0 : (uint8_t) strlen(session->ticket_alpn) + 1; #endif size_t needed = 4 /* ticket_age_add */ + 1 /* ticket_flags */ @@ -3858,7 +3859,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, *p++ = alpn_len; if (alpn_len > 0) { /* save chosen alpn */ - memcpy(p, session->alpn, alpn_len); + memcpy(p, session->ticket_alpn, alpn_len); p += alpn_len; } #endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ @@ -3951,6 +3952,7 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) uint8_t alpn_len; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; if (end - p < 1) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; @@ -3960,12 +3962,12 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, if (end - p < alpn_len) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + if (alpn_len > 0) { - session->alpn = mbedtls_calloc(alpn_len, sizeof(char)); - if (session->alpn == NULL) { - return MBEDTLS_ERR_SSL_ALLOC_FAILED; + ret = mbedtls_ssl_session_set_alpn(session, (const char *) p); + if (ret != 0) { + return ret; } - memcpy(session->alpn, p, alpn_len); p += alpn_len; } #endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ @@ -4917,11 +4919,12 @@ void mbedtls_ssl_session_free(mbedtls_ssl_session *session) defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) mbedtls_free(session->hostname); #endif + mbedtls_free(session->ticket); +#endif + #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) && \ defined(MBEDTLS_SSL_SRV_C) - mbedtls_free(session->alpn); -#endif - mbedtls_free(session->ticket); + mbedtls_free(session->ticket_alpn); #endif mbedtls_platform_zeroize(session, sizeof(mbedtls_ssl_session)); @@ -9870,4 +9873,37 @@ int mbedtls_ssl_session_set_hostname(mbedtls_ssl_session *session, MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_EARLY_DATA) && \ + defined(MBEDTLS_SSL_ALPN) +int mbedtls_ssl_session_set_alpn(mbedtls_ssl_session *session, + const char *alpn) +{ + size_t alpn_len = 0; + + if (alpn != NULL) { + alpn_len = strlen(alpn); + + if (alpn_len > MBEDTLS_SSL_MAX_ALPN_NAME_LEN) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + } + + if (session->ticket_alpn != NULL) { + mbedtls_zeroize_and_free(session->ticket_alpn, + strlen(session->ticket_alpn)); + } + + if (alpn == NULL) { + session->ticket_alpn = NULL; + } else { + session->ticket_alpn = mbedtls_calloc(strlen(alpn) + 1, sizeof(char)); + if (session->ticket_alpn == NULL) { + return MBEDTLS_ERR_SSL_ALLOC_FAILED; + } + memcpy(session->ticket_alpn, alpn, strlen(alpn) + 1); + } + + return 0; +} +#endif /* MBEDTLS_SSL_SRV_C && MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 291d64500d..9c73c7a1a5 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -469,12 +469,10 @@ static int ssl_tls13_session_copy_ticket(mbedtls_ssl_session *dst, dst->max_early_data_size = src->max_early_data_size; #if defined(MBEDTLS_SSL_ALPN) - if (src->alpn != NULL) { - dst->alpn = mbedtls_calloc(strlen(src->alpn) + 1, sizeof(char)); - if (dst->alpn == NULL) { - return MBEDTLS_ERR_SSL_ALLOC_FAILED; - } - memcpy(dst->alpn, src->alpn, strlen(src->alpn) + 1); + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + ret = mbedtls_ssl_session_set_alpn(dst, src->ticket_alpn); + if (ret != 0) { + return ret; } #endif /* MBEDTLS_SSL_ALPN */ #endif /* MBEDTLS_SSL_EARLY_DATA*/ @@ -3148,12 +3146,9 @@ static int ssl_tls13_prepare_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_PRINT_TICKET_FLAGS(4, session->ticket_flags); #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - if (ssl->alpn_chosen != NULL) { - session->alpn = mbedtls_calloc(strlen(ssl->alpn_chosen) + 1, sizeof(char)); - if (session->alpn == NULL) { - return MBEDTLS_ERR_SSL_ALLOC_FAILED; - } - memcpy(session->alpn, ssl->alpn_chosen, strlen(ssl->alpn_chosen) + 1); + ret = mbedtls_ssl_session_set_alpn(session, ssl->alpn_chosen); + if (ret != 0) { + return ret; } #endif diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 89c1bbf522..9c1676fc63 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1794,11 +1794,11 @@ int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_EARLY_DATA) session->max_early_data_size = 0x87654321; #if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) - session->alpn = mbedtls_calloc(strlen("ALPNExample")+1, sizeof(char)); - if (session->alpn == NULL) { + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + ret = mbedtls_ssl_session_set_alpn(session, "ALPNExample"); + if (ret != 0) { return -1; } - strcpy(session->alpn, "ALPNExample"); #endif /* MBEDTLS_SSL_ALPN && MBEDTLS_SSL_SRV_C */ #endif /* MBEDTLS_SSL_EARLY_DATA */ diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index da07f2c62f..e29667d0bc 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2106,11 +2106,10 @@ void ssl_serialize_session_save_load(int ticket_len, char *crt_file, original.max_early_data_size == restored.max_early_data_size); #if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) if (endpoint_type == MBEDTLS_SSL_IS_SERVER) { - TEST_ASSERT(original.alpn != NULL); - TEST_ASSERT(restored.alpn != NULL); - TEST_ASSERT(memcmp(original.alpn, - restored.alpn, - strlen(original.alpn)) == 0); + TEST_ASSERT(original.ticket_alpn != NULL); + TEST_ASSERT(restored.ticket_alpn != NULL); + TEST_MEMORY_COMPARE(original.ticket_alpn, strlen(original.ticket_alpn), + restored.ticket_alpn, strlen(restored.ticket_alpn)); } #endif #endif From 11025636859e4cacf0259c0cd32c7191b74bf6b0 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Thu, 7 Mar 2024 15:25:52 +0000 Subject: [PATCH 3/9] Add ALPN bit flag to session header Signed-off-by: Waleed Elmelegy --- library/ssl_tls.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index f78b97d444..e6023c03b8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3736,6 +3736,7 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) /* Serialization of TLS 1.3 sessions: * + * For more detail, see the description of ssl_session_save(). * struct { * opaque hostname<0..2^16-1>; * uint64 ticket_reception_time; @@ -4130,6 +4131,12 @@ static int ssl_tls13_session_load(const mbedtls_ssl_session *session, #define SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE 0 #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */ +#if defined(MBEDTLS_SSL_ALPN) +#define SSL_SERIALIZED_SESSION_CONFIG_ALPN 1 +#else +#define SSL_SERIALIZED_SESSION_CONFIG_ALPN 0 +#endif /* MBEDTLS_SSL_ALPN */ + #define SSL_SERIALIZED_SESSION_CONFIG_TIME_BIT 0 #define SSL_SERIALIZED_SESSION_CONFIG_CRT_BIT 1 #define SSL_SERIALIZED_SESSION_CONFIG_CLIENT_TICKET_BIT 2 @@ -4140,6 +4147,7 @@ static int ssl_tls13_session_load(const mbedtls_ssl_session *session, #define SSL_SERIALIZED_SESSION_CONFIG_SNI_BIT 7 #define SSL_SERIALIZED_SESSION_CONFIG_EARLY_DATA_BIT 8 #define SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE_BIT 9 +#define SSL_SERIALIZED_SESSION_CONFIG_ALPN_BIT 10 #define SSL_SERIALIZED_SESSION_CONFIG_BITFLAG \ ((uint16_t) ( \ @@ -4156,7 +4164,9 @@ static int ssl_tls13_session_load(const mbedtls_ssl_session *session, (SSL_SERIALIZED_SESSION_CONFIG_EARLY_DATA << \ SSL_SERIALIZED_SESSION_CONFIG_EARLY_DATA_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE << \ - SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE_BIT))) + SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE_BIT) | \ + (SSL_SERIALIZED_SESSION_CONFIG_ALPN << \ + SSL_SERIALIZED_SESSION_CONFIG_ALPN_BIT))) static const unsigned char ssl_serialized_session_header[] = { MBEDTLS_VERSION_MAJOR, From fe9ae085e384e95f0ee85cf8d6a6355ed43f73a6 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Thu, 7 Mar 2024 15:47:31 +0000 Subject: [PATCH 4/9] Update serialized session description with ALPN information Signed-off-by: Waleed Elmelegy --- library/ssl_tls.c | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e6023c03b8..ae42cf350c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3737,25 +3737,6 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, /* Serialization of TLS 1.3 sessions: * * For more detail, see the description of ssl_session_save(). - * struct { - * opaque hostname<0..2^16-1>; - * uint64 ticket_reception_time; - * uint32 ticket_lifetime; - * opaque ticket<1..2^16-1>; - * } ClientOnlyData; - * - * struct { - * uint32 ticket_age_add; - * uint8 ticket_flags; - * opaque resumption_key<0..255>; - * uint32 max_early_data_size; - * uint16 record_size_limit; - * select ( endpoint ) { - * case client: ClientOnlyData; - * case server: uint64 ticket_creation_time; - * }; - * } serialized_session_tls13; - * */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) MBEDTLS_CHECK_RETURN_CRITICAL @@ -4244,6 +4225,10 @@ static const unsigned char ssl_serialized_session_header[] = { * #if defined(MBEDTLS_HAVE_TIME) * case server: uint64 ticket_creation_time; * #endif + * #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) + * uint8 alpn_len; + * opaque ticket_alpn<0..255>; + * #endif * }; * } serialized_session_tls13; * From 75e33fa12e24ee4e930b45c5b8d69c35aa152e39 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Thu, 7 Mar 2024 16:57:58 +0000 Subject: [PATCH 5/9] Fix code style in ssl_tls.c Signed-off-by: Waleed Elmelegy --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ae42cf350c..de0057df90 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4146,7 +4146,7 @@ static int ssl_tls13_session_load(const mbedtls_ssl_session *session, SSL_SERIALIZED_SESSION_CONFIG_EARLY_DATA_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE << \ SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE_BIT) | \ - (SSL_SERIALIZED_SESSION_CONFIG_ALPN << \ + (SSL_SERIALIZED_SESSION_CONFIG_ALPN << \ SSL_SERIALIZED_SESSION_CONFIG_ALPN_BIT))) static const unsigned char ssl_serialized_session_header[] = { From 7dfba344752ba39b5c539d80964ccb0403133146 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 12 Mar 2024 16:22:39 +0000 Subject: [PATCH 6/9] Fix possible overflow in ALPN length when saving session Signed-off-by: Waleed Elmelegy --- library/ssl_tls.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index de0057df90..3ee2538b65 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2450,7 +2450,6 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite( #if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) - psa_status_t mbedtls_ssl_cipher_to_psa(mbedtls_cipher_type_t mbedtls_cipher_type, size_t taglen, psa_algorithm_t *alg, @@ -3755,7 +3754,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SRV_C) && \ defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) const uint8_t alpn_len = (session->ticket_alpn == NULL) ? - 0 : (uint8_t) strlen(session->ticket_alpn) + 1; + 0 : (uint8_t) strlen(session->ticket_alpn); #endif size_t needed = 4 /* ticket_age_add */ + 1 /* ticket_flags */ @@ -3934,7 +3933,6 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) uint8_t alpn_len; - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; if (end - p < 1) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; @@ -3945,11 +3943,20 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + if (alpn_len > MBEDTLS_SSL_MAX_ALPN_NAME_LEN) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + if (alpn_len > 0) { - ret = mbedtls_ssl_session_set_alpn(session, (const char *) p); - if (ret != 0) { - return ret; + if (session->ticket_alpn != NULL) { + mbedtls_zeroize_and_free(session->ticket_alpn, + strlen(session->ticket_alpn)); } + session->ticket_alpn = mbedtls_calloc(alpn_len + 1, sizeof(char)); + if (session->ticket_alpn == NULL) { + return MBEDTLS_ERR_SSL_ALLOC_FAILED; + } + memcpy(session->ticket_alpn, p, alpn_len); p += alpn_len; } #endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ @@ -4112,7 +4119,8 @@ static int ssl_tls13_session_load(const mbedtls_ssl_session *session, #define SSL_SERIALIZED_SESSION_CONFIG_RECORD_SIZE 0 #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */ -#if defined(MBEDTLS_SSL_ALPN) +#if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) && \ + defined(MBEDTLS_SSL_EARLY_DATA) #define SSL_SERIALIZED_SESSION_CONFIG_ALPN 1 #else #define SSL_SERIALIZED_SESSION_CONFIG_ALPN 0 @@ -4222,11 +4230,11 @@ static const unsigned char ssl_serialized_session_header[] = { * #endif * select ( endpoint ) { * case client: ClientOnlyData; + * case server: * #if defined(MBEDTLS_HAVE_TIME) - * case server: uint64 ticket_creation_time; + * uint64 ticket_creation_time; * #endif * #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - * uint8 alpn_len; * opaque ticket_alpn<0..255>; * #endif * }; @@ -9870,8 +9878,8 @@ int mbedtls_ssl_session_set_hostname(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_EARLY_DATA) && \ defined(MBEDTLS_SSL_ALPN) -int mbedtls_ssl_session_set_alpn(mbedtls_ssl_session *session, - const char *alpn) +int mbedtls_ssl_session_set_ticket_alpn(mbedtls_ssl_session *session, + const char *alpn) { size_t alpn_len = 0; @@ -9886,16 +9894,15 @@ int mbedtls_ssl_session_set_alpn(mbedtls_ssl_session *session, if (session->ticket_alpn != NULL) { mbedtls_zeroize_and_free(session->ticket_alpn, strlen(session->ticket_alpn)); + session->ticket_alpn = NULL; } - if (alpn == NULL) { - session->ticket_alpn = NULL; - } else { - session->ticket_alpn = mbedtls_calloc(strlen(alpn) + 1, sizeof(char)); + if (alpn != NULL) { + session->ticket_alpn = mbedtls_calloc(alpn_len + 1, 1); if (session->ticket_alpn == NULL) { return MBEDTLS_ERR_SSL_ALLOC_FAILED; } - memcpy(session->ticket_alpn, alpn, strlen(alpn) + 1); + memcpy(session->ticket_alpn, alpn, alpn_len); } return 0; From 5bc5263b2c501eadd9ae820a985a9c1b575a3fd4 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 12 Mar 2024 16:25:08 +0000 Subject: [PATCH 7/9] Add code improvments and refactoring in dealing with ALPN Signed-off-by: Waleed Elmelegy --- include/mbedtls/ssl.h | 3 ++- library/ssl_misc.h | 4 ++-- library/ssl_tls13_server.c | 11 ++++++----- tests/src/test_helpers/ssl_helpers.c | 3 +-- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b05bfe1b72..57d7bc67eb 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1305,7 +1305,8 @@ struct mbedtls_ssl_session { #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_CLI_C */ #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) - char *ticket_alpn; /*!< ALPN negotiated in the session */ + char *ticket_alpn; /*!< ALPN negotiated in the session + during which the ticket was generated. */ #endif #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 948c802299..a8807f67c6 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2855,8 +2855,8 @@ int mbedtls_ssl_session_set_hostname(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_EARLY_DATA) && \ defined(MBEDTLS_SSL_ALPN) MBEDTLS_CHECK_RETURN_CRITICAL -int mbedtls_ssl_session_set_alpn(mbedtls_ssl_session *session, - const char *alpn); +int mbedtls_ssl_session_set_ticket_alpn(mbedtls_ssl_session *session, + const char *alpn); #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 9c73c7a1a5..9453c69392 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -469,8 +469,7 @@ static int ssl_tls13_session_copy_ticket(mbedtls_ssl_session *dst, dst->max_early_data_size = src->max_early_data_size; #if defined(MBEDTLS_SSL_ALPN) - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - ret = mbedtls_ssl_session_set_alpn(dst, src->ticket_alpn); + int ret = mbedtls_ssl_session_set_ticket_alpn(dst, src->ticket_alpn); if (ret != 0) { return ret; } @@ -3146,9 +3145,11 @@ static int ssl_tls13_prepare_new_session_ticket(mbedtls_ssl_context *ssl, MBEDTLS_SSL_PRINT_TICKET_FLAGS(4, session->ticket_flags); #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - ret = mbedtls_ssl_session_set_alpn(session, ssl->alpn_chosen); - if (ret != 0) { - return ret; + if (session->ticket_alpn == NULL) { + ret = mbedtls_ssl_session_set_ticket_alpn(session, ssl->alpn_chosen); + if (ret != 0) { + return ret; + } } #endif diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 9c1676fc63..963938f1f8 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -1794,8 +1794,7 @@ int mbedtls_test_ssl_tls13_populate_session(mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_EARLY_DATA) session->max_early_data_size = 0x87654321; #if defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - ret = mbedtls_ssl_session_set_alpn(session, "ALPNExample"); + int ret = mbedtls_ssl_session_set_ticket_alpn(session, "ALPNExample"); if (ret != 0) { return -1; } From daa4da781ad32d1c44e72f5299504b21ecd37974 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 13 Mar 2024 16:25:34 +0000 Subject: [PATCH 8/9] Increase ALPN length in saved session to 2 bytes Signed-off-by: Waleed Elmelegy --- library/ssl_tls.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3ee2538b65..1e257e2099 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3753,8 +3753,8 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SRV_C) && \ defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - const uint8_t alpn_len = (session->ticket_alpn == NULL) ? - 0 : (uint8_t) strlen(session->ticket_alpn); + const size_t alpn_len = (session->ticket_alpn == NULL) ? + 0 : strlen(session->ticket_alpn) + 1; #endif size_t needed = 4 /* ticket_age_add */ + 1 /* ticket_flags */ @@ -3781,7 +3781,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SRV_C) if (session->endpoint == MBEDTLS_SSL_IS_SERVER) { #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - needed += 1 /* alpn_len */ + needed += 2 /* alpn_len */ + alpn_len; /* alpn */ #endif } @@ -3837,7 +3837,9 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #endif /* MBEDTLS_HAVE_TIME */ #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - *p++ = alpn_len; + MBEDTLS_PUT_UINT16_BE(alpn_len, p, 0); + p += 2; + if (alpn_len > 0) { /* save chosen alpn */ memcpy(p, session->ticket_alpn, alpn_len); @@ -3932,31 +3934,24 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, #endif /* MBEDTLS_HAVE_TIME */ #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - uint8_t alpn_len; + size_t alpn_len; - if (end - p < 1) { - return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; - } - alpn_len = *p++; - - if (end - p < alpn_len) { + if (end - p < 2) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - if (alpn_len > MBEDTLS_SSL_MAX_ALPN_NAME_LEN) { + alpn_len = MBEDTLS_GET_UINT16_BE(p, 0); + p += 2; + + if (end - p < (long int) alpn_len) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } if (alpn_len > 0) { - if (session->ticket_alpn != NULL) { - mbedtls_zeroize_and_free(session->ticket_alpn, - strlen(session->ticket_alpn)); + int ret = mbedtls_ssl_session_set_ticket_alpn(session, (char *) p); + if (ret != 0) { + return ret; } - session->ticket_alpn = mbedtls_calloc(alpn_len + 1, sizeof(char)); - if (session->ticket_alpn == NULL) { - return MBEDTLS_ERR_SSL_ALLOC_FAILED; - } - memcpy(session->ticket_alpn, p, alpn_len); p += alpn_len; } #endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ @@ -4235,7 +4230,7 @@ static const unsigned char ssl_serialized_session_header[] = { * uint64 ticket_creation_time; * #endif * #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) - * opaque ticket_alpn<0..255>; + * opaque ticket_alpn<0..256>; * #endif * }; * } serialized_session_tls13; From b28ab0a45a40e80f974fe888c8aec21f222fbd9a Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 13 Mar 2024 16:48:28 +0000 Subject: [PATCH 9/9] Fix code style in ssl_tls.c Signed-off-by: Waleed Elmelegy --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1e257e2099..ac8e0ea437 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3754,7 +3754,7 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SRV_C) && \ defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) const size_t alpn_len = (session->ticket_alpn == NULL) ? - 0 : strlen(session->ticket_alpn) + 1; + 0 : strlen(session->ticket_alpn) + 1; #endif size_t needed = 4 /* ticket_age_add */ + 1 /* ticket_flags */