Merge remote-tracking branch 'upstream-restricted/pr/426' into mbedtls-1.3-restricted
diff --git a/ChangeLog b/ChangeLog
index 27e9198..0873d46 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,12 @@
= mbed TLS 1.3.22 branch released 2017-xx-xx
Security
+ * Fix heap corruption in implementation of truncated HMAC extension.
+ When the truncated HMAC extension is enabled and CBC is used,
+ sending a malicious application packet can be used to selectively
+ corrupt 6 bytes on the peer's heap, potentially leading to crash or
+ remote code execution. This can be triggered remotely from either
+ side.
* Fix buffer overflow in RSA-PSS verification when the hash is too
large for the key size. Found by Seth Terashima, Qualcomm Product
Security Initiative, Qualcomm Technologies Inc.
@@ -26,6 +32,8 @@
after keypair generation.
Bugfix
+ * Fix typo in ssl.h leading to a too small value of SSL_MAC_ADD
+ in case CBC is disabled but ARC4 is enabled.
* Fix memory leak in ssl_set_hostname() when called multiple times.
Found by projectgus and jethrogb, #836.
* Fix usage help in ssl_server2 example. Found and fixed by Bei Lin.
diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
index 6e43f94..fffc9a3 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -303,7 +303,7 @@
#define SSL_COMPRESSION_ADD 0
#endif
-#if defined(POLARSSL_RC4_C) || defined(POLARSSL_CIPHER_MODE_CBC)
+#if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_MODE_CBC)
/* Ciphersuites using HMAC */
#if defined(POLARSSL_SHA512_C)
#define SSL_MAC_ADD 48 /* SHA-384 used for HMAC */
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index b08e490..ef8cd20 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1141,12 +1141,16 @@
defined(POLARSSL_SSL_PROTO_TLS1_2)
if( ssl->minor_ver >= SSL_MINOR_VERSION_1 )
{
+ unsigned char mac[SSL_MAC_ADD];
+
md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_ctr, 13 );
md_hmac_update( &ssl->transform_out->md_ctx_enc,
ssl->out_msg, ssl->out_msglen );
- md_hmac_finish( &ssl->transform_out->md_ctx_enc,
- ssl->out_msg + ssl->out_msglen );
+ md_hmac_finish( &ssl->transform_out->md_ctx_enc, mac );
md_hmac_reset( &ssl->transform_out->md_ctx_enc );
+
+ memcpy( ssl->out_msg + ssl->out_msglen, mac,
+ ssl->transform_out->maclen );
}
else
#endif
@@ -1155,7 +1159,7 @@
return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
}
- SSL_DEBUG_BUF( 4, "computed mac",
+ SSL_DEBUG_BUF( 4, "expected mac",
ssl->out_msg + ssl->out_msglen,
ssl->transform_out->maclen );
@@ -1419,8 +1423,6 @@
return( 0 );
}
-#define POLARSSL_SSL_MAX_MAC_SIZE 48
-
static int ssl_decrypt_buf( ssl_context *ssl )
{
size_t i;
@@ -1588,7 +1590,7 @@
#if defined(POLARSSL_SSL_ENCRYPT_THEN_MAC)
if( ssl->session_in->encrypt_then_mac == SSL_ETM_ENABLED )
{
- unsigned char computed_mac[POLARSSL_SSL_MAX_MAC_SIZE];
+ unsigned char mac_expect[SSL_MAC_ADD];
unsigned char pseudo_hdr[13];
SSL_DEBUG_MSG( 3, ( "using encrypt then mac" ) );
@@ -1606,15 +1608,15 @@
md_hmac_update( &ssl->transform_in->md_ctx_dec, pseudo_hdr, 13 );
md_hmac_update( &ssl->transform_in->md_ctx_dec,
ssl->in_iv, ssl->in_msglen );
- md_hmac_finish( &ssl->transform_in->md_ctx_dec, computed_mac );
+ md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect );
md_hmac_reset( &ssl->transform_in->md_ctx_dec );
SSL_DEBUG_BUF( 4, "message mac", ssl->in_iv + ssl->in_msglen,
ssl->transform_in->maclen );
- SSL_DEBUG_BUF( 4, "computed mac", computed_mac,
+ SSL_DEBUG_BUF( 4, "expected mac", mac_expect,
ssl->transform_in->maclen );
- if( safer_memcmp( ssl->in_iv + ssl->in_msglen, computed_mac,
+ if( safer_memcmp( ssl->in_iv + ssl->in_msglen, mac_expect,
ssl->transform_in->maclen ) != 0 )
{
SSL_DEBUG_MSG( 1, ( "message mac does not match" ) );
@@ -1775,15 +1777,13 @@
#if defined(POLARSSL_SOME_MODES_USE_MAC)
if( auth_done == 0 )
{
- unsigned char tmp[POLARSSL_SSL_MAX_MAC_SIZE];
+ unsigned char mac_expect[SSL_MAC_ADD];
ssl->in_msglen -= ssl->transform_in->maclen;
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, ssl->transform_in->maclen );
-
#if defined(POLARSSL_SSL_PROTO_SSL3)
if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
{
@@ -1820,8 +1820,8 @@
md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_ctr, 13 );
md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_msg,
ssl->in_msglen );
- md_hmac_finish( &ssl->transform_in->md_ctx_dec,
- ssl->in_msg + ssl->in_msglen );
+ md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect );
+
/* Call md_process at least once due to cache attacks */
for( j = 0; j < extra_run + 1; j++ )
md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg );
@@ -1836,11 +1836,11 @@
return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
}
- SSL_DEBUG_BUF( 4, "message mac", tmp, ssl->transform_in->maclen );
- SSL_DEBUG_BUF( 4, "computed mac", ssl->in_msg + ssl->in_msglen,
+ SSL_DEBUG_BUF( 4, "expected mac", mac_expect, ssl->transform_in->maclen );
+ SSL_DEBUG_BUF( 4, "message mac", ssl->in_msg + ssl->in_msglen,
ssl->transform_in->maclen );
- if( safer_memcmp( tmp, ssl->in_msg + ssl->in_msglen,
+ if( safer_memcmp( ssl->in_msg + ssl->in_msglen, mac_expect,
ssl->transform_in->maclen ) != 0 )
{
#if defined(POLARSSL_SSL_DEBUG_ALL)
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 51d31fd..b1b36b8 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -511,40 +511,40 @@
"$P_SRV debug_level=4" \
"$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \
0 \
- -s "dumping 'computed mac' (20 bytes)" \
- -S "dumping 'computed mac' (10 bytes)"
+ -s "dumping 'expected mac' (20 bytes)" \
+ -S "dumping 'expected mac' (10 bytes)"
run_test "Truncated HMAC: client disabled, server default" \
"$P_SRV debug_level=4" \
"$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
trunc_hmac=0" \
0 \
- -s "dumping 'computed mac' (20 bytes)" \
- -S "dumping 'computed mac' (10 bytes)"
+ -s "dumping 'expected mac' (20 bytes)" \
+ -S "dumping 'expected mac' (10 bytes)"
run_test "Truncated HMAC: client enabled, server default" \
"$P_SRV debug_level=4" \
"$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
trunc_hmac=1" \
0 \
- -S "dumping 'computed mac' (20 bytes)" \
- -s "dumping 'computed mac' (10 bytes)"
+ -S "dumping 'expected mac' (20 bytes)" \
+ -s "dumping 'expected mac' (10 bytes)"
run_test "Truncated HMAC: client enabled, server disabled" \
"$P_SRV debug_level=4 trunc_hmac=0" \
"$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
trunc_hmac=1" \
0 \
- -s "dumping 'computed mac' (20 bytes)" \
- -S "dumping 'computed mac' (10 bytes)"
+ -s "dumping 'expected mac' (20 bytes)" \
+ -S "dumping 'expected mac' (10 bytes)"
run_test "Truncated HMAC: client enabled, server enabled" \
"$P_SRV debug_level=4 trunc_hmac=1" \
"$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
trunc_hmac=1" \
0 \
- -S "dumping 'computed mac' (20 bytes)" \
- -s "dumping 'computed mac' (10 bytes)"
+ -S "dumping 'expected mac' (20 bytes)" \
+ -s "dumping 'expected mac' (10 bytes)"
# Tests for Encrypt-then-MAC extension