diff --git a/ChangeLog b/ChangeLog index 5f79eaabb..c647e0fc8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,10 @@ Security Found and fix proposed by Michael Schwarz, Samuel Weiser, Daniel Gruss, Clémentine Maurice and Stefan Mangard. +Bugfix + * Fix insufficient support for signature-hash-algorithm extension, + resulting in compatibility problems with Chrome. Found by hfloyrd. #823 + = mbed TLS 1.3.19 branch released 2017-03-08 Security diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index c7cd541cf..74b131702 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -539,6 +539,7 @@ typedef struct _ssl_session ssl_session; typedef struct _ssl_context ssl_context; typedef struct _ssl_transform ssl_transform; typedef struct _ssl_handshake_params ssl_handshake_params; +typedef struct _ssl_sig_hash_set_t ssl_sig_hash_set_t; #if defined(POLARSSL_SSL_SESSION_TICKETS) typedef struct _ssl_ticket_keys ssl_ticket_keys; #endif @@ -625,6 +626,24 @@ struct _ssl_transform #endif }; +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) +/* + * Abstraction for a grid of allowed signature-hash-algorithm pairs. + */ +struct _ssl_sig_hash_set_t +{ + /* At the moment, we only need to remember a single suitable + * hash algorithm per signature algorithm. As long as that's + * the case - and we don't need a general lookup function - + * we can implement the sig-hash-set as a map from signatures + * to hash algorithms. */ + md_type_t rsa; + md_type_t ecdsa; +}; +#endif /* POLARSSL_SSL_PROTO_TLS1_2) && + POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ + /* * This structure contains the parameters only needed during handshake. */ @@ -633,7 +652,10 @@ struct _ssl_handshake_params /* * Handshake specific crypto variables */ - int sig_alg; /*!< Hash algorithm for signature */ +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + ssl_sig_hash_set_t hash_algs; /*!< Set of suitable sig-hash pairs */ +#endif int cert_type; /*!< Requested cert type */ int verify_sig_alg; /*!< Signature algorithm for verify */ #if defined(POLARSSL_DHM_C) @@ -1957,15 +1979,40 @@ int ssl_psk_derive_premaster( ssl_context *ssl, key_exchange_type_t key_ex ); #if defined(POLARSSL_PK_C) unsigned char ssl_sig_from_pk( pk_context *pk ); +unsigned char ssl_sig_from_pk_alg( pk_type_t type ); pk_type_t ssl_pk_alg_from_sig( unsigned char sig ); #endif md_type_t ssl_md_alg_from_hash( unsigned char hash ); +unsigned char ssl_hash_from_md_alg( md_type_t md ); #if defined(POLARSSL_SSL_SET_CURVES) int ssl_curve_is_acceptable( const ssl_context *ssl, ecp_group_id grp_id ); #endif +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + +/* Find an entry in a signature-hash set matching a given hash algorithm. */ +md_type_t ssl_sig_hash_set_find( ssl_sig_hash_set_t *set, + pk_type_t sig_alg ); +/* Add a signature-hash-pair to a signature-hash set */ +void ssl_sig_hash_set_add( ssl_sig_hash_set_t *set, + pk_type_t sig_alg, + md_type_t md_alg ); +/* Allow exactly one hash algorithm for each signature. */ +void ssl_sig_hash_set_const_hash( ssl_sig_hash_set_t *set, + md_type_t md_alg ); + +/* Setup an empty signature-hash set */ +static inline void ssl_sig_hash_set_init( ssl_sig_hash_set_t *set ) +{ + ssl_sig_hash_set_const_hash( set, POLARSSL_MD_NONE ); +} + +#endif /* POLARSSL_SSL_PROTO_TLS1_2) && + POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ + #if defined(POLARSSL_X509_CRT_PARSE_C) static inline pk_context *ssl_own_key( ssl_context *ssl ) { @@ -1979,6 +2026,12 @@ static inline x509_crt *ssl_own_cert( ssl_context *ssl ) : ssl->handshake->key_cert->cert ); } +/* + * Check if a hash proposed by the peer is in our list. + * Return 0 if we're willing to use it, -1 otherwise. + */ +int ssl_check_sig_hash( md_type_t md ); + /* * Check usage of a certificate wrt extensions: * keyUsage, extendedKeyUsage (later), and nSCertType (later). diff --git a/include/polarssl/ssl_ciphersuites.h b/include/polarssl/ssl_ciphersuites.h index f0519ca36..2666b3be0 100644 --- a/include/polarssl/ssl_ciphersuites.h +++ b/include/polarssl/ssl_ciphersuites.h @@ -290,6 +290,7 @@ const ssl_ciphersuite_t *ssl_ciphersuite_from_id( int ciphersuite_id ); #if defined(POLARSSL_PK_C) pk_type_t ssl_get_ciphersuite_sig_pk_alg( const ssl_ciphersuite_t *info ); +pk_type_t ssl_get_ciphersuite_sig_alg( const ssl_ciphersuite_t *info ); #endif int ssl_ciphersuite_uses_ec( const ssl_ciphersuite_t *info ); diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c index 0fee1e6de..820473147 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -1803,6 +1803,24 @@ pk_type_t ssl_get_ciphersuite_sig_pk_alg( const ssl_ciphersuite_t *info ) return( POLARSSL_PK_NONE ); } } + +pk_type_t ssl_get_ciphersuite_sig_alg( const ssl_ciphersuite_t *info ) +{ + switch( info->key_exchange ) + { + case POLARSSL_KEY_EXCHANGE_RSA: + case POLARSSL_KEY_EXCHANGE_DHE_RSA: + case POLARSSL_KEY_EXCHANGE_ECDHE_RSA: + return( POLARSSL_PK_RSA ); + + case POLARSSL_KEY_EXCHANGE_ECDHE_ECDSA: + return( POLARSSL_PK_ECDSA ); + + default: + return( POLARSSL_PK_NONE ); + } +} + #endif /* POLARSSL_PK_C */ #if defined(POLARSSL_ECDH_C) || defined(POLARSSL_ECDSA_C) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 90d5ac7ff..8ad990b1a 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -467,6 +467,18 @@ static int ssl_parse_renegotiation_info( ssl_context *ssl, #if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) +/* + * Status of the implementation of signature-algorithms extension: + * + * Currently, we are only considering the signature-algorithm extension + * to pick a ciphersuite which allows us to send the ServerKeyExchange + * message with a signature-hash combination that the user allows. + * + * We do *not* check whether all certificates in our certificate + * chain are signed with an allowed signature-hash pair. + * This needs to be done at a later stage. + * + */ static int ssl_parse_signature_algorithms_ext( ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -474,8 +486,9 @@ static int ssl_parse_signature_algorithms_ext( ssl_context *ssl, size_t sig_alg_list_size; const unsigned char *p; const unsigned char *end = buf + len; - const int *md_cur; + md_type_t md_cur; + pk_type_t sig_cur; sig_alg_list_size = ( ( buf[0] << 8 ) | ( buf[1] ) ); if( sig_alg_list_size + 2 != len || @@ -485,35 +498,48 @@ static int ssl_parse_signature_algorithms_ext( ssl_context *ssl, return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); } - /* - * For now, ignore the SignatureAlgorithm part and rely on offered - * ciphersuites only for that part. To be fixed later. + /* Currently we only guarantee signing the ServerKeyExchange message according + * to the constraints specified in this extension (see above), so it suffices + * to remember only one suitable hash for each possible signature algorithm. * - * So, just look at the HashAlgorithm part. + * This will change when we also consider certificate signatures, + * in which case we will need to remember the whole signature-hash + * pair list from the extension. */ - for( md_cur = md_list(); *md_cur != POLARSSL_MD_NONE; md_cur++ ) { -#if !defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) - /* Skip MD5 */ - if( *md_cur == POLARSSL_MD_MD5 ) - continue; -#endif - for( p = buf + 2; p < end; p += 2 ) { - if( *md_cur == (int) ssl_md_alg_from_hash( p[0] ) ) { - ssl->handshake->sig_alg = p[0]; - goto have_sig_alg; - } + for( p = buf + 2; p < end; p += 2 ) { + + /* Silently ignore unknown signature or hash algorithms. */ + + if( (sig_cur = ssl_pk_alg_from_sig( p[1] ) ) == POLARSSL_PK_NONE ) + { + SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: unknown sig alg encoding %d", + p[1] ) ); + continue; + } + + /* Check if we support the hash the user proposes */ + md_cur = ssl_md_alg_from_hash( p[0] ); + if( md_cur == POLARSSL_MD_NONE ) + { + SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: " + "unknown hash alg encoding %d", p[0] ) ); + continue; + } + + if( ssl_check_sig_hash( md_cur ) == 0 ) + { + ssl_sig_hash_set_add( &ssl->handshake->hash_algs, sig_cur, md_cur ); + SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: match sig %d and hash %d", + sig_cur, md_cur ) ); + } + else + { + SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: hash alg %d not supported", + md_cur ) ); } } - /* Some key echanges do not need signatures at all */ - SSL_DEBUG_MSG( 3, ( "no signature_algorithm in common" ) ); - return( 0 ); - -have_sig_alg: - SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: %d", - ssl->handshake->sig_alg ) ); - return( 0 ); } #endif /* POLARSSL_SSL_PROTO_TLS1_2 && @@ -932,6 +958,11 @@ static int ssl_ciphersuite_match( ssl_context *ssl, int suite_id, { const ssl_ciphersuite_t *suite_info; +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + pk_type_t sig_type; +#endif + suite_info = ssl_ciphersuite_from_id( suite_id ); if( suite_info == NULL ) { @@ -979,6 +1010,26 @@ static int ssl_ciphersuite_match( ssl_context *ssl, int suite_id, } #endif +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + /* If the ciphersuite requires signing, check whether + * a suitable hash algorithm is present. */ + if( ssl->minor_ver == SSL_MINOR_VERSION_3 ) + { + sig_type = ssl_get_ciphersuite_sig_alg( suite_info ); + if( sig_type != POLARSSL_PK_NONE && + ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_type ) == POLARSSL_MD_NONE ) + { + SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: no suitable hash algorithm " + "for signature algorithm %d", sig_type ) ); + return( 0 ); + } + } + +#endif /* POLARSSL_SSL_PROTO_TLS1_2 && + POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ + + #if defined(POLARSSL_X509_CRT_PARSE_C) /* * Final check: if ciphersuite requires us to have a @@ -1287,6 +1338,15 @@ static int ssl_parse_client_hello( ssl_context *ssl ) const int *ciphersuites; const ssl_ciphersuite_t *ciphersuite_info; + /* If there is no signature-algorithm extension present, + * we need to fall back to the default values for allowed + * signature-hash pairs. */ +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + int sig_hash_alg_ext_present = 0; +#endif /* POLARSSL_SSL_PROTO_TLS1_2 && + POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ + SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); #if defined(POLARSSL_SSL_RENEGOTIATION) @@ -1621,6 +1681,8 @@ static int ssl_parse_client_hello( ssl_context *ssl ) ret = ssl_parse_signature_algorithms_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); + + sig_hash_alg_ext_present = 1; break; #endif /* POLARSSL_SSL_PROTO_TLS1_2 && POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ @@ -1764,6 +1826,27 @@ static int ssl_parse_client_hello( ssl_context *ssl ) return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); } + +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + + /* + * Try to fall back to default hash SHA1 if the client + * hasn't provided any preferred signature-hash combinations. + */ + if( sig_hash_alg_ext_present == 0 ) + { + md_type_t md_default = POLARSSL_MD_SHA1; + + if( ssl_check_sig_hash( md_default ) != 0 ) + md_default = POLARSSL_MD_NONE; + + ssl_sig_hash_set_const_hash( &ssl->handshake->hash_algs, md_default ); + } + +#endif /* POLARSSL_SSL_PROTO_TLS1_2 && + POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ + /* * Search for a matching ciphersuite * (At the end because we need information from the EC-based extensions @@ -1821,6 +1904,28 @@ have_ciphersuite: ssl->in_left = 0; ssl->state++; + /* Debugging-only output for testsuite */ +#if defined(POLARSSL_DEBUG_C) && \ + defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + if( ssl->minor_ver == SSL_MINOR_VERSION_3 ) + { + pk_type_t sig_alg = ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); + if( sig_alg != POLARSSL_PK_NONE ) + { + md_type_t md_alg = ssl_sig_hash_set_find( &ssl->handshake->hash_algs, + sig_alg ); + SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: %d", + ssl_hash_from_md_alg( md_alg ) ) ); + } + else + { + SSL_DEBUG_MSG( 3, ( "no hash algorithm for signature algorithm %d - should not happen", + sig_alg ) ); + } + } +#endif + SSL_DEBUG_MSG( 2, ( "<= parse client hello" ) ); return( 0 ); @@ -2665,17 +2770,25 @@ curve_matching_done: size_t signature_len = 0; unsigned int hashlen = 0; unsigned char hash[64]; - md_type_t md_alg = POLARSSL_MD_NONE; /* - * Choose hash algorithm. NONE means MD5 + SHA1 here. + * Choose hash algorithm: + * - For TLS 1.2, obey signature-hash-algorithm extension to choose appropriate hash. + * - For SSL3, TLS1.0, TLS1.1 and ECDHE_ECDSA, use SHA1 (RFC 4492, Sec. 5.4) + * - Otherwise, use MD5 + SHA1 (RFC 4346, Sec. 7.4.3) */ + + md_type_t md_alg; + #if defined(POLARSSL_SSL_PROTO_TLS1_2) + pk_type_t sig_alg = ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); if( ssl->minor_ver == SSL_MINOR_VERSION_3 ) { - md_alg = ssl_md_alg_from_hash( ssl->handshake->sig_alg ); + /* For TLS 1.2, obey signature-hash-algorithm extension + * (RFC 5246, Sec. 7.4.1.4.1). */ - if( md_alg == POLARSSL_MD_NONE ) + if( sig_alg == POLARSSL_PK_NONE || + ( md_alg = ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_alg ) ) == POLARSSL_MD_NONE ) { SSL_DEBUG_MSG( 1, ( "should never happen" ) ); return( POLARSSL_ERR_SSL_INTERNAL_ERROR ); @@ -2696,6 +2809,8 @@ curve_matching_done: md_alg = POLARSSL_MD_NONE; } + SSL_DEBUG_MSG( 3, ( "pick hash algorithm %d for signing", md_alg ) ); + /* * Compute the hash to be signed */ @@ -2794,8 +2909,8 @@ curve_matching_done: #if defined(POLARSSL_SSL_PROTO_TLS1_2) if( ssl->minor_ver == SSL_MINOR_VERSION_3 ) { - *(p++) = ssl->handshake->sig_alg; - *(p++) = ssl_sig_from_pk( ssl_own_key( ssl ) ); + *(p++) = ssl_hash_from_md_alg( md_alg ); + *(p++) = ssl_sig_from_pk_alg( sig_alg ); n += 2; } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 860499799..7cf968d92 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3526,7 +3526,11 @@ static void ssl_handshake_params_init( ssl_handshake_params *handshake ) #endif /* POLARSSL_SSL_PROTO_TLS1_2 */ handshake->update_checksum = ssl_update_checksum_start; - handshake->sig_alg = SSL_HASH_SHA1; + +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + ssl_sig_hash_set_init( &handshake->hash_algs ); +#endif #if defined(POLARSSL_DHM_C) dhm_init( &handshake->dhm_ctx ); @@ -5166,6 +5170,19 @@ unsigned char ssl_sig_from_pk( pk_context *pk ) return( SSL_SIG_ANON ); } +unsigned char ssl_sig_from_pk_alg( pk_type_t type ) +{ + switch( type ) { + case POLARSSL_PK_RSA: + return( SSL_SIG_RSA ); + case POLARSSL_PK_ECDSA: + case POLARSSL_PK_ECKEY: + return( SSL_SIG_ECDSA ); + default: + return( SSL_SIG_ANON ); + } +} + pk_type_t ssl_pk_alg_from_sig( unsigned char sig ) { switch( sig ) @@ -5184,6 +5201,57 @@ pk_type_t ssl_pk_alg_from_sig( unsigned char sig ) } #endif /* POLARSSL_PK_C */ +#if defined(POLARSSL_SSL_PROTO_TLS1_2) && \ + defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) + +/* Find an entry in a signature-hash set matching a given hash algorithm. */ +md_type_t ssl_sig_hash_set_find( ssl_sig_hash_set_t *set, + pk_type_t sig_alg ) +{ + switch( sig_alg ) + { + case POLARSSL_PK_RSA: + return( set->rsa ); + case POLARSSL_PK_ECDSA: + return( set->ecdsa ); + default: + return( POLARSSL_MD_NONE ); + } +} + +/* Add a signature-hash-pair to a signature-hash set */ +void ssl_sig_hash_set_add( ssl_sig_hash_set_t *set, + pk_type_t sig_alg, + md_type_t md_alg ) +{ + switch( sig_alg ) + { + case POLARSSL_PK_RSA: + if( set->rsa == POLARSSL_MD_NONE ) + set->rsa = md_alg; + break; + + case POLARSSL_PK_ECDSA: + if( set->ecdsa == POLARSSL_MD_NONE ) + set->ecdsa = md_alg; + break; + + default: + break; + } +} + +/* Allow exactly one hash algorithm for each signature. */ +void ssl_sig_hash_set_const_hash( ssl_sig_hash_set_t *set, + md_type_t md_alg ) +{ + set->rsa = md_alg; + set->ecdsa = md_alg; +} + +#endif /* POLARSSL_SSL_PROTO_TLS1_2) && + POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ + /* * Convert between SSL_HASH_XXX and POLARSSL_MD_XXX */ @@ -5216,6 +5284,38 @@ md_type_t ssl_md_alg_from_hash( unsigned char hash ) } } +/* + * Convert from POLARSSL_MD_XXX to SSL_HASH_XXX + */ +unsigned char ssl_hash_from_md_alg( md_type_t md ) +{ + switch( md ) + { +#if defined(POLARSSL_MD5_C) + case POLARSSL_MD_MD5: + return( SSL_HASH_MD5 ); +#endif +#if defined(POLARSSL_SHA1_C) + case POLARSSL_MD_SHA1: + return( SSL_HASH_SHA1 ); +#endif +#if defined(POLARSSL_SHA256_C) + case POLARSSL_MD_SHA224: + return( SSL_HASH_SHA224 ); + case POLARSSL_MD_SHA256: + return( SSL_HASH_SHA256 ); +#endif +#if defined(POLARSSL_SHA512_C) + case POLARSSL_MD_SHA384: + return( SSL_HASH_SHA384 ); + case POLARSSL_MD_SHA512: + return( SSL_HASH_SHA512 ); +#endif + default: + return( SSL_HASH_NONE ); + } +} + #if defined(POLARSSL_SSL_SET_CURVES) /* * Check is a curve proposed by the peer is in our list. @@ -5233,6 +5333,30 @@ int ssl_curve_is_acceptable( const ssl_context *ssl, ecp_group_id grp_id ) } #endif /* POLARSSL_SSL_SET_CURVES */ +#if defined(POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED) +/* + * Check if a hash proposed by the peer is in our list. + * Return 0 if we're willing to use it, -1 otherwise. + */ +int ssl_check_sig_hash( md_type_t md ) +{ + const int *cur; + + for( cur = md_list(); *cur != POLARSSL_MD_NONE; cur++ ) + { +#if !defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) + /* Skip MD5 */ + if( *cur == POLARSSL_MD_MD5 ) + continue; +#endif + if( *cur == (int) md ) + return( 0 ); + } + + return( -1 ); +} +#endif /* POLARSSL_KEY_EXCHANGE__WITH_CERT__ENABLED */ + #if defined(POLARSSL_X509_CRT_PARSE_C) int ssl_check_cert_usage( const x509_crt *cert, const ssl_ciphersuite_t *ciphersuite,