Update code base on review comments

Refine named_group parsing
Refine cipher_suites parsing
Remove hrr related part
Share code between client and server side
Some code style changes

Change-Id: Ia9ffd5ef9c0b64325f633241e0ea1669049fe33a
Signed-off-by: XiaokangQian <xiaokang.qian@arm.com>
This commit is contained in:
XiaokangQian 2022-04-20 07:16:41 +00:00
parent 17f974c63e
commit 0803755347
5 changed files with 113 additions and 83 deletions

View file

@ -98,8 +98,6 @@
/* Error space gap */ /* Error space gap */
/** Processing of the Certificate handshake message failed. */ /** Processing of the Certificate handshake message failed. */
#define MBEDTLS_ERR_SSL_BAD_CERTIFICATE -0x7A00 #define MBEDTLS_ERR_SSL_BAD_CERTIFICATE -0x7A00
/** Server needs to send a HelloRetryRequest */
#define MBEDTLS_ERR_SSL_HRR_REQUIRED -0x7A80
/* Error space gap */ /* Error space gap */
/* Error space gap */ /* Error space gap */
/* Error space gap */ /* Error space gap */

View file

@ -586,6 +586,11 @@ struct mbedtls_ssl_handshake_params
int hello_retry_request_count; int hello_retry_request_count;
#endif /* MBEDTLS_SSL_CLI_C */ #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 */
#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \
defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED)
mbedtls_ssl_sig_hash_set_t hash_algs; /*!< Set of suitable sig-hash pairs */ mbedtls_ssl_sig_hash_set_t hash_algs; /*!< Set of suitable sig-hash pairs */
@ -1856,6 +1861,39 @@ static inline int mbedtls_ssl_tls13_named_group_is_dhe( uint16_t named_group )
named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE8192 ); named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE8192 );
} }
static inline int mbedtls_ssl_named_group_is_offered(
const mbedtls_ssl_context *ssl, uint16_t named_group )
{
const uint16_t *group_list = mbedtls_ssl_get_groups( ssl );
if( group_list == NULL )
return( 0 );
for( ; *group_list != 0; group_list++ )
{
if( *group_list == named_group )
return( 1 );
}
return( 0 );
}
static inline int mbedtls_ssl_named_group_is_supported( uint16_t named_group )
{
#if defined(MBEDTLS_ECDH_C)
if( mbedtls_ssl_tls13_named_group_is_ecdhe( named_group ) )
{
const mbedtls_ecp_curve_info *curve_info =
mbedtls_ecp_curve_info_from_tls_id( named_group );
if( curve_info != NULL )
return( 1 );
}
#else
((void) named_group);
#endif /* MBEDTLS_ECDH_C */
return( 0 );
}
/* /*
* Return supported signature algorithms. * Return supported signature algorithms.
* *
@ -2180,4 +2218,7 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl,
#endif /* MBEDTLS_ECDH_C */ #endif /* MBEDTLS_ECDH_C */
int mbedtls_ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl,
int cipher_suite );
#endif /* ssl_misc.h */ #endif /* ssl_misc.h */

View file

@ -939,22 +939,6 @@ static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ss
return( 0 ); return( 0 );
} }
static int ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl,
int cipher_suite )
{
const int *ciphersuite_list = ssl->conf->ciphersuite_list;
/* Check whether we have offered this ciphersuite */
for ( size_t i = 0; ciphersuite_list[i] != 0; i++ )
{
if( ciphersuite_list[i] == cipher_suite )
{
return( 1 );
}
}
return( 0 );
}
/* Parse ServerHello message and configure context /* Parse ServerHello message and configure context
* *
* struct { * struct {
@ -1054,7 +1038,7 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
if( ( mbedtls_ssl_validate_ciphersuite( ssl, ciphersuite_info, if( ( mbedtls_ssl_validate_ciphersuite( ssl, ciphersuite_info,
ssl->tls_version, ssl->tls_version,
ssl->tls_version ) != 0 ) || ssl->tls_version ) != 0 ) ||
!ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) ) !mbedtls_ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) )
{ {
fatal_alert = MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER; fatal_alert = MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER;
} }

View file

@ -115,12 +115,8 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl,
MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x",
sig_alg ) ); sig_alg ) );
if( ! mbedtls_ssl_sig_alg_is_supported( ssl, sig_alg ) if( ! mbedtls_ssl_sig_alg_is_supported( ssl, sig_alg ) ||
#if defined(MBEDTLS_SSL_CLI_C) ! mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) )
|| ( ( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT )
&& ! mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) )
#endif /* MBEDTLS_SSL_CLI_C */
)
continue; continue;
if( common_idx + 1 < MBEDTLS_RECEIVED_SIG_ALGS_SIZE ) if( common_idx + 1 < MBEDTLS_RECEIVED_SIG_ALGS_SIZE )
@ -1541,4 +1537,20 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl,
} }
#endif /* MBEDTLS_ECDH_C */ #endif /* MBEDTLS_ECDH_C */
int mbedtls_ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl,
int cipher_suite )
{
const int *ciphersuite_list = ssl->conf->ciphersuite_list;
/* Check whether we have offered this ciphersuite */
for ( size_t i = 0; ciphersuite_list[i] != 0; i++ )
{
if( ciphersuite_list[i] == cipher_suite )
{
return( 1 );
}
}
return( 0 );
}
#endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */

View file

@ -24,6 +24,7 @@
#include "mbedtls/debug.h" #include "mbedtls/debug.h"
#include "ssl_misc.h" #include "ssl_misc.h"
#include "ssl_client.h"
#include "ssl_tls13_keys.h" #include "ssl_tls13_keys.h"
#include "ssl_debug_helpers.h" #include "ssl_debug_helpers.h"
#include <string.h> #include <string.h>
@ -110,7 +111,6 @@ static int ssl_tls13_parse_supported_groups_ext(
{ {
const unsigned char *p = buf; const unsigned char *p = buf;
size_t named_group_list_len; size_t named_group_list_len;
const mbedtls_ecp_curve_info *curve_info;
const unsigned char *named_group_list_end; const unsigned char *named_group_list_end;
MBEDTLS_SSL_DEBUG_BUF( 3, "supported_groups extension", p, end - buf ); MBEDTLS_SSL_DEBUG_BUF( 3, "supported_groups extension", p, end - buf );
@ -119,27 +119,30 @@ static int ssl_tls13_parse_supported_groups_ext(
p += 2; p += 2;
MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len );
named_group_list_end = p + named_group_list_len; named_group_list_end = p + named_group_list_len;
ssl->handshake->hrr_selected_group = 0;
while ( p < named_group_list_end ) while( p < named_group_list_end )
{ {
uint16_t tls_grp_id; uint16_t named_group;
MBEDTLS_SSL_CHK_BUF_READ_PTR( p, named_group_list_end, 2 ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, named_group_list_end, 2 );
tls_grp_id = MBEDTLS_GET_UINT16_BE( p, 0 ); named_group = MBEDTLS_GET_UINT16_BE( p, 0 );
curve_info = mbedtls_ecp_curve_info_from_tls_id( tls_grp_id ); p += 2;
if( curve_info != NULL ) MBEDTLS_SSL_DEBUG_MSG(
2, ( "got named group: %d",
named_group ) );
if( ! mbedtls_ssl_named_group_is_offered( ssl, named_group ) ||
! mbedtls_ssl_named_group_is_supported( named_group ) ||
ssl->handshake->hrr_selected_group != 0 )
{ {
continue;
MBEDTLS_SSL_DEBUG_MSG( 4, ( "supported curve: %s", curve_info->name ) );
/*
* Here we only update offered_group_id field with the first
* offered group
*/
ssl->handshake->offered_group_id = tls_grp_id;
break;
} }
p += 2; MBEDTLS_SSL_DEBUG_MSG(
2, ( "add named group (%04x) into received list.",
named_group ) );
ssl->handshake->hrr_selected_group = named_group;
} }
return( 0 ); return( 0 );
@ -147,6 +150,8 @@ static int ssl_tls13_parse_supported_groups_ext(
} }
#endif /* MBEDTLS_ECDH_C */ #endif /* MBEDTLS_ECDH_C */
#define SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH 1
#if defined(MBEDTLS_ECDH_C) #if defined(MBEDTLS_ECDH_C)
/* /*
* ssl_tls13_parse_key_shares_ext() verifies whether the information in the * ssl_tls13_parse_key_shares_ext() verifies whether the information in the
@ -155,7 +160,7 @@ static int ssl_tls13_parse_supported_groups_ext(
* *
* Possible return values are: * Possible return values are:
* - 0: Successful processing of the client provided key share extension. * - 0: Successful processing of the client provided key share extension.
* - MBEDTLS_ERR_SSL_HRR_REQUIRED: The key share provided by the client * - SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH: The key shares provided by the client
* does not match a group supported by the server. A HelloRetryRequest will * does not match a group supported by the server. A HelloRetryRequest will
* be needed. * be needed.
* - Another negative return value for fatal errors. * - Another negative return value for fatal errors.
@ -237,7 +242,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl,
MBEDTLS_SSL_DEBUG_RET( 1, "psa_crypto_init()", ret ); MBEDTLS_SSL_DEBUG_RET( 1, "psa_crypto_init()", ret );
return( ret ); return( ret );
} }
ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, end - p + 2 ); ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, key_exchange_len + 2 );
if( ret != 0 ) if( ret != 0 )
return( ret ); return( ret );
} }
@ -254,7 +259,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl,
if( match_found == 0 ) if( match_found == 0 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching key share" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching key share" ) );
return( MBEDTLS_ERR_SSL_HRR_REQUIRED ); return( SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH );
} }
return( 0 ); return( 0 );
} }
@ -388,7 +393,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl,
size_t extensions_len; size_t extensions_len;
const unsigned char *extensions_end; const unsigned char *extensions_end;
const int* cipher_suites;
const mbedtls_ssl_ciphersuite_t* ciphersuite_info; const mbedtls_ssl_ciphersuite_t* ciphersuite_info;
int hrr_required = 0; int hrr_required = 0;
@ -409,7 +413,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl,
*/ */
/* /*
* Needs to be updated due to mandatory extensions
* Minimal length ( with everything empty and extensions ommitted ) is * Minimal length ( with everything empty and extensions ommitted ) is
* 2 + 32 + 1 + 2 + 1 = 38 bytes. Check that first, so that we can * 2 + 32 + 1 + 2 + 1 = 38 bytes. Check that first, so that we can
* read at least up to session id length without worrying. * read at least up to session id length without worrying.
@ -438,20 +441,17 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl,
ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3;
ssl->minor_ver = MBEDTLS_SSL_MINOR_VERSION_4; ssl->minor_ver = MBEDTLS_SSL_MINOR_VERSION_4;
/* /* ---
* Save client random
*
* ---
* Random random; * Random random;
* --- * ---
* with Random defined as: * with Random defined as:
* opaque Random[32]; * opaque Random[32];
*/ */
MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes",
p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); p, MBEDTLS_CLIENT_HELLO_RANDOM_LEN );
memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_CLIENT_HELLO_RANDOM_LEN );
p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; p += MBEDTLS_CLIENT_HELLO_RANDOM_LEN;
/* --- /* ---
* opaque legacy_session_id<0..32>; * opaque legacy_session_id<0..32>;
@ -486,45 +486,41 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl,
*/ */
MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len + 2 + 2 ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len + 2 + 2 );
/* store pointer to ciphersuite list */ /* ---
* CipherSuite cipher_suites<2..2^16-2>;
* ---
* with CipherSuite defined as:
* uint8 CipherSuite[2];
*/
cipher_suites_start = p; cipher_suites_start = p;
MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, ciphersuitelist", MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, ciphersuitelist",
p, cipher_suites_len ); p, cipher_suites_len );
/* /*
* Search for a matching ciphersuite * Search for a matching ciphersuite
*/ */
size_t ciphersuite_exist = 0; size_t ciphersuite_exist = 0;
cipher_suites = ssl->conf->ciphersuite_list; uint16_t cipher_suite;
ciphersuite_info = NULL; ciphersuite_info = NULL;
for ( size_t j = 0; j < cipher_suites_len; for ( size_t j = 0; j < cipher_suites_len;
j += 2, p += 2 ) j += 2, p += 2 )
{ {
for ( size_t i = 0; cipher_suites[i] != 0; i++ ) cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 );
{ ciphersuite_info = mbedtls_ssl_ciphersuite_from_id(
if( MBEDTLS_GET_UINT16_BE(p, 0) == cipher_suites[i] ) cipher_suite );
{ /*
ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( * Check whether this ciphersuite is valid and offered.
cipher_suites[i] ); */
if( ( mbedtls_ssl_validate_ciphersuite(
ssl, ciphersuite_info, ssl->minor_ver, ssl->minor_ver ) != 0 ) ||
!mbedtls_ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) )
continue;
if( ciphersuite_info == NULL ) ssl->session_negotiate->ciphersuite = cipher_suite;
{ ssl->handshake->ciphersuite_info = ciphersuite_info;
MBEDTLS_SSL_DEBUG_MSG( ciphersuite_exist = 1;
1,
( "mbedtls_ssl_ciphersuite_from_id: should never happen" ) );
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
}
ssl->session_negotiate->ciphersuite = cipher_suites[i]; break;
ssl->handshake->ciphersuite_info = ciphersuite_info;
ciphersuite_exist = 1;
break;
}
}
} }
if( !ciphersuite_exist ) if( !ciphersuite_exist )
@ -614,7 +610,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl,
* ECDHE/DHE key establishment methods. * 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 == MBEDTLS_ERR_SSL_HRR_REQUIRED ) if( ret == SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH )
{ {
MBEDTLS_SSL_DEBUG_MSG( 2, ( "HRR needed " ) ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "HRR needed " ) );
ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE;
@ -703,7 +699,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl,
static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl )
{ {
int ret = 0; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
ret = mbedtls_ssl_tls13_key_schedule_stage_early( ssl ); ret = mbedtls_ssl_tls13_key_schedule_stage_early( ssl );
if( ret != 0 ) if( ret != 0 )
@ -725,7 +721,7 @@ static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl )
static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl )
{ {
int ret = 0; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
unsigned char* buf = NULL; unsigned char* buf = NULL;
size_t buflen = 0; size_t buflen = 0;
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) );
@ -751,7 +747,7 @@ cleanup:
*/ */
int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl )
{ {
int ret = 0; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
if( ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER || ssl->handshake == NULL ) if( ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER || ssl->handshake == NULL )
return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
@ -766,10 +762,9 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl )
case MBEDTLS_SSL_HELLO_REQUEST: case MBEDTLS_SSL_HELLO_REQUEST:
mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO );
ret = 0;
break; break;
/* ----- READ CLIENT HELLO ----*/
case MBEDTLS_SSL_CLIENT_HELLO: case MBEDTLS_SSL_CLIENT_HELLO:
ret = ssl_tls13_process_client_hello( ssl ); ret = ssl_tls13_process_client_hello( ssl );