From 5fbd27055d15c8ac234a229389ff4e31977487a0 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 14 Feb 2024 10:03:36 +0100 Subject: [PATCH] tls13: Use a flag not a counter for CCS and HRR handling Signed-off-by: Ronald Cron --- library/ssl_misc.h | 17 +++++++++++------ library/ssl_tls13_client.c | 8 ++++---- library/ssl_tls13_generic.c | 4 ++-- library/ssl_tls13_server.c | 6 +++--- tests/suites/test_suite_ssl.function | 10 +++++----- 5 files changed, 25 insertions(+), 20 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 942d4ad22f..30113d3eba 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -730,16 +730,21 @@ struct mbedtls_ssl_handshake_params { #if defined(MBEDTLS_SSL_PROTO_TLS1_3) uint8_t key_exchange_mode; /*!< Selected key exchange mode */ - /** Number of HelloRetryRequest messages received/sent from/to the server. */ - uint8_t hello_retry_request_count; + /** + * Flag indicating if, in the course of the current handshake, an + * HelloRetryRequest message has been sent by the server or received by + * the client (<> 0) or not (0). + */ + uint8_t hello_retry_request_flag; #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) /** - * Number of dummy change_cipher_spec (CCS) record sent. Used to send only - * one CCS per handshake without having to complicate the handshake state - * transitions. + * Flag indicating if, in the course of the current handshake, a dummy + * change_cipher_spec (CCS) record has already been sent. Used to send only + * one CCS per handshake while not complicating the handshake state + * transitions for that purpose. */ - uint8_t ccs_count; + uint8_t ccs_sent; #endif #if defined(MBEDTLS_SSL_SRV_C) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index cedebad29a..a055d4d0b2 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1180,7 +1180,7 @@ int mbedtls_ssl_tls13_write_client_hello_exts(mbedtls_ssl_context *ssl, #endif #if defined(MBEDTLS_SSL_EARLY_DATA) - if (ssl->handshake->hello_retry_request_count == 0) { + if (!ssl->handshake->hello_retry_request_flag) { 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) { @@ -1497,7 +1497,7 @@ static int ssl_tls13_preprocess_server_hello(mbedtls_ssl_context *ssl, * to a HelloRetryRequest), it MUST abort the handshake with an * "unexpected_message" alert. */ - if (handshake->hello_retry_request_count > 0) { + if (handshake->hello_retry_request_flag) { MBEDTLS_SSL_DEBUG_MSG(1, ("Multiple HRRs received")); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, @@ -1519,7 +1519,7 @@ static int ssl_tls13_preprocess_server_hello(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER; } - handshake->hello_retry_request_count++; + handshake->hello_retry_request_flag = 1; break; } @@ -1674,7 +1674,7 @@ static int ssl_tls13_parse_server_hello(mbedtls_ssl_context *ssl, * proposed in the HRR, we abort the handshake and send an * "illegal_parameter" alert. */ - else if ((!is_hrr) && (handshake->hello_retry_request_count > 0) && + else if ((!is_hrr) && handshake->hello_retry_request_flag && (cipher_suite != ssl->session_negotiate->ciphersuite)) { fatal_alert = MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER; } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index c6bb770278..3e6d8d068a 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1380,7 +1380,7 @@ int mbedtls_ssl_tls13_write_change_cipher_spec(mbedtls_ssl_context *ssl) MBEDTLS_SSL_DEBUG_MSG(2, ("=> write change cipher spec")); /* Only one CCS to send. */ - if (ssl->handshake->ccs_count > 0) { + if (ssl->handshake->ccs_sent) { ret = 0; goto cleanup; } @@ -1396,7 +1396,7 @@ int mbedtls_ssl_tls13_write_change_cipher_spec(mbedtls_ssl_context *ssl) /* Dispatch message */ MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_write_record(ssl, 0)); - ssl->handshake->ccs_count++; + ssl->handshake->ccs_sent = 1; cleanup: diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f9f3cc5714..b9f8870cc6 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1535,7 +1535,7 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, 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) { + if (ssl->handshake->hello_retry_request_flag) { /* Do not accept early data extension in 2nd ClientHello */ allowed_exts &= ~MBEDTLS_SSL_EXT_MASK(EARLY_DATA); } @@ -2431,7 +2431,7 @@ MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_prepare_hello_retry_request(mbedtls_ssl_context *ssl) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if (ssl->handshake->hello_retry_request_count > 0) { + if (ssl->handshake->hello_retry_request_flag) { MBEDTLS_SSL_DEBUG_MSG(1, ("Too many HRRs")); MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE); @@ -2478,7 +2478,7 @@ static int ssl_tls13_write_hello_retry_request(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg(ssl, buf_len, msg_len)); - ssl->handshake->hello_retry_request_count++; + ssl->handshake->hello_retry_request_flag = 1; #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) /* The server sends a dummy change_cipher_spec record immediately diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index d6a0d7487f..35eef692fc 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3903,7 +3903,7 @@ void tls13_cli_early_data_status(int scenario) break; case TEST_EARLY_DATA_HRR: - if (client_ep.ssl.handshake->hello_retry_request_count == 0) { + if (!client_ep.ssl.handshake->hello_retry_request_flag) { TEST_EQUAL(client_ep.ssl.early_data_status, MBEDTLS_SSL_EARLY_DATA_STATUS_UNKNOWN); } else { @@ -3928,7 +3928,7 @@ void tls13_cli_early_data_status(int scenario) break; case TEST_EARLY_DATA_HRR: - if (client_ep.ssl.handshake->hello_retry_request_count == 0) { + if (!client_ep.ssl.handshake->hello_retry_request_flag) { TEST_EQUAL(client_ep.ssl.early_data_status, MBEDTLS_SSL_EARLY_DATA_STATUS_CAN_WRITE); } else { @@ -4089,7 +4089,7 @@ void tls13_cli_early_data_status(int scenario) } while (client_ep.ssl.state != MBEDTLS_SSL_HANDSHAKE_OVER); #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) - TEST_EQUAL(client_ep.ssl.handshake->ccs_count, 1); + TEST_EQUAL(client_ep.ssl.handshake->ccs_sent, 1); #endif exit: @@ -4248,7 +4248,7 @@ void tls13_write_early_data(int scenario) break; case TEST_EARLY_DATA_HRR: - if (client_ep.ssl.handshake->hello_retry_request_count == 0) { + if (!client_ep.ssl.handshake->hello_retry_request_flag) { TEST_EQUAL(write_early_data_ret, early_data_len); TEST_EQUAL(client_ep.ssl.state, MBEDTLS_SSL_SERVER_HELLO); } else { @@ -4270,7 +4270,7 @@ void tls13_write_early_data(int scenario) break; case TEST_EARLY_DATA_HRR: - if (client_ep.ssl.handshake->hello_retry_request_count == 0) { + if (!client_ep.ssl.handshake->hello_retry_request_flag) { TEST_EQUAL(write_early_data_ret, early_data_len); TEST_EQUAL(client_ep.ssl.state, MBEDTLS_SSL_SERVER_HELLO); } else {