From 67a2c37039629bca8e3a0bb6ddd77d37601e6a60 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 14 Apr 2022 18:52:29 +0800 Subject: [PATCH 01/21] tls13:hrr:add empty frame work Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 775443cd61..c1ab3cefe0 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1276,6 +1276,16 @@ cleanup: } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ +/* + * Handler for MBEDTLS_SSL_HELLO_RETRY_REQUEST + */ + +static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) +{ + ((void) ssl); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +} + /* * TLS 1.3 State Machine -- server side */ @@ -1311,6 +1321,7 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) ret = ssl_tls13_write_server_hello( ssl ); break; +<<<<<<< HEAD case MBEDTLS_SSL_ENCRYPTED_EXTENSIONS: ret = ssl_tls13_write_encrypted_extensions( ssl ); if( ret != 0 ) @@ -1318,6 +1329,10 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_encrypted_extensions", ret ); return( ret ); } +======= + case MBEDTLS_SSL_HELLO_RETRY_REQUEST: + ret = ssl_tls13_write_hello_retry_request( ssl ); +>>>>>>> tls13:hrr:add empty frame work break; #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) From 93a13f2c3882def6d5b4d23c70dc89c7c9459f4b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 11 Apr 2022 23:00:01 +0800 Subject: [PATCH 02/21] Share magic word of HRR Signed-off-by: Jerry Yu --- library/ssl_misc.h | 2 +- library/ssl_tls13_client.c | 11 ++++------- library/ssl_tls13_generic.c | 6 ++++++ 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 4d8c479d46..941e796748 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1631,7 +1631,7 @@ static inline int mbedtls_ssl_conf_is_hybrid_tls12_tls13( const mbedtls_ssl_conf #endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) - +extern const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[32]; int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ); int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ); void mbedtls_ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 59e42c868c..320c5b4b23 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -750,11 +750,6 @@ static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - static const unsigned char magic_hrr_string[MBEDTLS_SERVER_HELLO_RANDOM_LEN] = - { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, - 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, - 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, - 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33 ,0x9C }; /* Check whether this message is a HelloRetryRequest ( HRR ) message. * @@ -771,9 +766,11 @@ static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, * } ServerHello; * */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2 + sizeof( magic_hrr_string ) ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, + 2 + sizeof( mbedtls_ssl_tls13_hello_retry_request_magic ) ); - if( memcmp( buf + 2, magic_hrr_string, sizeof( magic_hrr_string ) ) == 0 ) + if( memcmp( buf + 2, mbedtls_ssl_tls13_hello_retry_request_magic, + sizeof( mbedtls_ssl_tls13_hello_retry_request_magic ) ) == 0 ) { return( SSL_SERVER_HELLO_COORDINATE_HRR ); } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index f5d791f1bf..4332a1d0c0 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -34,6 +34,12 @@ #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" +const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[32] = + { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, + 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, + 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, 0x07, 0x9E, 0x09, + 0xE2, 0xC8, 0xA8, 0x33 ,0x9C }; + int mbedtls_ssl_tls13_fetch_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, unsigned char **buf, From cb03677f850da203a5cd3cd4cfafbb6d2db6aa31 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 15 Apr 2022 14:36:19 +0800 Subject: [PATCH 03/21] add hrr test Signed-off-by: Jerry Yu --- tests/opt-testcases/tls13-generic.sh | 37 ++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100755 tests/opt-testcases/tls13-generic.sh diff --git a/tests/opt-testcases/tls13-generic.sh b/tests/opt-testcases/tls13-generic.sh new file mode 100755 index 0000000000..ba1dd12f62 --- /dev/null +++ b/tests/opt-testcases/tls13-generic.sh @@ -0,0 +1,37 @@ +#!/bin/sh + +# tls13-generic.sh +# +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +run_test "TLS 1.3 m->M: HRR secp256r1 -> secp384r1" \ + "$P_SRV debug_level=4 force_version=tls13 curves=secp384r1" \ + "$P_CLI debug_level=4 force_version=tls13 curves=secp256r1,secp384r1" \ + 1 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_HELLO_RETRY_REQUEST" \ + -c "client state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "selected_group: secp384r1" \ + -s "SSL - The requested feature is not available" \ + -s "=> write hello retry request" \ + -s "<= write hello retry request" From fe24d1c9f5a9d481ba3d0ae141b79e579d650b81 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 11 Apr 2022 21:04:47 +0800 Subject: [PATCH 04/21] add named group debug helper Signed-off-by: Jerry Yu --- library/ssl_debug_helpers.h | 2 + scripts/generate_ssl_debug_helpers.py | 69 +++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/library/ssl_debug_helpers.h b/library/ssl_debug_helpers.h index 29b64dc4ea..9f1df736bd 100644 --- a/library/ssl_debug_helpers.h +++ b/library/ssl_debug_helpers.h @@ -41,6 +41,8 @@ const char *mbedtls_ssl_key_export_type_str( mbedtls_ssl_key_export_type in ); const char *mbedtls_ssl_sig_alg_to_str( uint16_t in ); +const char *mbedtls_ssl_named_group_to_str( uint16_t in ); + #endif /* MBEDTLS_DEBUG_C */ #endif /* SSL_DEBUG_HELPERS_H */ diff --git a/scripts/generate_ssl_debug_helpers.py b/scripts/generate_ssl_debug_helpers.py index 42e4fc82d5..86a54ea8a8 100755 --- a/scripts/generate_ssl_debug_helpers.py +++ b/scripts/generate_ssl_debug_helpers.py @@ -234,6 +234,7 @@ class EnumDefinition: prototype=self._prototype) return body + class SignatureAlgorithmDefinition: """ Generate helper functions for signature algorithms. @@ -267,6 +268,7 @@ class SignatureAlgorithmDefinition: def span(self): return self._definitions[0].span() + def __str__(self): """ Generate function for translating value to string @@ -277,7 +279,7 @@ class SignatureAlgorithmDefinition: translation_table.append( '\tcase {}:\n\t return "{}";'.format(name, name[len('MBEDTLS_TLS1_3_SIG_'):].lower()) - ) + ) body = textwrap.dedent('''\ const char *mbedtls_ssl_sig_alg_to_str( uint16_t in ) @@ -292,6 +294,65 @@ class SignatureAlgorithmDefinition: body = body.format(translation_table='\n'.join(translation_table)) return body + +class NamedGroupDefinition: + """ + Generate helper functions for named group + + It generates translation function from named group define to string. + Signature algorithm definition looks like: + #define MBEDTLS_SSL_IANA_TLS_GROUP_[ upper case named group ] [ value(hex) ] + + Known limitation: + - the definitions SHOULD exist in same macro blocks. + """ + + @classmethod + def extract(cls, source_code, start=0, end=-1): + sig_alg_pattern = re.compile(r'#define\s+(?PMBEDTLS_SSL_IANA_TLS_GROUP_\w+)\s+' + + r'(?P0[xX][0-9a-fA-F]+)$', + re.MULTILINE | re.DOTALL) + matches = list(sig_alg_pattern.finditer(source_code, start, end)) + if matches: + yield NamedGroupDefinition(source_code, definitions=matches) + + def __init__(self, source_code, definitions=None): + if definitions is None: + definitions = [] + assert isinstance(definitions, list) and definitions + self._definitions = definitions + self._source = source_code + + def __repr__(self): + return 'NamedGroup({})'.format(self._definitions[0].span()) + + def span(self): + return self._definitions[0].span() + + def __str__(self): + """ + Generate function for translating value to string + """ + translation_table = [] + for m in self._definitions: + name = m.groupdict()['name'] + iana_name = name[len('MBEDTLS_SSL_IANA_TLS_GROUP_'):].lower() + translation_table.append('\tcase {}:\n\t return "{}";'.format(name, iana_name)) + + body = textwrap.dedent('''\ + const char *mbedtls_ssl_named_group_to_str( uint16_t in ) + {{ + switch( in ) + {{ + {translation_table} + }}; + + return "UNKOWN"; + }}''') + body = body.format(translation_table='\n'.join(translation_table)) + return body + + OUTPUT_C_TEMPLATE = '''\ /* Automatically generated by generate_ssl_debug_helpers.py. DO NOT EDIT. */ @@ -335,14 +396,16 @@ def generate_ssl_debug_helpers(output_directory, mbedtls_root): """ Generate functions of debug helps """ - mbedtls_root = os.path.abspath(mbedtls_root or build_tree.guess_mbedtls_root()) + mbedtls_root = os.path.abspath( + mbedtls_root or build_tree.guess_mbedtls_root()) with open(os.path.join(mbedtls_root, 'include/mbedtls/ssl.h')) as f: source_code = remove_c_comments(f.read()) definitions = dict() for start, instance in preprocess_c_source_code(source_code, EnumDefinition, - SignatureAlgorithmDefinition): + SignatureAlgorithmDefinition, + NamedGroupDefinition): if start in definitions: continue if isinstance(instance, EnumDefinition): From 582dd069b72f33a5b09fcfa1cffadbf44198c879 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 22 Apr 2022 21:59:01 +0800 Subject: [PATCH 05/21] Add HRR handler Signed-off-by: Jerry Yu --- library/ssl_misc.h | 7 +- library/ssl_tls13_server.c | 269 ++++++++++++++++++++++++++++++++++++- 2 files changed, 265 insertions(+), 11 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 941e796748..7cf677a430 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -585,17 +585,14 @@ struct mbedtls_ssl_handshake_params */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) int tls13_kex_modes; /*!< key exchange modes for TLS 1.3 */ -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ -#if defined(MBEDTLS_SSL_CLI_C) - /** Number of Hello Retry Request messages received from the server. */ + /* Number of HelloRetryRequest messages received/sent from the server. */ int hello_retry_request_count; -#endif /* MBEDTLS_SSL_CLI_C */ - #if defined(MBEDTLS_SSL_SRV_C) /** selected_group of key_share extension in HelloRetryRequest message. */ uint16_t hrr_selected_group; #endif /* MBEDTLS_SSL_SRV_C */ +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index c1ab3cefe0..13fcb654bc 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -388,6 +388,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, const unsigned char *cipher_suites_end; size_t extensions_len; const unsigned char *extensions_end; + int hrr_required = SSL_CLIENT_HELLO_OK; const mbedtls_ssl_ciphersuite_t* ciphersuite_info; @@ -611,11 +612,15 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, if( ret == SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "HRR needed " ) ); - ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; + hrr_required |= SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH; } - if( ret != 0 ) + if( ret < 0 ) + { + MBEDTLS_SSL_DEBUG_RET( + 1, "ssl_tls13_parse_key_shares_ext", ret ); return( ret ); + } ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_KEY_SHARE; break; @@ -687,9 +692,11 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - return( 0 ); + return( hrr_required ); } +/* Update the handshake state machine */ + static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -716,6 +723,7 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char* buf = NULL; size_t buflen = 0; + int hrr_required ; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( @@ -724,8 +732,14 @@ 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_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); + + if( hrr_required == SSL_CLIENT_HELLO_OK ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); + else + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); cleanup: @@ -1280,10 +1294,252 @@ cleanup: * Handler for MBEDTLS_SSL_HELLO_RETRY_REQUEST */ +static int ssl_tls13_write_hello_retry_request_coordinate( + mbedtls_ssl_context *ssl ) +{ + int ret; + if( ssl->handshake->hello_retry_request_count > 1 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Too many HRRs" ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + /* + * 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 ); + + return( 0 ); +} + +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + uint16_t selected_group = ssl->handshake->hrr_selected_group; + /* key_share Extension + * + * struct { + * select (Handshake.msg_type) { + * ... + * case hello_retry_request: + * NamedGroup selected_group; + * ... + * }; + * } KeyShare; + */ + + *out_len = 0; + + /* For a pure PSK-based ciphersuite there is no key share to declare. */ + if( ! mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) + return( 0 ); + + /* We should only send the key_share extension if the client's initial + * key share was not acceptable. */ + if( ssl->handshake->offered_group_id != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "Skip key_share extension in HRR" ) ); + return( 0 ); + } + + if( selected_group == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching named group found" ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + if( ! mbedtls_ssl_named_group_is_offered( ssl, selected_group ) || + ! mbedtls_ssl_named_group_is_supported( selected_group ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + /* extension header, extension length, NamedGroup value */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 6 ); + + /* Write extension header */ + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, buf, 0 ); + + /* Write extension length */ + MBEDTLS_PUT_UINT16_BE( 2, buf, 2 ); + + /* Write selected group */ + MBEDTLS_PUT_UINT16_BE( selected_group, buf, 4 ); + + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "HRR selected_group: %s (%x)", + mbedtls_ssl_named_group_to_str( selected_group ), + selected_group ) ); + + *out_len = 6; + return( 0 ); + +} +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + +static int ssl_tls13_write_hello_retry_request_body( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + int ret; + unsigned char *p = buf; + unsigned char *start = buf; + size_t output_len; + unsigned char *extension_start; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write hello retry request" ) ); + + *out_len = 0; + + /* + * struct { + * ProtocolVersion legacy_version = 0x0303; + * Random random ( with magic value ); + * opaque legacy_session_id_echo<0..32>; + * CipherSuite cipher_suite; + * uint8 legacy_compression_method = 0; + * Extension extensions<0..2^16-1>; + * } ServerHello; --- aka HelloRetryRequest + */ + + + /* + * Write legacy_version + * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 + * + * For TLS 1.3 we use the legacy version number {0x03, 0x03} + * instead of the true version number. + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_PUT_UINT16_BE( 0x0303, p, 0 ); + p += 2; + + /* write magic string (as a replacement for the random value) */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + memcpy( p, mbedtls_ssl_tls13_hello_retry_request_magic, + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", + p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; + + /* + * Write legacy_session_id_echo + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 + ssl->session_negotiate->id_len ); + *p++ = (unsigned char)ssl->session_negotiate->id_len; + if( ssl->session_negotiate->id_len > 0 ) + { + memcpy( p, &ssl->session_negotiate->id[0], + ssl->session_negotiate->id_len ); + p += ssl->session_negotiate->id_len; + MBEDTLS_SSL_DEBUG_MSG( 3, ( "session id length ( %" + MBEDTLS_PRINTF_SIZET " )", + ssl->session_negotiate->id_len ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "session id", ssl->session_negotiate->id, + ssl->session_negotiate->id_len ); + } + + /* + * Write ciphersuite + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_PUT_UINT16_BE( ssl->session_negotiate->ciphersuite, p, 0 ); + p += 2; + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "server hello, chosen ciphersuite: %s ( id=%d )", + mbedtls_ssl_get_ciphersuite_name( + ssl->session_negotiate->ciphersuite ), + ssl->session_negotiate->ciphersuite ) ); + + /* write legacy_compression_method = ( 0 ) */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); + *p++ = 0x0; + + /* Extensions */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + extension_start = p; + p += 2; + + /* Add supported_version extension */ + if( ( ret = ssl_tls13_write_server_hello_supported_versions_ext( + ssl, p, end, &output_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_selected_version_ext", + ret ); + return( ret ); + } + p += output_len; + +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + /* Add key_share extension, if necessary */ + if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) + { + ret = ssl_tls13_write_hrr_key_share_ext( ssl, p, end, &output_len ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_hrr_key_share_ext", ret ); + return( ret ); + } + p += output_len; + } +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + + /* Write length information */ + MBEDTLS_PUT_UINT16_BE( p - extension_start - 2, extension_start, 0 ); + + MBEDTLS_SSL_DEBUG_BUF( 4, "hello retry request extensions", + extension_start, p - extension_start ); + + *out_len = p - start; + + MBEDTLS_SSL_DEBUG_BUF( 3, "hello retry request", start, *out_len ); + + *out_len = p - buf; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write hello retry request" ) ); + return( 0 ); +} + static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) { - ((void) ssl); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + int ret; + unsigned char *buf; + size_t buf_len, msg_len; + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_hello_retry_request_coordinate( ssl ) ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, + MBEDTLS_SSL_HS_SERVER_HELLO, &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_hello_retry_request_body( + ssl, buf, buf + buf_len, &msg_len ) ); + + mbedtls_ssl_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len ); + + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, + msg_len ) ); + + ssl->handshake->hello_retry_request_count++; + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); + +cleanup: + + return( ret ); } /* @@ -1304,6 +1560,7 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { /* start state */ case MBEDTLS_SSL_HELLO_REQUEST: + ssl->handshake->hello_retry_request_count = 0; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); ret = 0; From 23f7a6fc5cdfc97e0bfd63fd84bd1683f447286e Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sat, 23 Apr 2022 15:16:45 +0800 Subject: [PATCH 06/21] share write_body between HRR and ServerHello Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 293 +++++++++------------------ tests/opt-testcases/tls13-generic.sh | 2 +- 2 files changed, 92 insertions(+), 203 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 13fcb654bc..528409b57f 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -917,6 +917,72 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, return( 0 ); } +static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + uint16_t selected_group = ssl->handshake->hrr_selected_group; + /* key_share Extension + * + * struct { + * select (Handshake.msg_type) { + * ... + * case hello_retry_request: + * NamedGroup selected_group; + * ... + * }; + * } KeyShare; + */ + + *out_len = 0; + + /* For a pure PSK-based ciphersuite there is no key share to declare. */ + if( ! mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) + return( 0 ); + + /* We should only send the key_share extension if the client's initial + * key share was not acceptable. */ + if( ssl->handshake->offered_group_id != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "Skip key_share extension in HRR" ) ); + return( 0 ); + } + + if( selected_group == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching named group found" ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + if( ! mbedtls_ssl_named_group_is_offered( ssl, selected_group ) || + ! mbedtls_ssl_named_group_is_supported( selected_group ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + /* extension header, extension length, NamedGroup value */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 6 ); + + /* Write extension header */ + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, buf, 0 ); + + /* Write extension length */ + MBEDTLS_PUT_UINT16_BE( 2, buf, 2 ); + + /* Write selected group */ + MBEDTLS_PUT_UINT16_BE( selected_group, buf, 4 ); + + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "HRR selected_group: %s (%x)", + mbedtls_ssl_named_group_to_str( selected_group ), + selected_group ) ); + + *out_len = 6; + return( 0 ); + +} /* * Structure of ServerHello message: @@ -933,7 +999,8 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, - size_t *out_len ) + size_t *out_len, + int is_hrr ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *p = buf; @@ -959,8 +1026,16 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, * opaque Random[MBEDTLS_SERVER_HELLO_RANDOM_LEN]; */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - memcpy( p, &ssl->handshake->randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN], - MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + if( is_hrr ) + { + memcpy( p, mbedtls_ssl_tls13_hello_retry_request_magic, + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + } + else + { + memcpy( p, &ssl->handshake->randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN], + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + } MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; @@ -1026,7 +1101,10 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) { - ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); + if( is_hrr ) + ret = ssl_tls13_write_hrr_key_share_ext( ssl, p, end, &output_len ); + else + ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; @@ -1079,7 +1157,8 @@ static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_server_hello_body( ssl, buf, buf + buf_len, - &msg_len ) ); + &msg_len, + 0 ) ); mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len ); @@ -1319,213 +1398,23 @@ static int ssl_tls13_write_hello_retry_request_coordinate( return( 0 ); } -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) -static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - uint16_t selected_group = ssl->handshake->hrr_selected_group; - /* key_share Extension - * - * struct { - * select (Handshake.msg_type) { - * ... - * case hello_retry_request: - * NamedGroup selected_group; - * ... - * }; - * } KeyShare; - */ - - *out_len = 0; - - /* For a pure PSK-based ciphersuite there is no key share to declare. */ - if( ! mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) - return( 0 ); - - /* We should only send the key_share extension if the client's initial - * key share was not acceptable. */ - if( ssl->handshake->offered_group_id != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 4, ( "Skip key_share extension in HRR" ) ); - return( 0 ); - } - - if( selected_group == 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching named group found" ) ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - } - - if( ! mbedtls_ssl_named_group_is_offered( ssl, selected_group ) || - ! mbedtls_ssl_named_group_is_supported( selected_group ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 4, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* extension header, extension length, NamedGroup value */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 6 ); - - /* Write extension header */ - MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, buf, 0 ); - - /* Write extension length */ - MBEDTLS_PUT_UINT16_BE( 2, buf, 2 ); - - /* Write selected group */ - MBEDTLS_PUT_UINT16_BE( selected_group, buf, 4 ); - - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "HRR selected_group: %s (%x)", - mbedtls_ssl_named_group_to_str( selected_group ), - selected_group ) ); - - *out_len = 6; - return( 0 ); - -} -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ - -static int ssl_tls13_write_hello_retry_request_body( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - int ret; - unsigned char *p = buf; - unsigned char *start = buf; - size_t output_len; - unsigned char *extension_start; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write hello retry request" ) ); - - *out_len = 0; - - /* - * struct { - * ProtocolVersion legacy_version = 0x0303; - * Random random ( with magic value ); - * opaque legacy_session_id_echo<0..32>; - * CipherSuite cipher_suite; - * uint8 legacy_compression_method = 0; - * Extension extensions<0..2^16-1>; - * } ServerHello; --- aka HelloRetryRequest - */ - - - /* - * Write legacy_version - * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 - * - * For TLS 1.3 we use the legacy version number {0x03, 0x03} - * instead of the true version number. - */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - MBEDTLS_PUT_UINT16_BE( 0x0303, p, 0 ); - p += 2; - - /* write magic string (as a replacement for the random value) */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - memcpy( p, mbedtls_ssl_tls13_hello_retry_request_magic, - MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", - p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; - - /* - * Write legacy_session_id_echo - */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 + ssl->session_negotiate->id_len ); - *p++ = (unsigned char)ssl->session_negotiate->id_len; - if( ssl->session_negotiate->id_len > 0 ) - { - memcpy( p, &ssl->session_negotiate->id[0], - ssl->session_negotiate->id_len ); - p += ssl->session_negotiate->id_len; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "session id length ( %" - MBEDTLS_PRINTF_SIZET " )", - ssl->session_negotiate->id_len ) ); - MBEDTLS_SSL_DEBUG_BUF( 3, "session id", ssl->session_negotiate->id, - ssl->session_negotiate->id_len ); - } - - /* - * Write ciphersuite - */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - MBEDTLS_PUT_UINT16_BE( ssl->session_negotiate->ciphersuite, p, 0 ); - p += 2; - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "server hello, chosen ciphersuite: %s ( id=%d )", - mbedtls_ssl_get_ciphersuite_name( - ssl->session_negotiate->ciphersuite ), - ssl->session_negotiate->ciphersuite ) ); - - /* write legacy_compression_method = ( 0 ) */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); - *p++ = 0x0; - - /* Extensions */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - extension_start = p; - p += 2; - - /* Add supported_version extension */ - if( ( ret = ssl_tls13_write_server_hello_supported_versions_ext( - ssl, p, end, &output_len ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_selected_version_ext", - ret ); - return( ret ); - } - p += output_len; - -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - /* Add key_share extension, if necessary */ - if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) - { - ret = ssl_tls13_write_hrr_key_share_ext( ssl, p, end, &output_len ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_hrr_key_share_ext", ret ); - return( ret ); - } - p += output_len; - } -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ - - /* Write length information */ - MBEDTLS_PUT_UINT16_BE( p - extension_start - 2, extension_start, 0 ); - - MBEDTLS_SSL_DEBUG_BUF( 4, "hello retry request extensions", - extension_start, p - extension_start ); - - *out_len = p - start; - - MBEDTLS_SSL_DEBUG_BUF( 3, "hello retry request", start, *out_len ); - - *out_len = p - buf; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write hello retry request" ) ); - return( 0 ); -} - static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) { int ret; unsigned char *buf; size_t buf_len, msg_len; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write hello retry request" ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_hello_retry_request_coordinate( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, &buf, &buf_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_hello_retry_request_body( - ssl, buf, buf + buf_len, &msg_len ) ); - + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_server_hello_body( ssl, buf, + buf + buf_len, + &msg_len, + 1 ) ); mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len ); @@ -1538,7 +1427,7 @@ static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); cleanup: - + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write hello retry request" ) ); return( ret ); } diff --git a/tests/opt-testcases/tls13-generic.sh b/tests/opt-testcases/tls13-generic.sh index ba1dd12f62..67d62eee7e 100755 --- a/tests/opt-testcases/tls13-generic.sh +++ b/tests/opt-testcases/tls13-generic.sh @@ -22,7 +22,7 @@ requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 -run_test "TLS 1.3 m->M: HRR secp256r1 -> secp384r1" \ +run_test "TLS 1.3: server: HRR check - mbedtls" \ "$P_SRV debug_level=4 force_version=tls13 curves=secp384r1" \ "$P_CLI debug_level=4 force_version=tls13 curves=secp256r1,secp384r1" \ 1 \ From c1be19f22661d5080aae869e96464088def48f66 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sat, 23 Apr 2022 16:11:39 +0800 Subject: [PATCH 07/21] misc:minor improvement Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 98 +++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 45 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 528409b57f..f65a33ace0 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -106,9 +106,9 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, * NamedGroup named_group_list<2..2^16-1>; * } NamedGroupList; */ -static int ssl_tls13_parse_supported_groups_ext( - mbedtls_ssl_context *ssl, - const unsigned char *buf, const unsigned char *end ) +static int ssl_tls13_parse_supported_groups_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { const unsigned char *p = buf; size_t named_group_list_len; @@ -129,7 +129,10 @@ static int ssl_tls13_parse_supported_groups_ext( named_group = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "got named group: %d", named_group ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "got named group: %s(%04x)", + mbedtls_ssl_named_group_to_str( named_group ), + named_group ) ); if( ! mbedtls_ssl_named_group_is_offered( ssl, named_group ) || ! mbedtls_ssl_named_group_is_supported( named_group ) || @@ -138,9 +141,11 @@ static int ssl_tls13_parse_supported_groups_ext( continue; } - MBEDTLS_SSL_DEBUG_MSG( - 2, ( "add named group (%04x) into received list.", - named_group ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "add named group %s(%04x) into received list.", + mbedtls_ssl_named_group_to_str( named_group ), + named_group ) ); + ssl->handshake->hrr_selected_group = named_group; } @@ -162,8 +167,7 @@ static int ssl_tls13_parse_supported_groups_ext( * does not match a group supported by the server. A HelloRetryRequest will * be needed. * - A negative value for fatal errors. -*/ - + */ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -171,8 +175,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char const *p = buf; unsigned char const *client_shares_end; - size_t client_shares_len, key_exchange_len; - int match_found = 0; + size_t client_shares_len; /* From RFC 8446: * @@ -196,9 +199,10 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * dismiss it and send a HelloRetryRequest message. */ - for( ; p < client_shares_end; p += key_exchange_len ) + while( p < client_shares_end ) { uint16_t group; + size_t key_exchange_len; /* * struct { @@ -208,20 +212,18 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, client_shares_end, 4 ); group = MBEDTLS_GET_UINT16_BE( p, 0 ); - p += 2; - key_exchange_len = MBEDTLS_GET_UINT16_BE( p, 0 ); - p += 2; + key_exchange_len = MBEDTLS_GET_UINT16_BE( p, 2 ); + p += 4; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, client_shares_end, key_exchange_len ); /* Continue parsing even if we have already found a match, * for input validation purposes. */ - if( match_found == 1 ) - continue; - if( ! mbedtls_ssl_named_group_is_offered( ssl, group ) || - ! mbedtls_ssl_named_group_is_supported( group ) ) + ! mbedtls_ssl_named_group_is_supported( group ) || + ssl->handshake->offered_group_id != 0 ) { + p += key_exchange_len; continue; } @@ -230,28 +232,29 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, */ if( mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) { - const mbedtls_ecp_curve_info *curve_info = - mbedtls_ecp_curve_info_from_tls_id( group ); - ((void) curve_info); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH group: %s (%04x)", + mbedtls_ssl_named_group_to_str( group ), + group ) ); ret = mbedtls_ssl_tls13_read_public_ecdhe_share( - ssl, p - 2, key_exchange_len + 2 ); + ssl, p - 2, key_exchange_len + 2 ); if( ret != 0 ) return( ret ); - match_found = 1; } else { MBEDTLS_SSL_DEBUG_MSG( 4, ( "Unrecognized NamedGroup %u", (unsigned) group ) ); + p += key_exchange_len; continue; } ssl->handshake->offered_group_id = group; + p += key_exchange_len; } - if( match_found == 0 ) + + if( ssl->handshake->offered_group_id == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching key share" ) ); return( SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH ); @@ -436,9 +439,9 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, */ ssl->tls_version = MBEDTLS_SSL_VERSION_TLS1_3; - /* --- - * Random random; - * --- + /* ... + * Random random; + * ... * with Random defined as: * opaque Random[32]; */ @@ -448,9 +451,9 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_CLIENT_HELLO_RANDOM_LEN ); p += MBEDTLS_CLIENT_HELLO_RANDOM_LEN; - /* --- + /* ... * opaque legacy_session_id<0..32>; - * --- + * ... */ legacy_session_id_len = p[0]; p++; @@ -481,9 +484,9 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len + 2 + 2 ); - /* --- + /* ... * CipherSuite cipher_suites<2..2^16-2>; - * --- + * ... * with CipherSuite defined as: * uint8 CipherSuite[2]; */ @@ -505,10 +508,12 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * Check whether this ciphersuite is valid and offered. */ if( ( mbedtls_ssl_validate_ciphersuite( - ssl, ciphersuite_info, ssl->tls_version, - ssl->tls_version ) != 0 ) || - !mbedtls_ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) ) + ssl, ciphersuite_info, ssl->tls_version, + ssl->tls_version ) != 0 ) || + ! mbedtls_ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) ) + { continue; + } ssl->session_negotiate->ciphersuite = cipher_suite; ssl->handshake->ciphersuite_info = ciphersuite_info; @@ -518,7 +523,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, } - if( !ciphersuite_match ) + if( ! ciphersuite_match ) { MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); @@ -529,6 +534,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ciphersuite_info->name ) ); p = cipher_suites + cipher_suites_len; + /* ... * opaque legacy_compression_methods<1..2^8-1>; * ... @@ -542,9 +548,9 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, } p += 2; - /* --- + /* ... * Extension extensions<8..2^16-1>; - * --- + * ... * with Extension defined as: * struct { * ExtensionType extension_type; @@ -584,8 +590,8 @@ 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 = ssl_tls13_parse_supported_groups_ext( ssl, p, - extension_data_end ); + ret = ssl_tls13_parse_supported_groups_ext( + ssl, p, extension_data_end ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, @@ -608,7 +614,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * contains the endpoint's cryptographic parameters for * ECDHE/DHE key establishment methods. */ - ret = ssl_tls13_parse_key_shares_ext( ssl, p, extension_data_end ); + ret = ssl_tls13_parse_key_shares_ext( + ssl, p, extension_data_end ); if( ret == SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "HRR needed " ) ); @@ -630,7 +637,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported versions extension" ) ); ret = ssl_tls13_parse_supported_versions_ext( - ssl, p, extension_data_end ); + ssl, p, extension_data_end ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, @@ -644,8 +651,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, case MBEDTLS_TLS_EXT_SIG_ALG: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found signature_algorithms extension" ) ); - ret = mbedtls_ssl_tls13_parse_sig_alg_ext( ssl, p, - extension_data_end ); + ret = mbedtls_ssl_tls13_parse_sig_alg_ext( + ssl, p, extension_data_end ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, @@ -908,6 +915,7 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, if( ret != 0 ) return( ret ); p += key_exchange_length; + MBEDTLS_PUT_UINT16_BE( key_exchange_length, server_share + 2, 0 ); MBEDTLS_PUT_UINT16_BE( p - server_share, buf, 2 ); From fbe3e64b762e76ab2bd77f84077db9788c03db41 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 25 Apr 2022 19:31:51 +0800 Subject: [PATCH 08/21] fix various issues Signed-off-by: Jerry Yu --- library/ssl_misc.h | 3 ++- library/ssl_tls13_generic.c | 11 ++++++----- library/ssl_tls13_server.c | 13 +++++++------ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 7cf677a430..9b4ad3e73b 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1628,7 +1628,8 @@ static inline int mbedtls_ssl_conf_is_hybrid_tls12_tls13( const mbedtls_ssl_conf #endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) -extern const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[32]; +extern const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[ + MBEDTLS_SERVER_HELLO_RANDOM_LEN ]; int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ); int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ); void mbedtls_ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 4332a1d0c0..06ee46bd8d 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -34,11 +34,12 @@ #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" -const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[32] = - { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, - 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, - 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, 0x07, 0x9E, 0x09, - 0xE2, 0xC8, 0xA8, 0x33 ,0x9C }; +const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[ + MBEDTLS_SERVER_HELLO_RANDOM_LEN ] = + { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, + 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, + 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, + 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33, 0x9C }; int mbedtls_ssl_tls13_fetch_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f65a33ace0..4c9f466e3f 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1013,7 +1013,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *p = buf; unsigned char *p_extensions_len; - size_t output_len; /* Length of buffer used by function */ + size_t output_len; *out_len = 0; @@ -1037,12 +1037,12 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, if( is_hrr ) { memcpy( p, mbedtls_ssl_tls13_hello_retry_request_magic, - MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); } else { memcpy( p, &ssl->handshake->randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN], - MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); } MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); @@ -1385,7 +1385,7 @@ static int ssl_tls13_write_hello_retry_request_coordinate( mbedtls_ssl_context *ssl ) { int ret; - if( ssl->handshake->hello_retry_request_count > 1 ) + if( ssl->handshake->hello_retry_request_count > 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Too many HRRs" ) ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); @@ -1416,8 +1416,9 @@ static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_hello_retry_request_coordinate( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, - MBEDTLS_SSL_HS_SERVER_HELLO, &buf, &buf_len ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( + ssl, MBEDTLS_SSL_HS_SERVER_HELLO, + &buf, &buf_len ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_server_hello_body( ssl, buf, buf + buf_len, From 086edc2807e26633ae5db5e43a5c654594c8cc96 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 5 May 2022 10:50:38 +0800 Subject: [PATCH 09/21] refactor parse key_share ext Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 4c9f466e3f..e451ba4bb0 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -203,6 +203,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, { uint16_t group; size_t key_exchange_len; + const unsigned char *key_exchange; /* * struct { @@ -214,7 +215,9 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, group = MBEDTLS_GET_UINT16_BE( p, 0 ); key_exchange_len = MBEDTLS_GET_UINT16_BE( p, 2 ); p += 4; + key_exchange = p; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, client_shares_end, key_exchange_len ); + p += key_exchange_len; /* Continue parsing even if we have already found a match, * for input validation purposes. @@ -223,7 +226,6 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, ! mbedtls_ssl_named_group_is_supported( group ) || ssl->handshake->offered_group_id != 0 ) { - p += key_exchange_len; continue; } @@ -236,7 +238,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, mbedtls_ssl_named_group_to_str( group ), group ) ); ret = mbedtls_ssl_tls13_read_public_ecdhe_share( - ssl, p - 2, key_exchange_len + 2 ); + ssl, key_exchange - 2, key_exchange_len + 2 ); if( ret != 0 ) return( ret ); @@ -245,12 +247,10 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 4, ( "Unrecognized NamedGroup %u", (unsigned) group ) ); - p += key_exchange_len; continue; } ssl->handshake->offered_group_id = group; - p += key_exchange_len; } From 49ca92892d8ff4fdc303e4888ca36874339e9ab0 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 5 May 2022 11:05:22 +0800 Subject: [PATCH 10/21] refactor HRR routine Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index e451ba4bb0..b67aba5192 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -391,7 +391,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, const unsigned char *cipher_suites_end; size_t extensions_len; const unsigned char *extensions_end; - int hrr_required = SSL_CLIENT_HELLO_OK; + int hrr_required = 0; const mbedtls_ssl_ciphersuite_t* ciphersuite_info; @@ -619,7 +619,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, if( ret == SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "HRR needed " ) ); - hrr_required |= SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH; + hrr_required = 1; } if( ret < 0 ) @@ -699,7 +699,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - return( hrr_required ); + return( hrr_required ? SSL_CLIENT_HELLO_HRR_REQUIRED : SSL_CLIENT_HELLO_OK ); } /* Update the handshake state machine */ @@ -730,7 +730,7 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char* buf = NULL; size_t buflen = 0; - int hrr_required ; + int parse_client_hello_ret ; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( @@ -740,10 +740,10 @@ 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; + parse_client_hello_ret = ret; MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); - if( hrr_required == SSL_CLIENT_HELLO_OK ) + if( parse_client_hello_ret == SSL_CLIENT_HELLO_OK ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); else mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); From b0ac10b4a8e71bcc0d5b6bf41b557f96a65fac85 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 5 May 2022 11:10:08 +0800 Subject: [PATCH 11/21] Refactor hrr key_share Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b67aba5192..da6e1673e3 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -945,8 +945,12 @@ static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, *out_len = 0; - /* For a pure PSK-based ciphersuite there is no key share to declare. */ - if( ! mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) + /* + * For a pure PSK key exchange, there is no group to agree upon. The purpose + * of the HRR is then to transmit a cookie to force the client to demonstrate + * reachability at their apparent network address (primarily useful for DTLS). + */ + if( ! mbedtls_ssl_tls13_some_ephemeral_enabled( ssl ) ) return( 0 ); /* We should only send the key_share extension if the client's initial @@ -963,23 +967,15 @@ static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } - if( ! mbedtls_ssl_named_group_is_offered( ssl, selected_group ) || - ! mbedtls_ssl_named_group_is_supported( selected_group ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 4, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* extension header, extension length, NamedGroup value */ + /* Check if we have enough space: + * - extension_type (2 bytes) + * - extension_data_length (2 bytes) + * - selected_group (2 bytes) + */ MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 6 ); - /* Write extension header */ MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, buf, 0 ); - - /* Write extension length */ MBEDTLS_PUT_UINT16_BE( 2, buf, 2 ); - - /* Write selected group */ MBEDTLS_PUT_UINT16_BE( selected_group, buf, 4 ); MBEDTLS_SSL_DEBUG_MSG( 3, @@ -988,8 +984,8 @@ static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, selected_group ) ); *out_len = 6; - return( 0 ); + return( 0 ); } /* From 6a2cd9ebf5fc9ef7b5cd68a7a5b5fbeed3435398 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 5 May 2022 11:14:19 +0800 Subject: [PATCH 12/21] fix various issues Signed-off-by: Jerry Yu --- library/ssl_misc.h | 2 +- library/ssl_tls13_server.c | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 9b4ad3e73b..7bf57c2e5f 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -586,7 +586,7 @@ struct mbedtls_ssl_handshake_params #if defined(MBEDTLS_SSL_PROTO_TLS1_3) int tls13_kex_modes; /*!< key exchange modes for TLS 1.3 */ - /* Number of HelloRetryRequest messages received/sent from the server. */ + /** Number of HelloRetryRequest messages received/sent from/to the server. */ int hello_retry_request_count; #if defined(MBEDTLS_SSL_SRV_C) /** selected_group of key_share extension in HelloRetryRequest message. */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index da6e1673e3..ec0df65f90 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1380,7 +1380,7 @@ cleanup: static int ssl_tls13_write_hello_retry_request_coordinate( mbedtls_ssl_context *ssl ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; if( ssl->handshake->hello_retry_request_count > 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Too many HRRs" ) ); @@ -1404,7 +1404,7 @@ static int ssl_tls13_write_hello_retry_request_coordinate( static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *buf; size_t buf_len, msg_len; @@ -1454,9 +1454,7 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { /* start state */ case MBEDTLS_SSL_HELLO_REQUEST: - ssl->handshake->hello_retry_request_count = 0; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); - ret = 0; break; @@ -1472,7 +1470,6 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) ret = ssl_tls13_write_server_hello( ssl ); break; -<<<<<<< HEAD case MBEDTLS_SSL_ENCRYPTED_EXTENSIONS: ret = ssl_tls13_write_encrypted_extensions( ssl ); if( ret != 0 ) @@ -1480,10 +1477,14 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_encrypted_extensions", ret ); return( ret ); } -======= + case MBEDTLS_SSL_HELLO_RETRY_REQUEST: ret = ssl_tls13_write_hello_retry_request( ssl ); ->>>>>>> tls13:hrr:add empty frame work + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_hello_retry_request", ret ); + return( ret ); + } break; #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) From ab8bea23e6a5628e288553d073ce377428f23b2e Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 5 May 2022 11:19:38 +0800 Subject: [PATCH 13/21] fix comment and name issues in debug helper Signed-off-by: Jerry Yu --- scripts/generate_ssl_debug_helpers.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/generate_ssl_debug_helpers.py b/scripts/generate_ssl_debug_helpers.py index 86a54ea8a8..4be6fd6c06 100755 --- a/scripts/generate_ssl_debug_helpers.py +++ b/scripts/generate_ssl_debug_helpers.py @@ -300,19 +300,19 @@ class NamedGroupDefinition: Generate helper functions for named group It generates translation function from named group define to string. - Signature algorithm definition looks like: + Named group definition looks like: #define MBEDTLS_SSL_IANA_TLS_GROUP_[ upper case named group ] [ value(hex) ] Known limitation: - - the definitions SHOULD exist in same macro blocks. + - the definitions SHOULD exist in same macro blocks. """ @classmethod def extract(cls, source_code, start=0, end=-1): - sig_alg_pattern = re.compile(r'#define\s+(?PMBEDTLS_SSL_IANA_TLS_GROUP_\w+)\s+' + - r'(?P0[xX][0-9a-fA-F]+)$', - re.MULTILINE | re.DOTALL) - matches = list(sig_alg_pattern.finditer(source_code, start, end)) + named_group_pattern = re.compile(r'#define\s+(?PMBEDTLS_SSL_IANA_TLS_GROUP_\w+)\s+' + + r'(?P0[xX][0-9a-fA-F]+)$', + re.MULTILINE | re.DOTALL) + matches = list(named_group_pattern.finditer(source_code, start, end)) if matches: yield NamedGroupDefinition(source_code, definitions=matches) From ede50ea891b8f1765050e56b70cce369f853c62e Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 5 May 2022 11:21:20 +0800 Subject: [PATCH 14/21] move hrr tests Signed-off-by: Jerry Yu --- tests/opt-testcases/tls13-generic.sh | 37 ---------------------------- tests/ssl-opt.sh | 19 ++++++++++++++ 2 files changed, 19 insertions(+), 37 deletions(-) delete mode 100755 tests/opt-testcases/tls13-generic.sh diff --git a/tests/opt-testcases/tls13-generic.sh b/tests/opt-testcases/tls13-generic.sh deleted file mode 100755 index 67d62eee7e..0000000000 --- a/tests/opt-testcases/tls13-generic.sh +++ /dev/null @@ -1,37 +0,0 @@ -#!/bin/sh - -# tls13-generic.sh -# -# Copyright The Mbed TLS Contributors -# SPDX-License-Identifier: Apache-2.0 -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -requires_config_enabled MBEDTLS_DEBUG_C -requires_config_enabled MBEDTLS_SSL_CLI_C -requires_config_enabled MBEDTLS_SSL_SRV_C -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 -run_test "TLS 1.3: server: HRR check - mbedtls" \ - "$P_SRV debug_level=4 force_version=tls13 curves=secp384r1" \ - "$P_CLI debug_level=4 force_version=tls13 curves=secp256r1,secp384r1" \ - 1 \ - -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ - -s "tls13 server state: MBEDTLS_SSL_HELLO_RETRY_REQUEST" \ - -c "client state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ - -s "selected_group: secp384r1" \ - -s "SSL - The requested feature is not available" \ - -s "=> write hello retry request" \ - -s "<= write hello retry request" diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 591b6d39ff..6de870d83e 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11401,6 +11401,25 @@ run_test "TLS 1.3: Server side check - mbedtls with client authentication" \ -s "=> parse client hello" \ -s "<= parse client hello" + +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +run_test "TLS 1.3: server: HRR check - mbedtls" \ + "$P_SRV debug_level=4 force_version=tls13 curves=secp384r1" \ + "$P_CLI debug_level=4 force_version=tls13 curves=secp256r1,secp384r1" \ + 1 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_HELLO_RETRY_REQUEST" \ + -c "client state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "selected_group: secp384r1" \ + -s "SSL - The requested feature is not available" \ + -s "=> write hello retry request" \ + -s "<= write hello retry request" + for i in opt-testcases/*.sh do TEST_SUITE_NAME=${i##*/} From b8ac19a2961468c3ca20ed4ea64153e3765c2110 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 5 May 2022 11:35:53 +0800 Subject: [PATCH 15/21] send alert when second hrr needed Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index ec0df65f90..3730483d8a 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1384,6 +1384,8 @@ static int ssl_tls13_write_hello_retry_request_coordinate( if( ssl->handshake->hello_retry_request_count > 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Too many HRRs" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, + MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } From 7f157eb31fdf9e9bb7533075aa2558516a5fb7d3 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 6 May 2022 11:28:00 +0800 Subject: [PATCH 16/21] Change alert message Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 3730483d8a..bd3d608579 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1384,8 +1384,8 @@ static int ssl_tls13_write_hello_retry_request_coordinate( if( ssl->handshake->hello_retry_request_count > 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Too many HRRs" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, - MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } From 483305683322b1fd573ec578d97646cd29e60848 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 6 May 2022 21:35:44 +0800 Subject: [PATCH 17/21] fix ci test fails Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index bd3d608579..3c57f81fad 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1479,6 +1479,7 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_encrypted_extensions", ret ); return( ret ); } + break; case MBEDTLS_SSL_HELLO_RETRY_REQUEST: ret = ssl_tls13_write_hello_retry_request( ssl ); From 66d9e6f4058a925cd1c76c1ffa5f9b6d2e342d59 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sat, 7 May 2022 10:50:12 +0800 Subject: [PATCH 18/21] refactor next state of client hello Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 3c57f81fad..baf8c61fea 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -743,10 +743,21 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) parse_client_hello_ret = ret; MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); - if( parse_client_hello_ret == SSL_CLIENT_HELLO_OK ) - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); - else - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); + switch( parse_client_hello_ret ) + { + case SSL_CLIENT_HELLO_OK: + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); + break; + + case SSL_CLIENT_HELLO_HRR_REQUIRED: + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); + break; + + default: + MBEDTLS_SSL_DEBUG_MSG( 2, ( "should never happen" ) ); + ret = parse_client_hello_ret; + break; + } cleanup: From 4ca9140d433afaf254e533a5cf92db6461c97892 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 9 May 2022 15:50:57 +0800 Subject: [PATCH 19/21] fix coding style issues Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index baf8c61fea..d68403fc71 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -730,7 +730,8 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char* buf = NULL; size_t buflen = 0; - int parse_client_hello_ret ; + int parse_client_hello_ret; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( @@ -739,25 +740,14 @@ 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 ) ); - parse_client_hello_ret = ret; + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); - switch( parse_client_hello_ret ) - { - case SSL_CLIENT_HELLO_OK: - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); - break; - - case SSL_CLIENT_HELLO_HRR_REQUIRED: - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); - break; - - default: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "should never happen" ) ); - ret = parse_client_hello_ret; - break; - } + if( parse_client_hello_ret == SSL_CLIENT_HELLO_OK ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); + else + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); cleanup: From ead5cce22cc4b0d4d61c97846e8b6d23c60a9ae9 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 9 May 2022 15:58:50 +0800 Subject: [PATCH 20/21] improve readability Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index d68403fc71..bf863d5eaa 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -740,7 +740,9 @@ 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 ) ); - parse_client_hello_ret = ret; + parse_client_hello_ret = ret; /* store return reason of parse_client_hello + without error. on error, this statment will + not be called.*/ MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); From f41553b662a0bfee18f23e77c12000190e3f94e4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 9 May 2022 22:20:30 +0800 Subject: [PATCH 21/21] fix various issues Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index bf863d5eaa..46a6a49dda 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -740,9 +740,11 @@ 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 ) ); - parse_client_hello_ret = ret; /* store return reason of parse_client_hello - without error. on error, this statment will - not be called.*/ + parse_client_hello_ret = ret; /* Store return value of parse_client_hello, + * only SSL_CLIENT_HELLO_OK or + * SSL_CLIENT_HELLO_HRR_REQUIRED at this + * stage as negative error codes are handled + * by MBEDTLS_SSL_PROC_CHK_NEG. */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); @@ -1464,11 +1466,18 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_CLIENT_HELLO: - ret = ssl_tls13_process_client_hello( ssl ); if( ret != 0 ) MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_process_client_hello", ret ); + break; + case MBEDTLS_SSL_HELLO_RETRY_REQUEST: + ret = ssl_tls13_write_hello_retry_request( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_hello_retry_request", ret ); + return( ret ); + } break; case MBEDTLS_SSL_SERVER_HELLO: @@ -1484,15 +1493,6 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) } break; - case MBEDTLS_SSL_HELLO_RETRY_REQUEST: - ret = ssl_tls13_write_hello_retry_request( ssl ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_hello_retry_request", ret ); - return( ret ); - } - break; - #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) case MBEDTLS_SSL_CERTIFICATE_REQUEST: ret = ssl_tls13_write_certificate_request( ssl );