Fixed timing difference resulting from badly formatted padding.

(cherry picked from commit 4582999be6)

Conflicts:
	ChangeLog
	library/ssl_tls.c
This commit is contained in:
Paul Bakker 2013-03-11 15:56:17 +01:00
parent cb60e7c065
commit 6a229c1f8c
2 changed files with 53 additions and 18 deletions

View File

@ -4,6 +4,10 @@ PolarSSL ChangeLog
Changes Changes
* Allow enabling of dummy error_strerror() to support some use-cases * Allow enabling of dummy error_strerror() to support some use-cases
Security
* Removed timing differences during SSL message decryption in
ssl_decrypt_buf() due to badly formatted padding
= Version 1.1.5 released on 2013-01-16 = Version 1.1.5 released on 2013-01-16
Bugfix Bugfix
* Fixed MPI assembly for SPARC64 platform * Fixed MPI assembly for SPARC64 platform

View File

@ -652,7 +652,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
static int ssl_decrypt_buf( ssl_context *ssl ) static int ssl_decrypt_buf( ssl_context *ssl )
{ {
size_t i, padlen; size_t i, padlen = 0, correct = 1;
unsigned char tmp[20]; unsigned char tmp[20];
SSL_DEBUG_MSG( 2, ( "=> decrypt buf" ) ); SSL_DEBUG_MSG( 2, ( "=> decrypt buf" ) );
@ -677,12 +677,16 @@ static int ssl_decrypt_buf( ssl_context *ssl )
} }
else else
{ {
/*
* Decrypt and check the padding
*/
unsigned char *dec_msg; unsigned char *dec_msg;
unsigned char *dec_msg_result; unsigned char *dec_msg_result;
size_t dec_msglen; size_t dec_msglen;
size_t minlen = 0, fake_padlen;
/* /*
* Decrypt and check the padding * Check immediate ciphertext sanity
*/ */
if( ssl->in_msglen % ssl->ivlen != 0 ) if( ssl->in_msglen % ssl->ivlen != 0 )
{ {
@ -691,6 +695,17 @@ static int ssl_decrypt_buf( ssl_context *ssl )
return( POLARSSL_ERR_SSL_INVALID_MAC ); return( POLARSSL_ERR_SSL_INVALID_MAC );
} }
if( ssl->minor_ver >= SSL_MINOR_VERSION_2 )
minlen += ssl->ivlen;
if( ssl->in_msglen < minlen + ssl->ivlen ||
ssl->in_msglen < minlen + ssl->maclen + 1 )
{
SSL_DEBUG_MSG( 1, ( "msglen (%d) < max( ivlen(%d), maclen (%d) + 1 ) ( + expl IV )",
ssl->in_msglen, ssl->ivlen, ssl->maclen ) );
return( POLARSSL_ERR_SSL_INVALID_MAC );
}
dec_msglen = ssl->in_msglen; dec_msglen = ssl->in_msglen;
dec_msg = ssl->in_msg; dec_msg = ssl->in_msg;
dec_msg_result = ssl->in_msg; dec_msg_result = ssl->in_msg;
@ -750,6 +765,17 @@ static int ssl_decrypt_buf( ssl_context *ssl )
} }
padlen = 1 + ssl->in_msg[ssl->in_msglen - 1]; padlen = 1 + ssl->in_msg[ssl->in_msglen - 1];
fake_padlen = 256 - padlen;
if( ssl->in_msglen < ssl->maclen + padlen )
{
SSL_DEBUG_MSG( 1, ( "msglen (%d) < maclen (%d) + padlen (%d)",
ssl->in_msglen, ssl->maclen, padlen ) );
padlen = 0;
fake_padlen = 256;
correct = 0;
}
if( ssl->minor_ver == SSL_MINOR_VERSION_0 ) if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
{ {
@ -758,24 +784,33 @@ static int ssl_decrypt_buf( ssl_context *ssl )
SSL_DEBUG_MSG( 1, ( "bad padding length: is %d, " SSL_DEBUG_MSG( 1, ( "bad padding length: is %d, "
"should be no more than %d", "should be no more than %d",
padlen, ssl->ivlen ) ); padlen, ssl->ivlen ) );
padlen = 0; correct = 0;
} }
} }
else else
{ {
/* /*
* TLSv1: always check the padding * TLSv1+: always check the padding up to the first failure
* and fake check up to 256 bytes of padding
*/ */
for( i = 1; i <= padlen; i++ ) for( i = 1; i <= padlen; i++ )
{ {
if( ssl->in_msg[ssl->in_msglen - i] != padlen - 1 ) if( ssl->in_msg[ssl->in_msglen - i] != padlen - 1 )
{ {
SSL_DEBUG_MSG( 1, ( "bad padding byte: should be " correct = 0;
"%02x, but is %02x", padlen - 1, fake_padlen = 256 - i;
ssl->in_msg[ssl->in_msglen - i] ) );
padlen = 0; padlen = 0;
} }
} }
for( i = 1; i <= fake_padlen; i++ )
{
if( ssl->in_msg[i + 1] != fake_padlen - 1 )
minlen = 0;
else
minlen = 1;
}
if( padlen > 0 && correct == 0)
SSL_DEBUG_MSG( 1, ( "bad padding byte detected" ) );
} }
} }
@ -785,13 +820,6 @@ static int ssl_decrypt_buf( ssl_context *ssl )
/* /*
* Always compute the MAC (RFC4346, CBCTIME). * Always compute the MAC (RFC4346, CBCTIME).
*/ */
if( ssl->in_msglen < ssl->maclen + padlen )
{
SSL_DEBUG_MSG( 1, ( "msglen (%d) < maclen (%d) + padlen (%d)",
ssl->in_msglen, ssl->maclen, padlen ) );
return( POLARSSL_ERR_SSL_INVALID_MAC );
}
ssl->in_msglen -= ( ssl->maclen + padlen ); ssl->in_msglen -= ( ssl->maclen + padlen );
ssl->in_hdr[3] = (unsigned char)( ssl->in_msglen >> 8 ); ssl->in_hdr[3] = (unsigned char)( ssl->in_msglen >> 8 );
@ -812,6 +840,10 @@ static int ssl_decrypt_buf( ssl_context *ssl )
} }
else else
{ {
/*
* Process MAC and always update for padlen afterwards to make
* total time independent of padlen
*/
if( ssl->maclen == 16 ) if( ssl->maclen == 16 )
md5_hmac( ssl->mac_dec, 16, md5_hmac( ssl->mac_dec, 16,
ssl->in_ctr, ssl->in_msglen + 13, ssl->in_ctr, ssl->in_msglen + 13,
@ -830,14 +862,13 @@ static int ssl_decrypt_buf( ssl_context *ssl )
ssl->maclen ) != 0 ) ssl->maclen ) != 0 )
{ {
SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); SSL_DEBUG_MSG( 1, ( "message mac does not match" ) );
return( POLARSSL_ERR_SSL_INVALID_MAC ); correct = 0;
} }
/* /*
* Finally check the padding length; bad padding * Finally check the correct flag
* will produce the same error as an invalid MAC.
*/ */
if( ssl->ivlen != 0 && padlen == 0 ) if( correct == 0 )
return( POLARSSL_ERR_SSL_INVALID_MAC ); return( POLARSSL_ERR_SSL_INVALID_MAC );
if( ssl->in_msglen == 0 ) if( ssl->in_msglen == 0 )