Merge remote-tracking branch 'restricted/pr/494' into mbedtls-2.7
diff --git a/ChangeLog b/ChangeLog
index 35c8236..6cdb840 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,39 @@
= mbed TLS x.x.x branch released xxxx-xx-xx
+Security
+ * Fix a vulnerability in TLS ciphersuites based on CBC and using SHA-384,
+ in (D)TLS 1.0 to 1.2, that allowed an active network attacker to
+ partially recover the plaintext of messages under some conditions by
+ exploiting timing measurements. With DTLS, the attacker could perform
+ this recovery by sending many messages in the same connection. With TLS
+ or if mbedtls_ssl_conf_dtls_badmac_limit() was used, the attack only
+ worked if the same secret (for example a HTTP Cookie) has been repeatedly
+ sent over connections manipulated by the attacker. Connections using GCM
+ or CCM instead of CBC, using hash sizes other than SHA-384, or using
+ Encrypt-then-Mac (RFC 7366) were not affected. The vulnerability was
+ caused by a miscalculation (for SHA-384) in a countermeasure to the
+ original Lucky 13 attack. Found by Kenny Paterson, Eyal Ronen and Adi
+ Shamir.
+ * Fix a vulnerability in TLS ciphersuites based on CBC, in (D)TLS 1.0 to
+ 1.2, that allowed a local attacker, able to execute code on the local
+ machine as well as manipulate network packets, to partially recover the
+ plaintext of messages under some conditions by using a cache attack
+ targetting an internal MD/SHA buffer. With TLS or if
+ mbedtls_ssl_conf_dtls_badmac_limit() was used, the attack only worked if
+ the same secret (for example a HTTP Cookie) has been repeatedly sent over
+ connections manipulated by the attacker. Connections using GCM or CCM
+ instead of CBC or using Encrypt-then-Mac (RFC 7366) were not affected.
+ Found by Kenny Paterson, Eyal Ronen and Adi Shamir.
+ * Add a counter-measure against a vulnerability in TLS ciphersuites based
+ on CBC, in (D)TLS 1.0 to 1.2, that allowed a local attacker, able to
+ execute code on the local machine as well as manipulate network packets,
+ to partially recover the plaintext of messages under some conditions (see
+ previous entry) by using a cache attack targeting the SSL input record
+ buffer. Connections using GCM or CCM instead of CBC or using
+ Encrypt-then-Mac (RFC 7366) were not affected. Found by Kenny Paterson,
+ Eyal Ronen and Adi Shamir.
+
Bugfix
* Fix compilation error on C++, because of a variable named new.
Found and fixed by Hirotaka Niisato in #1783.
diff --git a/library/md5.c b/library/md5.c
index 8440ebf..3ba88cf 100644
--- a/library/md5.c
+++ b/library/md5.c
@@ -313,14 +313,6 @@
}
#endif
-static const unsigned char md5_padding[64] =
-{
- 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
-
/*
* MD5 final digest
*/
@@ -328,26 +320,48 @@
unsigned char output[16] )
{
int ret;
- uint32_t last, padn;
+ uint32_t used;
uint32_t high, low;
- unsigned char msglen[8];
+ /*
+ * Add padding: 0x80 then 0x00 until 8 bytes remain for the length
+ */
+ used = ctx->total[0] & 0x3F;
+
+ ctx->buffer[used++] = 0x80;
+
+ if( used <= 56 )
+ {
+ /* Enough room for padding + length in current block */
+ memset( ctx->buffer + used, 0, 56 - used );
+ }
+ else
+ {
+ /* We'll need an extra block */
+ memset( ctx->buffer + used, 0, 64 - used );
+
+ if( ( ret = mbedtls_internal_md5_process( ctx, ctx->buffer ) ) != 0 )
+ return( ret );
+
+ memset( ctx->buffer, 0, 56 );
+ }
+
+ /*
+ * Add message length
+ */
high = ( ctx->total[0] >> 29 )
| ( ctx->total[1] << 3 );
low = ( ctx->total[0] << 3 );
- PUT_UINT32_LE( low, msglen, 0 );
- PUT_UINT32_LE( high, msglen, 4 );
+ PUT_UINT32_LE( low, ctx->buffer, 56 );
+ PUT_UINT32_LE( high, ctx->buffer, 60 );
- last = ctx->total[0] & 0x3F;
- padn = ( last < 56 ) ? ( 56 - last ) : ( 120 - last );
+ if( ( ret = mbedtls_internal_md5_process( ctx, ctx->buffer ) ) != 0 )
+ return( ret );
- if( ( ret = mbedtls_md5_update_ret( ctx, md5_padding, padn ) ) != 0 )
- return( ret );
-
- if( ( ret = mbedtls_md5_update_ret( ctx, msglen, 8 ) ) != 0 )
- return( ret );
-
+ /*
+ * Output final state
+ */
PUT_UINT32_LE( ctx->state[0], output, 0 );
PUT_UINT32_LE( ctx->state[1], output, 4 );
PUT_UINT32_LE( ctx->state[2], output, 8 );
diff --git a/library/sha1.c b/library/sha1.c
index 1f29a0f..5d0335d 100644
--- a/library/sha1.c
+++ b/library/sha1.c
@@ -346,14 +346,6 @@
}
#endif
-static const unsigned char sha1_padding[64] =
-{
- 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
-
/*
* SHA-1 final digest
*/
@@ -361,25 +353,48 @@
unsigned char output[20] )
{
int ret;
- uint32_t last, padn;
+ uint32_t used;
uint32_t high, low;
- unsigned char msglen[8];
+ /*
+ * Add padding: 0x80 then 0x00 until 8 bytes remain for the length
+ */
+ used = ctx->total[0] & 0x3F;
+
+ ctx->buffer[used++] = 0x80;
+
+ if( used <= 56 )
+ {
+ /* Enough room for padding + length in current block */
+ memset( ctx->buffer + used, 0, 56 - used );
+ }
+ else
+ {
+ /* We'll need an extra block */
+ memset( ctx->buffer + used, 0, 64 - used );
+
+ if( ( ret = mbedtls_internal_sha1_process( ctx, ctx->buffer ) ) != 0 )
+ return( ret );
+
+ memset( ctx->buffer, 0, 56 );
+ }
+
+ /*
+ * Add message length
+ */
high = ( ctx->total[0] >> 29 )
| ( ctx->total[1] << 3 );
low = ( ctx->total[0] << 3 );
- PUT_UINT32_BE( high, msglen, 0 );
- PUT_UINT32_BE( low, msglen, 4 );
+ PUT_UINT32_BE( high, ctx->buffer, 56 );
+ PUT_UINT32_BE( low, ctx->buffer, 60 );
- last = ctx->total[0] & 0x3F;
- padn = ( last < 56 ) ? ( 56 - last ) : ( 120 - last );
-
- if( ( ret = mbedtls_sha1_update_ret( ctx, sha1_padding, padn ) ) != 0 )
- return( ret );
- if( ( ret = mbedtls_sha1_update_ret( ctx, msglen, 8 ) ) != 0 )
+ if( ( ret = mbedtls_internal_sha1_process( ctx, ctx->buffer ) ) != 0 )
return( ret );
+ /*
+ * Output final state
+ */
PUT_UINT32_BE( ctx->state[0], output, 0 );
PUT_UINT32_BE( ctx->state[1], output, 4 );
PUT_UINT32_BE( ctx->state[2], output, 8 );
diff --git a/library/sha256.c b/library/sha256.c
index f39bcba..4ec9164 100644
--- a/library/sha256.c
+++ b/library/sha256.c
@@ -315,14 +315,6 @@
}
#endif
-static const unsigned char sha256_padding[64] =
-{
- 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
-
/*
* SHA-256 final digest
*/
@@ -330,26 +322,48 @@
unsigned char output[32] )
{
int ret;
- uint32_t last, padn;
+ uint32_t used;
uint32_t high, low;
- unsigned char msglen[8];
+ /*
+ * Add padding: 0x80 then 0x00 until 8 bytes remain for the length
+ */
+ used = ctx->total[0] & 0x3F;
+
+ ctx->buffer[used++] = 0x80;
+
+ if( used <= 56 )
+ {
+ /* Enough room for padding + length in current block */
+ memset( ctx->buffer + used, 0, 56 - used );
+ }
+ else
+ {
+ /* We'll need an extra block */
+ memset( ctx->buffer + used, 0, 64 - used );
+
+ if( ( ret = mbedtls_internal_sha256_process( ctx, ctx->buffer ) ) != 0 )
+ return( ret );
+
+ memset( ctx->buffer, 0, 56 );
+ }
+
+ /*
+ * Add message length
+ */
high = ( ctx->total[0] >> 29 )
| ( ctx->total[1] << 3 );
low = ( ctx->total[0] << 3 );
- PUT_UINT32_BE( high, msglen, 0 );
- PUT_UINT32_BE( low, msglen, 4 );
+ PUT_UINT32_BE( high, ctx->buffer, 56 );
+ PUT_UINT32_BE( low, ctx->buffer, 60 );
- last = ctx->total[0] & 0x3F;
- padn = ( last < 56 ) ? ( 56 - last ) : ( 120 - last );
-
- if( ( ret = mbedtls_sha256_update_ret( ctx, sha256_padding, padn ) ) != 0 )
+ if( ( ret = mbedtls_internal_sha256_process( ctx, ctx->buffer ) ) != 0 )
return( ret );
- if( ( ret = mbedtls_sha256_update_ret( ctx, msglen, 8 ) ) != 0 )
- return( ret );
-
+ /*
+ * Output final state
+ */
PUT_UINT32_BE( ctx->state[0], output, 0 );
PUT_UINT32_BE( ctx->state[1], output, 4 );
PUT_UINT32_BE( ctx->state[2], output, 8 );
diff --git a/library/sha512.c b/library/sha512.c
index 97cee07..db2617e 100644
--- a/library/sha512.c
+++ b/library/sha512.c
@@ -345,18 +345,6 @@
}
#endif
-static const unsigned char sha512_padding[128] =
-{
- 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
-
/*
* SHA-512 final digest
*/
@@ -364,26 +352,48 @@
unsigned char output[64] )
{
int ret;
- size_t last, padn;
+ unsigned used;
uint64_t high, low;
- unsigned char msglen[16];
+ /*
+ * Add padding: 0x80 then 0x00 until 16 bytes remain for the length
+ */
+ used = ctx->total[0] & 0x7F;
+
+ ctx->buffer[used++] = 0x80;
+
+ if( used <= 112 )
+ {
+ /* Enough room for padding + length in current block */
+ memset( ctx->buffer + used, 0, 112 - used );
+ }
+ else
+ {
+ /* We'll need an extra block */
+ memset( ctx->buffer + used, 0, 128 - used );
+
+ if( ( ret = mbedtls_internal_sha512_process( ctx, ctx->buffer ) ) != 0 )
+ return( ret );
+
+ memset( ctx->buffer, 0, 112 );
+ }
+
+ /*
+ * Add message length
+ */
high = ( ctx->total[0] >> 61 )
| ( ctx->total[1] << 3 );
low = ( ctx->total[0] << 3 );
- PUT_UINT64_BE( high, msglen, 0 );
- PUT_UINT64_BE( low, msglen, 8 );
+ PUT_UINT64_BE( high, ctx->buffer, 112 );
+ PUT_UINT64_BE( low, ctx->buffer, 120 );
- last = (size_t)( ctx->total[0] & 0x7F );
- padn = ( last < 112 ) ? ( 112 - last ) : ( 240 - last );
+ if( ( ret = mbedtls_internal_sha512_process( ctx, ctx->buffer ) ) != 0 )
+ return( ret );
- if( ( ret = mbedtls_sha512_update_ret( ctx, sha512_padding, padn ) ) != 0 )
- return( ret );
-
- if( ( ret = mbedtls_sha512_update_ret( ctx, msglen, 16 ) ) != 0 )
- return( ret );
-
+ /*
+ * Output final state
+ */
PUT_UINT64_BE( ctx->state[0], output, 0 );
PUT_UINT64_BE( ctx->state[1], output, 8 );
PUT_UINT64_BE( ctx->state[2], output, 16 );
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index c19a26d..ca9b8c4 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1281,6 +1281,27 @@
#define SSL_SOME_MODES_USE_MAC
#endif
+/* The function below is only used in the Lucky 13 counter-measure in
+ * ssl_decrypt_buf(). These are the defines that guard the call site. */
+#if defined(SSL_SOME_MODES_USE_MAC) && \
+ ( defined(MBEDTLS_SSL_PROTO_TLS1) || \
+ defined(MBEDTLS_SSL_PROTO_TLS1_1) || \
+ defined(MBEDTLS_SSL_PROTO_TLS1_2) )
+/* This function makes sure every byte in the memory region is accessed
+ * (in ascending addresses order) */
+static void ssl_read_memory( unsigned char *p, size_t len )
+{
+ unsigned char acc = 0;
+ volatile unsigned char force;
+
+ for( ; len != 0; p++, len-- )
+ acc ^= *p;
+
+ force = acc;
+ (void) force;
+}
+#endif /* SSL_SOME_MODES_USE_MAC && ( TLS1 || TLS1_1 || TLS1_2 ) */
+
/*
* Encryption/decryption functions
*/
@@ -1956,8 +1977,10 @@
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
}
+#if defined(MBEDTLS_SSL_DEBUG_ALL)
MBEDTLS_SSL_DEBUG_BUF( 4, "raw buffer after decryption",
ssl->in_msg, ssl->in_msglen );
+#endif
/*
* Authenticate if not done yet.
@@ -1990,20 +2013,69 @@
{
/*
* Process MAC and always update for padlen afterwards to make
- * total time independent of padlen
- *
- * extra_run compensates MAC check for padlen
+ * total time independent of 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)
+ * To compensate for different timings for the MAC calculation
+ * depending on how much padding was removed (which is determined
+ * by padlen), process extra_run more blocks through the hash
+ * function.
+ *
+ * The formula in the paper is
+ * extra_run = ceil( (L1-55) / 64 ) - ceil( (L2-55) / 64 )
+ * where L1 is the size of the header plus the decrypted message
+ * plus CBC padding and L2 is the size of the header plus the
+ * decrypted message. This is for an underlying hash function
+ * with 64-byte blocks.
+ * 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.
+ *
+ * Repeat the formula rather than defining a block_size variable.
+ * This avoids requiring division by a variable at runtime
+ * (which would be marginally less efficient and would require
+ * linking an extra division function in some builds).
*/
size_t j, extra_run = 0;
- extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 -
- ( 13 + ssl->in_msglen + 8 ) / 64;
+
+ /*
+ * The next two sizes are the minimum and maximum values of
+ * in_msglen over all padlen values.
+ *
+ * They're independent of padlen, since we previously did
+ * in_msglen -= padlen.
+ *
+ * Note that max_len + maclen is never more than the buffer
+ * length, as we previously did in_msglen -= maclen too.
+ */
+ const size_t max_len = ssl->in_msglen + padlen;
+ const size_t min_len = ( max_len > 256 ) ? max_len - 256 : 0;
+
+ switch( ssl->transform_in->ciphersuite_info->mac )
+ {
+#if defined(MBEDTLS_MD5_C) || defined(MBEDTLS_SHA1_C) || \
+ defined(MBEDTLS_SHA256_C)
+ case MBEDTLS_MD_MD5:
+ case MBEDTLS_MD_SHA1:
+ case MBEDTLS_MD_SHA256:
+ /* 8 bytes of message size, 64-byte compression blocks */
+ extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 -
+ ( 13 + ssl->in_msglen + 8 ) / 64;
+ break;
+#endif
+#if defined(MBEDTLS_SHA512_C)
+ case MBEDTLS_MD_SHA384:
+ /* 16 bytes of message size, 128-byte compression blocks */
+ extra_run = ( 13 + ssl->in_msglen + padlen + 16 ) / 128 -
+ ( 13 + ssl->in_msglen + 16 ) / 128;
+ break;
+#endif
+ default:
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) );
+ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
+ }
extra_run &= correct * 0xFF;
@@ -2012,12 +2084,25 @@
mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_len, 2 );
mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_msg,
ssl->in_msglen );
+ /* Make sure we access everything even when padlen > 0. This
+ * makes the synchronisation requirements for just-in-time
+ * Prime+Probe attacks much tighter and hopefully impractical. */
+ ssl_read_memory( ssl->in_msg + ssl->in_msglen, padlen );
mbedtls_md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect );
- /* Call mbedtls_md_process at least once due to cache attacks */
+
+ /* Call mbedtls_md_process at least once due to cache attacks
+ * that observe whether md_process() was called of not */
for( j = 0; j < extra_run + 1; j++ )
mbedtls_md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg );
mbedtls_md_hmac_reset( &ssl->transform_in->md_ctx_dec );
+
+ /* Make sure we access all the memory that could contain the MAC,
+ * before we check it in the next code block. This makes the
+ * synchronisation requirements for just-in-time Prime+Probe
+ * attacks much tighter and hopefully impractical. */
+ ssl_read_memory( ssl->in_msg + min_len,
+ max_len - min_len + ssl->transform_in->maclen );
}
else
#endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \
@@ -2027,9 +2112,11 @@
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
}
+#if defined(MBEDTLS_SSL_DEBUG_ALL)
MBEDTLS_SSL_DEBUG_BUF( 4, "expected mac", mac_expect, ssl->transform_in->maclen );
MBEDTLS_SSL_DEBUG_BUF( 4, "message mac", ssl->in_msg + ssl->in_msglen,
ssl->transform_in->maclen );
+#endif
if( mbedtls_ssl_safer_memcmp( ssl->in_msg + ssl->in_msglen, mac_expect,
ssl->transform_in->maclen ) != 0 )