Send TLS alerts in many more cases
The TLS client and server code was usually closing the connection in case of a fatal error without sending an alert. This commit adds alerts in many cases. Added one test case to detect that we send the alert, where a server complains that the client's certificate is from an unknown CA (case tracked internally as IOTSSL-1330).
This commit is contained in:
Gilles Peskine
parent
071db41627
commit
1cc8e3472a
4 changed files with 265 additions and 4 deletions
|
|
@ -101,6 +101,8 @@ static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl,
|
|||
if( servername_list_size + 2 != len )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -111,6 +113,8 @@ static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl,
|
|||
if( hostname_len + 3 > servername_list_size )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -135,6 +139,8 @@ static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl,
|
|||
if( servername_list_size != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -201,6 +207,8 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl,
|
|||
sig_alg_list_size % 2 != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -247,6 +255,8 @@ static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl,
|
|||
list_size % 2 != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -254,6 +264,8 @@ static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl,
|
|||
if( ssl->handshake->curves != NULL )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -264,7 +276,11 @@ static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl,
|
|||
our_size = MBEDTLS_ECP_DP_MAX;
|
||||
|
||||
if( ( curves = mbedtls_calloc( our_size, sizeof( *curves ) ) ) == NULL )
|
||||
{
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_ALLOC_FAILED );
|
||||
}
|
||||
|
||||
ssl->handshake->curves = curves;
|
||||
|
||||
|
|
@ -297,6 +313,8 @@ static int ssl_parse_supported_point_formats( mbedtls_ssl_context *ssl,
|
|||
if( list_size + 1 != len )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -342,6 +360,8 @@ static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl,
|
|||
buf, len ) ) != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_one", ret );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER );
|
||||
return( ret );
|
||||
}
|
||||
|
||||
|
|
@ -360,6 +380,8 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl,
|
|||
if( len != 1 || buf[0] >= MBEDTLS_SSL_MAX_FRAG_LEN_INVALID )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -377,6 +399,8 @@ static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl,
|
|||
if( len != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -397,6 +421,8 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl,
|
|||
if( len != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -420,6 +446,8 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl,
|
|||
if( len != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -531,11 +559,19 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl,
|
|||
|
||||
/* Min length is 2 (list_len) + 1 (name_len) + 1 (name) */
|
||||
if( len < 4 )
|
||||
{
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
list_len = ( buf[0] << 8 ) | buf[1];
|
||||
if( list_len != len - 2 )
|
||||
{
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
/*
|
||||
* Use our order of preference
|
||||
|
|
@ -549,13 +585,21 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl,
|
|||
{
|
||||
/* If the list is well formed, we should get equality first */
|
||||
if( theirs > end )
|
||||
{
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
cur_len = *theirs++;
|
||||
|
||||
/* Empty strings MUST NOT be included */
|
||||
if( cur_len == 0 )
|
||||
{
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
if( cur_len == ours_len &&
|
||||
memcmp( theirs, *ours, cur_len ) == 0 )
|
||||
|
|
@ -1064,6 +1108,9 @@ have_ciphersuite_v2:
|
|||
}
|
||||
#endif /* MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO */
|
||||
|
||||
/* This function doesn't alert on errors that happen early during
|
||||
ClientHello parsing because they might indicate that the client is
|
||||
not talking SSL/TLS at all and would not understand our alert. */
|
||||
static int ssl_parse_client_hello( mbedtls_ssl_context *ssl )
|
||||
{
|
||||
int ret, got_common_suite;
|
||||
|
|
@ -1098,6 +1145,7 @@ read_record_header:
|
|||
{
|
||||
if( ( ret = mbedtls_ssl_fetch_input( ssl, 5 ) ) != 0 )
|
||||
{
|
||||
/* No alert on a read error. */
|
||||
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_fetch_input", ret );
|
||||
return( ret );
|
||||
}
|
||||
|
|
@ -1349,10 +1397,8 @@ read_record_header:
|
|||
" [%d:%d] < [%d:%d]",
|
||||
ssl->major_ver, ssl->minor_ver,
|
||||
ssl->conf->min_major_ver, ssl->conf->min_minor_ver ) );
|
||||
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION );
|
||||
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION );
|
||||
}
|
||||
|
||||
|
|
@ -1380,6 +1426,8 @@ read_record_header:
|
|||
sess_len + 34 + 2 > msg_len ) /* 2 for cipherlist length field */
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -1403,6 +1451,8 @@ read_record_header:
|
|||
if( cookie_offset + 1 + cookie_len + 2 > msg_len )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -1435,6 +1485,7 @@ read_record_header:
|
|||
/* We know we didn't send a cookie, so it should be empty */
|
||||
if( cookie_len != 0 )
|
||||
{
|
||||
/* This may be an attacker's probe, so don't send an alert */
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
|
@ -1459,6 +1510,8 @@ read_record_header:
|
|||
( ciph_len % 2 ) != 0 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -1477,6 +1530,8 @@ read_record_header:
|
|||
comp_len + comp_offset + 1 > msg_len )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -1515,6 +1570,8 @@ read_record_header:
|
|||
if( msg_len < ext_offset + 2 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
|
||||
|
|
@ -1525,6 +1582,8 @@ read_record_header:
|
|||
msg_len != ext_offset + 2 + ext_len )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
}
|
||||
|
|
@ -1544,6 +1603,8 @@ read_record_header:
|
|||
if( ext_size + 4 > ext_len )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
switch( ext_id )
|
||||
|
|
@ -1689,6 +1750,8 @@ read_record_header:
|
|||
if( ext_len > 0 && ext_len < 4 )
|
||||
{
|
||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
|
||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
|
||||
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue