From 378e7eb5cc149ac6267426b109ba1c68e2dd7729 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 19 Jul 2021 15:19:19 +0200 Subject: [PATCH] Unify memcmp functions Signed-off-by: Gabor Mezei --- library/cipher.c | 4 +-- library/constant_time.c | 65 ++--------------------------------------- library/constant_time.h | 18 ++---------- library/nist_kw.c | 4 +-- library/rsa.c | 6 ++-- library/ssl_cli.c | 4 +-- library/ssl_cookie.c | 2 +- library/ssl_msg.c | 10 +++---- library/ssl_srv.c | 4 +-- library/ssl_tls.c | 2 +- 10 files changed, 24 insertions(+), 95 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index 147241345..f38eb0403 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -1139,7 +1139,7 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, } /* Check the tag in "constant-time" */ - if( mbedtls_constant_time_memcmp( tag, check_tag, tag_len ) != 0 ) + if( mbedtls_cf_memcmp( tag, check_tag, tag_len ) != 0 ) return( MBEDTLS_ERR_CIPHER_AUTH_FAILED ); return( 0 ); @@ -1161,7 +1161,7 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, } /* Check the tag in "constant-time" */ - if( mbedtls_constant_time_memcmp( tag, check_tag, tag_len ) != 0 ) + if( mbedtls_cf_memcmp( tag, check_tag, tag_len ) != 0 ) return( MBEDTLS_ERR_CIPHER_AUTH_FAILED ); return( 0 ); diff --git a/library/constant_time.c b/library/constant_time.c index 822285a77..59e2c877a 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -31,10 +31,9 @@ #include -/* constant-time buffer comparison */ -int mbedtls_ssl_safer_memcmp( const void *a, - const void *b, - size_t n ) +int mbedtls_cf_memcmp( const void *a, + const void *b, + size_t n ) { size_t i; volatile const unsigned char *A = (volatile const unsigned char *) a; @@ -50,67 +49,9 @@ int mbedtls_ssl_safer_memcmp( const void *a, diff |= x ^ y; } - return( diff ); -} - -/* Compare the contents of two buffers in constant time. - * Returns 0 if the contents are bitwise identical, otherwise returns - * a non-zero value. - * This is currently only used by GCM and ChaCha20+Poly1305. - */ -int mbedtls_constant_time_memcmp( const void *v1, - const void *v2, - size_t len ) -{ - const unsigned char *p1 = (const unsigned char*) v1; - const unsigned char *p2 = (const unsigned char*) v2; - size_t i; - unsigned char diff; - - for( diff = 0, i = 0; i < len; i++ ) - diff |= p1[i] ^ p2[i]; - return( (int)diff ); } -/* constant-time buffer comparison */ -unsigned char mbedtls_nist_kw_safer_memcmp( const void *a, - const void *b, - size_t n ) -{ - size_t i; - volatile const unsigned char *A = (volatile const unsigned char *) a; - volatile const unsigned char *B = (volatile const unsigned char *) b; - volatile unsigned char diff = 0; - - for( i = 0; i < n; i++ ) - { - /* Read volatile data in order before computing diff. - * This avoids IAR compiler warning: - * 'the order of volatile accesses is undefined ..' */ - unsigned char x = A[i], y = B[i]; - diff |= x ^ y; - } - - return( diff ); -} - -/* constant-time buffer comparison */ -int mbedtls_safer_memcmp( const void *a, - const void *b, - size_t n ) -{ - size_t i; - const unsigned char *A = (const unsigned char *) a; - const unsigned char *B = (const unsigned char *) b; - unsigned char diff = 0; - - for( i = 0; i < n; i++ ) - diff |= A[i] ^ B[i]; - - return( diff ); -} - /** Turn zero-or-nonzero into zero-or-all-bits-one, without branches. * * \param value The value to analyze. diff --git a/library/constant_time.h b/library/constant_time.h index e6deb45b1..ae43081dc 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -29,22 +29,10 @@ #include -int mbedtls_ssl_safer_memcmp( const void *a, - const void *b, - size_t n ); - -int mbedtls_constant_time_memcmp( const void *v1, - const void *v2, - size_t len ); - -unsigned char mbedtls_nist_kw_safer_memcmp( const void *a, - const void *b, - size_t n ); - -int mbedtls_safer_memcmp( const void *a, - const void *b, - size_t n ); +int mbedtls_cf_memcmp( const void *a, + const void *b, + size_t n ); unsigned mbedtls_cf_uint_mask( unsigned value ); diff --git a/library/nist_kw.c b/library/nist_kw.c index 6262a7b0c..98c237be4 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -379,7 +379,7 @@ int mbedtls_nist_kw_unwrap( mbedtls_nist_kw_context *ctx, goto cleanup; /* Check ICV in "constant-time" */ - diff = mbedtls_nist_kw_safer_memcmp( NIST_KW_ICV1, A, KW_SEMIBLOCK_LENGTH ); + diff = mbedtls_cf_memcmp( NIST_KW_ICV1, A, KW_SEMIBLOCK_LENGTH ); if( diff != 0 ) { @@ -428,7 +428,7 @@ int mbedtls_nist_kw_unwrap( mbedtls_nist_kw_context *ctx, } /* Check ICV in "constant-time" */ - diff = mbedtls_nist_kw_safer_memcmp( NIST_KW_ICV2, A, KW_SEMIBLOCK_LENGTH / 2 ); + diff = mbedtls_cf_memcmp( NIST_KW_ICV2, A, KW_SEMIBLOCK_LENGTH / 2 ); if( diff != 0 ) { diff --git a/library/rsa.c b/library/rsa.c index 3f076f0e5..a387d0989 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1933,7 +1933,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_rsa_private( ctx, f_rng, p_rng, sig, sig_try ) ); MBEDTLS_MPI_CHK( mbedtls_rsa_public( ctx, sig_try, verif ) ); - if( mbedtls_safer_memcmp( verif, sig, ctx->len ) != 0 ) + if( mbedtls_cf_memcmp( verif, sig, ctx->len ) != 0 ) { ret = MBEDTLS_ERR_RSA_PRIVATE_FAILED; goto cleanup; @@ -2231,8 +2231,8 @@ int mbedtls_rsa_rsassa_pkcs1_v15_verify( mbedtls_rsa_context *ctx, * Compare */ - if( ( ret = mbedtls_safer_memcmp( encoded, encoded_expected, - sig_len ) ) != 0 ) + if( ( ret = mbedtls_cf_memcmp( encoded, encoded_expected, + sig_len ) ) != 0 ) { ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; goto cleanup; diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 912b5d007..0bc39fa74 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1459,9 +1459,9 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, /* Check verify-data in constant-time. The length OTOH is no secret */ if( len != 1 + ssl->verify_data_len * 2 || buf[0] != ssl->verify_data_len * 2 || - mbedtls_ssl_safer_memcmp( buf + 1, + mbedtls_cf_memcmp( buf + 1, ssl->own_verify_data, ssl->verify_data_len ) != 0 || - mbedtls_ssl_safer_memcmp( buf + 1 + ssl->verify_data_len, + mbedtls_cf_memcmp( buf + 1 + ssl->verify_data_len, ssl->peer_verify_data, ssl->verify_data_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching renegotiation info" ) ); diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c index 84fbab1a4..d19a13a35 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -225,7 +225,7 @@ int mbedtls_ssl_cookie_check( void *p_ctx, if( ret != 0 ) return( ret ); - if( mbedtls_ssl_safer_memcmp( cookie + 4, ref_hmac, sizeof( ref_hmac ) ) != 0 ) + if( mbedtls_cf_memcmp( cookie + 4, ref_hmac, sizeof( ref_hmac ) ) != 0 ) return( -1 ); #if defined(MBEDTLS_HAVE_TIME) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 0b940ec36..57a56f4cd 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -1281,7 +1281,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, * * Afterwards, we know that data + data_len is followed by at * least maclen Bytes, which justifies the call to - * mbedtls_ssl_safer_memcmp() below. + * mbedtls_cf_memcmp() below. * * Further, we still know that data_len > minlen */ rec->data_len -= transform->maclen; @@ -1304,8 +1304,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, transform->maclen ); /* Compare expected MAC with MAC at the end of the record. */ - if( mbedtls_ssl_safer_memcmp( data + rec->data_len, mac_expect, - transform->maclen ) != 0 ) + if( mbedtls_cf_memcmp( data + rec->data_len, mac_expect, + transform->maclen ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); return( MBEDTLS_ERR_SSL_INVALID_MAC ); @@ -1582,8 +1582,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, MBEDTLS_SSL_DEBUG_BUF( 4, "message mac", mac_peer, transform->maclen ); #endif - if( mbedtls_ssl_safer_memcmp( mac_peer, mac_expect, - transform->maclen ) != 0 ) + if( mbedtls_cf_memcmp( mac_peer, mac_expect, + transform->maclen ) != 0 ) { #if defined(MBEDTLS_SSL_DEBUG_ALL) MBEDTLS_SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index b9e4bc0f6..9c1bea2bc 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -197,7 +197,7 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, /* Check verify-data in constant-time. The length OTOH is no secret */ if( len != 1 + ssl->verify_data_len || buf[0] != ssl->verify_data_len || - mbedtls_ssl_safer_memcmp( buf + 1, ssl->peer_verify_data, + mbedtls_cf_memcmp( buf + 1, ssl->peer_verify_data, ssl->verify_data_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching renegotiation info" ) ); @@ -4064,7 +4064,7 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha /* Identity is not a big secret since clients send it in the clear, * but treat it carefully anyway, just in case */ if( n != ssl->conf->psk_identity_len || - mbedtls_ssl_safer_memcmp( ssl->conf->psk_identity, *p, n ) != 0 ) + mbedtls_cf_memcmp( ssl->conf->psk_identity, *p, n ) != 0 ) { ret = MBEDTLS_ERR_SSL_UNKNOWN_IDENTITY; } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 607981a8d..3fafb7d84 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3604,7 +3604,7 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_FINISHED ); } - if( mbedtls_ssl_safer_memcmp( ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ), + if( mbedtls_cf_memcmp( ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ), buf, hash_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) );