Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed timing difference resulting from badly formatted padding.

  • Loading branch information...
commit 4582999be608c9794d4518ae336b265084db9f93 1 parent 8fe40dc
Paul Bakker authored
Showing with 54 additions and 23 deletions.
  1. +4 −0 ChangeLog
  2. +50 −23 library/ssl_tls.c
4 ChangeLog
View
@@ -4,6 +4,10 @@ PolarSSL ChangeLog
Changes
* 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.2.4 released 2013-01-25
Changes
* Added ssl_handshake_step() to allow single stepping the handshake process
73 library/ssl_tls.c
View
@@ -1196,7 +1196,7 @@ static int ssl_encrypt_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[POLARSSL_SSL_MAX_MAC_SIZE];
SSL_DEBUG_MSG( 2, ( "=> decrypt buf" ) );
@@ -1211,7 +1211,6 @@ static int ssl_decrypt_buf( ssl_context *ssl )
if( ssl->transform_in->ivlen == 0 )
{
#if defined(POLARSSL_ARC4_C)
- padlen = 0;
if( ssl->session_in->ciphersuite == TLS_RSA_WITH_RC4_128_MD5 ||
ssl->session_in->ciphersuite == TLS_RSA_WITH_RC4_128_SHA )
{
@@ -1237,8 +1236,6 @@ static int ssl_decrypt_buf( ssl_context *ssl )
unsigned char add_data[13];
int ret = POLARSSL_ERR_SSL_FEATURE_UNAVAILABLE;
- padlen = 0;
-
#if defined(POLARSSL_AES_C) && defined(POLARSSL_GCM_C)
if( ssl->session_in->ciphersuite == TLS_RSA_WITH_AES_128_GCM_SHA256 ||
ssl->session_in->ciphersuite == TLS_RSA_WITH_AES_256_GCM_SHA384 ||
@@ -1296,12 +1293,16 @@ static int ssl_decrypt_buf( ssl_context *ssl )
}
else
{
+ /*
+ * Decrypt and check the padding
+ */
unsigned char *dec_msg;
unsigned char *dec_msg_result;
size_t dec_msglen;
+ size_t minlen = 0, fake_padlen;
/*
- * Decrypt and check the padding
+ * Check immediate ciphertext sanity
*/
if( ssl->in_msglen % ssl->transform_in->ivlen != 0 )
{
@@ -1310,6 +1311,17 @@ static int ssl_decrypt_buf( ssl_context *ssl )
return( POLARSSL_ERR_SSL_INVALID_MAC );
}
+ if( ssl->minor_ver >= SSL_MINOR_VERSION_2 )
+ minlen += ssl->transform_in->ivlen;
+
+ if( ssl->in_msglen < minlen + ssl->transform_in->ivlen ||
+ ssl->in_msglen < minlen + ssl->transform_in->maclen + 1 )
+ {
+ SSL_DEBUG_MSG( 1, ( "msglen (%d) < max( ivlen(%d), maclen (%d) + 1 ) ( + expl IV )",
+ ssl->in_msglen, ssl->transform_in->ivlen, ssl->transform_in->maclen ) );
+ return( POLARSSL_ERR_SSL_INVALID_MAC );
+ }
+
dec_msglen = ssl->in_msglen;
dec_msg = ssl->in_msg;
dec_msg_result = ssl->in_msg;
@@ -1387,6 +1399,17 @@ 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->transform_in->maclen + padlen )
+ {
+ SSL_DEBUG_MSG( 1, ( "msglen (%d) < maclen (%d) + padlen (%d)",
+ ssl->in_msglen, ssl->transform_in->maclen, padlen ) );
+
+ padlen = 0;
+ fake_padlen = 256;
+ correct = 0;
+ }
if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
{
@@ -1395,24 +1418,33 @@ static int ssl_decrypt_buf( ssl_context *ssl )
SSL_DEBUG_MSG( 1, ( "bad padding length: is %d, "
"should be no more than %d",
padlen, ssl->transform_in->ivlen ) );
- padlen = 0;
+ correct = 0;
}
}
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++ )
{
if( ssl->in_msg[ssl->in_msglen - i] != padlen - 1 )
{
- SSL_DEBUG_MSG( 1, ( "bad padding byte: should be "
- "%02x, but is %02x", padlen - 1,
- ssl->in_msg[ssl->in_msglen - i] ) );
+ 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;
+ }
+ if( padlen > 0 && correct == 0)
+ SSL_DEBUG_MSG( 1, ( "bad padding byte detected" ) );
}
}
@@ -1422,19 +1454,12 @@ static int ssl_decrypt_buf( ssl_context *ssl )
/*
* Always compute the MAC (RFC4346, CBCTIME).
*/
- if( ssl->in_msglen < ssl->transform_in->maclen + padlen )
- {
- SSL_DEBUG_MSG( 1, ( "msglen (%d) < maclen (%d) + padlen (%d)",
- ssl->in_msglen, ssl->transform_in->maclen, padlen ) );
- return( POLARSSL_ERR_SSL_INVALID_MAC );
- }
-
ssl->in_msglen -= ( ssl->transform_in->maclen + padlen );
ssl->in_hdr[3] = (unsigned char)( ssl->in_msglen >> 8 );
ssl->in_hdr[4] = (unsigned char)( ssl->in_msglen );
- memcpy( tmp, ssl->in_msg + ssl->in_msglen, POLARSSL_SSL_MAX_MAC_SIZE );
+ memcpy( tmp, ssl->in_msg + ssl->in_msglen, ssl->transform_in->maclen );
if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
{
@@ -1459,6 +1484,10 @@ static int ssl_decrypt_buf( ssl_context *ssl )
}
else
{
+ /*
+ * Process MAC and always update for padlen afterwards to make
+ * total time independent of padlen
+ */
if( ssl->transform_in->maclen == 16 )
md5_hmac( ssl->transform_in->mac_dec, 16,
ssl->in_ctr, ssl->in_msglen + 13,
@@ -1487,15 +1516,13 @@ static int ssl_decrypt_buf( ssl_context *ssl )
ssl->transform_in->maclen ) != 0 )
{
SSL_DEBUG_MSG( 1, ( "message mac does not match" ) );
- return( POLARSSL_ERR_SSL_INVALID_MAC );
+ correct = 0;
}
/*
- * Finally check the padding length; bad padding
- * will produce the same error as an invalid MAC.
+ * Finally check the correct flag
*/
- if( ssl->transform_in->ivlen != 0 && ssl->transform_in->ivlen != 12 &&
- padlen == 0 )
+ if( correct == 0 )
return( POLARSSL_ERR_SSL_INVALID_MAC );
if( ssl->in_msglen == 0 )
Please sign in to comment.
Something went wrong with that request. Please try again.