From 064dd2b87091704227e64e975beba56e3ee69bb3 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 8 Dec 2023 14:58:08 +0800 Subject: [PATCH 01/11] Adjust check order Signed-off-by: Jerry Yu --- tests/opt-testcases/tls13-misc.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/opt-testcases/tls13-misc.sh b/tests/opt-testcases/tls13-misc.sh index b6894de81..4e6bf876d 100755 --- a/tests/opt-testcases/tls13-misc.sh +++ b/tests/opt-testcases/tls13-misc.sh @@ -502,8 +502,8 @@ run_test "TLS 1.3 G->m: EarlyData: feature is enabled, good." \ "$G_NEXT_CLI localhost --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+GROUP-ALL:+KX-ALL \ -d 10 -r --earlydata $EARLY_DATA_INPUT " \ 0 \ - -s "NewSessionTicket: early_data(42) extension exists." \ -s "Sent max_early_data_size=$EARLY_DATA_INPUT_LEN" \ + -s "NewSessionTicket: early_data(42) extension exists." \ -s "ClientHello: early_data(42) extension exists." \ -s "EncryptedExtensions: early_data(42) extension exists." \ -s "$( head -1 $EARLY_DATA_INPUT )" \ From 4caf3ca08cb3b1b206477ce07a7a09fa14b7da4c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 15 Nov 2023 16:13:47 +0800 Subject: [PATCH 02/11] tls13: srv: Add discard_early_data_record SSL field Add discard_early_data_record in SSL context for the record layer to know if it has to discard some potential early data record and how. Signed-off-by: Ronald Cron --- include/mbedtls/ssl.h | 30 +++++++++++++++++++++ library/ssl_tls.c | 7 ++++- library/ssl_tls13_server.c | 54 ++++++++++++++++++-------------------- 3 files changed, 61 insertions(+), 30 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b0633609d..e0cd79d02 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -353,6 +353,26 @@ #define MBEDTLS_SSL_DTLS_TIMEOUT_DFL_MIN 1000 #define MBEDTLS_SSL_DTLS_TIMEOUT_DFL_MAX 60000 +/* + * Whether early data record should be discarded or not and how. + * + * The client has indicated early data and the server has rejected them. + * The server has then to skip past early data by either: + * - attempting to deprotect received records using the handshake traffic + * key, discarding records which fail deprotection (up to the configured + * max_early_data_size). Once a record is deprotected successfully, + * it is treated as the start of the client's second flight and the + * server proceeds as with an ordinary 1-RTT handshake. + * - skipping all records with an external content type of + * "application_data" (indicating that they are encrypted), up to the + * configured max_early_data_size. This is the expected behavior if the + * server has sent an HelloRetryRequest message. The server ignores + * application data message before 2nd ClientHello. + */ +#define MBEDTLS_SSL_EARLY_DATA_NO_DISCARD 0 +#define MBEDTLS_SSL_EARLY_DATA_TRY_TO_DEPROTECT_AND_DISCARD 1 +#define MBEDTLS_SSL_EARLY_DATA_DISCARD 2 + /** * \name SECTION: Module settings * @@ -1782,6 +1802,16 @@ struct mbedtls_ssl_context { * within a single datagram. */ #endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_SRV_C) + /* + * One of: + * MBEDTLS_SSL_EARLY_DATA_NO_DISCARD + * MBEDTLS_SSL_EARLY_DATA_TRY_TO_DEPROTECT_AND_DISCARD + * MBEDTLS_SSL_EARLY_DATA_DISCARD + */ + uint8_t MBEDTLS_PRIVATE(discard_early_data_record); +#endif + /* * Record layer (outgoing data) */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c952add9b..c2f874b45 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1098,9 +1098,14 @@ static int ssl_handshake_init(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_ALLOC_FAILED; } -#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_CLI_C) +#if defined(MBEDTLS_SSL_EARLY_DATA) +#if defined(MBEDTLS_SSL_CLI_C) ssl->early_data_status = MBEDTLS_SSL_EARLY_DATA_STATUS_NOT_SENT; #endif +#if defined(MBEDTLS_SSL_SRV_C) + ssl->discard_early_data_record = MBEDTLS_SSL_EARLY_DATA_NO_DISCARD; +#endif +#endif /* MBEDTLS_SSL_EARLY_DATA */ /* Initialize structures */ mbedtls_ssl_session_init(ssl->session_negotiate); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8bd70ef02..6e87d7b9d 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1780,28 +1780,15 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_EARLY_DATA) -static int ssl_tls13_is_early_data_accepted(mbedtls_ssl_context *ssl, - int hrr_required) +static int ssl_tls13_check_early_data_requirements(mbedtls_ssl_context *ssl) { mbedtls_ssl_handshake_params *handshake = ssl->handshake; - if ((handshake->received_extensions & - MBEDTLS_SSL_EXT_MASK(EARLY_DATA)) == 0) { - MBEDTLS_SSL_DEBUG_MSG( - 1, ("EarlyData: no early data extension received.")); - return 0; - } - if (ssl->conf->early_data_enabled == MBEDTLS_SSL_EARLY_DATA_DISABLED) { MBEDTLS_SSL_DEBUG_MSG( 1, ("EarlyData: rejected, feature disabled in server configuration.")); - return 0; - } - - if (hrr_required) { - MBEDTLS_SSL_DEBUG_MSG(1, ("EarlyData: rejected, HRR required.")); - return 0; + return -1; } if (!handshake->resume) { @@ -1810,7 +1797,7 @@ static int ssl_tls13_is_early_data_accepted(mbedtls_ssl_context *ssl, resumption. */ MBEDTLS_SSL_DEBUG_MSG( 1, ("EarlyData: rejected, not a session resumption.")); - return 0; + return -1; } /* RFC 8446 4.2.10 @@ -1833,7 +1820,7 @@ static int ssl_tls13_is_early_data_accepted(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ("EarlyData: rejected, the selected key in " "`pre_shared_key` is not the first one.")); - return 0; + return -1; } if (handshake->ciphersuite_info->id != @@ -1841,7 +1828,7 @@ static int ssl_tls13_is_early_data_accepted(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ("EarlyData: rejected, the selected ciphersuite is not the one " "of the selected pre-shared key.")); - return 0; + return -1; } @@ -1850,10 +1837,10 @@ static int ssl_tls13_is_early_data_accepted(mbedtls_ssl_context *ssl, 1, ("EarlyData: rejected, early_data not allowed in ticket " "permission bits.")); - return 0; + return -1; } - return 1; + return 0; } #endif /* MBEDTLS_SSL_EARLY_DATA */ @@ -1885,15 +1872,24 @@ static int ssl_tls13_postprocess_client_hello(mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_EARLY_DATA) - ssl->handshake->early_data_accepted = - ssl_tls13_is_early_data_accepted(ssl, hrr_required); - - if (ssl->handshake->early_data_accepted) { - ret = mbedtls_ssl_tls13_compute_early_transform(ssl); - if (ret != 0) { - MBEDTLS_SSL_DEBUG_RET( - 1, "mbedtls_ssl_tls13_compute_early_transform", ret); - return ret; + if (ssl->handshake->received_extensions & MBEDTLS_SSL_EXT_MASK(EARLY_DATA)) { + ssl->handshake->early_data_accepted = 0; + if (!hrr_required) { + ssl->handshake->early_data_accepted = + (ssl_tls13_check_early_data_requirements(ssl) == 0); + } + if (ssl->handshake->early_data_accepted) { + ret = mbedtls_ssl_tls13_compute_early_transform(ssl); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET( + 1, "mbedtls_ssl_tls13_compute_early_transform", ret); + return ret; + } + } else { + ssl->discard_early_data_record = + hrr_required ? + MBEDTLS_SSL_EARLY_DATA_DISCARD : + MBEDTLS_SSL_EARLY_DATA_TRY_TO_DEPROTECT_AND_DISCARD; } } #else From 2995d35ac344376acc5e18d3f39e1b6afc6917cb Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 18 Jan 2024 16:59:39 +0100 Subject: [PATCH 03/11] tls13: srv: Deprotect and discard early data records Signed-off-by: Ronald Cron --- library/ssl_msg.c | 39 ++++++++++++++++++ tests/suites/test_suite_ssl.data | 7 +++- tests/suites/test_suite_ssl.function | 61 ++++++++++++++++++++++------ 3 files changed, 93 insertions(+), 14 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 20501c940..bf9a8ca7c 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3985,6 +3985,31 @@ static int ssl_prepare_record_content(mbedtls_ssl_context *ssl, rec)) != 0) { MBEDTLS_SSL_DEBUG_RET(1, "ssl_decrypt_buf", ret); +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_SRV_C) + /* + * Although the server rejected early data, it might receive early + * data as long as it has not received the client Finished message. + * It is encrypted with early keys and should be ignored as stated + * in section 4.2.10 of RFC 8446: + * + * "Ignore the extension and return a regular 1-RTT response. The + * server then skips past early data by attempting to deprotect + * received records using the handshake traffic key, discarding + * records which fail deprotection (up to the configured + * max_early_data_size). Once a record is deprotected successfully, + * it is treated as the start of the client's second flight and the + * server proceeds as with an ordinary 1-RTT handshake." + */ + if ((old_msg_type == MBEDTLS_SSL_MSG_APPLICATION_DATA) && + (ssl->discard_early_data_record == + MBEDTLS_SSL_EARLY_DATA_TRY_TO_DEPROTECT_AND_DISCARD)) { + MBEDTLS_SSL_DEBUG_MSG( + 3, ("EarlyData: deprotect and discard app data records.")); + /* TODO: Add max_early_data_size check here. */ + ret = MBEDTLS_ERR_SSL_CONTINUE_PROCESSING; + } +#endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_SRV_C */ + #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) if (ret == MBEDTLS_ERR_SSL_UNEXPECTED_CID && ssl->conf->ignore_unexpected_cid @@ -3997,6 +4022,20 @@ static int ssl_prepare_record_content(mbedtls_ssl_context *ssl, return ret; } +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_SRV_C) + /* + * If the server were discarding protected records that it fails to + * deprotect because it has rejected early data, as we have just + * deprotected successfully a record, the server has to resume normal + * operation and fail the connection if the deprotection of a record + * fails. + */ + if (ssl->discard_early_data_record == + MBEDTLS_SSL_EARLY_DATA_TRY_TO_DEPROTECT_AND_DISCARD) { + ssl->discard_early_data_record = MBEDTLS_SSL_EARLY_DATA_NO_DISCARD; + } +#endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_SRV_C */ + if (old_msg_type != rec->type) { MBEDTLS_SSL_DEBUG_MSG(4, ("record type after decrypt (before %d): %d", old_msg_type, rec->type)); diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index c06c0a746..404818dfc 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3274,5 +3274,8 @@ elliptic_curve_get_properties TLS 1.3 resume session with ticket tls13_resume_session_with_ticket -TLS 1.3 early data -tls13_early_data +TLS 1.3 early data, reference +tls13_early_data:"reference" + +TLS 1.3 early data, deprotect and discard +tls13_early_data:"deprotect and discard" diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 2d1a757e4..31a973b6f 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3662,9 +3662,10 @@ exit: /* END_CASE */ /* BEGIN_CASE depends_on:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_DEBUG_C:MBEDTLS_TEST_AT_LEAST_ONE_TLS1_3_CIPHERSUITE:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_HAVE_SECP256R1:MBEDTLS_ECP_HAVE_SECP384R1:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_SSL_SESSION_TICKETS */ -void tls13_early_data() +void tls13_early_data(char *scenario_string) { int ret = -1; + int scenario = 0; unsigned char buf[64]; const char *early_data = "This is early data."; size_t early_data_len = strlen(early_data); @@ -3672,6 +3673,18 @@ void tls13_early_data() mbedtls_test_handshake_test_options client_options; mbedtls_test_handshake_test_options server_options; mbedtls_ssl_session saved_session; + mbedtls_test_ssl_log_pattern server_pattern = { NULL, 0 }; + + /* + * Determine scenario. + */ + if (strcmp(scenario_string, "reference") == 0) { + scenario = 0; + } else if (strcmp(scenario_string, "deprotect and discard") == 0) { + scenario = 1; + } else { + TEST_FAIL("Unknown scenario."); + } /* * Test set-up @@ -3692,15 +3705,17 @@ void tls13_early_data() mbedtls_ssl_conf_early_data(&client_ep.conf, MBEDTLS_SSL_EARLY_DATA_ENABLED); server_options.pk_alg = MBEDTLS_PK_ECDSA; + server_options.srv_log_fun = mbedtls_test_ssl_log_analyzer; + server_options.srv_log_obj = &server_pattern; ret = mbedtls_test_ssl_endpoint_init(&server_ep, MBEDTLS_SSL_IS_SERVER, &server_options, NULL, NULL, NULL, NULL); TEST_EQUAL(ret, 0); + mbedtls_ssl_conf_early_data(&server_ep.conf, MBEDTLS_SSL_EARLY_DATA_ENABLED); mbedtls_ssl_conf_session_tickets_cb(&server_ep.conf, mbedtls_test_ticket_write, mbedtls_test_ticket_parse, NULL); - mbedtls_ssl_conf_early_data(&server_ep.conf, MBEDTLS_SSL_EARLY_DATA_ENABLED); ret = mbedtls_test_mock_socket_connect(&(client_ep.socket), &(server_ep.socket), 1024); @@ -3740,6 +3755,16 @@ void tls13_early_data() ret = mbedtls_ssl_set_session(&(client_ep.ssl), &saved_session); TEST_EQUAL(ret, 0); + switch (scenario) { + case 1: /* deprotect and discard */ + mbedtls_debug_set_threshold(3); + server_pattern.pattern = + "EarlyData: deprotect and discard app data records."; + mbedtls_ssl_conf_early_data(&server_ep.conf, + MBEDTLS_SSL_EARLY_DATA_DISABLED); + break; + } + TEST_EQUAL(mbedtls_test_move_handshake_to_state( &(client_ep.ssl), &(server_ep.ssl), MBEDTLS_SSL_SERVER_HELLO), 0); @@ -3751,18 +3776,29 @@ void tls13_early_data() early_data_len); TEST_EQUAL(ret, early_data_len); - TEST_EQUAL(mbedtls_test_move_handshake_to_state( - &(server_ep.ssl), &(client_ep.ssl), - MBEDTLS_SSL_CLIENT_FINISHED), MBEDTLS_ERR_SSL_RECEIVED_EARLY_DATA); + ret = mbedtls_test_move_handshake_to_state( + &(server_ep.ssl), &(client_ep.ssl), + MBEDTLS_SSL_HANDSHAKE_WRAPUP); - TEST_EQUAL(server_ep.ssl.handshake->early_data_accepted, 1); - TEST_EQUAL(mbedtls_ssl_read_early_data(&(server_ep.ssl), buf, sizeof(buf)), - early_data_len); - TEST_MEMORY_COMPARE(buf, early_data_len, early_data, early_data_len); + switch (scenario) { + case 0: + 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), + buf, sizeof(buf)), early_data_len); + TEST_MEMORY_COMPARE(buf, early_data_len, early_data, early_data_len); - TEST_EQUAL(mbedtls_test_move_handshake_to_state( - &(server_ep.ssl), &(client_ep.ssl), - MBEDTLS_SSL_HANDSHAKE_OVER), 0); + TEST_EQUAL(mbedtls_test_move_handshake_to_state( + &(server_ep.ssl), &(client_ep.ssl), + MBEDTLS_SSL_HANDSHAKE_WRAPUP), 0); + break; + + case 1: + TEST_EQUAL(ret, 0); + TEST_EQUAL(server_ep.ssl.handshake->early_data_accepted, 0); + TEST_EQUAL(server_pattern.counter, 1); + break; + } exit: mbedtls_test_ssl_endpoint_free(&client_ep, NULL); @@ -3770,6 +3806,7 @@ exit: mbedtls_test_free_handshake_options(&client_options); mbedtls_test_free_handshake_options(&server_options); mbedtls_ssl_session_free(&saved_session); + mbedtls_debug_set_threshold(0); PSA_DONE(); } /* END_CASE */ From 1483dc3bdedfc3279f84e4aa26427cf7e0c82851 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 19 Jan 2024 10:00:47 +0100 Subject: [PATCH 04/11] tls13: cli: Indicate early data only in first ClientHello Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 76f0f1896..2598bae75 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1182,7 +1182,8 @@ int mbedtls_ssl_tls13_write_client_hello_exts(mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_EARLY_DATA) if (mbedtls_ssl_conf_tls13_is_some_psk_enabled(ssl) && ssl_tls13_early_data_has_valid_ticket(ssl) && - ssl->conf->early_data_enabled == MBEDTLS_SSL_EARLY_DATA_ENABLED) { + ssl->conf->early_data_enabled == MBEDTLS_SSL_EARLY_DATA_ENABLED && + ssl->handshake->hello_retry_request_count == 0) { ret = mbedtls_ssl_tls13_write_early_data_ext( ssl, 0, p, end, &ext_len); From 263dbf71679c359be8549e111fcd0160a1ed1ed4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 26 Oct 2022 10:51:27 +0800 Subject: [PATCH 05/11] tls13: srv: Do not allow early data indication in 2nd ClientHello Signed-off-by: Jerry Yu Signed-off-by: Ronald Cron --- library/ssl_tls13_server.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 6e87d7b9d..93748a6a2 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1533,6 +1533,12 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, unsigned int extension_type; size_t extension_data_len; const unsigned char *extension_data_end; + uint32_t allowed_exts = MBEDTLS_SSL_TLS1_3_ALLOWED_EXTS_OF_CH; + + if (ssl->handshake->hello_retry_request_count > 0) { + /* Do not accept early data extension in 2nd ClientHello */ + allowed_exts &= ~MBEDTLS_SSL_EXT_MASK(EARLY_DATA); + } /* RFC 8446, section 4.2.11 * @@ -1560,7 +1566,7 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, ret = mbedtls_ssl_tls13_check_received_extension( ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, extension_type, - MBEDTLS_SSL_TLS1_3_ALLOWED_EXTS_OF_CH); + allowed_exts); if (ret != 0) { return ret; } From f57d14bed4dbefb7419cbd439afeddcd096058d2 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 15 Nov 2023 16:40:09 +0800 Subject: [PATCH 06/11] Ignore early data app msg before 2nd client hello Signed-off-by: Jerry Yu Signed-off-by: Ronald Cron --- library/ssl_msg.c | 26 ++++++++++++++++++++++++++ tests/suites/test_suite_ssl.data | 3 +++ tests/suites/test_suite_ssl.function | 27 ++++++++++++++++++++++++--- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index bf9a8ca7c..2fe084c4d 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -4109,6 +4109,32 @@ static int ssl_prepare_record_content(mbedtls_ssl_context *ssl, } +#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_SRV_C) + /* + * Although the server rejected early data because it needed to send an + * HelloRetryRequest message, it might receive early data as long as it has + * not received the client Finished message. + * The early data is encrypted with early keys and should be ignored as + * stated in section 4.2.10 of RFC 8446 (second case): + * + * "The server then ignores early data by skipping all records with an + * external content type of "application_data" (indicating that they are + * encrypted), up to the configured max_early_data_size. Ignore application + * data message before 2nd ClientHello when early_data was received in 1st + * ClientHello." + */ + if (ssl->discard_early_data_record == MBEDTLS_SSL_EARLY_DATA_DISCARD) { + if (rec->type == MBEDTLS_SSL_MSG_APPLICATION_DATA) { + MBEDTLS_SSL_DEBUG_MSG( + 3, ("EarlyData: Ignore application message before 2nd ClientHello")); + /* TODO: Add max_early_data_size check here. */ + return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING; + } else if (rec->type == MBEDTLS_SSL_MSG_HANDSHAKE) { + ssl->discard_early_data_record = MBEDTLS_SSL_EARLY_DATA_NO_DISCARD; + } + } +#endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_SRV_C */ + #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { mbedtls_ssl_dtls_replay_update(ssl); diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 404818dfc..e5e4c1e00 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3279,3 +3279,6 @@ tls13_early_data:"reference" TLS 1.3 early data, deprotect and discard tls13_early_data:"deprotect and discard" + +TLS 1.3 early data, discard after HRR +tls13_early_data:"discard after HRR" diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 31a973b6f..949356a1c 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3674,6 +3674,11 @@ void tls13_early_data(char *scenario_string) mbedtls_test_handshake_test_options server_options; mbedtls_ssl_session saved_session; mbedtls_test_ssl_log_pattern server_pattern = { NULL, 0 }; + uint16_t group_list[3] = { + MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1, + MBEDTLS_SSL_IANA_TLS_GROUP_SECP384R1, + MBEDTLS_SSL_IANA_TLS_GROUP_NONE + }; /* * Determine scenario. @@ -3682,6 +3687,8 @@ void tls13_early_data(char *scenario_string) scenario = 0; } else if (strcmp(scenario_string, "deprotect and discard") == 0) { scenario = 1; + } else if (strcmp(scenario_string, "discard after HRR") == 0) { + scenario = 2; } else { TEST_FAIL("Unknown scenario."); } @@ -3700,7 +3707,7 @@ void tls13_early_data(char *scenario_string) client_options.pk_alg = MBEDTLS_PK_ECDSA; ret = mbedtls_test_ssl_endpoint_init(&client_ep, MBEDTLS_SSL_IS_CLIENT, &client_options, NULL, NULL, NULL, - NULL); + group_list); TEST_EQUAL(ret, 0); mbedtls_ssl_conf_early_data(&client_ep.conf, MBEDTLS_SSL_EARLY_DATA_ENABLED); @@ -3709,7 +3716,7 @@ void tls13_early_data(char *scenario_string) server_options.srv_log_obj = &server_pattern; ret = mbedtls_test_ssl_endpoint_init(&server_ep, MBEDTLS_SSL_IS_SERVER, &server_options, NULL, NULL, NULL, - NULL); + group_list); TEST_EQUAL(ret, 0); mbedtls_ssl_conf_early_data(&server_ep.conf, MBEDTLS_SSL_EARLY_DATA_ENABLED); mbedtls_ssl_conf_session_tickets_cb(&server_ep.conf, @@ -3763,6 +3770,19 @@ void tls13_early_data(char *scenario_string) mbedtls_ssl_conf_early_data(&server_ep.conf, MBEDTLS_SSL_EARLY_DATA_DISABLED); break; + + case 2: /* discard after HRR */ + mbedtls_debug_set_threshold(3); + server_pattern.pattern = + "EarlyData: Ignore application message before 2nd ClientHello"; + mbedtls_ssl_conf_groups(&server_ep.conf, group_list + 1); + /* + * Need to reset again to reconstruct the group list in the + * handshake structure from the configured one. + */ + ret = mbedtls_ssl_session_reset(&(server_ep.ssl)); + TEST_EQUAL(ret, 0); + break; } TEST_EQUAL(mbedtls_test_move_handshake_to_state( @@ -3793,7 +3813,8 @@ void tls13_early_data(char *scenario_string) MBEDTLS_SSL_HANDSHAKE_WRAPUP), 0); break; - case 1: + case 1: /* Intentional fallthrough */ + case 2: TEST_EQUAL(ret, 0); TEST_EQUAL(server_ep.ssl.handshake->early_data_accepted, 0); TEST_EQUAL(server_pattern.counter, 1); From ae2d81c314c15350484c6188995a1bac9791f3ef Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 22 Jan 2024 09:13:41 +0100 Subject: [PATCH 07/11] tests: tls13: Run early data test only in TLS 1.3 only config Temporary workaround to not run the early data test in Windows-2013 where there is an issue with mbedtls_vsnprintf(). Signed-off-by: Ronald Cron --- tests/suites/test_suite_ssl.function | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 949356a1c..807b5ab71 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -12,7 +12,8 @@ #define SSL_MESSAGE_QUEUE_INIT { NULL, 0, 0, 0 } -#if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_CLI_C) && \ +#if (!defined(MBEDTLS_SSL_PROTO_TLS1_2)) && \ + defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_CLI_C) && \ defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_DEBUG_C) && \ defined(MBEDTLS_TEST_AT_LEAST_ONE_TLS1_3_CIPHERSUITE) && \ defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED) && \ @@ -3661,7 +3662,12 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_DEBUG_C:MBEDTLS_TEST_AT_LEAST_ONE_TLS1_3_CIPHERSUITE:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_HAVE_SECP256R1:MBEDTLS_ECP_HAVE_SECP384R1:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_SSL_SESSION_TICKETS */ +/* + * The !MBEDTLS_SSL_PROTO_TLS1_2 dependency of tls13_early_data() below is + * a temporary workaround to not run the test in Windows-2013 where there is + * an issue with mbedtls_vsnprintf(). + */ +/* BEGIN_CASE depends_on:!MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_DEBUG_C:MBEDTLS_TEST_AT_LEAST_ONE_TLS1_3_CIPHERSUITE:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_HAVE_SECP256R1:MBEDTLS_ECP_HAVE_SECP384R1:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_SSL_SESSION_TICKETS */ void tls13_early_data(char *scenario_string) { int ret = -1; From 31e2d83eeef6a4dc564b1791a5ab31ecc5a3c593 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 5 Feb 2024 16:45:57 +0100 Subject: [PATCH 08/11] tls13: srv: Improve coding Signed-off-by: Ronald Cron --- library/ssl_tls13_server.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 93748a6a2..5f6d1a18a 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1879,11 +1879,9 @@ static int ssl_tls13_postprocess_client_hello(mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_EARLY_DATA) if (ssl->handshake->received_extensions & MBEDTLS_SSL_EXT_MASK(EARLY_DATA)) { - ssl->handshake->early_data_accepted = 0; - if (!hrr_required) { - ssl->handshake->early_data_accepted = - (ssl_tls13_check_early_data_requirements(ssl) == 0); - } + ssl->handshake->early_data_accepted = + (!hrr_required) && (ssl_tls13_check_early_data_requirements(ssl) == 0); + if (ssl->handshake->early_data_accepted) { ret = mbedtls_ssl_tls13_compute_early_transform(ssl); if (ret != 0) { From 71c6e65d83844da1bc15451743bc44a6db75eca4 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 5 Feb 2024 16:48:10 +0100 Subject: [PATCH 09/11] tls13: ssl_msg.c: Improve/add comments Signed-off-by: Ronald Cron --- library/ssl_msg.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 2fe084c4d..7af9fd2b4 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -4005,7 +4005,7 @@ static int ssl_prepare_record_content(mbedtls_ssl_context *ssl, MBEDTLS_SSL_EARLY_DATA_TRY_TO_DEPROTECT_AND_DISCARD)) { MBEDTLS_SSL_DEBUG_MSG( 3, ("EarlyData: deprotect and discard app data records.")); - /* TODO: Add max_early_data_size check here. */ + /* TODO: Add max_early_data_size check here, see issue 6347 */ ret = MBEDTLS_ERR_SSL_CONTINUE_PROCESSING; } #endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_SRV_C */ @@ -4019,6 +4019,10 @@ static int ssl_prepare_record_content(mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ + /* + * The decryption of the record failed, no reason to ignore it, + * return in error with the decryption error code. + */ return ret; } @@ -4127,7 +4131,7 @@ static int ssl_prepare_record_content(mbedtls_ssl_context *ssl, if (rec->type == MBEDTLS_SSL_MSG_APPLICATION_DATA) { MBEDTLS_SSL_DEBUG_MSG( 3, ("EarlyData: Ignore application message before 2nd ClientHello")); - /* TODO: Add max_early_data_size check here. */ + /* TODO: Add max_early_data_size check here, see issue 6347 */ return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING; } else if (rec->type == MBEDTLS_SSL_MSG_HANDSHAKE) { ssl->discard_early_data_record = MBEDTLS_SSL_EARLY_DATA_NO_DISCARD; From 33327dab8564bff155ec067a281e0e961390e7bf Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 5 Feb 2024 17:46:41 +0100 Subject: [PATCH 10/11] tests: early data: Switch to mnemonics for test scenarios Signed-off-by: Ronald Cron --- tests/suites/test_suite_ssl.data | 6 ++--- tests/suites/test_suite_ssl.function | 37 +++++++++++++--------------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index e5e4c1e00..86945cc7b 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3275,10 +3275,10 @@ TLS 1.3 resume session with ticket tls13_resume_session_with_ticket TLS 1.3 early data, reference -tls13_early_data:"reference" +tls13_early_data:TEST_EARLY_DATA_REFERENCE TLS 1.3 early data, deprotect and discard -tls13_early_data:"deprotect and discard" +tls13_early_data:TEST_EARLY_DATA_DEPROTECT_AND_DISCARD TLS 1.3 early data, discard after HRR -tls13_early_data:"discard after HRR" +tls13_early_data:TEST_EARLY_DATA_DISCARD_AFTER_HRR diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 807b5ab71..cbb29b6fb 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -12,6 +12,11 @@ #define SSL_MESSAGE_QUEUE_INIT { NULL, 0, 0, 0 } +/* Mnemonics for the early data test scenarios */ +#define TEST_EARLY_DATA_REFERENCE 0 +#define TEST_EARLY_DATA_DEPROTECT_AND_DISCARD 1 +#define TEST_EARLY_DATA_DISCARD_AFTER_HRR 2 + #if (!defined(MBEDTLS_SSL_PROTO_TLS1_2)) && \ defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_CLI_C) && \ defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_DEBUG_C) && \ @@ -3668,10 +3673,9 @@ exit: * an issue with mbedtls_vsnprintf(). */ /* BEGIN_CASE depends_on:!MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_DEBUG_C:MBEDTLS_TEST_AT_LEAST_ONE_TLS1_3_CIPHERSUITE:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_HAVE_SECP256R1:MBEDTLS_ECP_HAVE_SECP384R1:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_SSL_SESSION_TICKETS */ -void tls13_early_data(char *scenario_string) +void tls13_early_data(int scenario) { int ret = -1; - int scenario = 0; unsigned char buf[64]; const char *early_data = "This is early data."; size_t early_data_len = strlen(early_data); @@ -3686,19 +3690,6 @@ void tls13_early_data(char *scenario_string) MBEDTLS_SSL_IANA_TLS_GROUP_NONE }; - /* - * Determine scenario. - */ - if (strcmp(scenario_string, "reference") == 0) { - scenario = 0; - } else if (strcmp(scenario_string, "deprotect and discard") == 0) { - scenario = 1; - } else if (strcmp(scenario_string, "discard after HRR") == 0) { - scenario = 2; - } else { - TEST_FAIL("Unknown scenario."); - } - /* * Test set-up */ @@ -3769,7 +3760,10 @@ void tls13_early_data(char *scenario_string) TEST_EQUAL(ret, 0); switch (scenario) { - case 1: /* deprotect and discard */ + case TEST_EARLY_DATA_REFERENCE: + break; + + case TEST_EARLY_DATA_DEPROTECT_AND_DISCARD: mbedtls_debug_set_threshold(3); server_pattern.pattern = "EarlyData: deprotect and discard app data records."; @@ -3777,7 +3771,7 @@ void tls13_early_data(char *scenario_string) MBEDTLS_SSL_EARLY_DATA_DISABLED); break; - case 2: /* discard after HRR */ + case TEST_EARLY_DATA_DISCARD_AFTER_HRR: mbedtls_debug_set_threshold(3); server_pattern.pattern = "EarlyData: Ignore application message before 2nd ClientHello"; @@ -3789,6 +3783,9 @@ void tls13_early_data(char *scenario_string) ret = mbedtls_ssl_session_reset(&(server_ep.ssl)); TEST_EQUAL(ret, 0); break; + + default: + TEST_FAIL("Unknown scenario."); } TEST_EQUAL(mbedtls_test_move_handshake_to_state( @@ -3807,7 +3804,7 @@ void tls13_early_data(char *scenario_string) MBEDTLS_SSL_HANDSHAKE_WRAPUP); switch (scenario) { - case 0: + case TEST_EARLY_DATA_REFERENCE: 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), @@ -3819,8 +3816,8 @@ void tls13_early_data(char *scenario_string) MBEDTLS_SSL_HANDSHAKE_WRAPUP), 0); break; - case 1: /* Intentional fallthrough */ - case 2: + case TEST_EARLY_DATA_DEPROTECT_AND_DISCARD: /* Intentional fallthrough */ + case TEST_EARLY_DATA_DISCARD_AFTER_HRR: TEST_EQUAL(ret, 0); TEST_EQUAL(server_ep.ssl.handshake->early_data_accepted, 0); TEST_EQUAL(server_pattern.counter, 1); From d0a772740ec78c153b13a3337e2a4e40fbad341e Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 5 Feb 2024 17:57:05 +0100 Subject: [PATCH 11/11] tests: early data: Complete the handshake Signed-off-by: Ronald Cron --- tests/suites/test_suite_ssl.function | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index cbb29b6fb..8687a4d6f 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3810,10 +3810,6 @@ void tls13_early_data(int scenario) TEST_EQUAL(mbedtls_ssl_read_early_data(&(server_ep.ssl), buf, sizeof(buf)), early_data_len); TEST_MEMORY_COMPARE(buf, early_data_len, early_data, early_data_len); - - TEST_EQUAL(mbedtls_test_move_handshake_to_state( - &(server_ep.ssl), &(client_ep.ssl), - MBEDTLS_SSL_HANDSHAKE_WRAPUP), 0); break; case TEST_EARLY_DATA_DEPROTECT_AND_DISCARD: /* Intentional fallthrough */ @@ -3824,6 +3820,10 @@ void tls13_early_data(int scenario) break; } + TEST_EQUAL(mbedtls_test_move_handshake_to_state( + &(server_ep.ssl), &(client_ep.ssl), + MBEDTLS_SSL_HANDSHAKE_OVER), 0); + exit: mbedtls_test_ssl_endpoint_free(&client_ep, NULL); mbedtls_test_ssl_endpoint_free(&server_ep, NULL);