Address various issues

Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
This commit is contained in:
Jerry Yu 2021-10-13 13:36:05 +08:00
parent 435208a949
commit b85277e3af
4 changed files with 95 additions and 114 deletions

View file

@ -808,24 +808,21 @@ int mbedtls_ecdh_setup_no_everest( mbedtls_ecdh_context *ctx,
static int ecdh_tls13_read_public_internal( mbedtls_ecdh_context_mbed *ctx,
const unsigned char *buf,
size_t blen )
size_t buf_len )
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
const unsigned char *p = buf;
size_t data_len;
if( blen < 3 )
if( buf_len < 3 )
return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
data_len = MBEDTLS_GET_UINT16_BE( p, 0 );
p += 2;
if( data_len < 1 || data_len != ( blen - 2 ) )
if( data_len < 1 || data_len != ( buf_len - 2 ) )
return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
/*
* Save buffer start for read_binary and update buf
*/
if( ( ret = mbedtls_ecp_point_read_binary( &ctx->grp,
&ctx->Qp, p, data_len ) ) != 0)
{
@ -840,13 +837,13 @@ static int ecdh_tls13_read_public_internal( mbedtls_ecdh_context_mbed *ctx,
*/
int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx,
const unsigned char *buf,
size_t blen )
size_t buf_len )
{
ECDH_VALIDATE_RET( ctx != NULL );
ECDH_VALIDATE_RET( buf != NULL );
#if defined(MBEDTLS_ECDH_LEGACY_CONTEXT)
return( ecdh_tls13_read_public_internal( ctx, buf, blen ) );
return( ecdh_tls13_read_public_internal( ctx, buf, buf_len ) );
#else
switch( ctx->var )
{
@ -856,7 +853,7 @@ int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx,
#endif
case MBEDTLS_ECDH_VARIANT_MBEDTLS_2_0:
return( ecdh_tls13_read_public_internal( &ctx->ctx.mbed_ecdh,
buf, blen ) );
buf, buf_len ) );
default:
return MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
}

View file

@ -36,19 +36,19 @@ int mbedtls_ecdh_setup_no_everest( mbedtls_ecdh_context *ctx,
mbedtls_ecp_group_id grp_id );
/*
* TLS 1.3 version of mbedtls_ecdh_make_params in ecdh.h
* TLS 1.3 version of mbedtls_ecdh_make_params
*/
int mbedtls_ecdh_tls13_make_params( mbedtls_ecdh_context *ctx, size_t *olen,
unsigned char *buf, size_t blen,
unsigned char *buf, size_t buf_len,
int ( *f_rng )( void *, unsigned char *, size_t ),
void *p_rng );
/*
* TLS 1.3 version of mbedtls_ecdh_read_public in ecdh.h
* TLS 1.3 version of mbedtls_ecdh_read_public
*/
int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx,
const unsigned char *buf,
size_t blen );
size_t buf_len );
#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */

View file

@ -97,12 +97,12 @@ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl,
static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl,
const unsigned char *buf,
size_t buf_len )
const unsigned char *end )
{
((void) ssl);
if( buf_len != 2 ||
buf[0] != MBEDTLS_SSL_MAJOR_VERSION_3 ||
MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2);
if( buf[0] != MBEDTLS_SSL_MAJOR_VERSION_3 ||
buf[1] != MBEDTLS_SSL_MINOR_VERSION_4 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "unexpected version" ) );
@ -497,7 +497,7 @@ static int ssl_tls13_check_ecdh_params( const mbedtls_ssl_context *ssl )
MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) );
if( mbedtls_ssl_check_curve( ssl, grp_id ) != 0 )
return( -1 );
return( -1 );
MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx,
MBEDTLS_DEBUG_ECDH_QP );
@ -505,12 +505,6 @@ static int ssl_tls13_check_ecdh_params( const mbedtls_ssl_context *ssl )
return( 0 );
}
/* The ssl_tls13_parse_key_share_ext() function is used
* by the client to parse a KeyShare extension in
* a Server Hello message.
*
* The server only provides a single KeyShareEntry.
*/
static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl,
const unsigned char *buf,
size_t buf_len )
@ -522,12 +516,16 @@ static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl,
if( ret != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ecdh_tls13_read_public" ), ret );
return( ret );
MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
}
if( ssl_tls13_check_ecdh_params( ssl ) != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "ssl_tls13_check_ecdh_params() failed!" ) );
MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
@ -538,7 +536,9 @@ static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl,
#endif /* MBEDTLS_ECDH_C */
/*
* Parse key_share extension in Server Hello
* ssl_tls13_parse_key_share_ext()
* Parse key_share extension in Server Hello
*
* struct {
* KeyShareEntry server_share;
* } KeyShareServerHello;
@ -551,7 +551,7 @@ static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end )
{
int ret = 0;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
const unsigned char *p = buf;
uint16_t group, offered_group;
@ -583,8 +583,9 @@ static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl,
if( ret != 0 )
return( ret );
}
else
#endif /* MBEDTLS_ECDH_C */
else if( 0 /* other KEMs? */ )
if( 0 /* other KEMs? */ )
{
/* Do something */
}
@ -883,9 +884,18 @@ cleanup:
/*
* Functions for parsing and processing Server Hello
*/
static int ssl_server_hello_is_hrr( unsigned const char *buf, size_t blen )
/* Fetch and preprocess
* Returns a negative value on failure, and otherwise
* - SSL_SERVER_HELLO_COORDINATE_HELLO or
* - SSL_SERVER_HELLO_COORDINATE_HRR
* to indicate which message is expected and to be parsed next. */
#define SSL_SERVER_HELLO_COORDINATE_HELLO 0
#define SSL_SERVER_HELLO_COORDINATE_HRR 1
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[32] =
static const unsigned char magic_hrr_string[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,
@ -902,31 +912,23 @@ static int ssl_server_hello_is_hrr( unsigned const char *buf, size_t blen )
* opaque legacy_session_id_echo<0..32>;
* CipherSuite cipher_suite;
* uint8 legacy_compression_method = 0;
* Extension extensions<6..2 ^ 16 - 1>;
* Extension extensions<6..2^16-1>;
* } ServerHello;
*
*/
if( blen < 2 + sizeof( magic_hrr_string ) )
return (MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL );
MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2 + sizeof( magic_hrr_string ) );
if( memcmp( buf + 2, magic_hrr_string, sizeof( magic_hrr_string ) ) == 0 )
{
return( 1 );
return( SSL_SERVER_HELLO_COORDINATE_HRR );
}
return( 0 );
return( SSL_SERVER_HELLO_COORDINATE_HELLO );
}
/* Fetch and preprocess
* Returns a negative value on failure, and otherwise
* - SSL_SERVER_HELLO_COORDINATE_HELLO or
* - SSL_SERVER_HELLO_COORDINATE_HRR
* to indicate which message is expected and to be parsed next. */
#define SSL_SERVER_HELLO_COORDINATE_HELLO 0
#define SSL_SERVER_HELLO_COORDINATE_HRR 1
static int ssl_server_hello_coordinate( mbedtls_ssl_context *ssl,
unsigned char **buf,
size_t *buf_len )
static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl,
unsigned char **buf,
size_t *buf_len )
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
@ -945,15 +947,15 @@ static int ssl_server_hello_coordinate( mbedtls_ssl_context *ssl,
*buf = ssl->in_msg + 4;
*buf_len = ssl->in_hslen - 4;
if( ssl_server_hello_is_hrr( *buf, *buf_len ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) );
ret = SSL_SERVER_HELLO_COORDINATE_HRR;
}
else
ret = ssl_server_hello_is_hrr( ssl, *buf, *buf + *buf_len );
switch( ret )
{
case SSL_SERVER_HELLO_COORDINATE_HELLO:
MBEDTLS_SSL_DEBUG_MSG( 2, ( "received ServerHello message" ) );
ret = SSL_SERVER_HELLO_COORDINATE_HELLO;
break;
case SSL_SERVER_HELLO_COORDINATE_HRR:
MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) );
break;
}
cleanup:
@ -977,10 +979,6 @@ static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ss
if( ssl->session_negotiate->id_len != legacy_session_id_echo_len ||
memcmp( ssl->session_negotiate->id, p , legacy_session_id_echo_len ) != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "Mismatch of session id length:"
" id_len = %" MBEDTLS_PRINTF_SIZET
" , legacy_session_id_echo_len = %" MBEDTLS_PRINTF_SIZET,
ssl->session_negotiate->id_len, legacy_session_id_echo_len ) );
MBEDTLS_SSL_DEBUG_BUF( 3, "Expected Session ID",
ssl->session_negotiate->id,
ssl->session_negotiate->id_len );
@ -1025,17 +1023,17 @@ static int ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl,
* opaque legacy_session_id_echo<0..32>;
* CipherSuite cipher_suite;
* uint8 legacy_compression_method = 0;
* Extension extensions<6..2 ^ 16 - 1>;
* Extension extensions<6..2^16-1>;
* } ServerHello;
*/
static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end )
{
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
const unsigned char *p = buf;
size_t extensions_len; /* Length of field */
const unsigned char *extensions_end; /* Pointer to end of individual extension */
size_t extensions_len;
const unsigned char *extensions_end;
uint16_t cipher_suite;
const mbedtls_ssl_ciphersuite_t *ciphersuite_info;
@ -1054,7 +1052,7 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, version", p, 2 );
/* ...
* ProtocaolVersion legacy_version = 0x0303; // TLS 1.2
* ProtocolVersion legacy_version = 0x0303; // TLS 1.2
* ...
* with ProtocolVersion defined as:
* uint16 ProtocolVersion;
@ -1112,9 +1110,8 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
if( ciphersuite_info == NULL ||
ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) == 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite info for %04x not found",
MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite(%04x) not found or not offered",
cipher_suite ) );
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) );
MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
@ -1142,17 +1139,16 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 );
if( p[0] != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) );
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad legacy compression method" ) );
MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
}
p++;
/*
* ....
* Extension extensions<6..2 ^ 16 - 1>;
* ....
/* ...
* Extension extensions<6..2^16-1>;
* ...
* struct {
* ExtensionType extension_type; (2 bytes)
* opaque extension_data<0..2^16-1>;
@ -1166,9 +1162,6 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len );
extensions_end = p + extensions_len;
MBEDTLS_SSL_DEBUG_MSG( 3,
( "server hello, total extension length: %" MBEDTLS_PRINTF_SIZET ,
extensions_len ) );
MBEDTLS_SSL_DEBUG_BUF( 3, "server hello extensions", p, extensions_len );
while( p < extensions_end )
@ -1190,7 +1183,8 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
( "found supported_versions extension" ) );
ret = ssl_tls13_parse_supported_versions_ext( ssl,
p, extension_data_len );
p,
p + extension_data_len );
if( ret != 0 )
return( ret );
break;
@ -1238,45 +1232,39 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl )
{
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
mbedtls_ssl_key_set traffic_keys;
mbedtls_ssl_transform *transform_handshake = NULL;
mbedtls_ssl_handshake_params *handshake = ssl->handshake;
/* We need to set the key exchange algorithm based on the
* following rules:
*
* 1) IF PRE_SHARED_KEY extension was received
* THEN set KEY_EXCHANGE_MODE_PSK_EPHEMERAL;
* 2) IF PRE_SHARED_KEY extension && KEY_SHARE was received
* THEN set KEY_EXCHANGE_MODE_PSK;
* 3) IF KEY_SHARES extension was received && SIG_ALG extension received
* THEN set KEY_EXCHANGE_MODE_EPHEMERAL
* ELSE unknown key exchange mechanism.
/* Determine the key exchange mode:
* 1) If both the pre_shared_key and key_share extensions were received
* then the key exchange mode is PSK with EPHEMERAL.
* 2) If only the pre_shared_key extension was received then the key
* exchange mode is PSK-only.
* 3) If only the key_share extension was received then the key
* exchange mode is EPHEMERAL-only.
*/
if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PRE_SHARED_KEY )
switch( handshake->extensions_present &
( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ) )
{
if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE )
{
/* Condition 2) */
ssl->handshake->tls1_3_kex_modes =
MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL;
}
else
{
/* Condition 1) */
ssl->handshake->tls1_3_kex_modes =
MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK;
}
}
else if( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) )
{
/* Condition 3) */
ssl->handshake->tls1_3_kex_modes =
MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL;
}
else
{
/* ELSE case */
/* Only the pre_shared_key extension was received */
case MBEDTLS_SSL_EXT_PRE_SHARED_KEY:
handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK;
break;
/* Only the key_share extension was received */
case MBEDTLS_SSL_EXT_KEY_SHARE:
handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL;
break;
/* Both the pre_shared_key and key_share extensions were received */
case ( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ):
handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL;
break;
/* Neither pre_shared_key nor key_share extension was received */
default:
MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unknown key exchange." ) );
ret = MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE;
goto cleanup;
@ -1313,8 +1301,7 @@ static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl )
goto cleanup;
}
transform_handshake =
mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) );
transform_handshake = mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) );
if( transform_handshake == NULL )
{
ret = MBEDTLS_ERR_SSL_ALLOC_FAILED;
@ -1332,8 +1319,8 @@ static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl )
goto cleanup;
}
ssl->handshake->transform_handshake = transform_handshake;
mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake );
handshake->transform_handshake = transform_handshake;
mbedtls_ssl_set_inbound_transform( ssl, transform_handshake );
MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) );
ssl->session_in = ssl->session_negotiate;
@ -1348,8 +1335,7 @@ cleanup:
mbedtls_platform_zeroize( &traffic_keys, sizeof( traffic_keys ) );
if( ret != 0 )
{
if( transform_handshake != NULL )
mbedtls_free( transform_handshake );
mbedtls_free( transform_handshake );
MBEDTLS_SSL_PEND_FATAL_ALERT(
MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE,
@ -1375,11 +1361,10 @@ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl )
* - Make sure it's either a ServerHello or a HRR.
* - Switch processing routine in case of HRR
*/
ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3;
ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE;
ret = ssl_server_hello_coordinate( ssl, &buf, &buf_len );
ret = ssl_tls13_server_hello_coordinate( ssl, &buf, &buf_len );
/* Parsing step
* We know what message to expect by now and call
* the respective parsing function.

View file

@ -1000,7 +1000,6 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl )
}
else if( mbedtls_ssl_tls13_named_group_is_dhe( handshake->offered_group_id ) )
{
/* TODO: Not supported yet */
MBEDTLS_SSL_DEBUG_MSG( 1, ( "DHE not supported." ) );
return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE );
}