From fe59ff794d2e198665ca9456c097eae0b30fab66 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 24 Jan 2024 14:31:50 +0100 Subject: [PATCH] tls13: Send dummy CCS only once Fix cases where the client was sending two CCS, no harm but better to send only one. Prevent to send even more CCS when early data are involved without having to add conditional state transitions. Signed-off-by: Ronald Cron --- library/ssl_misc.h | 15 ++++++++++++--- library/ssl_tls13_client.c | 23 ++++++++++++++--------- library/ssl_tls13_generic.c | 2 ++ library/ssl_tls13_server.c | 10 +++++++--- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 16cd62e28..dff19c88e 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -731,14 +731,23 @@ struct mbedtls_ssl_handshake_params { uint8_t key_exchange_mode; /*!< Selected key exchange mode */ /** Number of HelloRetryRequest messages received/sent from/to the server. */ - int hello_retry_request_count; + uint8_t hello_retry_request_count; + +#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. + */ + uint8_t ccs_count; +#endif #if defined(MBEDTLS_SSL_SRV_C) - /** selected_group of key_share extension in HelloRetryRequest message. */ - uint16_t hrr_selected_group; #if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_PSK_ENABLED) uint8_t tls13_kex_modes; /*!< Key exchange modes supported by the client */ #endif + /** selected_group of key_share extension in HelloRetryRequest message. */ + uint16_t hrr_selected_group; #if defined(MBEDTLS_SSL_SESSION_TICKETS) uint16_t new_session_tickets_count; /*!< number of session tickets */ #endif diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index f4987b316..e6680c7a6 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2568,8 +2568,6 @@ static int ssl_tls13_process_server_finished(mbedtls_ssl_context *ssl) #if defined(MBEDTLS_SSL_EARLY_DATA) if (ssl->early_data_status == MBEDTLS_SSL_EARLY_DATA_STATUS_ACCEPTED) { mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_END_OF_EARLY_DATA); - } else if (ssl->early_data_status == MBEDTLS_SSL_EARLY_DATA_STATUS_REJECTED) { - mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE); } else #endif /* MBEDTLS_SSL_EARLY_DATA */ { @@ -3059,18 +3057,25 @@ int mbedtls_ssl_tls13_handshake_client_step(mbedtls_ssl_context *ssl) */ #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) case MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO: - ret = mbedtls_ssl_tls13_write_change_cipher_spec(ssl); - if (ret == 0) { - mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_CLIENT_HELLO); + ret = 0; + if (ssl->handshake->ccs_count == 0) { + ret = mbedtls_ssl_tls13_write_change_cipher_spec(ssl); + if (ret != 0) { + break; + } } + mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_CLIENT_HELLO); break; case MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED: - ret = mbedtls_ssl_tls13_write_change_cipher_spec(ssl); - if (ret == 0) { - mbedtls_ssl_handshake_set_state( - ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE); + ret = 0; + if (ssl->handshake->ccs_count == 0) { + ret = mbedtls_ssl_tls13_write_change_cipher_spec(ssl); + if (ret != 0) { + break; + } } + mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE); break; #if defined(MBEDTLS_SSL_EARLY_DATA) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 2666067b7..386a75402 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1390,6 +1390,8 @@ 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++; + cleanup: MBEDTLS_SSL_DEBUG_MSG(2, ("<= write change cipher spec")); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 62b117cfa..05693f3bf 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -3482,10 +3482,14 @@ int mbedtls_ssl_tls13_handshake_server_step(mbedtls_ssl_context *ssl) break; case MBEDTLS_SSL_SERVER_CCS_AFTER_SERVER_HELLO: - ret = mbedtls_ssl_tls13_write_change_cipher_spec(ssl); - if (ret == 0) { - mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS); + ret = 0; + if (ssl->handshake->ccs_count == 0) { + ret = mbedtls_ssl_tls13_write_change_cipher_spec(ssl); + if (ret != 0) { + break; + } } + mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS); break; #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */