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 <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine 2021-12-10 21:33:21 +01:00
parent 476b157dc4
commit d5ba50e239
2 changed files with 22 additions and 2 deletions

View File

@ -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.

View File

@ -685,6 +685,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl,
rec->data_len += transform->maclen; rec->data_len += transform->maclen;
post_avail -= transform->maclen; post_avail -= transform->maclen;
auth_done++; auth_done++;
mbedtls_platform_zeroize( mac, transform->maclen );
} }
#endif /* MBEDTLS_SSL_SOME_SUITES_USE_MAC */ #endif /* MBEDTLS_SSL_SOME_SUITES_USE_MAC */
@ -939,6 +940,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl,
rec->data_len += transform->maclen; rec->data_len += transform->maclen;
post_avail -= transform->maclen; post_avail -= transform->maclen;
auth_done++; auth_done++;
mbedtls_platform_zeroize( mac, transform->maclen );
} }
#endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */
} }
@ -1222,13 +1224,20 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl,
transform->maclen ); transform->maclen );
/* Compare expected MAC with MAC at the end of the record. */ /* Compare expected MAC with MAC at the end of the record. */
ret = 0;
if( mbedtls_ct_memcmp( data + rec->data_len, mac_expect, if( mbedtls_ct_memcmp( data + rec->data_len, mac_expect,
transform->maclen ) != 0 ) transform->maclen ) != 0 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); 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++; auth_done++;
hmac_failed_etm_enabled:
mbedtls_platform_zeroize( mac_expect, transform->maclen );
if( ret != 0 )
return( ret );
} }
#endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */
@ -1420,7 +1429,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl,
if( ret != 0 ) if( ret != 0 )
{ {
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ct_hmac", ret ); MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ct_hmac", ret );
return( ret ); goto hmac_failed_etm_disabled;
} }
mbedtls_ct_memcpy_offset( mac_peer, data, mbedtls_ct_memcpy_offset( mac_peer, data,
@ -1443,6 +1452,12 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl,
correct = 0; correct = 0;
} }
auth_done++; auth_done++;
hmac_failed_etm_disabled:
mbedtls_platform_zeroize( mac_peer, transform->maclen );
mbedtls_platform_zeroize( mac_expect, transform->maclen );
if( ret != 0 )
return( ret );
} }
/* /*