Fix potential timing issue in RSA pms handling

This commit is contained in:
Manuel Pégourié-Gonnard 2015-02-06 10:30:58 +00:00
parent 555fbf8758
commit 6674cce892
2 changed files with 41 additions and 18 deletions

View file

@ -18,6 +18,9 @@ Security
* Fix potential stack overflow while parsing crafted X.509 certificates * Fix potential stack overflow while parsing crafted X.509 certificates
(TLS server is not affected if it doesn't ask for a client certificate) (TLS server is not affected if it doesn't ask for a client certificate)
(found using Codenomicon Defensics). (found using Codenomicon Defensics).
* Fix timing difference that could theoretically lead to a
Bleichenbacher-style attack in the RSA and RSA-PSK key exchanges
(reported by Sebastian Schinzel).
Features Features
* Add support for FALLBACK_SCSV (draft-ietf-tls-downgrade-scsv). * Add support for FALLBACK_SCSV (draft-ietf-tls-downgrade-scsv).

View file

@ -2884,6 +2884,10 @@ static int ssl_parse_encrypted_pms( ssl_context *ssl,
int ret; int ret;
size_t len = pk_get_len( ssl_own_key( ssl ) ); size_t len = pk_get_len( ssl_own_key( ssl ) );
unsigned char *pms = ssl->handshake->premaster + pms_offset; unsigned char *pms = ssl->handshake->premaster + pms_offset;
unsigned char fake_pms[48], peer_pms[48];
unsigned char mask;
unsigned int uret;
size_t i;
if( ! pk_can_do( ssl_own_key( ssl ), POLARSSL_PK_RSA ) ) if( ! pk_can_do( ssl_own_key( ssl ), POLARSSL_PK_RSA ) )
{ {
@ -2913,31 +2917,47 @@ static int ssl_parse_encrypted_pms( ssl_context *ssl,
return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );
} }
/*
* Protection against Bleichenbacher's attack: invalid PKCS#1 v1.5 padding
* must not cause the connection to end immediately; instead, send a
* bad_record_mac later in the handshake.
* Also, avoid data-dependant branches here to protect against
* timing-based variants.
*/
ret = ssl->f_rng( ssl->p_rng, fake_pms, sizeof( fake_pms ) );
if( ret != 0 )
return( ret );
ret = pk_decrypt( ssl_own_key( ssl ), p, len, ret = pk_decrypt( ssl_own_key( ssl ), p, len,
pms, &ssl->handshake->pmslen, peer_pms, &ssl->handshake->pmslen,
sizeof( ssl->handshake->premaster ) - pms_offset, sizeof( peer_pms ),
ssl->f_rng, ssl->p_rng ); ssl->f_rng, ssl->p_rng );
if( ret != 0 || ssl->handshake->pmslen != 48 || ret |= ssl->handshake->pmslen - 48;
pms[0] != ssl->handshake->max_major_ver || ret |= peer_pms[0] - ssl->handshake->max_major_ver;
pms[1] != ssl->handshake->max_minor_ver ) ret |= peer_pms[1] - ssl->handshake->max_minor_ver;
{
#if defined(POLARSSL_SSL_DEBUG_ALL)
if( ret != 0 )
SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) ); SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) );
#endif
/* if( sizeof( ssl->handshake->premaster ) < pms_offset ||
* Protection against Bleichenbacher's attack: sizeof( ssl->handshake->premaster ) - pms_offset < 48 )
* invalid PKCS#1 v1.5 padding must not cause {
* the connection to end immediately; instead, SSL_DEBUG_MSG( 1, ( "should never happen" ) );
* send a bad_record_mac later in the handshake. return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
*/
ssl->handshake->pmslen = 48;
ret = ssl->f_rng( ssl->p_rng, pms, ssl->handshake->pmslen );
if( ret != 0 )
return( ret );
} }
ssl->handshake->pmslen = 48;
return( ret ); uret = (unsigned) ret;
uret |= -uret; /* msb = ( ret != 0 ) */
uret >>= 8 * sizeof( uret ) - 1; /* uret = ( ret != 0 ) */
mask = (unsigned char)( -uret ) ; /* ret ? 0xff : 0x00 */
for( i = 0; i < ssl->handshake->pmslen; i++ )
pms[i] = ( mask & fake_pms[i] ) | ( (~mask) & peer_pms[i] );
return( 0 );
} }
#endif /* POLARSSL_KEY_EXCHANGE_RSA_ENABLED || #endif /* POLARSSL_KEY_EXCHANGE_RSA_ENABLED ||
POLARSSL_KEY_EXCHANGE_RSA_PSK_ENABLED */ POLARSSL_KEY_EXCHANGE_RSA_PSK_ENABLED */