From a500ef98f5fb433beecfd1e093ee4e568f0cfe79 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 6 Mar 2024 15:11:44 +0000 Subject: [PATCH] Add ALPN checking when accepting early data Signed-off-by: Waleed Elmelegy --- library/ssl_tls.c | 14 +++++++-- library/ssl_tls13_server.c | 16 ++++++++++- tests/include/test/ssl_helpers.h | 3 ++ tests/src/test_helpers/ssl_helpers.c | 6 ++++ tests/suites/test_suite_ssl.data | 12 ++++++++ tests/suites/test_suite_ssl.function | 43 ++++++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e454d2e240e7..03a15c756a28 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -275,6 +275,17 @@ int mbedtls_ssl_session_copy(mbedtls_ssl_session *dst, #endif /* MBEDTLS_X509_CRT_PARSE_C */ +#if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_ALPN) && \ + defined(MBEDTLS_SSL_EARLY_DATA) + { + 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_SRV_C && MBEDTLS_SSL_ALPN && MBEDTLS_SSL_EARLY_DATA */ + #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) if (src->ticket != NULL) { dst->ticket = mbedtls_calloc(1, src->ticket_len); @@ -2444,7 +2455,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, @@ -5414,7 +5424,7 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, /* alpn_chosen should point to an item in the configured list */ for (cur = ssl->conf->alpn_list; *cur != NULL; cur++) { if (strlen(*cur) == alpn_len && - memcmp(p, cur, alpn_len) == 0) { + memcmp(p, *cur, alpn_len) == 0) { ssl->alpn_chosen = *cur; break; } diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 33a341c1afd9..06d7cd64b452 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1823,7 +1823,6 @@ static int ssl_tls13_check_early_data_requirements(mbedtls_ssl_context *ssl) * NOTE: * - The TLS version number is checked in * ssl_tls13_offered_psks_check_identity_match_ticket(). - * - ALPN is not checked for the time being (TODO). */ if (handshake->selected_identity != 0) { @@ -1850,6 +1849,21 @@ static int ssl_tls13_check_early_data_requirements(mbedtls_ssl_context *ssl) return -1; } +#if defined(MBEDTLS_SSL_ALPN) + const char *alpn = mbedtls_ssl_get_alpn_protocol(ssl); + + if (alpn == NULL && ssl->session_negotiate->ticket_alpn == NULL) { + return 0; + } + + if (alpn == NULL || ssl->session_negotiate->ticket_alpn == NULL + || memcmp(alpn, ssl->session_negotiate->ticket_alpn, strlen(alpn)) != 0) { + MBEDTLS_SSL_DEBUG_MSG(1, ("EarlyData: rejected, the selected ALPN is different " + "from the one associated with the pre-shared key.")); + return -1; + } +#endif + return 0; } #endif /* MBEDTLS_SSL_EARLY_DATA */ diff --git a/tests/include/test/ssl_helpers.h b/tests/include/test/ssl_helpers.h index 5b071f75aa1c..ee3fe4c85718 100644 --- a/tests/include/test/ssl_helpers.h +++ b/tests/include/test/ssl_helpers.h @@ -117,6 +117,9 @@ typedef struct mbedtls_test_handshake_test_options { #if defined(MBEDTLS_SSL_CACHE_C) mbedtls_ssl_cache_context *cache; #endif +#if defined(MBEDTLS_SSL_ALPN) + const char *alpn_list[MBEDTLS_SSL_MAX_ALPN_LIST_LEN/MBEDTLS_SSL_MAX_ALPN_NAME_LEN]; +#endif } mbedtls_test_handshake_test_options; /* diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index d45fc38d5b33..31112ca917d0 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -826,6 +826,12 @@ int mbedtls_test_ssl_endpoint_init( #if defined(MBEDTLS_SSL_EARLY_DATA) mbedtls_ssl_conf_early_data(&(ep->conf), options->early_data); +#if defined(MBEDTLS_SSL_ALPN) + /* check that alpn_list contains at least one valid entry */ + if (options->alpn_list[0] != NULL) { + mbedtls_ssl_conf_alpn_protocols(&(ep->conf), options->alpn_list); + } +#endif #endif #if defined(MBEDTLS_SSL_CACHE_C) && defined(MBEDTLS_SSL_SRV_C) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 385682ae12f3..c5ce4a5ddccb 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3286,6 +3286,18 @@ tls13_read_early_data:TEST_EARLY_DATA_SERVER_REJECTS TLS 1.3 read early data, discard after HRR tls13_read_early_data:TEST_EARLY_DATA_HRR +TLS 1.3 cli, early data, same ALPN +depends_on:MBEDTLS_SSL_ALPN +tls13_read_early_data:TEST_EARLY_DATA_SAME_ALPN + +TLS 1.3 cli, early data, different ALPN +depends_on:MBEDTLS_SSL_ALPN +tls13_read_early_data:TEST_EARLY_DATA_DIFF_ALPN + +TLS 1.3 cli, early data, no initial ALPN +depends_on:MBEDTLS_SSL_ALPN +tls13_read_early_data:TEST_EARLY_DATA_NO_INITIAL_ALPN + TLS 1.3 cli, early data status, early data accepted tls13_cli_early_data_status:TEST_EARLY_DATA_ACCEPTED diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 042a01abbe6e..a03e1572dfec 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -17,6 +17,11 @@ #define TEST_EARLY_DATA_NO_INDICATION_SENT 1 #define TEST_EARLY_DATA_SERVER_REJECTS 2 #define TEST_EARLY_DATA_HRR 3 +#if defined(MBEDTLS_SSL_ALPN) +#define TEST_EARLY_DATA_SAME_ALPN 4 +#define TEST_EARLY_DATA_DIFF_ALPN 5 +#define TEST_EARLY_DATA_NO_INITIAL_ALPN 6 +#endif /* END_HEADER */ @@ -3630,6 +3635,18 @@ void tls13_read_early_data(int scenario) server_options.group_list = group_list; server_options.early_data = MBEDTLS_SSL_EARLY_DATA_ENABLED; +#if defined(MBEDTLS_SSL_ALPN) + switch (scenario) { + case TEST_EARLY_DATA_SAME_ALPN: + case TEST_EARLY_DATA_DIFF_ALPN: + client_options.alpn_list[0] = "ALPNExample"; + client_options.alpn_list[1] = NULL; + server_options.alpn_list[0] = "ALPNExample"; + server_options.alpn_list[1] = NULL; + break; + } +#endif + ret = mbedtls_test_get_tls13_ticket(&client_options, &server_options, &saved_session); TEST_EQUAL(ret, 0); @@ -3658,6 +3675,25 @@ void tls13_read_early_data(int scenario) "EarlyData: Ignore application message before 2nd ClientHello"; server_options.group_list = group_list + 1; break; +#if defined(MBEDTLS_SSL_ALPN) + case TEST_EARLY_DATA_SAME_ALPN: + client_options.alpn_list[0] = "ALPNExample"; + client_options.alpn_list[1] = NULL; + server_options.alpn_list[0] = "ALPNExample"; + server_options.alpn_list[1] = NULL; + break; + case TEST_EARLY_DATA_DIFF_ALPN: + case TEST_EARLY_DATA_NO_INITIAL_ALPN: + client_options.alpn_list[0] = "ALPNExample2"; + client_options.alpn_list[1] = NULL; + server_options.alpn_list[0] = "ALPNExample2"; + server_options.alpn_list[1] = NULL; + mbedtls_debug_set_threshold(3); + server_pattern.pattern = + "EarlyData: rejected, the selected ALPN is different " + "from the one associated with the pre-shared key."; + break; +#endif default: TEST_FAIL("Unknown scenario."); @@ -3709,6 +3745,9 @@ void tls13_read_early_data(int scenario) switch (scenario) { case TEST_EARLY_DATA_ACCEPTED: +#if defined(MBEDTLS_SSL_ALPN) + case TEST_EARLY_DATA_SAME_ALPN: +#endif TEST_EQUAL(ret, MBEDTLS_ERR_SSL_RECEIVED_EARLY_DATA); TEST_EQUAL(server_ep.ssl.handshake->early_data_accepted, 1); TEST_EQUAL(mbedtls_ssl_read_early_data(&(server_ep.ssl), @@ -3723,6 +3762,10 @@ void tls13_read_early_data(int scenario) case TEST_EARLY_DATA_SERVER_REJECTS: /* Intentional fallthrough */ case TEST_EARLY_DATA_HRR: +#if defined(MBEDTLS_SSL_ALPN) + case TEST_EARLY_DATA_DIFF_ALPN: + case TEST_EARLY_DATA_NO_INITIAL_ALPN: +#endif TEST_EQUAL(ret, 0); TEST_EQUAL(server_ep.ssl.handshake->early_data_accepted, 0); TEST_EQUAL(server_pattern.counter, 1);