From 8c28d3666be2b2591052e5e71b4e272ae264094c Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Fri, 8 Jan 2021 17:20:12 +0000 Subject: [PATCH 1/3] Introduce helper function to keep track of present extensions and avoid writing or parsing more than one extension of the same type in a given extension block (RFC requirement) --- include/mbedtls/ssl_internal.h | 23 ++++++++- library/ssl_tls13_client.c | 34 ++++++------ library/ssl_tls13_generic.c | 2 +- library/ssl_tls13_server.c | 94 +++++++++++++++++----------------- 4 files changed, 87 insertions(+), 66 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index f93e44f5d964..de5fd3baef36 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -833,7 +833,9 @@ struct mbedtls_ssl_handshake_params int max_minor_ver; /*!< max. minor version client*/ int cli_exts; /*!< client extension presence*/ #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) - int extensions_present; /*!< which extension were present; the */ + int extensions_present; /*!< which extensions are present; used + and set by + `mbedtls_ssl_extensions_present()`*/ #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) && defined(MBEDTLS_SSL_TLS13_CTLS) @@ -1743,4 +1745,23 @@ int mbedtls_ssl_double_retransmit_timeout( mbedtls_ssl_context *ssl ); void mbedtls_ssl_reset_retransmit_timeout( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ +/* Reset list of present extensions */ +static inline void mbedtls_ssl_reset_extensions_present( mbedtls_ssl_context *ssl ) +{ + ssl->handshake->extensions_present = NO_EXTENSION; +} + +/* Check presence of extensions and optionally set the extension flag */ +static inline int mbedtls_ssl_extensions_present( mbedtls_ssl_context *ssl, + int extension_flag, + int set_extension_flag ) +{ + int ret = 0; + + ret = ssl->handshake->extensions_present & extension_flag; + if( set_extension_flag ) + ssl->handshake->extensions_present |= extension_flag; + return( ret ); +} + #endif /* ssl_internal.h */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 7749405c3bf8..fec998a4e5e9 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -966,7 +966,7 @@ int mbedtls_ssl_write_pre_shared_key_ext( mbedtls_ssl_context *ssl, *olen = 0; - if( !( ssl->handshake->extensions_present & PSK_KEY_EXCHANGE_MODES_EXTENSION ) ) + if( !( mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 0 ) ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "The psk_key_exchange_modes extension has not been added." ) ); return( MBEDTLS_ERR_SSL_BAD_HS_PSK_KEY_EXCHANGE_MODES_EXT ); @@ -1593,8 +1593,8 @@ static int ssl_client_hello_write( mbedtls_ssl_context* ssl, unsigned char* ciphersuite_start; size_t ciphersuite_count; - /* Keeping track of the included extensions */ - ssl->handshake->extensions_present = NO_EXTENSION; + /* Keeping track of the included extension */ + mbedtls_ssl_reset_extensions_present( ssl ); #if defined(MBEDTLS_SSL_TLS13_CTLS) if( ssl->handshake->ctls == MBEDTLS_SSL_TLS13_CTLS_USE ) @@ -1910,7 +1910,7 @@ static int ssl_client_hello_write( mbedtls_ssl_context* ssl, buf += cur_ext_len; if( ret == 0 ) - ssl->handshake->extensions_present |= PSK_KEY_EXCHANGE_MODES_EXTENSION; + mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 1 ); } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ @@ -1927,7 +1927,7 @@ static int ssl_client_hello_write( mbedtls_ssl_context* ssl, total_ext_len += cur_ext_len; buf += cur_ext_len; - if( ret == 0 ) ssl->handshake->extensions_present |= SUPPORTED_GROUPS_EXTENSION; + if( ret == 0 ) mbedtls_ssl_extensions_present( ssl, SUPPORTED_GROUPS_EXTENSION, 1 ); } /* The supported_signature_algorithms extension is REQUIRED for @@ -1940,7 +1940,7 @@ static int ssl_client_hello_write( mbedtls_ssl_context* ssl, total_ext_len += cur_ext_len; buf += cur_ext_len; - if( ret == 0 ) ssl->handshake->extensions_present |= SIGNATURE_ALGORITHM_EXTENSION; + if( ret == 0 ) mbedtls_ssl_extensions_present( ssl, SIGNATURE_ALGORITHM_EXTENSION, 1 ); } /* We need to send the key shares under three conditions: * 1 ) A certificate-based ciphersuite is being offered. In this case @@ -1957,16 +1957,16 @@ static int ssl_client_hello_write( mbedtls_ssl_context* ssl, total_ext_len += cur_ext_len; buf += cur_ext_len; - if( ret == 0 ) ssl->handshake->extensions_present |= KEY_SHARE_EXTENSION; + if( ret == 0 ) mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 1 ); } - else if( ssl->handshake->extensions_present & SUPPORTED_GROUPS_EXTENSION && ssl->handshake->extensions_present & SIGNATURE_ALGORITHM_EXTENSION ) + else if( mbedtls_ssl_extensions_present( ssl, SUPPORTED_GROUPS_EXTENSION | SIGNATURE_ALGORITHM_EXTENSION, 0 ) ) { /* We are using a certificate-based key exchange */ ret = ssl_write_key_shares_ext( ssl, buf, end, &cur_ext_len ); total_ext_len += cur_ext_len; buf += cur_ext_len; - if( ret == 0 ) ssl->handshake->extensions_present |= KEY_SHARE_EXTENSION; + if( ret == 0 ) mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 1 ); } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ @@ -1986,7 +1986,7 @@ static int ssl_client_hello_write( mbedtls_ssl_context* ssl, buf += cur_ext_len; if( ret == 0 ) - ssl->handshake->extensions_present |= PRE_SHARED_KEY_EXTENSION; + mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 1 ); } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ @@ -2114,7 +2114,7 @@ static int ssl_parse_server_psk_identity_ext( mbedtls_ssl_context *ssl, } /* buf += 2; */ - ssl->handshake->extensions_present |= PRE_SHARED_KEY_EXTENSION; + mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 1 ); return( 0 ); } @@ -2212,7 +2212,7 @@ static int ssl_parse_key_shares_ext( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } - ssl->handshake->extensions_present |= KEY_SHARE_EXTENSION; + mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 1 ); return( ret ); } #endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ @@ -3279,14 +3279,14 @@ static int ssl_server_hello_postprocess( mbedtls_ssl_context* ssl ) * ELSE unknown key exchange mechanism. */ - if( ssl->handshake->extensions_present & PRE_SHARED_KEY_EXTENSION ) + if( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) ) { - if( ssl->handshake->extensions_present & KEY_SHARE_EXTENSION ) + if( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) ssl->session_negotiate->key_exchange = MBEDTLS_KEY_EXCHANGE_ECDHE_PSK; else ssl->session_negotiate->key_exchange = MBEDTLS_KEY_EXCHANGE_PSK; } - else if( ssl->handshake->extensions_present & KEY_SHARE_EXTENSION ) + else if( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) ssl->session_negotiate->key_exchange = MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA; else { @@ -4120,7 +4120,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) * * Reset extensions we have seen so far. */ - ssl->handshake->extensions_present = NO_EXTENSION; + mbedtls_ssl_reset_extensions_present( ssl ); ret = ssl_server_hello_process( ssl ); if( ret != 0 ) @@ -4210,7 +4210,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) * message and not the HRR anymore. */ /* reset extensions we have seen so far */ - ssl->handshake->extensions_present = NO_EXTENSION; + mbedtls_ssl_reset_extensions_present( ssl ); ret = ssl_server_hello_process( ssl ); if( ret != 0 ) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 57c4e09b205e..acf53c286aac 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -4982,7 +4982,7 @@ int mbedtls_ssl_write_early_data_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_SRV_C) if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER ) { - if( ( ssl->handshake->extensions_present & EARLY_DATA_EXTENSION ) == 0 ) + if( !( mbedtls_ssl_extensions_present( ssl, EARLY_DATA_EXTENSION, 0 ) ) ) return( 0 ); if( ssl->conf->key_exchange_modes != MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_KE || diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index d8705e4d8a2d..f62a8941b91f 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -73,7 +73,7 @@ static int ssl_write_sni_server_ext( unsigned char *p = buf; *olen = 0; - if( ( ssl->handshake->extensions_present & SERVERNAME_EXTENSION ) == 0 ) + if( !( mbedtls_ssl_extensions_present( ssl, SERVERNAME_EXTENSION, 0 ) ) ) { return( 0 ); } @@ -2316,7 +2316,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, const int* ciphersuites; const mbedtls_ssl_ciphersuite_t* ciphersuite_info; - ssl->handshake->extensions_present = NO_EXTENSION; + mbedtls_ssl_reset_extensions_present( ssl ); ssl->session_negotiate->key_exchange = MBEDTLS_KEY_EXCHANGE_NONE; /* TBD: Refactor */ @@ -2574,7 +2574,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, MBEDTLS_SSL_DEBUG_RET( 1, "ssl_parse_servername_ext", ret ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVERNAME_EXT ); } - ssl->handshake->extensions_present |= SERVERNAME_EXTENSION; + mbedtls_ssl_extensions_present( ssl, SERVERNAME_EXTENSION, 1 ); break; #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ @@ -2591,7 +2591,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, } else if( ret == 0 ) /* cid extension present and processed succesfully */ { - ssl->handshake->extensions_present |= CID_EXTENSION; + mbedtls_ssl_extensions_present( ssl, CID_EXTENSION, 1 ); } break; #endif /* MBEDTLS_CID */ @@ -2609,7 +2609,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, } else if( ret == 0 ) /* cookie extension present and processed succesfully */ { - ssl->handshake->extensions_present |= COOKIE_EXTENSION; + mbedtls_ssl_extensions_present( ssl, COOKIE_EXTENSION, 1 ); } break; #endif /* MBEDTLS_SSL_COOKIE_C */ @@ -2624,7 +2624,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, ext_len_psk_ext = ext_size; ext_psk_ptr = ext + 4; - ssl->handshake->extensions_present |= PRE_SHARED_KEY_EXTENSION; + mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 1 ); break; #endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ @@ -2640,7 +2640,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, return( MBEDTLS_ERR_SSL_BAD_HS_SUPPORTED_GROUPS ); } */ - ssl->handshake->extensions_present |= EARLY_DATA_EXTENSION; + mbedtls_ssl_extensions_present( ssl, EARLY_DATA_EXTENSION, 1 ); break; #endif /* MBEDTLS_ZERO_RTT */ @@ -2662,7 +2662,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, return( MBEDTLS_ERR_SSL_BAD_HS_SUPPORTED_GROUPS ); } - ssl->handshake->extensions_present |= SUPPORTED_GROUPS_EXTENSION; + mbedtls_ssl_extensions_present( ssl, SUPPORTED_GROUPS_EXTENSION, 1 ); break; #endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ @@ -2677,7 +2677,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, return( MBEDTLS_ERR_SSL_BAD_HS_PSK_KEY_EXCHANGE_MODES_EXT ); } - ssl->handshake->extensions_present |= PSK_KEY_EXCHANGE_MODES_EXTENSION; + mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 1 ); break; #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ @@ -2708,10 +2708,10 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, * the content of it. */ final_ret = MBEDTLS_ERR_SSL_BAD_HS_WRONG_KEY_SHARE; - ssl->handshake->extensions_present |= KEY_SHARE_EXTENSION; + mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 1 ); break; } - ssl->handshake->extensions_present |= KEY_SHARE_EXTENSION; + mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 1 ); break; #endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ @@ -2725,7 +2725,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_max_fragment_length_ext" ), ret ); return( MBEDTLS_ERR_SSL_BAD_HS_MAX_FRAGMENT_LENGTH_EXT ); } - ssl->handshake->extensions_present |= MAX_FRAGMENT_LENGTH_EXTENSION; + mbedtls_ssl_extensions_present( ssl, MAX_FRAGMENT_LENGTH_EXTENSION, 1 ); break; #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ @@ -2738,7 +2738,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_supported_versions_ext" ), ret ); return( MBEDTLS_ERR_SSL_BAD_HS_SUPPORTED_VERSIONS_EXT ); } - ssl->handshake->extensions_present |= SUPPORTED_VERSION_EXTENSION; + mbedtls_ssl_extensions_present( ssl, SUPPORTED_VERSION_EXTENSION, 1 ); break; #if defined(MBEDTLS_SSL_ALPN) @@ -2751,7 +2751,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_alpn_ext" ), ret ); return( MBEDTLS_ERR_SSL_BAD_HS_ALPN_EXT ); } - ssl->handshake->extensions_present |= ALPN_EXTENSION; + mbedtls_ssl_extensions_present( ssl, ALPN_EXTENSION, 1 ); break; #endif /* MBEDTLS_SSL_ALPN */ @@ -2765,7 +2765,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ( "ssl_parse_supported_signature_algorithms_server_ext ( %d )", ret ) ); return( ret ); } - ssl->handshake->extensions_present |= SIGNATURE_ALGORITHM_EXTENSION; + mbedtls_ssl_extensions_present( ssl, SIGNATURE_ALGORITHM_EXTENSION, 1 ); break; #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ @@ -2849,29 +2849,29 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, /* List all the extensions we have received */ MBEDTLS_SSL_DEBUG_MSG( 3, ( "Supported Extensions:" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- KEY_SHARE_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & KEY_SHARE_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PSK_KEY_EXCHANGE_MODES_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & PSK_KEY_EXCHANGE_MODES_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PRE_SHARED_KEY_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & PRE_SHARED_KEY_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SIGNATURE_ALGORITHM_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & SIGNATURE_ALGORITHM_EXTENSION ) >0 ) ? "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_GROUPS_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & SUPPORTED_GROUPS_EXTENSION ) >0 ) ? "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_VERSION_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & SUPPORTED_VERSION_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- KEY_SHARE_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PSK_KEY_EXCHANGE_MODES_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PRE_SHARED_KEY_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SIGNATURE_ALGORITHM_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, SIGNATURE_ALGORITHM_EXTENSION, 0 ) ) >0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_GROUPS_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, SUPPORTED_GROUPS_EXTENSION, 0 ) ) >0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_VERSION_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, SUPPORTED_VERSION_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); #if defined(MBEDTLS_CID) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- CID_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & CID_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- CID_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, CID_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_CID */ #if defined ( MBEDTLS_SSL_SERVER_NAME_INDICATION ) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SERVERNAME_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & SERVERNAME_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SERVERNAME_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, SERVERNAME_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined ( MBEDTLS_SSL_ALPN ) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- ALPN_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & ALPN_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- ALPN_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, ALPN_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_ALPN */ #if defined ( MBEDTLS_SSL_MAX_FRAGMENT_LENGTH ) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- MAX_FRAGMENT_LENGTH_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & MAX_FRAGMENT_LENGTH_EXTENSION ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- MAX_FRAGMENT_LENGTH_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, MAX_FRAGMENT_LENGTH_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined ( MBEDTLS_SSL_COOKIE_C ) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- COOKIE_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & COOKIE_EXTENSION ) >0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- COOKIE_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, COOKIE_EXTENSION, 0 ) ) >0 ) ? "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_COOKIE_C */ #if defined(MBEDTLS_ZERO_RTT) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- EARLY_DATA_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & EARLY_DATA_EXTENSION ) >0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- EARLY_DATA_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, EARLY_DATA_EXTENSION, 0 ) ) >0 ) ? "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_ZERO_RTT*/ /* Determine key exchange algorithm to use. There are three types of key exchanges @@ -2886,12 +2886,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, * the pre_shared_key extension. It may additionally provide key share * and supported_groups. */ - if( ssl->handshake->extensions_present & EARLY_DATA_EXTENSION ) + if( mbedtls_ssl_extensions_present( ssl, EARLY_DATA_EXTENSION, 0 ) ) { /* Pure PSK mode */ if( ( ssl->conf->early_data == MBEDTLS_SSL_EARLY_DATA_ENABLED ) && - ( ssl->handshake->extensions_present & PRE_SHARED_KEY_EXTENSION ) && - ( ssl->handshake->extensions_present & PSK_KEY_EXCHANGE_MODES_EXTENSION ) ) + ( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 0 ) ) ) { /* Test whether we are allowed to use this mode ( server-side check ) */ if( ( ssl->conf->key_exchange_modes == @@ -2925,9 +2925,9 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, } /* ECDHE-PSK mode */ if( ( ssl->conf->early_data == MBEDTLS_SSL_EARLY_DATA_ENABLED ) && - ( ssl->handshake->extensions_present & PRE_SHARED_KEY_EXTENSION ) && - ( ssl->handshake->extensions_present & KEY_SHARE_EXTENSION ) && - ( ssl->handshake->extensions_present & PSK_KEY_EXCHANGE_MODES_EXTENSION ) ) + ( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 0 ) ) ) { /* Test whether we are allowed to use this mode ( server-side check ) */ if( ssl->conf->key_exchange_modes == @@ -2975,8 +2975,8 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, * Requires key_exchange_modes and the pre_shared_key extension * */ - if( ( ssl->handshake->extensions_present & PRE_SHARED_KEY_EXTENSION ) && - ( ssl->handshake->extensions_present & PSK_KEY_EXCHANGE_MODES_EXTENSION ) ) + if( ( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 0 ) ) ) { /* Test whether we are allowed to use this mode ( server-side check ) */ if( ssl->conf->key_exchange_modes == @@ -3011,9 +3011,9 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, * Requires key share, supported_groups, key_exchange_modes and * the pre_shared_key extension. */ - if( ( ssl->handshake->extensions_present & PRE_SHARED_KEY_EXTENSION ) && - ( ssl->handshake->extensions_present & KEY_SHARE_EXTENSION ) && - ( ssl->handshake->extensions_present & PSK_KEY_EXCHANGE_MODES_EXTENSION ) ) + if( ( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 0 ) ) ) { /* Test whether we are allowed to use this mode ( server-side check ) */ if( ssl->conf->key_exchange_modes == @@ -3049,9 +3049,9 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, * It requires supported_groups, supported_signature extensions, and key share * */ - if( ( ssl->handshake->extensions_present & SUPPORTED_GROUPS_EXTENSION ) && - ( ssl->handshake->extensions_present & SIGNATURE_ALGORITHM_EXTENSION ) && - ( ssl->handshake->extensions_present & KEY_SHARE_EXTENSION ) ) + if( ( mbedtls_ssl_extensions_present( ssl, SUPPORTED_GROUPS_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, SIGNATURE_ALGORITHM_EXTENSION, 0 ) ) && + ( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) ) { /* Test whether we are allowed to use this mode ( server-side check ) */ if( ssl->conf->key_exchange_modes == @@ -3090,7 +3090,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, * the verification check. */ if( ( ssl->conf->rr_config == MBEDTLS_SSL_FORCE_RR_CHECK_ON ) && - !( ssl->handshake->extensions_present & COOKIE_EXTENSION ) ) { + !( mbedtls_ssl_extensions_present( ssl, COOKIE_EXTENSION, 0 ) ) ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "Cookie extension missing. Need to send a HRR." ) ); final_ret = MBEDTLS_ERR_SSL_BAD_HS_MISSING_COOKIE_EXT; } @@ -3308,7 +3308,7 @@ static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, unsigned char *p = buf; *olen = 0; - if( ( ssl->handshake->extensions_present & MAX_FRAGMENT_LENGTH_EXTENSION ) + if( ( mbedtls_ssl_extensions_present( ssl, MAX_FRAGMENT_LENGTH_EXTENSION, 0 ) ) == 0 ) { return( 0 ); @@ -3341,7 +3341,7 @@ static void ssl_write_alpn_ext( mbedtls_ssl_context *ssl, { *olen = 0; - if( ( ssl->handshake->extensions_present & ALPN_EXTENSION ) == 0 || + if( ( mbedtls_ssl_extensions_present( ssl, ALPN_EXTENSION, 0 ) ) == 0 || ssl->alpn_chosen == NULL ) { return( 0 ); @@ -4100,7 +4100,7 @@ static int ssl_server_hello_write( mbedtls_ssl_context* ssl, /* Only add the pre_shared_key extension if the client provided it in the ClientHello * and if the key exchange supports PSK */ - if( ssl->handshake->extensions_present & PRE_SHARED_KEY_EXTENSION && ( + if( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) && ( ssl->session_negotiate->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK || ssl->session_negotiate->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK ) ) { @@ -4114,7 +4114,7 @@ static int ssl_server_hello_write( mbedtls_ssl_context* ssl, /* Only add the key_share extension if the client provided it in the ClientHello * and if the appropriate key exchange mechanism was selected */ - if( ssl->handshake->extensions_present & KEY_SHARE_EXTENSION && ( + if( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) && ( ssl->session_negotiate->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK || ssl->session_negotiate->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) ) { @@ -4140,7 +4140,7 @@ static int ssl_server_hello_write( mbedtls_ssl_context* ssl, buf += cur_ext_len; #if defined(MBEDTLS_CID) - if( ssl->handshake->extensions_present & CID_EXTENSION ) + if( mbedtls_ssl_extensions_present( ssl, CID_EXTENSION, 0 ) ) { if( ( ret = ssl_write_cid_ext( ssl, buf, end, &cur_ext_len ) ) != 0 ) { From e03cdd8c0272231203dfec959731dccce9c777b9 Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Fri, 8 Jan 2021 20:23:32 +0000 Subject: [PATCH 2/3] Enforce the unicity of extension types in ClientHello, HelloRetryRequest, ServerHello, EncryptedExtensions and CertificateRequest --- include/mbedtls/ssl_internal.h | 16 +++++++ library/ssl_tls13_client.c | 71 ++++++++++++++++++++++++++++++ library/ssl_tls13_server.c | 79 +++++++++++++++++++++++++++++++++- 3 files changed, 165 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index de5fd3baef36..16f1012f905c 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1764,4 +1764,20 @@ static inline int mbedtls_ssl_extensions_present( mbedtls_ssl_context *ssl, return( ret ); } +/* Check presence of extensions and send an alert if one of them is already + * present. Update the list of present extensions */ +static inline int mbedtls_ssl_check_extensions_present( mbedtls_ssl_context *ssl, + int extension_flag ) +{ + int ret = 0; + + if( ( ret = mbedtls_ssl_extensions_present( ssl, extension_flag, 0 ) ) ) + SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + + /* Update extension list */ + ssl->handshake->extensions_present |= extension_flag; + + return( ret ); +} + #endif /* ssl_internal.h */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index fec998a4e5e9..9067ad07fd8e 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2467,6 +2467,9 @@ static int ssl_certificate_request_parse( mbedtls_ssl_context* ssl, /* * Parse extensions */ + + mbedtls_ssl_reset_extensions_present( ssl ); + ext_len = ( p[0] << 8 ) | ( p[1] ); /* At least one extension needs to be present, namely signature_algorithms ext. */ @@ -2500,6 +2503,12 @@ static int ssl_certificate_request_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_SIG_ALG: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found signature_algorithms extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, SIGNATURE_ALGORITHM_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + if( ( ret = mbedtls_ssl_parse_signature_algorithms_ext( ssl, ext + 4, (size_t)ext_size ) ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_parse_signature_algorithms_ext" ) ); @@ -2659,6 +2668,8 @@ static int ssl_encrypted_extensions_parse( mbedtls_ssl_context* ssl, /* skip handshake header */ buf += mbedtls_ssl_hs_hdr_len( ssl ); + mbedtls_ssl_reset_extensions_present( ssl ); + ext_len = ( ( buf[0] << 8 ) | ( buf[1] ) ); buf += 2; /* skip extension length */ @@ -2707,6 +2718,12 @@ static int ssl_encrypted_extensions_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found max_fragment_length extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, MAX_FRAGMENT_LENGTH_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_ENCRYPTED_EXTENSIONS ); + } + if( ( ret = ssl_parse_max_fragment_length_ext( ssl, ext + 4, ext_size ) ) != 0 ) { @@ -2721,6 +2738,12 @@ static int ssl_encrypted_extensions_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_ALPN: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found alpn extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, ALPN_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_ENCRYPTED_EXTENSIONS ); + } + if( ( ret = ssl_parse_alpn_ext( ssl, ext + 4, (size_t)ext_size ) ) != 0 ) { return( ret ); @@ -3172,6 +3195,8 @@ static int ssl_server_hello_parse( mbedtls_ssl_context* ssl, return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } + mbedtls_ssl_reset_extensions_present( ssl ); + ext_len = ( ( buf[0] << 8 ) | ( buf[1] ) ); buf += 2; /* skip extension length */ @@ -3207,6 +3232,13 @@ static int ssl_server_hello_parse( mbedtls_ssl_context* ssl, #if defined(MBEDTLS_CID) case MBEDTLS_TLS_EXT_CID: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found CID extension" ) ); + + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, CID_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); + } + if( ssl->conf->cid == MBEDTLS_CID_CONF_DISABLED ) break; @@ -3219,6 +3251,12 @@ static int ssl_server_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported_versions extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, SUPPORTED_VERSION_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); + } + ret = ssl_parse_supported_version_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); @@ -3227,6 +3265,13 @@ static int ssl_server_hello_parse( mbedtls_ssl_context* ssl, #if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) case MBEDTLS_TLS_EXT_PRE_SHARED_KEY: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found pre_shared_key extension" ) ); + + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); + } + if( ( ret = ssl_parse_server_psk_identity_ext( ssl, ext + 4, (size_t)ext_size ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_server_psk_identity_ext" ), ret ); @@ -3239,6 +3284,12 @@ static int ssl_server_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_KEY_SHARES: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found key_shares extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, KEY_SHARE_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); + } + if( ( ret = ssl_parse_key_shares_ext( ssl, ext + 4, (size_t)ext_size ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_parse_key_shares_ext", ret ); @@ -3555,6 +3606,8 @@ static int ssl_hrr_parse( mbedtls_ssl_context* ssl, return( MBEDTLS_ERR_SSL_BAD_HS_HELLO_RETRY_REQUEST ); } + mbedtls_ssl_reset_extensions_present( ssl ); + ext_len = ( ( buf[0] << 8 ) | ( buf[1] ) ); buf += 2; /* skip extension length */ @@ -3589,6 +3642,12 @@ static int ssl_hrr_parse( mbedtls_ssl_context* ssl, #if defined(MBEDTLS_SSL_COOKIE_C) case MBEDTLS_TLS_EXT_COOKIE: + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, COOKIE_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_HELLO_RETRY_REQUEST ); + } + /* Retrieve length field of cookie */ if( ext_size >= 2 ) { @@ -3628,6 +3687,12 @@ static int ssl_hrr_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported_versions extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, SUPPORTED_VERSION_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_HELLO_RETRY_REQUEST ); + } + ret = ssl_parse_supported_version_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); @@ -3638,6 +3703,12 @@ static int ssl_hrr_parse( mbedtls_ssl_context* ssl, MBEDTLS_SSL_DEBUG_BUF( 3, "key_share extension", ext + 4, ext_size ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, KEY_SHARE_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_HELLO_RETRY_REQUEST ); + } + /* Read selected_group */ tls_id = ( ( ext[4] << 8 ) | ( ext[5] ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "selected_group ( %d )", tls_id ) ); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f62a8941b91f..a118f13af43c 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -2537,6 +2537,8 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); } + mbedtls_ssl_reset_extensions_present( ssl ); + ext_len = ( buf[0] << 8 ) | ( buf[1] ); if( ( ext_len > 0 && ext_len < 4 ) || @@ -2568,6 +2570,13 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) case MBEDTLS_TLS_EXT_SERVERNAME: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found ServerName extension" ) ); + + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, SERVERNAME_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ret = ssl_parse_servername_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) { @@ -2581,6 +2590,13 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, #if defined(MBEDTLS_CID) case MBEDTLS_TLS_EXT_CID: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found CID extension" ) ); + + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, CID_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + if( ssl->conf->cid == MBEDTLS_CID_CONF_DISABLED ) break; @@ -2600,6 +2616,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_COOKIE: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found cookie extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, COOKIE_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ret = ssl_parse_cookie_ext( ssl, ext + 4, ext_size ); /* if cookie verification failed then we return a hello retry message */ @@ -2617,6 +2639,13 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, #if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) case MBEDTLS_TLS_EXT_PRE_SHARED_KEY: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found pre_shared_key extension" ) ); + + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + /* Delay processing of the PSK identity once we have * found out which algorithms to use. We keep a pointer * to the buffer and the size for later processing. @@ -2632,6 +2661,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_EARLY_DATA: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found early_data extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, EARLY_DATA_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + /* There is nothing really to process with this extension. ret = ssl_parse_early_data_ext( ssl, ext + 4, ext_size ); @@ -2648,6 +2683,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_SUPPORTED_GROUPS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported group extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, SUPPORTED_GROUPS_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + /* Supported Groups Extension * * When sent by the client, the "supported_groups" extension @@ -2670,6 +2711,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_PSK_KEY_EXCHANGE_MODES: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found psk key exchange modes extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ret = ssl_parse_key_exchange_modes_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) { @@ -2685,6 +2732,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_KEY_SHARES: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found key share extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, KEY_SHARE_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + /* * Key Share Extension * @@ -2719,6 +2772,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found max fragment length extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, MAX_FRAGMENT_LENGTH_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ret = ssl_parse_max_fragment_length_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) { @@ -2732,6 +2791,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported versions extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, SUPPORTED_VERSION_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ret = ssl_parse_supported_versions_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) { @@ -2745,6 +2810,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_ALPN: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found alpn extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, ALPN_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ret = ssl_parse_alpn_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) { @@ -2759,6 +2830,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, case MBEDTLS_TLS_EXT_SIG_ALG: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found signature_algorithms extension" ) ); + if( ( ret = mbedtls_ssl_check_extensions_present( ssl, SIGNATURE_ALGORITHM_EXTENSION ) ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "mbedtls_ssl_check_extensions_present" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + ret = mbedtls_ssl_parse_signature_algorithms_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) { @@ -2849,7 +2926,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context* ssl, /* List all the extensions we have received */ MBEDTLS_SSL_DEBUG_MSG( 3, ( "Supported Extensions:" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- KEY_SHARE_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- KEY_SHARE_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, KEY_SHARE_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PSK_KEY_EXCHANGE_MODES_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, PSK_KEY_EXCHANGE_MODES_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PRE_SHARED_KEY_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, PRE_SHARED_KEY_EXTENSION, 0 ) ) > 0 ) ? "TRUE" : "FALSE" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SIGNATURE_ALGORITHM_EXTENSION ( %s )", ( ( mbedtls_ssl_extensions_present( ssl, SIGNATURE_ALGORITHM_EXTENSION, 0 ) ) >0 ) ? "TRUE" : "FALSE" ) ); From 92913a186af04de310f1503af4f9adf5d1f92078 Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Thu, 14 Jan 2021 11:58:28 +0000 Subject: [PATCH 3/3] Update `mbedtls_ssl_extensions_present()` to support detection of a conjunction of extensions --- include/mbedtls/ssl_internal.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 16f1012f905c..aa8393b036c3 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1751,14 +1751,15 @@ static inline void mbedtls_ssl_reset_extensions_present( mbedtls_ssl_context *ss ssl->handshake->extensions_present = NO_EXTENSION; } -/* Check presence of extensions and optionally set the extension flag */ +/* Check presence of extension (or conjunction of extensions) and optionally set + * the extension flag */ static inline int mbedtls_ssl_extensions_present( mbedtls_ssl_context *ssl, int extension_flag, int set_extension_flag ) { int ret = 0; - ret = ssl->handshake->extensions_present & extension_flag; + ret = ( ssl->handshake->extensions_present & extension_flag ) == extension_flag; if( set_extension_flag ) ssl->handshake->extensions_present |= extension_flag; return( ret );