Removed further timing differences during SSL message decryption in ssl_decrypt_buf()
New padding checking is unbiased on correct or incorrect padding and
has no branch prediction timing differences.
The additional MAC checks further straighten out the timing differences.
(cherry picked from commit e47b34bdc8507b63758402f69e7623d11dfb6984)
Conflicts:
ChangeLog
library/ssl_tls.c
diff --git a/library/md5.c b/library/md5.c
index 7a449b2..2c660bb 100644
--- a/library/md5.c
+++ b/library/md5.c
@@ -75,7 +75,7 @@
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 72ca063..cda40b4 100644
--- a/library/sha1.c
+++ b/library/sha1.c
@@ -76,7 +76,7 @@
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 4b5e696..ec87d18 100644
--- a/library/sha2.c
+++ b/library/sha2.c
@@ -97,7 +97,7 @@
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 d7413d9..0134834 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -683,7 +683,7 @@
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 @@
}
padlen = 1 + ssl->in_msg[ssl->in_msglen - 1];
- fake_padlen = 256 - padlen;
if( ssl->in_msglen < ssl->maclen + padlen )
{
@@ -774,7 +773,6 @@
ssl->in_msglen, ssl->maclen, padlen ) );
#endif
padlen = 0;
- fake_padlen = 256;
correct = 0;
}
@@ -796,26 +794,23 @@
* 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 @@
/*
* 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 @@
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;
}