Merge pull request #5325 from gilles-peskine-arm/zeroize-tag-3.1
Zeroize expected MAC/tag intermediate variables
diff --git a/ChangeLog.d/mac-zeroize.txt b/ChangeLog.d/mac-zeroize.txt
new file mode 100644
index 0000000..a43e34f
--- /dev/null
+++ b/ChangeLog.d/mac-zeroize.txt
@@ -0,0 +1,6 @@
+Security
+ * Zeroize several intermediate variables used to calculate the expected
+ value when verifying a MAC or AEAD tag. This hardens the library in
+ case the value leaks through a memory disclosure vulnerability. For
+ example, a memory disclosure vulnerability could have allowed a
+ man-in-the-middle to inject fake ciphertext into a DTLS connection.
diff --git a/ChangeLog.d/ssl-mac-zeroize.txt b/ChangeLog.d/ssl-mac-zeroize.txt
deleted file mode 100644
index b49c7ac..0000000
--- a/ChangeLog.d/ssl-mac-zeroize.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-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/cipher.c b/library/cipher.c
index 0d9d710..03e84c6 100644
--- a/library/cipher.c
+++ b/library/cipher.c
@@ -1175,6 +1175,12 @@
}
#endif /* MBEDTLS_USE_PSA_CRYPTO */
+ /* Status to return on a non-authenticated algorithm. It would make sense
+ * to return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT or perhaps
+ * MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, but at the time I write this our
+ * unit tests assume 0. */
+ ret = 0;
+
#if defined(MBEDTLS_GCM_C)
if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode )
{
@@ -1195,9 +1201,10 @@
/* Check the tag in "constant-time" */
if( mbedtls_ct_memcmp( tag, check_tag, tag_len ) != 0 )
- return( MBEDTLS_ERR_CIPHER_AUTH_FAILED );
-
- return( 0 );
+ {
+ ret = MBEDTLS_ERR_CIPHER_AUTH_FAILED;
+ goto exit;
+ }
}
#endif /* MBEDTLS_GCM_C */
@@ -1217,13 +1224,16 @@
/* Check the tag in "constant-time" */
if( mbedtls_ct_memcmp( tag, check_tag, tag_len ) != 0 )
- return( MBEDTLS_ERR_CIPHER_AUTH_FAILED );
-
- return( 0 );
+ {
+ ret = MBEDTLS_ERR_CIPHER_AUTH_FAILED;
+ goto exit;
+ }
}
#endif /* MBEDTLS_CHACHAPOLY_C */
- return( 0 );
+exit:
+ mbedtls_platform_zeroize( check_tag, tag_len );
+ return( ret );
}
#endif /* MBEDTLS_GCM_C || MBEDTLS_CHACHAPOLY_C */
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 9f5b946..f257651 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -2210,6 +2210,7 @@
status = PSA_ERROR_INVALID_SIGNATURE;
exit:
+ mbedtls_platform_zeroize( actual_hash, sizeof( actual_hash ) );
if( status != PSA_SUCCESS )
psa_hash_abort(operation);
@@ -2244,12 +2245,18 @@
actual_hash, sizeof(actual_hash),
&actual_hash_length );
if( status != PSA_SUCCESS )
- return( status );
+ goto exit;
if( actual_hash_length != hash_length )
- return( PSA_ERROR_INVALID_SIGNATURE );
+ {
+ status = PSA_ERROR_INVALID_SIGNATURE;
+ goto exit;
+ }
if( mbedtls_psa_safer_memcmp( hash, actual_hash, actual_hash_length ) != 0 )
- return( PSA_ERROR_INVALID_SIGNATURE );
- return( PSA_SUCCESS );
+ status = PSA_ERROR_INVALID_SIGNATURE;
+
+exit:
+ mbedtls_platform_zeroize( actual_hash, sizeof( actual_hash ) );
+ return( status );
}
psa_status_t psa_hash_clone( const psa_hash_operation_t *source_operation,
diff --git a/library/rsa.c b/library/rsa.c
index e3ec056..36f487f 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -1896,9 +1896,13 @@
memcpy( sig, sig_try, ctx->len );
cleanup:
+ mbedtls_platform_zeroize( sig_try, ctx->len );
+ mbedtls_platform_zeroize( verif, ctx->len );
mbedtls_free( sig_try );
mbedtls_free( verif );
+ if( ret != 0 )
+ memset( sig, '!', ctx->len );
return( ret );
}
#endif /* MBEDTLS_PKCS1_V15 */
diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c
index 7516786..358169e 100644
--- a/library/ssl_cookie.c
+++ b/library/ssl_cookie.c
@@ -217,15 +217,20 @@
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &ctx->mutex ) != 0 )
- return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_SSL_INTERNAL_ERROR,
- MBEDTLS_ERR_THREADING_MUTEX_ERROR ) );
+ {
+ ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_SSL_INTERNAL_ERROR,
+ MBEDTLS_ERR_THREADING_MUTEX_ERROR );
+ }
#endif
if( ret != 0 )
- return( ret );
+ goto exit;
if( mbedtls_ct_memcmp( cookie + 4, ref_hmac, sizeof( ref_hmac ) ) != 0 )
- return( -1 );
+ {
+ ret = -1;
+ goto exit;
+ }
#if defined(MBEDTLS_HAVE_TIME)
cur_time = (unsigned long) mbedtls_time( NULL );
@@ -239,8 +244,13 @@
( (unsigned long) cookie[3] );
if( ctx->timeout != 0 && cur_time - cookie_time > ctx->timeout )
- return( -1 );
+ {
+ ret = -1;
+ goto exit;
+ }
- return( 0 );
+exit:
+ mbedtls_platform_zeroize( ref_hmac, sizeof( ref_hmac ) );
+ return( ret );
}
#endif /* MBEDTLS_SSL_COOKIE_C */
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index a974dbb..d868e49 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -2874,7 +2874,7 @@
int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl )
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
- unsigned int hash_len;
+ unsigned int hash_len = 12;
unsigned char buf[SSL_MAX_HASH_LEN];
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished" ) );
@@ -2884,7 +2884,7 @@
if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
- return( ret );
+ goto exit;
}
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE )
@@ -2892,16 +2892,16 @@
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) );
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE );
- return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
+ ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE;
+ goto exit;
}
- hash_len = 12;
-
if( ssl->in_msg[0] != MBEDTLS_SSL_HS_FINISHED )
{
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE );
- return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
+ ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE;
+ goto exit;
}
if( ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) + hash_len )
@@ -2909,7 +2909,8 @@
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) );
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
- return( MBEDTLS_ERR_SSL_DECODE_ERROR );
+ ret = MBEDTLS_ERR_SSL_DECODE_ERROR;
+ goto exit;
}
if( mbedtls_ct_memcmp( ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ),
@@ -2918,7 +2919,8 @@
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) );
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR );
- return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE );
+ ret = MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE;
+ goto exit;
}
#if defined(MBEDTLS_SSL_RENEGOTIATION)
@@ -2947,7 +2949,9 @@
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished" ) );
- return( 0 );
+exit:
+ mbedtls_platform_zeroize( buf, hash_len );
+ return( ret );
}
static void ssl_handshake_params_init( mbedtls_ssl_handshake_params *handshake )
