diff --git a/ChangeLog b/ChangeLog index 5afc2a1dc..1257e6133 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,8 +7,8 @@ Changes disabled by default and can be enabled with POLARSSL_SSL_DEBUG_ALL Security - * Removed timing differences during SSL message decryption in - ssl_decrypt_buf() due to badly formatted padding + * Removed further timing differences during SSL message decryption in + ssl_decrypt_buf() = Version 1.1.5 released on 2013-01-16 Bugfix diff --git a/include/polarssl/md5.h b/include/polarssl/md5.h index 936e9c958..316786019 100644 --- a/include/polarssl/md5.h +++ b/include/polarssl/md5.h @@ -147,6 +147,9 @@ void md5_hmac( const unsigned char *key, size_t keylen, */ int md5_self_test( int verbose ); +/* Internal use */ +void md5_process( md5_context *ctx, const unsigned char data[64] ); + #ifdef __cplusplus } #endif diff --git a/include/polarssl/sha1.h b/include/polarssl/sha1.h index 0d5e67eb2..a512edd0f 100644 --- a/include/polarssl/sha1.h +++ b/include/polarssl/sha1.h @@ -145,6 +145,9 @@ void sha1_hmac( const unsigned char *key, size_t keylen, */ int sha1_self_test( int verbose ); +/* Internal use */ +void sha1_process( sha1_context *ctx, const unsigned char data[64] ); + #ifdef __cplusplus } #endif diff --git a/include/polarssl/sha2.h b/include/polarssl/sha2.h index 811b0fd61..da3970ce0 100644 --- a/include/polarssl/sha2.h +++ b/include/polarssl/sha2.h @@ -153,6 +153,9 @@ void sha2_hmac( const unsigned char *key, size_t keylen, */ int sha2_self_test( int verbose ); +/* Internal use */ +void sha2_process( sha2_context *ctx, const unsigned char data[64] ); + #ifdef __cplusplus } #endif diff --git a/library/md5.c b/library/md5.c index 7a449b246..2c660bb88 100644 --- a/library/md5.c +++ b/library/md5.c @@ -75,7 +75,7 @@ void md5_starts( md5_context *ctx ) ctx->state[3] = 0x10325476; } -static void md5_process( md5_context *ctx, const unsigned char data[64] ) +void md5_process( md5_context *ctx, const unsigned char data[64] ) { unsigned long X[16], A, B, C, D; diff --git a/library/sha1.c b/library/sha1.c index 72ca06338..cda40b4a0 100644 --- a/library/sha1.c +++ b/library/sha1.c @@ -76,7 +76,7 @@ void sha1_starts( sha1_context *ctx ) ctx->state[4] = 0xC3D2E1F0; } -static void sha1_process( sha1_context *ctx, const unsigned char data[64] ) +void sha1_process( sha1_context *ctx, const unsigned char data[64] ) { unsigned long temp, W[16], A, B, C, D, E; diff --git a/library/sha2.c b/library/sha2.c index 4b5e6962f..ec87d18f5 100644 --- a/library/sha2.c +++ b/library/sha2.c @@ -97,7 +97,7 @@ void sha2_starts( sha2_context *ctx, int is224 ) ctx->is224 = is224; } -static void sha2_process( sha2_context *ctx, const unsigned char data[64] ) +void sha2_process( sha2_context *ctx, const unsigned char data[64] ) { unsigned long temp1, temp2, W[64]; unsigned long A, B, C, D, E, F, G, H; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d7413d9a8..013483494 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -683,7 +683,7 @@ static int ssl_decrypt_buf( ssl_context *ssl ) unsigned char *dec_msg; unsigned char *dec_msg_result; size_t dec_msglen; - size_t minlen = 0, fake_padlen; + size_t minlen = 0; /* * Check immediate ciphertext sanity @@ -765,7 +765,6 @@ static int ssl_decrypt_buf( ssl_context *ssl ) } padlen = 1 + ssl->in_msg[ssl->in_msglen - 1]; - fake_padlen = 256 - padlen; if( ssl->in_msglen < ssl->maclen + padlen ) { @@ -774,7 +773,6 @@ static int ssl_decrypt_buf( ssl_context *ssl ) ssl->in_msglen, ssl->maclen, padlen ) ); #endif padlen = 0; - fake_padlen = 256; correct = 0; } @@ -796,26 +794,23 @@ static int ssl_decrypt_buf( ssl_context *ssl ) * TLSv1+: always check the padding up to the first failure * and fake check up to 256 bytes of padding */ + size_t pad_count = 0, fake_pad_count = 0; + size_t padding_idx = ssl->in_msglen - padlen - 1; + for( i = 1; i <= padlen; i++ ) - { - if( ssl->in_msg[ssl->in_msglen - i] != padlen - 1 ) - { - correct = 0; - fake_padlen = 256 - i; - padlen = 0; - } - } - for( i = 1; i <= fake_padlen; i++ ) - { - if( ssl->in_msg[i + 1] != fake_padlen - 1 ) - minlen = 0; - else - minlen = 1; - } + pad_count += ( ssl->in_msg[padding_idx + i] == padlen - 1 ); + + for( ; i <= 256; i++ ) + fake_pad_count += ( ssl->in_msg[padding_idx + i] == padlen - 1 ); + + correct &= ( pad_count == padlen ); /* Only 1 on correct padding */ + correct &= ( pad_count + fake_pad_count < 512 ); /* Always 1 */ + #if defined(POLARSSL_SSL_DEBUG_ALL) if( padlen > 0 && correct == 0) SSL_DEBUG_MSG( 1, ( "bad padding byte detected" ) ); #endif + padlen &= correct * 0x1FF; } } @@ -848,15 +843,48 @@ static int ssl_decrypt_buf( ssl_context *ssl ) /* * Process MAC and always update for padlen afterwards to make * total time independent of padlen + * + * extra_run compensates MAC check for padlen + * + * Known timing attacks: + * - Lucky Thirteen (http://www.isg.rhul.ac.uk/tls/TLStiming.pdf) + * + * We use ( ( Lx + 8 ) / 64 ) to handle 'negative Lx' values + * correctly. (We round down instead of up, so -56 is the correct + * value for our calculations instead of -55) */ + int j, extra_run = 0; + extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 - + ( 13 + ssl->in_msglen + 8 ) / 64; + + extra_run &= correct * 0xFF; + if( ssl->maclen == 16 ) - md5_hmac( ssl->mac_dec, 16, - ssl->in_ctr, ssl->in_msglen + 13, - ssl->in_msg + ssl->in_msglen ); - else - sha1_hmac( ssl->mac_dec, 20, - ssl->in_ctr, ssl->in_msglen + 13, - ssl->in_msg + ssl->in_msglen ); + { + md5_context ctx; + md5_hmac_starts( &ctx, ssl->mac_dec, 16 ); + md5_hmac_update( &ctx, ssl->in_ctr, ssl->in_msglen + 13 ); + md5_hmac_finish( &ctx, ssl->in_msg + ssl->in_msglen ); + + for( j = 0; j < extra_run; j++ ) + md5_process( &ctx, ssl->in_msg ); + } + else if( ssl->maclen == 20 ) + { + sha1_context ctx; + sha1_hmac_starts( &ctx, ssl->mac_dec, 20 ); + sha1_hmac_update( &ctx, ssl->in_ctr, ssl->in_msglen + 13 ); + sha1_hmac_finish( &ctx, ssl->in_msg + ssl->in_msglen ); + + for( j = 0; j < extra_run; j++ ) + sha1_process( &ctx, ssl->in_msg ); + } + else if( ssl->maclen != 0 ) + { + SSL_DEBUG_MSG( 1, ( "invalid MAC len: %d", + ssl->maclen ) ); + return( POLARSSL_ERR_SSL_FEATURE_UNAVAILABLE ); + } } SSL_DEBUG_BUF( 4, "message mac", tmp, ssl->maclen ); @@ -866,7 +894,9 @@ static int ssl_decrypt_buf( ssl_context *ssl ) if( memcmp( tmp, ssl->in_msg + ssl->in_msglen, ssl->maclen ) != 0 ) { +#if defined(POLARSSL_SSL_DEBUG_ALL) SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); +#endif correct = 0; }