From 957e383f87a97e0429bec1d274ef94a3540dbc89 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Dec 2021 21:33:21 +0100 Subject: Zeroize local MAC variables Zeroize local MAC variables used for CBC+HMAC cipher suites. In encryption, this is just good hygiene but probably not needed for security since the data protected by the MAC that could leak is about to be transmitted anyway. In DTLS decryption, this could be a security issue since an adversary could learn the MAC of data that they were trying to inject. At least with encrypt-then-MAC, the adversary could then easily inject a datagram with a corrected packet. TLS would still be safe since the receiver would close the connection after the bad MAC. Signed-off-by: Gilles Peskine --- ChangeLog.d/ssl-mac-zeroize.txt | 5 +++++ library/ssl_tls.c | 22 +++++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 ChangeLog.d/ssl-mac-zeroize.txt diff --git a/ChangeLog.d/ssl-mac-zeroize.txt b/ChangeLog.d/ssl-mac-zeroize.txt new file mode 100644 index 0000000..b49c7ac --- /dev/null +++ b/ChangeLog.d/ssl-mac-zeroize.txt @@ -0,0 +1,5 @@ +Security + * Zeroize intermediate variables used to calculate the MAC in CBC cipher + suites. This hardens the library in case stack memory leaks through a + memory disclosure vulnerabilty, which could formerly have allowed a + man-in-the-middle to inject fake ciphertext into a DTLS connection. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3c1e917..155867b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1479,6 +1479,7 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl ) mac ); memcpy( ssl->out_msg + ssl->out_msglen, mac, ssl->transform_out->maclen ); + mbedtls_platform_zeroize( mac, ssl->transform_out->maclen ); } else #endif @@ -1497,6 +1498,7 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl ) mbedtls_md_hmac_reset( &ssl->transform_out->md_ctx_enc ); memcpy( ssl->out_msg + ssl->out_msglen, mac, ssl->transform_out->maclen ); + mbedtls_platform_zeroize( mac, ssl->transform_out->maclen ); } else #endif @@ -1760,6 +1762,7 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl ) ssl->out_msglen += ssl->transform_out->maclen; auth_done++; + mbedtls_platform_zeroize( mac, ssl->transform_out->maclen ); } #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ } @@ -2156,14 +2159,21 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_BUF( 4, "expected mac", mac_expect, ssl->transform_in->maclen ); + ret = 0; if( mbedtls_ssl_safer_memcmp( ssl->in_iv + ssl->in_msglen, mac_expect, ssl->transform_in->maclen ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); - return( MBEDTLS_ERR_SSL_INVALID_MAC ); + ret = MBEDTLS_ERR_SSL_INVALID_MAC; + goto hmac_failed_etm_enabled; } auth_done++; + + hmac_failed_etm_enabled: + mbedtls_platform_zeroize( mac_expect, ssl->transform_in->maclen ); + if( ret != 0 ) + return( ret ); } #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ @@ -2322,6 +2332,7 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) { unsigned char mac_expect[MBEDTLS_SSL_MAC_ADD]; unsigned char mac_peer[MBEDTLS_SSL_MAC_ADD]; + int ret = 0; ssl->in_msglen -= ssl->transform_in->maclen; @@ -2345,7 +2356,6 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver > MBEDTLS_SSL_MINOR_VERSION_0 ) { - int ret; unsigned char add_data[13]; /* @@ -2373,7 +2383,7 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_cf_hmac", ret ); - return( ret ); + goto hmac_failed_etm_disabled; } mbedtls_ssl_cf_memcpy_offset( mac_peer, ssl->in_msg, @@ -2403,6 +2413,12 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) correct = 0; } auth_done++; + + hmac_failed_etm_disabled: + mbedtls_platform_zeroize( mac_peer, ssl->transform_in->maclen ); + mbedtls_platform_zeroize( mac_expect, ssl->transform_in->maclen ); + if( ret != 0 ) + return( ret ); } /* -- cgit v1.1 From 95f869c9fb97ff641de68b0ce4ff9b51a709f78f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 11 Dec 2021 16:01:16 +0100 Subject: Move changelog entry file that was in the wrong directory Signed-off-by: Gilles Peskine --- ChangeLog.d/check-return.txt | 7 +++++++ check-return.txt | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 ChangeLog.d/check-return.txt delete mode 100644 check-return.txt diff --git a/ChangeLog.d/check-return.txt b/ChangeLog.d/check-return.txt new file mode 100644 index 0000000..47d31de --- /dev/null +++ b/ChangeLog.d/check-return.txt @@ -0,0 +1,7 @@ +Bugfix + * Failures of alternative implementations of AES or DES single-block + functions enabled with MBEDTLS_AES_ENCRYPT_ALT, MBEDTLS_AES_DECRYPT_ALT, + MBEDTLS_DES_CRYPT_ECB_ALT or MBEDTLS_DES3_CRYPT_ECB_ALT were ignored. + This does not concern the implementation provided with Mbed TLS, + where this function cannot fail, or full-module replacements with + MBEDTLS_AES_ALT or MBEDTLS_DES_ALT. Reported by Armelle Duboc in #1092. diff --git a/check-return.txt b/check-return.txt deleted file mode 100644 index 47d31de..0000000 --- a/check-return.txt +++ /dev/null @@ -1,7 +0,0 @@ -Bugfix - * Failures of alternative implementations of AES or DES single-block - functions enabled with MBEDTLS_AES_ENCRYPT_ALT, MBEDTLS_AES_DECRYPT_ALT, - MBEDTLS_DES_CRYPT_ECB_ALT or MBEDTLS_DES3_CRYPT_ECB_ALT were ignored. - This does not concern the implementation provided with Mbed TLS, - where this function cannot fail, or full-module replacements with - MBEDTLS_AES_ALT or MBEDTLS_DES_ALT. Reported by Armelle Duboc in #1092. -- cgit v1.1 From a89bdf03ba38846531dd91c0730019042d4b2f0f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Dec 2021 21:35:10 +0100 Subject: Catch failures of md_hmac operations Check the return values of mbedtls_md_xx functions in our code. We were already doing that everywhere for hash calculations, but not for HMAC calculations. Signed-off-by: Gilles Peskine --- ChangeLog.d/check-return.txt | 3 + library/ssl_tls.c | 307 ++++++++++++++++++++++++++++++++----------- 2 files changed, 231 insertions(+), 79 deletions(-) diff --git a/ChangeLog.d/check-return.txt b/ChangeLog.d/check-return.txt index 47d31de..6eb1629 100644 --- a/ChangeLog.d/check-return.txt +++ b/ChangeLog.d/check-return.txt @@ -5,3 +5,6 @@ Bugfix This does not concern the implementation provided with Mbed TLS, where this function cannot fail, or full-module replacements with MBEDTLS_AES_ALT or MBEDTLS_DES_ALT. Reported by Armelle Duboc in #1092. + * Some failures of HMAC operations were ignored. These failures could only + happen with an alternative implementation of the underlying hash module. + diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 155867b..04275a4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -449,24 +449,45 @@ static int tls1_prf( const unsigned char *secret, size_t slen, * First compute P_md5(secret,label+random)[0..dlen] */ if( ( md_info = mbedtls_md_info_from_type( MBEDTLS_MD_MD5 ) ) == NULL ) - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + { + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + goto exit; + } if( ( ret = mbedtls_md_setup( &md_ctx, md_info, 1 ) ) != 0 ) - return( ret ); + goto exit; - mbedtls_md_hmac_starts( &md_ctx, S1, hs ); - mbedtls_md_hmac_update( &md_ctx, tmp + 20, nb ); - mbedtls_md_hmac_finish( &md_ctx, 4 + tmp ); + ret = mbedtls_md_hmac_starts( &md_ctx, S1, hs ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, tmp + 20, nb ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, 4 + tmp ); + if( ret != 0 ) + goto exit; for( i = 0; i < dlen; i += 16 ) { - mbedtls_md_hmac_reset ( &md_ctx ); - mbedtls_md_hmac_update( &md_ctx, 4 + tmp, 16 + nb ); - mbedtls_md_hmac_finish( &md_ctx, h_i ); + ret = mbedtls_md_hmac_reset ( &md_ctx ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, 4 + tmp, 16 + nb ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, h_i ); + if( ret != 0 ) + goto exit; - mbedtls_md_hmac_reset ( &md_ctx ); - mbedtls_md_hmac_update( &md_ctx, 4 + tmp, 16 ); - mbedtls_md_hmac_finish( &md_ctx, 4 + tmp ); + ret = mbedtls_md_hmac_reset ( &md_ctx ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, 4 + tmp, 16 ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, 4 + tmp ); + if( ret != 0 ) + goto exit; k = ( i + 16 > dlen ) ? dlen % 16 : 16; @@ -480,24 +501,45 @@ static int tls1_prf( const unsigned char *secret, size_t slen, * XOR out with P_sha1(secret,label+random)[0..dlen] */ if( ( md_info = mbedtls_md_info_from_type( MBEDTLS_MD_SHA1 ) ) == NULL ) - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + { + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + goto exit; + } if( ( ret = mbedtls_md_setup( &md_ctx, md_info, 1 ) ) != 0 ) - return( ret ); + goto exit; - mbedtls_md_hmac_starts( &md_ctx, S2, hs ); - mbedtls_md_hmac_update( &md_ctx, tmp + 20, nb ); - mbedtls_md_hmac_finish( &md_ctx, tmp ); + ret = mbedtls_md_hmac_starts( &md_ctx, S2, hs ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, tmp + 20, nb ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, tmp ); + if( ret != 0 ) + goto exit; for( i = 0; i < dlen; i += 20 ) { - mbedtls_md_hmac_reset ( &md_ctx ); - mbedtls_md_hmac_update( &md_ctx, tmp, 20 + nb ); - mbedtls_md_hmac_finish( &md_ctx, h_i ); + ret = mbedtls_md_hmac_reset ( &md_ctx ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, tmp, 20 + nb ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, h_i ); + if( ret != 0 ) + goto exit; - mbedtls_md_hmac_reset ( &md_ctx ); - mbedtls_md_hmac_update( &md_ctx, tmp, 20 ); - mbedtls_md_hmac_finish( &md_ctx, tmp ); + ret = mbedtls_md_hmac_reset ( &md_ctx ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, tmp, 20 ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, tmp ); + if( ret != 0 ) + goto exit; k = ( i + 20 > dlen ) ? dlen % 20 : 20; @@ -505,6 +547,7 @@ static int tls1_prf( const unsigned char *secret, size_t slen, dstbuf[i + j] = (unsigned char)( dstbuf[i + j] ^ h_i[j] ); } +exit: mbedtls_md_free( &md_ctx ); mbedtls_platform_zeroize( tmp, sizeof( tmp ) ); @@ -548,21 +591,39 @@ static int tls_prf_generic( mbedtls_md_type_t md_type, * Compute P_(secret, label + random)[0..dlen] */ if ( ( ret = mbedtls_md_setup( &md_ctx, md_info, 1 ) ) != 0 ) - return( ret ); + goto exit; - mbedtls_md_hmac_starts( &md_ctx, secret, slen ); - mbedtls_md_hmac_update( &md_ctx, tmp + md_len, nb ); - mbedtls_md_hmac_finish( &md_ctx, tmp ); + ret = mbedtls_md_hmac_starts( &md_ctx, secret, slen ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, tmp + md_len, nb ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, tmp ); + if( ret != 0 ) + goto exit; for( i = 0; i < dlen; i += md_len ) { - mbedtls_md_hmac_reset ( &md_ctx ); - mbedtls_md_hmac_update( &md_ctx, tmp, md_len + nb ); - mbedtls_md_hmac_finish( &md_ctx, h_i ); + ret = mbedtls_md_hmac_reset ( &md_ctx ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, tmp, md_len + nb ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, h_i ); + if( ret != 0 ) + goto exit; - mbedtls_md_hmac_reset ( &md_ctx ); - mbedtls_md_hmac_update( &md_ctx, tmp, md_len ); - mbedtls_md_hmac_finish( &md_ctx, tmp ); + ret = mbedtls_md_hmac_reset ( &md_ctx ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_update( &md_ctx, tmp, md_len ); + if( ret != 0 ) + goto exit; + ret = mbedtls_md_hmac_finish( &md_ctx, tmp ); + if( ret != 0 ) + goto exit; k = ( i + md_len > dlen ) ? dlen % md_len : md_len; @@ -570,6 +631,7 @@ static int tls_prf_generic( mbedtls_md_type_t md_type, dstbuf[i + j] = h_i[j]; } +exit: mbedtls_md_free( &md_ctx ); mbedtls_platform_zeroize( tmp, sizeof( tmp ) ); @@ -1015,8 +1077,14 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) For AEAD-based ciphersuites, there is nothing to do here. */ if( mac_key_len != 0 ) { - mbedtls_md_hmac_starts( &transform->md_ctx_enc, mac_enc, mac_key_len ); - mbedtls_md_hmac_starts( &transform->md_ctx_dec, mac_dec, mac_key_len ); + ret = mbedtls_md_hmac_starts( &transform->md_ctx_enc, + mac_enc, mac_key_len ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_hmac_starts( &transform->md_ctx_dec, + mac_dec, mac_key_len ); + if( ret != 0 ) + return( ret ); } } else @@ -1390,17 +1458,18 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch * SSLv3.0 MAC functions */ #define SSL_MAC_MAX_BYTES 20 /* MD-5 or SHA-1 */ -static void ssl_mac( mbedtls_md_context_t *md_ctx, - const unsigned char *secret, - const unsigned char *buf, size_t len, - const unsigned char *ctr, int type, - unsigned char out[SSL_MAC_MAX_BYTES] ) +static int ssl_mac( mbedtls_md_context_t *md_ctx, + const unsigned char *secret, + const unsigned char *buf, size_t len, + const unsigned char *ctr, int type, + unsigned char out[SSL_MAC_MAX_BYTES] ) { unsigned char header[11]; unsigned char padding[48]; int padlen; int md_size = mbedtls_md_get_size( md_ctx->md_info ); int md_type = mbedtls_md_get_type( md_ctx->md_info ); + int ret; /* Only MD5 and SHA-1 supported */ if( md_type == MBEDTLS_MD_MD5 ) @@ -1414,19 +1483,43 @@ static void ssl_mac( mbedtls_md_context_t *md_ctx, header[10] = (unsigned char)( len ); memset( padding, 0x36, padlen ); - mbedtls_md_starts( md_ctx ); - mbedtls_md_update( md_ctx, secret, md_size ); - mbedtls_md_update( md_ctx, padding, padlen ); - mbedtls_md_update( md_ctx, header, 11 ); - mbedtls_md_update( md_ctx, buf, len ); - mbedtls_md_finish( md_ctx, out ); + ret = mbedtls_md_starts( md_ctx ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_update( md_ctx, secret, md_size ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_update( md_ctx, padding, padlen ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_update( md_ctx, header, 11 ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_update( md_ctx, buf, len ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_finish( md_ctx, out ); + if( ret != 0 ) + return( ret ); memset( padding, 0x5C, padlen ); - mbedtls_md_starts( md_ctx ); - mbedtls_md_update( md_ctx, secret, md_size ); - mbedtls_md_update( md_ctx, padding, padlen ); - mbedtls_md_update( md_ctx, out, md_size ); - mbedtls_md_finish( md_ctx, out ); + ret = mbedtls_md_starts( md_ctx ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_update( md_ctx, secret, md_size ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_update( md_ctx, padding, padlen ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_update( md_ctx, out, md_size ); + if( ret != 0 ) + return( ret ); + ret = mbedtls_md_finish( md_ctx, out ); + if( ret != 0 ) + return( ret ); + + return( 0 ); } #endif /* MBEDTLS_SSL_PROTO_SSL3 */ @@ -1471,15 +1564,22 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl ) if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) { unsigned char mac[SSL_MAC_MAX_BYTES]; + int ret; - ssl_mac( &ssl->transform_out->md_ctx_enc, - ssl->transform_out->mac_enc, - ssl->out_msg, ssl->out_msglen, - ssl->out_ctr, ssl->out_msgtype, - mac ); + ret = ssl_mac( &ssl->transform_out->md_ctx_enc, + ssl->transform_out->mac_enc, + ssl->out_msg, ssl->out_msglen, + ssl->out_ctr, ssl->out_msgtype, + mac ); - memcpy( ssl->out_msg + ssl->out_msglen, mac, ssl->transform_out->maclen ); + if( ret == 0 ) + memcpy( ssl->out_msg + ssl->out_msglen, mac, ssl->transform_out->maclen ); mbedtls_platform_zeroize( mac, ssl->transform_out->maclen ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_mac", ret ); + return( ret ); + } } else #endif @@ -1488,17 +1588,35 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl ) if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 ) { unsigned char mac[MBEDTLS_SSL_MAC_ADD]; + int ret; - mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_ctr, 8 ); - mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_hdr, 3 ); - mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_len, 2 ); - mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, - ssl->out_msg, ssl->out_msglen ); - mbedtls_md_hmac_finish( &ssl->transform_out->md_ctx_enc, mac ); - mbedtls_md_hmac_reset( &ssl->transform_out->md_ctx_enc ); + ret = mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_ctr, 8 ); + if( ret != 0 ) + goto hmac_failed_etm_disabled; + ret = mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_hdr, 3 ); + if( ret != 0 ) + goto hmac_failed_etm_disabled; + ret = mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_len, 2 ); + if( ret != 0 ) + goto hmac_failed_etm_disabled; + ret = mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, + ssl->out_msg, ssl->out_msglen ); + ret = mbedtls_md_hmac_finish( &ssl->transform_out->md_ctx_enc, mac ); + if( ret != 0 ) + goto hmac_failed_etm_disabled; + ret = mbedtls_md_hmac_reset( &ssl->transform_out->md_ctx_enc ); + if( ret != 0 ) + goto hmac_failed_etm_disabled; memcpy( ssl->out_msg + ssl->out_msglen, mac, ssl->transform_out->maclen ); + + hmac_failed_etm_disabled: mbedtls_platform_zeroize( mac, ssl->transform_out->maclen ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_md_hmac_xxx", ret ); + return( ret ); + } } else #endif @@ -1751,18 +1869,33 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_BUF( 4, "MAC'd meta-data", pseudo_hdr, 13 ); - mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, pseudo_hdr, 13 ); - mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, - ssl->out_iv, ssl->out_msglen ); - mbedtls_md_hmac_finish( &ssl->transform_out->md_ctx_enc, mac ); - mbedtls_md_hmac_reset( &ssl->transform_out->md_ctx_enc ); + ret = mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, pseudo_hdr, 13 ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; + ret = mbedtls_md_hmac_update( &ssl->transform_out->md_ctx_enc, + ssl->out_iv, ssl->out_msglen ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; + ret = mbedtls_md_hmac_finish( &ssl->transform_out->md_ctx_enc, mac ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; + ret = mbedtls_md_hmac_reset( &ssl->transform_out->md_ctx_enc ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; memcpy( ssl->out_iv + ssl->out_msglen, mac, ssl->transform_out->maclen ); ssl->out_msglen += ssl->transform_out->maclen; auth_done++; + + hmac_failed_etm_enabled: mbedtls_platform_zeroize( mac, ssl->transform_out->maclen ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "HMAC calculation failed", ret ); + return( ret ); + } } #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ } @@ -2148,18 +2281,25 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_BUF( 4, "MAC'd meta-data", pseudo_hdr, 13 ); - mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, pseudo_hdr, 13 ); - mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, - ssl->in_iv, ssl->in_msglen ); - mbedtls_md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect ); - mbedtls_md_hmac_reset( &ssl->transform_in->md_ctx_dec ); + ret = mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, pseudo_hdr, 13 ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; + ret = mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, + ssl->in_iv, ssl->in_msglen ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; + ret = mbedtls_md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; + ret = mbedtls_md_hmac_reset( &ssl->transform_in->md_ctx_dec ); + if( ret != 0 ) + goto hmac_failed_etm_enabled; MBEDTLS_SSL_DEBUG_BUF( 4, "message mac", ssl->in_iv + ssl->in_msglen, ssl->transform_in->maclen ); MBEDTLS_SSL_DEBUG_BUF( 4, "expected mac", mac_expect, ssl->transform_in->maclen ); - ret = 0; if( mbedtls_ssl_safer_memcmp( ssl->in_iv + ssl->in_msglen, mac_expect, ssl->transform_in->maclen ) != 0 ) { @@ -2173,7 +2313,11 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) hmac_failed_etm_enabled: mbedtls_platform_zeroize( mac_expect, ssl->transform_in->maclen ); if( ret != 0 ) + { + if( ret != MBEDTLS_ERR_SSL_INVALID_MAC ) + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_hmac_xxx", ret ); return( ret ); + } } #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ @@ -2342,11 +2486,16 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_SSL3) if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) { - ssl_mac( &ssl->transform_in->md_ctx_dec, - ssl->transform_in->mac_dec, - ssl->in_msg, ssl->in_msglen, - ssl->in_ctr, ssl->in_msgtype, - mac_expect ); + ret = ssl_mac( &ssl->transform_in->md_ctx_dec, + ssl->transform_in->mac_dec, + ssl->in_msg, ssl->in_msglen, + ssl->in_ctr, ssl->in_msgtype, + mac_expect ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_mac", ret ); + return( ret ); + } memcpy( mac_peer, ssl->in_msg + ssl->in_msglen, ssl->transform_in->maclen ); } -- cgit v1.1