From cfd925f3e8e2b27a6a5fe0b329c886d3e4f9cc93 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 14 Apr 2022 07:10:37 +0000 Subject: [PATCH] Fix comments and remove hrr related code Change-Id: Iab1fc5415b3b7f7b5bcb0a41a01f4234cc3497d6 Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 3 +- library/ssl_misc.h | 4 --- library/ssl_tls13_generic.c | 5 +-- library/ssl_tls13_server.c | 59 ++++++-------------------------- tests/ssl-opt.sh | 8 ++--- tests/suites/test_suite_ssl.data | 2 +- 6 files changed, 18 insertions(+), 63 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 09fe01294..3fbe6f3e2 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -648,7 +648,6 @@ typedef enum MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) MBEDTLS_SSL_HELLO_RETRY_REQUEST, - MBEDTLS_SSL_SECOND_CLIENT_HELLO, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY, #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) @@ -1362,7 +1361,7 @@ struct mbedtls_ssl_config void *MBEDTLS_PRIVATE(p_psk); /*!< context for PSK callback */ #endif -#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) +#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) /** Callback to create & write a cookie for ClientHello veirifcation */ int (*MBEDTLS_PRIVATE(f_cookie_write))( void *, unsigned char **, unsigned char *, const unsigned char *, size_t ); diff --git a/library/ssl_misc.h b/library/ssl_misc.h index b23fc1df1..9a7c50325 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -585,10 +585,6 @@ struct mbedtls_ssl_handshake_params /*!< Number of Hello Retry Request messages received from the server. */ int hello_retry_request_count; #endif /* MBEDTLS_SSL_CLI_C */ -#if defined(MBEDTLS_SSL_SRV_C) - /*!< Number of Hello Retry Request messages sent by the server. */ - int hello_retry_requests_sent; -#endif /* MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index a6bcac318..fabc5d158 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1518,15 +1518,16 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, size_t buf_len ) { uint8_t *p = (uint8_t*)buf; + const uint8_t *end = buf + buf_len; mbedtls_ssl_handshake_params *handshake = ssl->handshake; /* Get size of the TLS opaque key_exchange field of the KeyShareEntry struct. */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); uint16_t peerkey_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; /* Check if key size is consistent with given buffer length. */ - if ( peerkey_len > ( buf_len - 2 ) ) - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, peerkey_len ); /* Store peer's ECDH public key. */ memcpy( handshake->ecdh_psa_peerkey, p, peerkey_len ); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b172ec9b5..d181cd294 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -41,12 +41,7 @@ /* From RFC 8446: * struct { - * select (Handshake.msg_type) { - * case client_hello: - * ProtocolVersion versions<2..254>; - * case server_hello: // and HelloRetryRequest - * ProtocolVersion selected_version; - * }; + * ProtocolVersion versions<2..254>; * } SupportedVersions; */ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, @@ -60,21 +55,15 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *versions_end; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); - versions_len = p[0]; p += 1; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, versions_len ); - if( versions_len % 2 != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid supported version list length %" MBEDTLS_PRINTF_SIZET, - versions_len ) ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } versions_end = p + versions_len; while( p < versions_end ) { + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, versions_end, 2 ); mbedtls_ssl_read_version( &major_ver, &minor_ver, ssl->conf->transport, p ); /* In this implementation we only support TLS 1.3 and DTLS 1.3. */ @@ -121,7 +110,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, * NamedGroup named_group_list<2..2^16-1>; * } NamedGroupList; */ -static int mbedtls_ssl_tls13_parse_supported_groups_ext( +static int ssl_tls13_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { @@ -307,6 +296,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_ECDH_C */ +#if defined(MBEDTLS_SSL_DEBUG_C) static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) { ((void) ssl); @@ -336,6 +326,7 @@ static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ } +#endif /* MBEDTLS_SSL_DEBUG_C */ static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, int ext_id_mask ) @@ -523,6 +514,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * uint8 legacy_compression_method = 0; * ... */ + p += 1; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); if( p[0] != 0 ) { @@ -570,7 +562,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * indicates the named groups which the client supports, * ordered from most preferred to least preferred. */ - ret = mbedtls_ssl_tls13_parse_supported_groups_ext( ssl, p, + ret = ssl_tls13_parse_supported_groups_ext( ssl, p, extension_data_end ); if( ret != 0 ) { @@ -693,7 +685,9 @@ have_ciphersuite: ssl->handshake->ciphersuite_info = ciphersuite_info; /* List all the extensions we have received */ +#if defined(MBEDTLS_SSL_DEBUG_C) ssl_tls13_debug_print_client_hello_exts( ssl ); +#endif /* MBEDTLS_SSL_DEBUG_C */ /* * Determine the key exchange algorithm to use. @@ -728,36 +722,10 @@ have_ciphersuite: /* Update the handshake state machine */ -static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl, - int hrr_required ) +static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) { int ret = 0; - if( ssl->handshake->hello_retry_requests_sent == 0 ) - { - hrr_required = SSL_CLIENT_HELLO_HRR_REQUIRED; - } - - if( hrr_required == SSL_CLIENT_HELLO_HRR_REQUIRED ) - { - /* - * Create stateless transcript hash for HRR - */ - MBEDTLS_SSL_DEBUG_MSG( 4, ( "Reset transcript for HRR" ) ); - ret = mbedtls_ssl_reset_transcript_for_hrr( ssl ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_reset_transcript_for_hrr", - ret ); - return( ret ); - } - mbedtls_ssl_session_reset_msg_layer( ssl, 0 ); - - /* Transmit Hello Retry Request */ - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); - return( 0 ); - } - ret = mbedtls_ssl_tls13_key_schedule_stage_early( ssl ); if( ret != 0 ) { @@ -779,7 +747,6 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) { int ret = 0; - int hrr_required = SSL_CLIENT_HELLO_OK; unsigned char* buf = NULL; size_t buflen = 0; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); @@ -791,11 +758,8 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_parse_client_hello( ssl, buf, buf + buflen ) ); - hrr_required = ret; - MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess" ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl, - hrr_required ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); cleanup: @@ -821,7 +785,6 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { /* start state */ case MBEDTLS_SSL_HELLO_REQUEST: - ssl->handshake->hello_retry_requests_sent = 0; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); break; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 416b89f6c..dac868f49 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10218,9 +10218,7 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - op "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$O_NEXT_CLI -msg -tls1_3" \ 1 \ - -s " tls13 server state: MBEDTLS_CLIENT_HELLO" \ - -s " tls13 server state: MBEDTLS_SERVER_HELLO" \ - -s ""SSL - The requested feature is not available"" \ + -s " tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ -s "=> parse client hello" \ -s "<= parse client hello" @@ -10235,9 +10233,7 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_128_GCM_SHA256 - gn "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI -d 4 localhost --priority=NONE:+AES-128-GCM:+SHA256:+AEAD:+VERS-TLS1.3:%NO_TICKETS" \ 1 \ - -s " tls13 server state: MBEDTLS_CLIENT_HELLO" \ - -s " tls13 server state: MBEDTLS_SERVER_HELLO" \ - -s ""SSL - The requested feature is not available"" \ + -s " tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ -s "=> parse client hello" \ -s "<= parse client hello" diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 6d1408682..5aa8dd0b4 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3279,7 +3279,7 @@ conf_version:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_TRANSPORT_DATAGRAM:MBEDTLS_SSL_VE Version config: unsupported server TLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 -conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE +conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:3:4:3:4:0 Version config: unsupported server DTLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3