diff --git a/ChangeLog b/ChangeLog index 27e9198c85..0873d46fa9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,12 @@ mbed TLS ChangeLog (Sorted per branch, date) = 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 @@ Security 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 6e43f94324..fffc9a30db 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 b08e490fb8..ef8cd203ea 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1141,12 +1141,16 @@ static int ssl_encrypt_buf( ssl_context *ssl ) 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 @@ static int ssl_encrypt_buf( ssl_context *ssl ) 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 @@ static int ssl_encrypt_buf( ssl_context *ssl ) return( 0 ); } -#define POLARSSL_SSL_MAX_MAC_SIZE 48 - static int ssl_decrypt_buf( ssl_context *ssl ) { size_t i; @@ -1588,7 +1590,7 @@ static int ssl_decrypt_buf( ssl_context *ssl ) #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 @@ static int ssl_decrypt_buf( ssl_context *ssl ) 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 @@ static int ssl_decrypt_buf( ssl_context *ssl ) #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 @@ static int ssl_decrypt_buf( ssl_context *ssl ) 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 @@ static int ssl_decrypt_buf( ssl_context *ssl ) 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 51d31fdfdc..b1b36b849d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -511,40 +511,40 @@ run_test "Truncated HMAC: client default, server default" \ "$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