From d112534585095e4a04f275b6a0cad5505624f399 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 12 Jul 2021 16:31:22 +0200 Subject: [PATCH 01/52] Add a new file for constant-time functions Signed-off-by: gabor-mezei-arm --- library/CMakeLists.txt | 1 + library/Makefile | 1 + library/constant_time.c | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 library/constant_time.c diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 5adc128c96..bad8839f3e 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -24,6 +24,7 @@ set(src_crypto chachapoly.c cipher.c cipher_wrap.c + constant_time.c cmac.c ctr_drbg.c des.c diff --git a/library/Makefile b/library/Makefile index 8c58fb8501..1677a29675 100644 --- a/library/Makefile +++ b/library/Makefile @@ -84,6 +84,7 @@ OBJS_CRYPTO= \ cipher.o \ cipher_wrap.o \ cmac.o \ + constant_time.o \ ctr_drbg.o \ des.o \ dhm.o \ diff --git a/library/constant_time.c b/library/constant_time.c new file mode 100644 index 0000000000..9f0229bd52 --- /dev/null +++ b/library/constant_time.c @@ -0,0 +1,20 @@ +/** + * Constant-time functions + * + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "common.h" From 9fa43ce2380f81537dc28adf35cba4edc5d388bf Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Tue, 28 Sep 2021 16:14:47 +0200 Subject: [PATCH 02/52] Rename function to have suitable name Signed-off-by: gabor-mezei-arm --- library/bignum.c | 30 ++++++------ library/rsa.c | 59 ++++++++++++----------- library/ssl_invasive.h | 12 ++--- library/ssl_msg.c | 70 ++++++++++++++-------------- tests/suites/test_suite_ssl.function | 14 +++--- 5 files changed, 94 insertions(+), 91 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 42ec7ac314..0df325d431 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -280,7 +280,7 @@ void mbedtls_mpi_swap( mbedtls_mpi *X, mbedtls_mpi *Y ) * * \return The selected sign value. */ -static int mpi_safe_cond_select_sign( int a, int b, unsigned char second ) +static int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ) { /* In order to avoid questions about what we can reasonnably assume about * the representations of signed integers, move everything to unsigned @@ -304,10 +304,10 @@ static int mpi_safe_cond_select_sign( int a, int b, unsigned char second ) * dest and src must be arrays of limbs of size n. * assign must be 0 or 1. */ -static void mpi_safe_cond_assign( size_t n, - mbedtls_mpi_uint *dest, - const mbedtls_mpi_uint *src, - unsigned char assign ) +void mbedtls_cf_mpi_uint_cond_assign( size_t n, + mbedtls_mpi_uint *dest, + const mbedtls_mpi_uint *src, + unsigned char assign ) { size_t i; @@ -360,9 +360,9 @@ int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, const mbedtls_mpi *Y, unsigned MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); - X->s = mpi_safe_cond_select_sign( X->s, Y->s, assign ); + X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, assign ); - mpi_safe_cond_assign( Y->n, X->p, Y->p, assign ); + mbedtls_cf_mpi_uint_cond_assign( Y->n, X->p, Y->p, assign ); for( i = Y->n; i < X->n; i++ ) X->p[i] &= ~limb_mask; @@ -409,8 +409,8 @@ int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, mbedtls_mpi *Y, unsigned char sw MBEDTLS_MPI_CHK( mbedtls_mpi_grow( Y, X->n ) ); s = X->s; - X->s = mpi_safe_cond_select_sign( X->s, Y->s, swap ); - Y->s = mpi_safe_cond_select_sign( Y->s, s, swap ); + X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, swap ); + Y->s = mbedtls_cf_cond_select_sign( Y->s, s, swap ); for( i = 0; i < X->n; i++ ) @@ -1253,7 +1253,7 @@ int mbedtls_mpi_cmp_mpi( const mbedtls_mpi *X, const mbedtls_mpi *Y ) * * \return 1 if \p x is less than \p y, 0 otherwise */ -static unsigned ct_lt_mpi_uint( const mbedtls_mpi_uint x, +static unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, const mbedtls_mpi_uint y ) { mbedtls_mpi_uint ret; @@ -1328,7 +1328,7 @@ int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, const mbedtls_mpi *Y, * Again even if we can make a decision, we just mark the result and * the fact that we are done and continue looping. */ - cond = ct_lt_mpi_uint( Y->p[i - 1], X->p[i - 1] ); + cond = mbedtls_cf_mpi_uint_lt( Y->p[i - 1], X->p[i - 1] ); *ret |= cond & ( 1 - done ) & X_is_negative; done |= cond; @@ -1339,7 +1339,7 @@ int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, const mbedtls_mpi *Y, * Again even if we can make a decision, we just mark the result and * the fact that we are done and continue looping. */ - cond = ct_lt_mpi_uint( X->p[i - 1], Y->p[i - 1] ); + cond = mbedtls_cf_mpi_uint_lt( X->p[i - 1], Y->p[i - 1] ); *ret |= cond & ( 1 - done ) & ( 1 - X_is_negative ); done |= cond; } @@ -2207,7 +2207,7 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi * so d[n] == 1 and we want to set A to the result of the subtraction * which is d - (2^biL)^n, i.e. the n least significant limbs of d. * This exactly corresponds to a conditional assignment. */ - mpi_safe_cond_assign( n, A->p, d, (unsigned char) d[n] ); + mbedtls_cf_mpi_uint_cond_assign( n, A->p, d, (unsigned char) d[n] ); } /* @@ -2238,7 +2238,7 @@ static void mpi_montred( mbedtls_mpi *A, const mbedtls_mpi *N, * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -static size_t mbedtls_mpi_cf_bool_eq( size_t x, size_t y ) +static size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) { /* diff = 0 if x == y, non-zero otherwise */ const size_t diff = x ^ y; @@ -2285,7 +2285,7 @@ static int mpi_select( mbedtls_mpi *R, const mbedtls_mpi *T, size_t T_size, size for( size_t i = 0; i < T_size; i++ ) { MBEDTLS_MPI_CHK( mbedtls_mpi_safe_cond_assign( R, &T[i], - (unsigned char) mbedtls_mpi_cf_bool_eq( i, idx ) ) ); + (unsigned char) mbedtls_cf_size_bool_eq( i, idx ) ) ); } cleanup: diff --git a/library/rsa.c b/library/rsa.c index e818e6daeb..9c110378c7 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1478,7 +1478,7 @@ cleanup: * \param value The value to analyze. * \return Zero if \p value is zero, otherwise all-bits-one. */ -static unsigned all_or_nothing_int( unsigned value ) +static unsigned mbedtls_cf_uint_mask( unsigned value ) { /* MSVC has a warning about unary minus on unsigned, but this is * well-defined and precisely what we want to do here */ @@ -1502,7 +1502,7 @@ static unsigned all_or_nothing_int( unsigned value ) * \return \c 0 if `size <= max`. * \return \c 1 if `size > max`. */ -static unsigned size_greater_than( size_t size, size_t max ) +static unsigned mbedtls_cf_size_gt( size_t size, size_t max ) { /* Return the sign bit (1 for negative) of (max - size). */ return( ( max - size ) >> ( sizeof( size_t ) * 8 - 1 ) ); @@ -1518,16 +1518,17 @@ static unsigned size_greater_than( size_t size, size_t max ) * \param if0 Value to use if \p cond is zero. * \return \c if1 if \p cond is nonzero, otherwise \c if0. */ -static unsigned if_int( unsigned cond, unsigned if1, unsigned if0 ) +static unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ) { - unsigned mask = all_or_nothing_int( cond ); + unsigned mask = mbedtls_cf_uint_mask( cond ); return( ( mask & if1 ) | (~mask & if0 ) ); } /** Shift some data towards the left inside a buffer without leaking * the length of the data through side channels. * - * `mem_move_to_left(start, total, offset)` is functionally equivalent to + * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally + * equivalent to * ``` * memmove(start, start + offset, total - offset); * memset(start + offset, 0, total - offset); @@ -1540,9 +1541,9 @@ static unsigned if_int( unsigned cond, unsigned if1, unsigned if0 ) * \param total Total size of the buffer. * \param offset Offset from which to copy \p total - \p offset bytes. */ -static void mem_move_to_left( void *start, - size_t total, - size_t offset ) +static void mbedtls_cf_mem_move_to_left( void *start, + size_t total, + size_t offset ) { volatile unsigned char *buf = start; size_t i, n; @@ -1550,7 +1551,7 @@ static void mem_move_to_left( void *start, return; for( i = 0; i < total; i++ ) { - unsigned no_op = size_greater_than( total - offset, i ); + unsigned no_op = mbedtls_cf_size_gt( total - offset, i ); /* The first `total - offset` passes are a no-op. The last * `offset` passes shift the data one byte to the left and * zero out the last byte. */ @@ -1558,9 +1559,9 @@ static void mem_move_to_left( void *start, { unsigned char current = buf[n]; unsigned char next = buf[n+1]; - buf[n] = if_int( no_op, current, next ); + buf[n] = mbedtls_cf_uint_if( no_op, current, next ); } - buf[total-1] = if_int( no_op, buf[total-1], 0 ); + buf[total-1] = mbedtls_cf_uint_if( no_op, buf[total-1], 0 ); } } @@ -1634,10 +1635,10 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, /* If pad_done is still zero, there's no data, only unfinished padding. */ - bad |= if_int( pad_done, 0, 1 ); + bad |= mbedtls_cf_uint_if( pad_done, 0, 1 ); /* There must be at least 8 bytes of padding. */ - bad |= size_greater_than( 8, pad_count ); + bad |= mbedtls_cf_size_gt( 8, pad_count ); /* If the padding is valid, set plaintext_size to the number of * remaining bytes after stripping the padding. If the padding @@ -1646,23 +1647,25 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, * buffer. Do it without branches to avoid leaking the padding * validity through timing. RSA keys are small enough that all the * size_t values involved fit in unsigned int. */ - plaintext_size = if_int( bad, - (unsigned) plaintext_max_size, - (unsigned) ( ilen - pad_count - 3 ) ); + plaintext_size = mbedtls_cf_uint_if( + bad, (unsigned) plaintext_max_size, + (unsigned) ( ilen - pad_count - 3 ) ); /* Set output_too_large to 0 if the plaintext fits in the output * buffer and to 1 otherwise. */ - output_too_large = size_greater_than( plaintext_size, - plaintext_max_size ); + output_too_large = mbedtls_cf_size_gt( plaintext_size, + plaintext_max_size ); /* Set ret without branches to avoid timing attacks. Return: * - INVALID_PADDING if the padding is bad (bad != 0). * - OUTPUT_TOO_LARGE if the padding is good but the decrypted * plaintext does not fit in the output buffer. * - 0 if the padding is correct. */ - ret = - (int) if_int( bad, - MBEDTLS_ERR_RSA_INVALID_PADDING, - if_int( output_too_large, - MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, - 0 ) ); + ret = - (int) mbedtls_cf_uint_if( + bad, - MBEDTLS_ERR_RSA_INVALID_PADDING, + mbedtls_cf_uint_if( output_too_large, + - MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, + 0 ) ); /* If the padding is bad or the plaintext is too large, zero the * data that we're about to copy to the output buffer. @@ -1670,7 +1673,7 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, * from the same buffer whether the padding is good or not to * avoid leaking the padding validity through overall timing or * through memory or cache access patterns. */ - bad = all_or_nothing_int( bad | output_too_large ); + bad = mbedtls_cf_uint_mask( bad | output_too_large ); for( i = 11; i < ilen; i++ ) buf[i] &= ~bad; @@ -1678,9 +1681,9 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, * Copy anyway to avoid revealing the length through timing, because * revealing the length is as bad as revealing the padding validity * for a Bleichenbacher attack. */ - plaintext_size = if_int( output_too_large, - (unsigned) plaintext_max_size, - (unsigned) plaintext_size ); + plaintext_size = mbedtls_cf_uint_if( output_too_large, + (unsigned) plaintext_max_size, + (unsigned) plaintext_size ); /* Move the plaintext to the leftmost position where it can start in * the working buffer, i.e. make it start plaintext_max_size from @@ -1688,9 +1691,9 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, * does not depend on the plaintext size. After this move, the * starting location of the plaintext is no longer sensitive * information. */ - mem_move_to_left( buf + ilen - plaintext_max_size, - plaintext_max_size, - plaintext_max_size - plaintext_size ); + mbedtls_cf_mem_move_to_left( buf + ilen - plaintext_max_size, + plaintext_max_size, + plaintext_max_size - plaintext_size ); /* Finally copy the decrypted plaintext plus trailing zeros into the output * buffer. If output_max_len is 0, then output may be an invalid pointer diff --git a/library/ssl_invasive.h b/library/ssl_invasive.h index babbc2768c..5cb29cb0a8 100644 --- a/library/ssl_invasive.h +++ b/library/ssl_invasive.h @@ -65,7 +65,7 @@ * \retval MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED * The hardware accelerator failed. */ -int mbedtls_ssl_cf_hmac( +int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, const unsigned char *add_data, size_t add_data_len, const unsigned char *data, size_t data_len_secret, @@ -90,11 +90,11 @@ int mbedtls_ssl_cf_hmac( * \param offset_max The maximal value of \p offset_secret. * \param len The number of bytes to copy. */ -void mbedtls_ssl_cf_memcpy_offset( unsigned char *dst, - const unsigned char *src_base, - size_t offset_secret, - size_t offset_min, size_t offset_max, - size_t len ); +void mbedtls_cf_memcpy_offset( unsigned char *dst, + const unsigned char *src_base, + size_t offset_secret, + size_t offset_min, size_t offset_max, + size_t len ); #endif /* MBEDTLS_TEST_HOOKS && MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ #endif /* MBEDTLS_SSL_INVASIVE_H */ diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 76cc2b17d4..34b28b1cb0 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -949,7 +949,7 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -static size_t mbedtls_ssl_cf_mask_from_bit( size_t bit ) +static size_t mbedtls_cf_size_mask( size_t bit ) { /* MSVC has a warning about unary minus on unsigned integer types, * but this is well-defined and precisely what we want to do here. */ @@ -974,7 +974,7 @@ static size_t mbedtls_ssl_cf_mask_from_bit( size_t bit ) * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -static size_t mbedtls_ssl_cf_mask_lt( size_t x, size_t y ) +static size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ) { /* This has the most significant bit set if and only if x < y */ const size_t sub = x - y; @@ -983,7 +983,7 @@ static size_t mbedtls_ssl_cf_mask_lt( size_t x, size_t y ) const size_t sub1 = sub >> ( sizeof( sub ) * 8 - 1 ); /* mask = (x < y) ? 0xff... : 0x00... */ - const size_t mask = mbedtls_ssl_cf_mask_from_bit( sub1 ); + const size_t mask = mbedtls_cf_size_mask( sub1 ); return( mask ); } @@ -999,9 +999,9 @@ static size_t mbedtls_ssl_cf_mask_lt( size_t x, size_t y ) * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -static size_t mbedtls_ssl_cf_mask_ge( size_t x, size_t y ) +static size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ) { - return( ~mbedtls_ssl_cf_mask_lt( x, y ) ); + return( ~mbedtls_cf_size_mask_lt( x, y ) ); } /* @@ -1010,12 +1010,12 @@ static size_t mbedtls_ssl_cf_mask_ge( size_t x, size_t y ) * * This function can be used to write constant-time code by replacing branches * with bit operations - it can be used in conjunction with - * mbedtls_ssl_cf_mask_from_bit(). + * mbedtls_cf_size_mask(). * * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -static size_t mbedtls_ssl_cf_bool_eq( size_t x, size_t y ) +static size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) { /* diff = 0 if x == y, non-zero otherwise */ const size_t diff = x ^ y; @@ -1049,14 +1049,14 @@ static size_t mbedtls_ssl_cf_bool_eq( size_t x, size_t y ) * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -static void mbedtls_ssl_cf_memcpy_if_eq( unsigned char *dst, - const unsigned char *src, - size_t len, - size_t c1, size_t c2 ) +static void mbedtls_cf_memcpy_if_eq( unsigned char *dst, + const unsigned char *src, + size_t len, + size_t c1, size_t c2 ) { /* mask = c1 == c2 ? 0xff : 0x00 */ - const size_t equal = mbedtls_ssl_cf_bool_eq( c1, c2 ); - const unsigned char mask = (unsigned char) mbedtls_ssl_cf_mask_from_bit( equal ); + const size_t equal = mbedtls_cf_size_bool_eq( c1, c2 ); + const unsigned char mask = (unsigned char) mbedtls_cf_size_mask( equal ); /* dst[i] = c1 == c2 ? src[i] : dst[i] */ for( size_t i = 0; i < len; i++ ) @@ -1069,7 +1069,7 @@ static void mbedtls_ssl_cf_memcpy_if_eq( unsigned char *dst, * Only works with MD-5, SHA-1, SHA-256 and SHA-384. * (Otherwise, computation of block_size needs to be adapted.) */ -MBEDTLS_STATIC_TESTABLE int mbedtls_ssl_cf_hmac( +MBEDTLS_STATIC_TESTABLE int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, const unsigned char *add_data, size_t add_data_len, const unsigned char *data, size_t data_len_secret, @@ -1125,8 +1125,8 @@ MBEDTLS_STATIC_TESTABLE int mbedtls_ssl_cf_hmac( MD_CHK( mbedtls_md_clone( &aux, ctx ) ); MD_CHK( mbedtls_md_finish( &aux, aux_out ) ); /* Keep only the correct inner_hash in the output buffer */ - mbedtls_ssl_cf_memcpy_if_eq( output, aux_out, hash_size, - offset, data_len_secret ); + mbedtls_cf_memcpy_if_eq( output, aux_out, hash_size, + offset, data_len_secret ); if( offset < max_data_len ) MD_CHK( mbedtls_md_update( ctx, data + offset, 1 ) ); @@ -1156,7 +1156,7 @@ cleanup: * - functionally equivalent to memcpy(dst, src + offset_secret, len) * - but with execution flow independent from the value of offset_secret. */ -MBEDTLS_STATIC_TESTABLE void mbedtls_ssl_cf_memcpy_offset( +MBEDTLS_STATIC_TESTABLE void mbedtls_cf_memcpy_offset( unsigned char *dst, const unsigned char *src_base, size_t offset_secret, @@ -1167,8 +1167,8 @@ MBEDTLS_STATIC_TESTABLE void mbedtls_ssl_cf_memcpy_offset( for( offset = offset_min; offset <= offset_max; offset++ ) { - mbedtls_ssl_cf_memcpy_if_eq( dst, src_base + offset, len, - offset, offset_secret ); + mbedtls_cf_memcpy_if_eq( dst, src_base + offset, len, + offset, offset_secret ); } } #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ @@ -1494,7 +1494,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, if( auth_done == 1 ) { - const size_t mask = mbedtls_ssl_cf_mask_ge( + const size_t mask = mbedtls_cf_size_mask_ge( rec->data_len, padlen + 1 ); correct &= mask; @@ -1514,7 +1514,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, } #endif - const size_t mask = mbedtls_ssl_cf_mask_ge( + const size_t mask = mbedtls_cf_size_mask_ge( rec->data_len, transform->maclen + padlen + 1 ); correct &= mask; @@ -1548,18 +1548,18 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, /* pad_count += (idx >= padding_idx) && * (check[idx] == padlen - 1); */ - const size_t mask = mbedtls_ssl_cf_mask_ge( idx, padding_idx ); - const size_t equal = mbedtls_ssl_cf_bool_eq( check[idx], - padlen - 1 ); + const size_t mask = mbedtls_cf_size_mask_ge( idx, padding_idx ); + const size_t equal = mbedtls_cf_size_bool_eq( check[idx], + padlen - 1 ); pad_count += mask & equal; } - correct &= mbedtls_ssl_cf_bool_eq( pad_count, padlen ); + correct &= mbedtls_cf_size_bool_eq( pad_count, padlen ); #if defined(MBEDTLS_SSL_DEBUG_ALL) if( padlen > 0 && correct == 0 ) MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad padding byte detected" ) ); #endif - padlen &= mbedtls_ssl_cf_mask_from_bit( correct ); + padlen &= mbedtls_cf_size_mask( correct ); #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ @@ -1622,20 +1622,20 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, const size_t max_len = rec->data_len + padlen; const size_t min_len = ( max_len > 256 ) ? max_len - 256 : 0; - ret = mbedtls_ssl_cf_hmac( &transform->md_ctx_dec, - add_data, add_data_len, - data, rec->data_len, min_len, max_len, - mac_expect ); + ret = mbedtls_cf_hmac( &transform->md_ctx_dec, + add_data, add_data_len, + data, rec->data_len, min_len, max_len, + mac_expect ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_cf_hmac", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cf_hmac", ret ); return( ret ); } - mbedtls_ssl_cf_memcpy_offset( mac_peer, data, - rec->data_len, - min_len, max_len, - transform->maclen ); + mbedtls_cf_memcpy_offset( mac_peer, data, + rec->data_len, + min_len, max_len, + transform->maclen ); #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ #if defined(MBEDTLS_SSL_DEBUG_ALL) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 72b0cdb8ac..26b99a446b 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -4555,7 +4555,7 @@ void resize_buffers_renegotiate_mfl( int mfl, int legacy_renegotiation, void ssl_cf_hmac( int hash ) { /* - * Test the function mbedtls_ssl_cf_hmac() against a reference + * Test the function mbedtls_cf_hmac() against a reference * implementation. */ mbedtls_md_context_t ctx, ref_ctx; @@ -4614,10 +4614,10 @@ void ssl_cf_hmac( int hash ) /* Get the function's result */ TEST_CF_SECRET( &in_len, sizeof( in_len ) ); - TEST_EQUAL( 0, mbedtls_ssl_cf_hmac( &ctx, add_data, sizeof( add_data ), - data, in_len, - min_in_len, max_in_len, - out ) ); + TEST_EQUAL( 0, mbedtls_cf_hmac( &ctx, add_data, sizeof( add_data ), + data, in_len, + min_in_len, max_in_len, + out ) ); TEST_CF_PUBLIC( &in_len, sizeof( in_len ) ); TEST_CF_PUBLIC( out, out_len ); @@ -4664,8 +4664,8 @@ void ssl_cf_memcpy_offset( int offset_min, int offset_max, int len ) mbedtls_test_set_step( (int) secret ); TEST_CF_SECRET( &secret, sizeof( secret ) ); - mbedtls_ssl_cf_memcpy_offset( dst, src, secret, - offset_min, offset_max, len ); + mbedtls_cf_memcpy_offset( dst, src, secret, + offset_min, offset_max, len ); TEST_CF_PUBLIC( &secret, sizeof( secret ) ); TEST_CF_PUBLIC( dst, len ); From db9a38c6721435d73196a69541bc67e9ccf61744 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 11:28:54 +0200 Subject: [PATCH 03/52] Move contatnt-time memcmp functions to the contant-time module Signed-off-by: gabor-mezei-arm --- library/cipher.c | 22 +----------- library/constant_time.c | 74 +++++++++++++++++++++++++++++++++++++++++ library/constant_time.h | 30 +++++++++++++++++ library/nist_kw.c | 21 +----------- library/rsa.c | 17 +--------- library/ssl_cli.c | 1 + library/ssl_cookie.c | 1 + library/ssl_misc.h | 20 ----------- library/ssl_msg.c | 1 + library/ssl_srv.c | 1 + library/ssl_tls.c | 1 + 11 files changed, 112 insertions(+), 77 deletions(-) create mode 100644 library/constant_time.h diff --git a/library/cipher.c b/library/cipher.c index 546cace552..a53609e4eb 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -29,6 +29,7 @@ #include "cipher_wrap.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "constant_time.h" #include #include @@ -74,27 +75,6 @@ #define CIPHER_VALIDATE( cond ) \ MBEDTLS_INTERNAL_VALIDATE( cond ) -#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) -/* 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. - */ -static 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 ); -} -#endif /* MBEDTLS_GCM_C || MBEDTLS_CHACHAPOLY_C */ - static int supported_init = 0; const int *mbedtls_cipher_list( void ) diff --git a/library/constant_time.c b/library/constant_time.c index 9f0229bd52..cb156bc9b7 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -18,3 +18,77 @@ */ #include "common.h" +#include "constant_time.h" + +/* constant-time buffer comparison */ +int mbedtls_ssl_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 ); +} + +/* 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 ); +} diff --git a/library/constant_time.h b/library/constant_time.h new file mode 100644 index 0000000000..e14232b8bf --- /dev/null +++ b/library/constant_time.h @@ -0,0 +1,30 @@ +/** + * Constant-time functions + * + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "common.h" + +#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 ); diff --git a/library/nist_kw.c b/library/nist_kw.c index 5054ca206b..aaed42a18c 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -34,6 +34,7 @@ #include "mbedtls/nist_kw.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "constant_time.h" #include #include @@ -52,26 +53,6 @@ #define KW_SEMIBLOCK_LENGTH 8 #define MIN_SEMIBLOCKS_COUNT 3 -/* constant-time buffer comparison */ -static inline 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 ); -} - /*! The 64-bit default integrity check value (ICV) for KW mode. */ static const unsigned char NIST_KW_ICV1[] = {0xA6, 0xA6, 0xA6, 0xA6, 0xA6, 0xA6, 0xA6, 0xA6}; /*! The 32-bit default integrity check value (ICV) for KWP mode. */ diff --git a/library/rsa.c b/library/rsa.c index 9c110378c7..f1b8489eee 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -44,6 +44,7 @@ #include "mbedtls/oid.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "constant_time.h" #include @@ -72,22 +73,6 @@ #define RSA_VALIDATE( cond ) \ MBEDTLS_INTERNAL_VALIDATE( cond ) -#if defined(MBEDTLS_PKCS1_V15) -/* constant-time buffer comparison */ -static inline 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 ); -} -#endif /* MBEDTLS_PKCS1_V15 */ - int mbedtls_rsa_import( mbedtls_rsa_context *ctx, const mbedtls_mpi *N, const mbedtls_mpi *P, const mbedtls_mpi *Q, diff --git a/library/ssl_cli.c b/library/ssl_cli.c index e0a1c24ec1..3ef318c963 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -33,6 +33,7 @@ #include "ssl_misc.h" #include "mbedtls/debug.h" #include "mbedtls/error.h" +#include "constant_time.h" #if defined(MBEDTLS_USE_PSA_CRYPTO) #include "mbedtls/psa_util.h" diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c index 40b8913b8b..5936d3598b 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -36,6 +36,7 @@ #include "ssl_misc.h" #include "mbedtls/error.h" #include "mbedtls/platform_util.h" +#include "constant_time.h" #include diff --git a/library/ssl_misc.h b/library/ssl_misc.h index c8e2f4c884..856dd1e16f 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1179,26 +1179,6 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ); int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, const mbedtls_ssl_session *src ); -/* constant-time buffer comparison */ -static inline int mbedtls_ssl_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 ); -} - #if defined(MBEDTLS_SSL_PROTO_TLS1_2) /* The hash buffer must have at least MBEDTLS_MD_MAX_SIZE bytes of length. */ int mbedtls_ssl_get_key_exchange_md_tls1_2( mbedtls_ssl_context *ssl, diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 34b28b1cb0..168f186210 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -40,6 +40,7 @@ #include "mbedtls/error.h" #include "mbedtls/platform_util.h" #include "mbedtls/version.h" +#include "constant_time.h" #include "ssl_invasive.h" diff --git a/library/ssl_srv.c b/library/ssl_srv.c index d82ec0471f..716fa7de15 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -34,6 +34,7 @@ #include "mbedtls/debug.h" #include "mbedtls/error.h" #include "mbedtls/platform_util.h" +#include "constant_time.h" #include diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 923c671a7b..c5ffa4dbbd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -39,6 +39,7 @@ #include "mbedtls/error.h" #include "mbedtls/platform_util.h" #include "mbedtls/version.h" +#include "constant_time.h" #include From 340948e4a5bab405732c3ca9625f1d418d1e3c1d Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 11:40:03 +0200 Subject: [PATCH 04/52] Move mbedtls_cf_uint_mask function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 19 +++++++++++++++++++ library/constant_time.h | 3 +++ library/rsa.c | 19 ------------------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index cb156bc9b7..0c5c04c1fc 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -92,3 +92,22 @@ int mbedtls_safer_memcmp( const void *a, const void *b, size_t n ) return( diff ); } + +/** Turn zero-or-nonzero into zero-or-all-bits-one, without branches. + * + * \param value The value to analyze. + * \return Zero if \p value is zero, otherwise all-bits-one. + */ +unsigned mbedtls_cf_uint_mask( unsigned value ) +{ + /* MSVC has a warning about unary minus on unsigned, but this is + * well-defined and precisely what we want to do here */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif + return( - ( ( value | - value ) >> ( sizeof( value ) * 8 - 1 ) ) ); +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif +} diff --git a/library/constant_time.h b/library/constant_time.h index e14232b8bf..0d6c0fdfc1 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -28,3 +28,6 @@ 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 ); + + +unsigned mbedtls_cf_uint_mask( unsigned value ); diff --git a/library/rsa.c b/library/rsa.c index f1b8489eee..3e19ad950c 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1458,25 +1458,6 @@ cleanup: #endif /* MBEDTLS_PKCS1_V21 */ #if defined(MBEDTLS_PKCS1_V15) -/** Turn zero-or-nonzero into zero-or-all-bits-one, without branches. - * - * \param value The value to analyze. - * \return Zero if \p value is zero, otherwise all-bits-one. - */ -static unsigned mbedtls_cf_uint_mask( unsigned value ) -{ - /* MSVC has a warning about unary minus on unsigned, but this is - * well-defined and precisely what we want to do here */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - return( - ( ( value | - value ) >> ( sizeof( value ) * 8 - 1 ) ) ); -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif -} - /** Check whether a size is out of bounds, without branches. * * This is equivalent to `size > max`, but is likely to be compiled to From 3733bf805a49381223ce6249063be59baca96f84 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 11:49:42 +0200 Subject: [PATCH 05/52] Move mbedtls_cf_size_mask function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 25 +++++++++++++++++++++++++ library/constant_time.h | 2 ++ library/ssl_msg.c | 25 ------------------------- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 0c5c04c1fc..604859f0fe 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -111,3 +111,28 @@ unsigned mbedtls_cf_uint_mask( unsigned value ) #pragma warning( pop ) #endif } + +/* + * Turn a bit into a mask: + * - if bit == 1, return the all-bits 1 mask, aka (size_t) -1 + * - if bit == 0, return the all-bits 0 mask, aka 0 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + */ +size_t mbedtls_cf_size_mask( size_t bit ) +{ + /* MSVC has a warning about unary minus on unsigned integer types, + * but this is well-defined and precisely what we want to do here. */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif + return -bit; +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif +} diff --git a/library/constant_time.h b/library/constant_time.h index 0d6c0fdfc1..3cbabe1d3b 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -31,3 +31,5 @@ int mbedtls_safer_memcmp( const void *a, const void *b, size_t n ); unsigned mbedtls_cf_uint_mask( unsigned value ); + +size_t mbedtls_cf_size_mask( size_t bit ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 168f186210..df57cb0ca0 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -939,31 +939,6 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -/* - * Turn a bit into a mask: - * - if bit == 1, return the all-bits 1 mask, aka (size_t) -1 - * - if bit == 0, return the all-bits 0 mask, aka 0 - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ -static size_t mbedtls_cf_size_mask( size_t bit ) -{ - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - return -bit; -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif -} - /* * Constant-flow mask generation for "less than" comparison: * - if x < y, return all bits 1, that is (size_t) -1 From c76227d8084f5da857670e8294e054d60a5ba01f Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 11:53:54 +0200 Subject: [PATCH 06/52] Move mbedtls_cf_size_mask_lt function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 25 +++++++++++++++++++++++++ library/constant_time.h | 2 ++ library/ssl_msg.c | 25 ------------------------- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 604859f0fe..928b9b7fe1 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -136,3 +136,28 @@ size_t mbedtls_cf_size_mask( size_t bit ) #pragma warning( pop ) #endif } + +/* + * Constant-flow mask generation for "less than" comparison: + * - if x < y, return all bits 1, that is (size_t) -1 + * - otherwise, return all bits 0, that is 0 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + */ +size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ) +{ + /* This has the most significant bit set if and only if x < y */ + const size_t sub = x - y; + + /* sub1 = (x < y) ? 1 : 0 */ + const size_t sub1 = sub >> ( sizeof( sub ) * 8 - 1 ); + + /* mask = (x < y) ? 0xff... : 0x00... */ + const size_t mask = mbedtls_cf_size_mask( sub1 ); + + return( mask ); +} diff --git a/library/constant_time.h b/library/constant_time.h index 3cbabe1d3b..0b759000a7 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -33,3 +33,5 @@ int mbedtls_safer_memcmp( const void *a, const void *b, size_t n ); unsigned mbedtls_cf_uint_mask( unsigned value ); size_t mbedtls_cf_size_mask( size_t bit ); + +size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index df57cb0ca0..94f263d004 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -939,31 +939,6 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -/* - * Constant-flow mask generation for "less than" comparison: - * - if x < y, return all bits 1, that is (size_t) -1 - * - otherwise, return all bits 0, that is 0 - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ -static size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ) -{ - /* This has the most significant bit set if and only if x < y */ - const size_t sub = x - y; - - /* sub1 = (x < y) ? 1 : 0 */ - const size_t sub1 = sub >> ( sizeof( sub ) * 8 - 1 ); - - /* mask = (x < y) ? 0xff... : 0x00... */ - const size_t mask = mbedtls_cf_size_mask( sub1 ); - - return( mask ); -} - /* * Constant-flow mask generation for "greater or equal" comparison: * - if x >= y, return all bits 1, that is (size_t) -1 From 16fc57bcc4fb2cec17af716abd70de76f82e1f64 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 11:58:31 +0200 Subject: [PATCH 07/52] Move mbedtls_cf_size_mask_ge function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 16 ++++++++++++++++ library/constant_time.h | 2 ++ library/ssl_msg.c | 16 ---------------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 928b9b7fe1..b53d06ba69 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -161,3 +161,19 @@ size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ) return( mask ); } + +/* + * Constant-flow mask generation for "greater or equal" comparison: + * - if x >= y, return all bits 1, that is (size_t) -1 + * - otherwise, return all bits 0, that is 0 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + */ +size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ) +{ + return( ~mbedtls_cf_size_mask_lt( x, y ) ); +} diff --git a/library/constant_time.h b/library/constant_time.h index 0b759000a7..8e8992d6f5 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -35,3 +35,5 @@ unsigned mbedtls_cf_uint_mask( unsigned value ); size_t mbedtls_cf_size_mask( size_t bit ); size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); + +size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 94f263d004..fedaab78ef 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -939,22 +939,6 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -/* - * Constant-flow mask generation for "greater or equal" comparison: - * - if x >= y, return all bits 1, that is (size_t) -1 - * - otherwise, return all bits 0, that is 0 - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ -static size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ) -{ - return( ~mbedtls_cf_size_mask_lt( x, y ) ); -} - /* * Constant-flow boolean "equal" comparison: * return x == y From 8d1d5fd204087234656f85ca969d6b90311bbbc5 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 12:15:19 +0200 Subject: [PATCH 08/52] Move mbedtls_cf_size_bool_eq function to the constant-time module There were multiple functions called mbedtls_cf_size_bool_eq. They had exactly the same behavior, so move the one in bignum.c and remove the other. Signed-off-by: gabor-mezei-arm --- library/bignum.c | 37 +------------------------------------ library/constant_time.c | 36 ++++++++++++++++++++++++++++++++++++ library/constant_time.h | 2 ++ library/ssl_msg.c | 35 ----------------------------------- 4 files changed, 39 insertions(+), 71 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 0df325d431..da115a711d 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -41,6 +41,7 @@ #include "bn_mul.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "constant_time.h" #include @@ -2227,42 +2228,6 @@ static void mpi_montred( mbedtls_mpi *A, const mbedtls_mpi *N, mpi_montmul( A, &U, N, mm, T ); } -/* - * Constant-flow boolean "equal" comparison: - * return x == y - * - * This function can be used to write constant-time code by replacing branches - * with bit operations - it can be used in conjunction with - * mbedtls_ssl_cf_mask_from_bit(). - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ -static size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) -{ - /* diff = 0 if x == y, non-zero otherwise */ - const size_t diff = x ^ y; - - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - - /* diff_msb's most significant bit is equal to x != y */ - const size_t diff_msb = ( diff | (size_t) -diff ); - -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif - - /* diff1 = (x != y) ? 1 : 0 */ - const size_t diff1 = diff_msb >> ( sizeof( diff_msb ) * 8 - 1 ); - - return( 1 ^ diff1 ); -} - /** * Select an MPI from a table without leaking the index. * diff --git a/library/constant_time.c b/library/constant_time.c index b53d06ba69..d73f4fe448 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -177,3 +177,39 @@ size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ) { return( ~mbedtls_cf_size_mask_lt( x, y ) ); } + +/* + * Constant-flow boolean "equal" comparison: + * return x == y + * + * This function can be used to write constant-time code by replacing branches + * with bit operations - it can be used in conjunction with + * mbedtls_ssl_cf_mask_from_bit(). + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + */ +size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) +{ + /* diff = 0 if x == y, non-zero otherwise */ + const size_t diff = x ^ y; + + /* MSVC has a warning about unary minus on unsigned integer types, + * but this is well-defined and precisely what we want to do here. */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif + + /* diff_msb's most significant bit is equal to x != y */ + const size_t diff_msb = ( diff | (size_t) -diff ); + +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif + + /* diff1 = (x != y) ? 1 : 0 */ + const size_t diff1 = diff_msb >> ( sizeof( diff_msb ) * 8 - 1 ); + + return( 1 ^ diff1 ); +} diff --git a/library/constant_time.h b/library/constant_time.h index 8e8992d6f5..50108d7d91 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -37,3 +37,5 @@ size_t mbedtls_cf_size_mask( size_t bit ); size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); + +size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index fedaab78ef..301e50e345 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -939,41 +939,6 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -/* - * Constant-flow boolean "equal" comparison: - * return x == y - * - * This function can be used to write constant-time code by replacing branches - * with bit operations - it can be used in conjunction with - * mbedtls_cf_size_mask(). - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ -static size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) -{ - /* diff = 0 if x == y, non-zero otherwise */ - const size_t diff = x ^ y; - - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - - /* diff_msb's most significant bit is equal to x != y */ - const size_t diff_msb = ( diff | -diff ); - -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif - - /* diff1 = (x != y) ? 1 : 0 */ - const size_t diff1 = diff_msb >> ( sizeof( diff_msb ) * 8 - 1 ); - - return( 1 ^ diff1 ); -} /* * Constant-flow conditional memcpy: From 5a85442604b3b749aca50e0459bcbafd8e6830e3 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 12:25:07 +0200 Subject: [PATCH 09/52] Move mbedtls_cf_size_gt function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 16 ++++++++++++++++ library/constant_time.h | 2 ++ library/rsa.c | 16 ---------------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index d73f4fe448..7da4046621 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -213,3 +213,19 @@ size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) return( 1 ^ diff1 ); } + +/** Check whether a size is out of bounds, without branches. + * + * This is equivalent to `size > max`, but is likely to be compiled to + * to code using bitwise operation rather than a branch. + * + * \param size Size to check. + * \param max Maximum desired value for \p size. + * \return \c 0 if `size <= max`. + * \return \c 1 if `size > max`. + */ +unsigned mbedtls_cf_size_gt( size_t size, size_t max ) +{ + /* Return the sign bit (1 for negative) of (max - size). */ + return( ( max - size ) >> ( sizeof( size_t ) * 8 - 1 ) ); +} diff --git a/library/constant_time.h b/library/constant_time.h index 50108d7d91..eff7f446f1 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -39,3 +39,5 @@ size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ); + +unsigned mbedtls_cf_size_gt( size_t size, size_t max ); diff --git a/library/rsa.c b/library/rsa.c index 3e19ad950c..21d6d12dff 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1458,22 +1458,6 @@ cleanup: #endif /* MBEDTLS_PKCS1_V21 */ #if defined(MBEDTLS_PKCS1_V15) -/** Check whether a size is out of bounds, without branches. - * - * This is equivalent to `size > max`, but is likely to be compiled to - * to code using bitwise operation rather than a branch. - * - * \param size Size to check. - * \param max Maximum desired value for \p size. - * \return \c 0 if `size <= max`. - * \return \c 1 if `size > max`. - */ -static unsigned mbedtls_cf_size_gt( size_t size, size_t max ) -{ - /* Return the sign bit (1 for negative) of (max - size). */ - return( ( max - size ) >> ( sizeof( size_t ) * 8 - 1 ) ); -} - /** Choose between two integer values, without branches. * * This is equivalent to `cond ? if1 : if0`, but is likely to be compiled From 84dc02c8f532e344b719fd03d764db2ac7099669 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 12:47:06 +0200 Subject: [PATCH 10/52] Remove module dependency Elinimate macros defined by modules locally in the functions that are moving to the new constant-time module. Signed-off-by: gabor-mezei-arm --- library/bignum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/bignum.c b/library/bignum.c index da115a711d..48ab24c2a3 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1277,7 +1277,7 @@ static unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, ret |= y & cond; - ret = ret >> ( biL - 1 ); + ret = ret >> ( sizeof( mbedtls_mpi_uint ) * 8 - 1 ); return (unsigned) ret; } From 3f90fd540a48ecc45aceeb47aa4537bfb3632805 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 12:55:33 +0200 Subject: [PATCH 11/52] Move mbedtls_cf_mpi_uint_lt function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/bignum.c | 35 -------------------------------- library/constant_time.c | 44 +++++++++++++++++++++++++++++++++++++++++ library/constant_time.h | 11 +++++++++++ 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 48ab24c2a3..cd8e69e49e 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1247,41 +1247,6 @@ int mbedtls_mpi_cmp_mpi( const mbedtls_mpi *X, const mbedtls_mpi *Y ) return( 0 ); } -/** Decide if an integer is less than the other, without branches. - * - * \param x First integer. - * \param y Second integer. - * - * \return 1 if \p x is less than \p y, 0 otherwise - */ -static unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, - const mbedtls_mpi_uint y ) -{ - mbedtls_mpi_uint ret; - mbedtls_mpi_uint cond; - - /* - * Check if the most significant bits (MSB) of the operands are different. - */ - cond = ( x ^ y ); - /* - * If the MSB are the same then the difference x-y will be negative (and - * have its MSB set to 1 during conversion to unsigned) if and only if x> ( sizeof( mbedtls_mpi_uint ) * 8 - 1 ); - - return (unsigned) ret; -} - /* * Compare signed values in constant time */ diff --git a/library/constant_time.c b/library/constant_time.c index 7da4046621..b513c6a9d4 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -20,6 +20,11 @@ #include "common.h" #include "constant_time.h" +#if defined(MBEDTLS_BIGNUM_C) +#include "mbedtls/bignum.h" +#endif + + /* constant-time buffer comparison */ int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ) { @@ -229,3 +234,42 @@ unsigned mbedtls_cf_size_gt( size_t size, size_t max ) /* Return the sign bit (1 for negative) of (max - size). */ return( ( max - size ) >> ( sizeof( size_t ) * 8 - 1 ) ); } + +#if defined(MBEDTLS_BIGNUM_C) + +/** Decide if an integer is less than the other, without branches. + * + * \param x First integer. + * \param y Second integer. + * + * \return 1 if \p x is less than \p y, 0 otherwise + */ +unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, + const mbedtls_mpi_uint y ) +{ + mbedtls_mpi_uint ret; + mbedtls_mpi_uint cond; + + /* + * Check if the most significant bits (MSB) of the operands are different. + */ + cond = ( x ^ y ); + /* + * If the MSB are the same then the difference x-y will be negative (and + * have its MSB set to 1 during conversion to unsigned) if and only if x> ( sizeof( mbedtls_mpi_uint ) * 8 - 1 ); + + return (unsigned) ret; +} + +#endif /* MBEDTLS_BIGNUM_C */ diff --git a/library/constant_time.h b/library/constant_time.h index eff7f446f1..3c18b4ef96 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -19,6 +19,10 @@ #include "common.h" +#if defined(MBEDTLS_BIGNUM_C) +#include "mbedtls/bignum.h" +#endif + #include int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ); @@ -41,3 +45,10 @@ size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ); unsigned mbedtls_cf_size_gt( size_t size, size_t max ); + +#if defined(MBEDTLS_BIGNUM_C) + +unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, + const mbedtls_mpi_uint y ); + +#endif /* MBEDTLS_BIGNUM_C */ From b2dbf2c113aa4daa0c375b5102872b7ac97a4131 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 12:59:30 +0200 Subject: [PATCH 12/52] Move mbedtls_cf_uint_if function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 16 ++++++++++++++++ library/constant_time.h | 3 +++ library/rsa.c | 16 ---------------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index b513c6a9d4..6d531345cc 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -273,3 +273,19 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, } #endif /* MBEDTLS_BIGNUM_C */ + +/** Choose between two integer values, without branches. + * + * This is equivalent to `cond ? if1 : if0`, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * \param cond Condition to test. + * \param if1 Value to use if \p cond is nonzero. + * \param if0 Value to use if \p cond is zero. + * \return \c if1 if \p cond is nonzero, otherwise \c if0. + */ +unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ) +{ + unsigned mask = mbedtls_cf_uint_mask( cond ); + return( ( mask & if1 ) | (~mask & if0 ) ); +} diff --git a/library/constant_time.h b/library/constant_time.h index 3c18b4ef96..973e856d6b 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -52,3 +52,6 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, const mbedtls_mpi_uint y ); #endif /* MBEDTLS_BIGNUM_C */ + +unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ); + diff --git a/library/rsa.c b/library/rsa.c index 21d6d12dff..a75906a3c3 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1458,22 +1458,6 @@ cleanup: #endif /* MBEDTLS_PKCS1_V21 */ #if defined(MBEDTLS_PKCS1_V15) -/** Choose between two integer values, without branches. - * - * This is equivalent to `cond ? if1 : if0`, but is likely to be compiled - * to code using bitwise operation rather than a branch. - * - * \param cond Condition to test. - * \param if1 Value to use if \p cond is nonzero. - * \param if0 Value to use if \p cond is zero. - * \return \c if1 if \p cond is nonzero, otherwise \c if0. - */ -static unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ) -{ - unsigned mask = mbedtls_cf_uint_mask( cond ); - return( ( mask & if1 ) | (~mask & if0 ) ); -} - /** Shift some data towards the left inside a buffer without leaking * the length of the data through side channels. * From d3230d533c3e0256281c970a0eea76db362a0610 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 13:03:57 +0200 Subject: [PATCH 13/52] Move mbedtls_cf_cond_select_sign function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/bignum.c | 30 ------------------------------ library/constant_time.c | 30 ++++++++++++++++++++++++++++++ library/constant_time.h | 1 + 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index cd8e69e49e..abe3252ba2 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -269,36 +269,6 @@ void mbedtls_mpi_swap( mbedtls_mpi *X, mbedtls_mpi *Y ) memcpy( Y, &T, sizeof( mbedtls_mpi ) ); } -/** - * Select between two sign values in constant-time. - * - * This is functionally equivalent to second ? a : b but uses only bit - * operations in order to avoid branches. - * - * \param[in] a The first sign; must be either +1 or -1. - * \param[in] b The second sign; must be either +1 or -1. - * \param[in] second Must be either 1 (return b) or 0 (return a). - * - * \return The selected sign value. - */ -static int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ) -{ - /* In order to avoid questions about what we can reasonnably assume about - * the representations of signed integers, move everything to unsigned - * by taking advantage of the fact that a and b are either +1 or -1. */ - unsigned ua = a + 1; - unsigned ub = b + 1; - - /* second was 0 or 1, mask is 0 or 2 as are ua and ub */ - const unsigned mask = second << 1; - - /* select ua or ub */ - unsigned ur = ( ua & ~mask ) | ( ub & mask ); - - /* ur is now 0 or 2, convert back to -1 or +1 */ - return( (int) ur - 1 ); -} - /* * Conditionally assign dest = src, without leaking information * about whether the assignment was made or not. diff --git a/library/constant_time.c b/library/constant_time.c index 6d531345cc..6f59884efc 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -289,3 +289,33 @@ unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ) unsigned mask = mbedtls_cf_uint_mask( cond ); return( ( mask & if1 ) | (~mask & if0 ) ); } + +/** + * Select between two sign values in constant-time. + * + * This is functionally equivalent to second ? a : b but uses only bit + * operations in order to avoid branches. + * + * \param[in] a The first sign; must be either +1 or -1. + * \param[in] b The second sign; must be either +1 or -1. + * \param[in] second Must be either 1 (return b) or 0 (return a). + * + * \return The selected sign value. + */ +int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ) +{ + /* In order to avoid questions about what we can reasonnably assume about + * the representations of signed integers, move everything to unsigned + * by taking advantage of the fact that a and b are either +1 or -1. */ + unsigned ua = a + 1; + unsigned ub = b + 1; + + /* second was 0 or 1, mask is 0 or 2 as are ua and ub */ + const unsigned mask = second << 1; + + /* select ua or ub */ + unsigned ur = ( ua & ~mask ) | ( ub & mask ); + + /* ur is now 0 or 2, convert back to -1 or +1 */ + return( (int) ur - 1 ); +} diff --git a/library/constant_time.h b/library/constant_time.h index 973e856d6b..f97c57e7c2 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -55,3 +55,4 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ); +int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ); From be8d98b0be9161d8d7d90970a54de97ccf9a0832 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 13:17:15 +0200 Subject: [PATCH 14/52] Move mbedtls_cf_mpi_uint_cond_assign function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/bignum.c | 31 ------------------------------- library/constant_time.c | 35 +++++++++++++++++++++++++++++++++++ library/constant_time.h | 9 +++++++++ 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index abe3252ba2..3967dbe78f 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -269,37 +269,6 @@ void mbedtls_mpi_swap( mbedtls_mpi *X, mbedtls_mpi *Y ) memcpy( Y, &T, sizeof( mbedtls_mpi ) ); } -/* - * Conditionally assign dest = src, without leaking information - * about whether the assignment was made or not. - * dest and src must be arrays of limbs of size n. - * assign must be 0 or 1. - */ -void mbedtls_cf_mpi_uint_cond_assign( size_t n, - mbedtls_mpi_uint *dest, - const mbedtls_mpi_uint *src, - unsigned char assign ) -{ - size_t i; - - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - - /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */ - const mbedtls_mpi_uint mask = -assign; - -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif - - for( i = 0; i < n; i++ ) - dest[i] = ( src[i] & mask ) | ( dest[i] & ~mask ); -} - /* * Conditionally assign X = Y, without leaking information * about whether the assignment was made or not. diff --git a/library/constant_time.c b/library/constant_time.c index 6f59884efc..b48305a1f8 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -319,3 +319,38 @@ int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ) /* ur is now 0 or 2, convert back to -1 or +1 */ return( (int) ur - 1 ); } + +#if defined(MBEDTLS_BIGNUM_C) + +/* + * Conditionally assign dest = src, without leaking information + * about whether the assignment was made or not. + * dest and src must be arrays of limbs of size n. + * assign must be 0 or 1. + */ +void mbedtls_cf_mpi_uint_cond_assign( size_t n, + mbedtls_mpi_uint *dest, + const mbedtls_mpi_uint *src, + unsigned char assign ) +{ + size_t i; + + /* MSVC has a warning about unary minus on unsigned integer types, + * but this is well-defined and precisely what we want to do here. */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif + + /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */ + const mbedtls_mpi_uint mask = -assign; + +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif + + for( i = 0; i < n; i++ ) + dest[i] = ( src[i] & mask ) | ( dest[i] & ~mask ); +} + +#endif /* MBEDTLS_BIGNUM_C */ diff --git a/library/constant_time.h b/library/constant_time.h index f97c57e7c2..588181ec98 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -56,3 +56,12 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ); int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ); + +#if defined(MBEDTLS_BIGNUM_C) + +void mbedtls_cf_mpi_uint_cond_assign( size_t n, + mbedtls_mpi_uint *dest, + const mbedtls_mpi_uint *src, + unsigned char assign ); + +#endif /* MBEDTLS_BIGNUM_C */ From 394aeaaefbb678f3ede5d5c24a5afacb0ed97cf9 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 13:31:06 +0200 Subject: [PATCH 15/52] Move mbedtls_cf_mem_move_to_left function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 41 +++++++++++++++++++++++++++++++++++++++++ library/constant_time.h | 4 ++++ library/rsa.c | 41 ----------------------------------------- 3 files changed, 45 insertions(+), 41 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index b48305a1f8..281df6400a 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -354,3 +354,44 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, } #endif /* MBEDTLS_BIGNUM_C */ + +/** Shift some data towards the left inside a buffer without leaking + * the length of the data through side channels. + * + * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally + * equivalent to + * ``` + * memmove(start, start + offset, total - offset); + * memset(start + offset, 0, total - offset); + * ``` + * but it strives to use a memory access pattern (and thus total timing) + * that does not depend on \p offset. This timing independence comes at + * the expense of performance. + * + * \param start Pointer to the start of the buffer. + * \param total Total size of the buffer. + * \param offset Offset from which to copy \p total - \p offset bytes. + */ +void mbedtls_cf_mem_move_to_left( void *start, + size_t total, + size_t offset ) +{ + volatile unsigned char *buf = start; + size_t i, n; + if( total == 0 ) + return; + for( i = 0; i < total; i++ ) + { + unsigned no_op = mbedtls_cf_size_gt( total - offset, i ); + /* The first `total - offset` passes are a no-op. The last + * `offset` passes shift the data one byte to the left and + * zero out the last byte. */ + for( n = 0; n < total - 1; n++ ) + { + unsigned char current = buf[n]; + unsigned char next = buf[n+1]; + buf[n] = mbedtls_cf_uint_if( no_op, current, next ); + } + buf[total-1] = mbedtls_cf_uint_if( no_op, buf[total-1], 0 ); + } +} diff --git a/library/constant_time.h b/library/constant_time.h index 588181ec98..5a932cc6f7 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -65,3 +65,7 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, unsigned char assign ); #endif /* MBEDTLS_BIGNUM_C */ + +void mbedtls_cf_mem_move_to_left( void *start, + size_t total, + size_t offset ); diff --git a/library/rsa.c b/library/rsa.c index a75906a3c3..889b94457d 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1458,47 +1458,6 @@ cleanup: #endif /* MBEDTLS_PKCS1_V21 */ #if defined(MBEDTLS_PKCS1_V15) -/** Shift some data towards the left inside a buffer without leaking - * the length of the data through side channels. - * - * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally - * equivalent to - * ``` - * memmove(start, start + offset, total - offset); - * memset(start + offset, 0, total - offset); - * ``` - * but it strives to use a memory access pattern (and thus total timing) - * that does not depend on \p offset. This timing independence comes at - * the expense of performance. - * - * \param start Pointer to the start of the buffer. - * \param total Total size of the buffer. - * \param offset Offset from which to copy \p total - \p offset bytes. - */ -static void mbedtls_cf_mem_move_to_left( void *start, - size_t total, - size_t offset ) -{ - volatile unsigned char *buf = start; - size_t i, n; - if( total == 0 ) - return; - for( i = 0; i < total; i++ ) - { - unsigned no_op = mbedtls_cf_size_gt( total - offset, i ); - /* The first `total - offset` passes are a no-op. The last - * `offset` passes shift the data one byte to the left and - * zero out the last byte. */ - for( n = 0; n < total - 1; n++ ) - { - unsigned char current = buf[n]; - unsigned char next = buf[n+1]; - buf[n] = mbedtls_cf_uint_if( no_op, current, next ); - } - buf[total-1] = mbedtls_cf_uint_if( no_op, buf[total-1], 0 ); - } -} - /* * Implementation of the PKCS#1 v2.1 RSAES-PKCS1-V1_5-DECRYPT function */ From dee0fd33f10804f6af3b6f818545f2d26e3da731 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 13:34:25 +0200 Subject: [PATCH 16/52] Move mbedtls_cf_memcpy_if_eq function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 23 +++++++++++++++++++++++ library/constant_time.h | 5 +++++ library/ssl_msg.c | 23 ----------------------- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 281df6400a..cbfc8e59ad 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -395,3 +395,26 @@ void mbedtls_cf_mem_move_to_left( void *start, buf[total-1] = mbedtls_cf_uint_if( no_op, buf[total-1], 0 ); } } + +/* + * Constant-flow conditional memcpy: + * - if c1 == c2, equivalent to memcpy(dst, src, len), + * - otherwise, a no-op, + * but with execution flow independent of the values of c1 and c2. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + */ +void mbedtls_cf_memcpy_if_eq( unsigned char *dst, + const unsigned char *src, + size_t len, + size_t c1, size_t c2 ) +{ + /* mask = c1 == c2 ? 0xff : 0x00 */ + const size_t equal = mbedtls_cf_size_bool_eq( c1, c2 ); + const unsigned char mask = (unsigned char) mbedtls_cf_size_mask( equal ); + + /* dst[i] = c1 == c2 ? src[i] : dst[i] */ + for( size_t i = 0; i < len; i++ ) + dst[i] = ( src[i] & mask ) | ( dst[i] & ~mask ); +} diff --git a/library/constant_time.h b/library/constant_time.h index 5a932cc6f7..ae491b8921 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -69,3 +69,8 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, void mbedtls_cf_mem_move_to_left( void *start, size_t total, size_t offset ); + +void mbedtls_cf_memcpy_if_eq( unsigned char *dst, + const unsigned char *src, + size_t len, + size_t c1, size_t c2 ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 301e50e345..bc4f9d1911 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -940,29 +940,6 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -/* - * Constant-flow conditional memcpy: - * - if c1 == c2, equivalent to memcpy(dst, src, len), - * - otherwise, a no-op, - * but with execution flow independent of the values of c1 and c2. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ -static void mbedtls_cf_memcpy_if_eq( unsigned char *dst, - const unsigned char *src, - size_t len, - size_t c1, size_t c2 ) -{ - /* mask = c1 == c2 ? 0xff : 0x00 */ - const size_t equal = mbedtls_cf_size_bool_eq( c1, c2 ); - const unsigned char mask = (unsigned char) mbedtls_cf_size_mask( equal ); - - /* dst[i] = c1 == c2 ? src[i] : dst[i] */ - for( size_t i = 0; i < len; i++ ) - dst[i] = ( src[i] & mask ) | ( dst[i] & ~mask ); -} - /* * Compute HMAC of variable-length data with constant flow. * From 0e7f71e1a9c3b23e170afb555ae67a48b3bee376 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 13:57:45 +0200 Subject: [PATCH 17/52] Move mbedtls_cf_memcpy_offset function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 21 +++++++++++++++++++++ library/constant_time.h | 24 ++++++++++++++++++++++++ library/ssl_msg.c | 21 --------------------- 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index cbfc8e59ad..d48d646eeb 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -418,3 +418,24 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dst, for( size_t i = 0; i < len; i++ ) dst[i] = ( src[i] & mask ) | ( dst[i] & ~mask ); } + +/* + * Constant-flow memcpy from variable position in buffer. + * - functionally equivalent to memcpy(dst, src + offset_secret, len) + * - but with execution flow independent from the value of offset_secret. + */ +void mbedtls_cf_memcpy_offset( + unsigned char *dst, + const unsigned char *src_base, + size_t offset_secret, + size_t offset_min, size_t offset_max, + size_t len ) +{ + size_t offset; + + for( offset = offset_min; offset <= offset_max; offset++ ) + { + mbedtls_cf_memcpy_if_eq( dst, src_base + offset, len, + offset, offset_secret ); + } +} diff --git a/library/constant_time.h b/library/constant_time.h index ae491b8921..060aca3883 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -74,3 +74,27 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dst, const unsigned char *src, size_t len, size_t c1, size_t c2 ); + +/** Copy data from a secret position with constant flow. + * + * This function copies \p len bytes from \p src_base + \p offset_secret to \p + * dst, with a code flow and memory access pattern that does not depend on \p + * offset_secret, but only on \p offset_min, \p offset_max and \p len. + * + * \param dst The destination buffer. This must point to a writable + * buffer of at least \p len bytes. + * \param src_base The base of the source buffer. This must point to a + * readable buffer of at least \p offset_max + \p len + * bytes. + * \param offset_secret The offset in the source buffer from which to copy. + * This must be no less than \p offset_min and no greater + * than \p offset_max. + * \param offset_min The minimal value of \p offset_secret. + * \param offset_max The maximal value of \p offset_secret. + * \param len The number of bytes to copy. + */ +void mbedtls_cf_memcpy_offset( unsigned char *dst, + const unsigned char *src_base, + size_t offset_secret, + size_t offset_min, size_t offset_max, + size_t len ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index bc4f9d1911..fcd63ac86e 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -1027,27 +1027,6 @@ cleanup: mbedtls_md_free( &aux ); return( ret ); } - -/* - * Constant-flow memcpy from variable position in buffer. - * - functionally equivalent to memcpy(dst, src + offset_secret, len) - * - but with execution flow independent from the value of offset_secret. - */ -MBEDTLS_STATIC_TESTABLE void mbedtls_cf_memcpy_offset( - unsigned char *dst, - const unsigned char *src_base, - size_t offset_secret, - size_t offset_min, size_t offset_max, - size_t len ) -{ - size_t offset; - - for( offset = offset_min; offset <= offset_max; offset++ ) - { - mbedtls_cf_memcpy_if_eq( dst, src_base + offset, len, - offset, offset_secret ); - } -} #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, From 1349ffde84882af3057eb7d8dd0465bb865d4aa3 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 14:28:31 +0200 Subject: [PATCH 18/52] Move mbedtls_cf_hmac function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 97 +++++++++++++++++++++++++++++++++++++++++ library/constant_time.h | 48 ++++++++++++++++++++ library/ssl_msg.c | 91 -------------------------------------- 3 files changed, 145 insertions(+), 91 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index d48d646eeb..60352ae714 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -19,11 +19,16 @@ #include "common.h" #include "constant_time.h" +#include "mbedtls/error.h" #if defined(MBEDTLS_BIGNUM_C) #include "mbedtls/bignum.h" #endif +#if defined(MBEDTLS_SSL_TLS_C) +#include "ssl_misc.h" +#endif + /* constant-time buffer comparison */ int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ) @@ -439,3 +444,95 @@ void mbedtls_cf_memcpy_offset( offset, offset_secret ); } } + +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) + +/* + * Compute HMAC of variable-length data with constant flow. + * + * Only works with MD-5, SHA-1, SHA-256 and SHA-384. + * (Otherwise, computation of block_size needs to be adapted.) + */ +int mbedtls_cf_hmac( + mbedtls_md_context_t *ctx, + const unsigned char *add_data, size_t add_data_len, + const unsigned char *data, size_t data_len_secret, + size_t min_data_len, size_t max_data_len, + unsigned char *output ) +{ + /* + * This function breaks the HMAC abstraction and uses the md_clone() + * extension to the MD API in order to get constant-flow behaviour. + * + * HMAC(msg) is defined as HASH(okey + HASH(ikey + msg)) where + means + * concatenation, and okey/ikey are the XOR of the key with some fixed bit + * patterns (see RFC 2104, sec. 2), which are stored in ctx->hmac_ctx. + * + * We'll first compute inner_hash = HASH(ikey + msg) by hashing up to + * minlen, then cloning the context, and for each byte up to maxlen + * finishing up the hash computation, keeping only the correct result. + * + * Then we only need to compute HASH(okey + inner_hash) and we're done. + */ + const mbedtls_md_type_t md_alg = mbedtls_md_get_type( ctx->md_info ); + /* TLS 1.2 only supports SHA-384, SHA-256, SHA-1, MD-5, + * all of which have the same block size except SHA-384. */ + const size_t block_size = md_alg == MBEDTLS_MD_SHA384 ? 128 : 64; + const unsigned char * const ikey = ctx->hmac_ctx; + const unsigned char * const okey = ikey + block_size; + const size_t hash_size = mbedtls_md_get_size( ctx->md_info ); + + unsigned char aux_out[MBEDTLS_MD_MAX_SIZE]; + mbedtls_md_context_t aux; + size_t offset; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + mbedtls_md_init( &aux ); + +#define MD_CHK( func_call ) \ + do { \ + ret = (func_call); \ + if( ret != 0 ) \ + goto cleanup; \ + } while( 0 ) + + MD_CHK( mbedtls_md_setup( &aux, ctx->md_info, 0 ) ); + + /* After hmac_start() of hmac_reset(), ikey has already been hashed, + * so we can start directly with the message */ + MD_CHK( mbedtls_md_update( ctx, add_data, add_data_len ) ); + MD_CHK( mbedtls_md_update( ctx, data, min_data_len ) ); + + /* For each possible length, compute the hash up to that point */ + for( offset = min_data_len; offset <= max_data_len; offset++ ) + { + MD_CHK( mbedtls_md_clone( &aux, ctx ) ); + MD_CHK( mbedtls_md_finish( &aux, aux_out ) ); + /* Keep only the correct inner_hash in the output buffer */ + mbedtls_cf_memcpy_if_eq( output, aux_out, hash_size, + offset, data_len_secret ); + + if( offset < max_data_len ) + MD_CHK( mbedtls_md_update( ctx, data + offset, 1 ) ); + } + + /* The context needs to finish() before it starts() again */ + MD_CHK( mbedtls_md_finish( ctx, aux_out ) ); + + /* Now compute HASH(okey + inner_hash) */ + MD_CHK( mbedtls_md_starts( ctx ) ); + MD_CHK( mbedtls_md_update( ctx, okey, block_size ) ); + MD_CHK( mbedtls_md_update( ctx, output, hash_size ) ); + MD_CHK( mbedtls_md_finish( ctx, output ) ); + + /* Done, get ready for next time */ + MD_CHK( mbedtls_md_hmac_reset( ctx ) ); + +#undef MD_CHK + +cleanup: + mbedtls_md_free( &aux ); + return( ret ); +} + +#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ diff --git a/library/constant_time.h b/library/constant_time.h index 060aca3883..782e89a6a8 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -23,6 +23,10 @@ #include "mbedtls/bignum.h" #endif +#if defined(MBEDTLS_SSL_TLS_C) +#include "ssl_misc.h" +#endif + #include int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ); @@ -98,3 +102,47 @@ void mbedtls_cf_memcpy_offset( unsigned char *dst, size_t offset_secret, size_t offset_min, size_t offset_max, size_t len ); + +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) + +/** Compute the HMAC of variable-length data with constant flow. + * + * This function computes the HMAC of the concatenation of \p add_data and \p + * data, and does with a code flow and memory access pattern that does not + * depend on \p data_len_secret, but only on \p min_data_len and \p + * max_data_len. In particular, this function always reads exactly \p + * max_data_len bytes from \p data. + * + * \param ctx The HMAC context. It must have keys configured + * with mbedtls_md_hmac_starts() and use one of the + * following hashes: SHA-384, SHA-256, SHA-1 or MD-5. + * It is reset using mbedtls_md_hmac_reset() after + * the computation is complete to prepare for the + * next computation. + * \param add_data The additional data prepended to \p data. This + * must point to a readable buffer of \p add_data_len + * bytes. + * \param add_data_len The length of \p add_data in bytes. + * \param data The data appended to \p add_data. This must point + * to a readable buffer of \p max_data_len bytes. + * \param data_len_secret The length of the data to process in \p data. + * This must be no less than \p min_data_len and no + * greater than \p max_data_len. + * \param min_data_len The minimal length of \p data in bytes. + * \param max_data_len The maximal length of \p data in bytes. + * \param output The HMAC will be written here. This must point to + * a writable buffer of sufficient size to hold the + * HMAC value. + * + * \retval 0 on success. + * \retval MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED + * The hardware accelerator failed. + */ +int mbedtls_ssl_cf_hmac( + mbedtls_md_context_t *ctx, + const unsigned char *add_data, size_t add_data_len, + const unsigned char *data, size_t data_len_secret, + size_t min_data_len, size_t max_data_len, + unsigned char *output ); + +#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ diff --git a/library/ssl_msg.c b/library/ssl_msg.c index fcd63ac86e..be9c5f2f83 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -938,97 +938,6 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, return( 0 ); } -#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) - -/* - * Compute HMAC of variable-length data with constant flow. - * - * Only works with MD-5, SHA-1, SHA-256 and SHA-384. - * (Otherwise, computation of block_size needs to be adapted.) - */ -MBEDTLS_STATIC_TESTABLE int mbedtls_cf_hmac( - mbedtls_md_context_t *ctx, - const unsigned char *add_data, size_t add_data_len, - const unsigned char *data, size_t data_len_secret, - size_t min_data_len, size_t max_data_len, - unsigned char *output ) -{ - /* - * This function breaks the HMAC abstraction and uses the md_clone() - * extension to the MD API in order to get constant-flow behaviour. - * - * HMAC(msg) is defined as HASH(okey + HASH(ikey + msg)) where + means - * concatenation, and okey/ikey are the XOR of the key with some fixed bit - * patterns (see RFC 2104, sec. 2), which are stored in ctx->hmac_ctx. - * - * We'll first compute inner_hash = HASH(ikey + msg) by hashing up to - * minlen, then cloning the context, and for each byte up to maxlen - * finishing up the hash computation, keeping only the correct result. - * - * Then we only need to compute HASH(okey + inner_hash) and we're done. - */ - const mbedtls_md_type_t md_alg = mbedtls_md_get_type( ctx->md_info ); - /* TLS 1.2 only supports SHA-384, SHA-256, SHA-1, MD-5, - * all of which have the same block size except SHA-384. */ - const size_t block_size = md_alg == MBEDTLS_MD_SHA384 ? 128 : 64; - const unsigned char * const ikey = ctx->hmac_ctx; - const unsigned char * const okey = ikey + block_size; - const size_t hash_size = mbedtls_md_get_size( ctx->md_info ); - - unsigned char aux_out[MBEDTLS_MD_MAX_SIZE]; - mbedtls_md_context_t aux; - size_t offset; - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - - mbedtls_md_init( &aux ); - -#define MD_CHK( func_call ) \ - do { \ - ret = (func_call); \ - if( ret != 0 ) \ - goto cleanup; \ - } while( 0 ) - - MD_CHK( mbedtls_md_setup( &aux, ctx->md_info, 0 ) ); - - /* After hmac_start() of hmac_reset(), ikey has already been hashed, - * so we can start directly with the message */ - MD_CHK( mbedtls_md_update( ctx, add_data, add_data_len ) ); - MD_CHK( mbedtls_md_update( ctx, data, min_data_len ) ); - - /* For each possible length, compute the hash up to that point */ - for( offset = min_data_len; offset <= max_data_len; offset++ ) - { - MD_CHK( mbedtls_md_clone( &aux, ctx ) ); - MD_CHK( mbedtls_md_finish( &aux, aux_out ) ); - /* Keep only the correct inner_hash in the output buffer */ - mbedtls_cf_memcpy_if_eq( output, aux_out, hash_size, - offset, data_len_secret ); - - if( offset < max_data_len ) - MD_CHK( mbedtls_md_update( ctx, data + offset, 1 ) ); - } - - /* The context needs to finish() before it starts() again */ - MD_CHK( mbedtls_md_finish( ctx, aux_out ) ); - - /* Now compute HASH(okey + inner_hash) */ - MD_CHK( mbedtls_md_starts( ctx ) ); - MD_CHK( mbedtls_md_update( ctx, okey, block_size ) ); - MD_CHK( mbedtls_md_update( ctx, output, hash_size ) ); - MD_CHK( mbedtls_md_finish( ctx, output ) ); - - /* Done, get ready for next time */ - MD_CHK( mbedtls_md_hmac_reset( ctx ) ); - -#undef MD_CHK - -cleanup: - mbedtls_md_free( &aux ); - return( ret ); -} -#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ - int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, mbedtls_ssl_transform *transform, mbedtls_record *rec ) From 9c1203fd67af6da2ccc8c6f34213707f4b276941 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 14:36:10 +0200 Subject: [PATCH 19/52] Delete ssl_invasive.h due to duplicated function declarations All function declaration provided by ssl_invasive.h is needed only for testing purposes and all of them are provided by constant_time.h as well. Signed-off-by: gabor-mezei-arm --- library/ssl_invasive.h | 100 --------------------------- library/ssl_msg.c | 2 - tests/suites/test_suite_ssl.function | 2 +- 3 files changed, 1 insertion(+), 103 deletions(-) delete mode 100644 library/ssl_invasive.h diff --git a/library/ssl_invasive.h b/library/ssl_invasive.h deleted file mode 100644 index 5cb29cb0a8..0000000000 --- a/library/ssl_invasive.h +++ /dev/null @@ -1,100 +0,0 @@ -/** - * \file ssl_invasive.h - * - * \brief SSL module: interfaces for invasive testing only. - * - * The interfaces in this file are intended for testing purposes only. - * They SHOULD NOT be made available in library integrations except when - * building the library for testing. - */ -/* - * Copyright The Mbed TLS Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -#ifndef MBEDTLS_SSL_INVASIVE_H -#define MBEDTLS_SSL_INVASIVE_H - -#include "common.h" -#include "mbedtls/md.h" - -#if defined(MBEDTLS_TEST_HOOKS) && \ - defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -/** \brief Compute the HMAC of variable-length data with constant flow. - * - * This function computes the HMAC of the concatenation of \p add_data and \p - * data, and does with a code flow and memory access pattern that does not - * depend on \p data_len_secret, but only on \p min_data_len and \p - * max_data_len. In particular, this function always reads exactly \p - * max_data_len bytes from \p data. - * - * \param ctx The HMAC context. It must have keys configured - * with mbedtls_md_hmac_starts() and use one of the - * following hashes: SHA-384, SHA-256, SHA-1 or MD-5. - * It is reset using mbedtls_md_hmac_reset() after - * the computation is complete to prepare for the - * next computation. - * \param add_data The additional data prepended to \p data. This - * must point to a readable buffer of \p add_data_len - * bytes. - * \param add_data_len The length of \p add_data in bytes. - * \param data The data appended to \p add_data. This must point - * to a readable buffer of \p max_data_len bytes. - * \param data_len_secret The length of the data to process in \p data. - * This must be no less than \p min_data_len and no - * greater than \p max_data_len. - * \param min_data_len The minimal length of \p data in bytes. - * \param max_data_len The maximal length of \p data in bytes. - * \param output The HMAC will be written here. This must point to - * a writable buffer of sufficient size to hold the - * HMAC value. - * - * \retval 0 - * Success. - * \retval MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED - * The hardware accelerator failed. - */ -int mbedtls_cf_hmac( - mbedtls_md_context_t *ctx, - const unsigned char *add_data, size_t add_data_len, - const unsigned char *data, size_t data_len_secret, - size_t min_data_len, size_t max_data_len, - unsigned char *output ); - -/** \brief Copy data from a secret position with constant flow. - * - * This function copies \p len bytes from \p src_base + \p offset_secret to \p - * dst, with a code flow and memory access pattern that does not depend on \p - * offset_secret, but only on \p offset_min, \p offset_max and \p len. - * - * \param dst The destination buffer. This must point to a writable - * buffer of at least \p len bytes. - * \param src_base The base of the source buffer. This must point to a - * readable buffer of at least \p offset_max + \p len - * bytes. - * \param offset_secret The offset in the source buffer from which to copy. - * This must be no less than \p offset_min and no greater - * than \p offset_max. - * \param offset_min The minimal value of \p offset_secret. - * \param offset_max The maximal value of \p offset_secret. - * \param len The number of bytes to copy. - */ -void mbedtls_cf_memcpy_offset( unsigned char *dst, - const unsigned char *src_base, - size_t offset_secret, - size_t offset_min, size_t offset_max, - size_t len ); -#endif /* MBEDTLS_TEST_HOOKS && MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ - -#endif /* MBEDTLS_SSL_INVASIVE_H */ diff --git a/library/ssl_msg.c b/library/ssl_msg.c index be9c5f2f83..f8f366021a 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -42,8 +42,6 @@ #include "mbedtls/version.h" #include "constant_time.h" -#include "ssl_invasive.h" - #include #if defined(MBEDTLS_USE_PSA_CRYPTO) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 26b99a446b..5623a1c2d6 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -10,7 +10,7 @@ #include -#include +#include #include From 40a49251286512420e405dc561aad40ae67e1701 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 15:33:35 +0200 Subject: [PATCH 20/52] Move mbedtls_mpi_safe_cond_assign function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/bignum.c | 42 ----------------------------------- library/constant_time.c | 49 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 3967dbe78f..b637101294 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -269,48 +269,6 @@ void mbedtls_mpi_swap( mbedtls_mpi *X, mbedtls_mpi *Y ) memcpy( Y, &T, sizeof( mbedtls_mpi ) ); } -/* - * Conditionally assign X = Y, without leaking information - * about whether the assignment was made or not. - * (Leaking information about the respective sizes of X and Y is ok however.) - */ -int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, const mbedtls_mpi *Y, unsigned char assign ) -{ - int ret = 0; - size_t i; - mbedtls_mpi_uint limb_mask; - MPI_VALIDATE_RET( X != NULL ); - MPI_VALIDATE_RET( Y != NULL ); - - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - - /* make sure assign is 0 or 1 in a time-constant manner */ - assign = (assign | (unsigned char)-assign) >> (sizeof( assign ) * 8 - 1); - /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */ - limb_mask = -assign; - -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif - - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); - - X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, assign ); - - mbedtls_cf_mpi_uint_cond_assign( Y->n, X->p, Y->p, assign ); - - for( i = Y->n; i < X->n; i++ ) - X->p[i] &= ~limb_mask; - -cleanup: - return( ret ); -} - /* * Conditionally swap X and Y, without leaking information * about whether the swap was made or not. diff --git a/library/constant_time.c b/library/constant_time.c index 60352ae714..2952992cc0 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -536,3 +536,52 @@ cleanup: } #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ + +#if defined(MBEDTLS_BIGNUM_C) + +#define MPI_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_MPI_BAD_INPUT_DATA ) + +/* + * Conditionally assign X = Y, without leaking information + * about whether the assignment was made or not. + * (Leaking information about the respective sizes of X and Y is ok however.) + */ +int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, const mbedtls_mpi *Y, unsigned char assign ) +{ + int ret = 0; + size_t i; + mbedtls_mpi_uint limb_mask; + MPI_VALIDATE_RET( X != NULL ); + MPI_VALIDATE_RET( Y != NULL ); + + /* MSVC has a warning about unary minus on unsigned integer types, + * but this is well-defined and precisely what we want to do here. */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif + + /* make sure assign is 0 or 1 in a time-constant manner */ + assign = (assign | (unsigned char)-assign) >> (sizeof( assign ) * 8 - 1); + /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */ + limb_mask = -assign; + +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif + + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); + + X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, assign ); + + mbedtls_cf_mpi_uint_cond_assign( Y->n, X->p, Y->p, assign ); + + for( i = Y->n; i < X->n; i++ ) + X->p[i] &= ~limb_mask; + +cleanup: + return( ret ); +} + +#endif /* MBEDTLS_BIGNUM_C */ From 5c9762121567e958d034d9d3201b6327eb8b8758 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 15:37:50 +0200 Subject: [PATCH 21/52] Move mbedtls_mpi_safe_cond_swap function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/bignum.c | 53 ----------------------------------------- library/constant_time.c | 53 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 53 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index b637101294..e0391eca15 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -269,59 +269,6 @@ void mbedtls_mpi_swap( mbedtls_mpi *X, mbedtls_mpi *Y ) memcpy( Y, &T, sizeof( mbedtls_mpi ) ); } -/* - * Conditionally swap X and Y, without leaking information - * about whether the swap was made or not. - * Here it is not ok to simply swap the pointers, which whould lead to - * different memory access patterns when X and Y are used afterwards. - */ -int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, mbedtls_mpi *Y, unsigned char swap ) -{ - int ret, s; - size_t i; - mbedtls_mpi_uint limb_mask; - mbedtls_mpi_uint tmp; - MPI_VALIDATE_RET( X != NULL ); - MPI_VALIDATE_RET( Y != NULL ); - - if( X == Y ) - return( 0 ); - - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - - /* make sure swap is 0 or 1 in a time-constant manner */ - swap = (swap | (unsigned char)-swap) >> (sizeof( swap ) * 8 - 1); - /* all-bits 1 if swap is 1, all-bits 0 if swap is 0 */ - limb_mask = -swap; - -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif - - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( Y, X->n ) ); - - s = X->s; - X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, swap ); - Y->s = mbedtls_cf_cond_select_sign( Y->s, s, swap ); - - - for( i = 0; i < X->n; i++ ) - { - tmp = X->p[i]; - X->p[i] = ( X->p[i] & ~limb_mask ) | ( Y->p[i] & limb_mask ); - Y->p[i] = ( Y->p[i] & ~limb_mask ) | ( tmp & limb_mask ); - } - -cleanup: - return( ret ); -} - /* * Set value from integer */ diff --git a/library/constant_time.c b/library/constant_time.c index 2952992cc0..b2203ba9a4 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -584,4 +584,57 @@ cleanup: return( ret ); } +/* + * Conditionally swap X and Y, without leaking information + * about whether the swap was made or not. + * Here it is not ok to simply swap the pointers, which whould lead to + * different memory access patterns when X and Y are used afterwards. + */ +int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, mbedtls_mpi *Y, unsigned char swap ) +{ + int ret, s; + size_t i; + mbedtls_mpi_uint limb_mask; + mbedtls_mpi_uint tmp; + MPI_VALIDATE_RET( X != NULL ); + MPI_VALIDATE_RET( Y != NULL ); + + if( X == Y ) + return( 0 ); + + /* MSVC has a warning about unary minus on unsigned integer types, + * but this is well-defined and precisely what we want to do here. */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif + + /* make sure swap is 0 or 1 in a time-constant manner */ + swap = (swap | (unsigned char)-swap) >> (sizeof( swap ) * 8 - 1); + /* all-bits 1 if swap is 1, all-bits 0 if swap is 0 */ + limb_mask = -swap; + +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif + + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( Y, X->n ) ); + + s = X->s; + X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, swap ); + Y->s = mbedtls_cf_cond_select_sign( Y->s, s, swap ); + + + for( i = 0; i < X->n; i++ ) + { + tmp = X->p[i]; + X->p[i] = ( X->p[i] & ~limb_mask ) | ( Y->p[i] & limb_mask ); + Y->p[i] = ( Y->p[i] & ~limb_mask ) | ( tmp & limb_mask ); + } + +cleanup: + return( ret ); +} + #endif /* MBEDTLS_BIGNUM_C */ From c29a3da5994b0bb7844db84ed32e474b0a4e59f9 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 15:41:30 +0200 Subject: [PATCH 22/52] Move mbedtls_mpi_lt_mpi_ct function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/bignum.c | 66 ----------------------------------------- library/constant_time.c | 66 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 66 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index e0391eca15..ee5f27d706 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1091,72 +1091,6 @@ int mbedtls_mpi_cmp_mpi( const mbedtls_mpi *X, const mbedtls_mpi *Y ) return( 0 ); } -/* - * Compare signed values in constant time - */ -int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, const mbedtls_mpi *Y, - unsigned *ret ) -{ - size_t i; - /* The value of any of these variables is either 0 or 1 at all times. */ - unsigned cond, done, X_is_negative, Y_is_negative; - - MPI_VALIDATE_RET( X != NULL ); - MPI_VALIDATE_RET( Y != NULL ); - MPI_VALIDATE_RET( ret != NULL ); - - if( X->n != Y->n ) - return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; - - /* - * Set sign_N to 1 if N >= 0, 0 if N < 0. - * We know that N->s == 1 if N >= 0 and N->s == -1 if N < 0. - */ - X_is_negative = ( X->s & 2 ) >> 1; - Y_is_negative = ( Y->s & 2 ) >> 1; - - /* - * If the signs are different, then the positive operand is the bigger. - * That is if X is negative (X_is_negative == 1), then X < Y is true and it - * is false if X is positive (X_is_negative == 0). - */ - cond = ( X_is_negative ^ Y_is_negative ); - *ret = cond & X_is_negative; - - /* - * This is a constant-time function. We might have the result, but we still - * need to go through the loop. Record if we have the result already. - */ - done = cond; - - for( i = X->n; i > 0; i-- ) - { - /* - * If Y->p[i - 1] < X->p[i - 1] then X < Y is true if and only if both - * X and Y are negative. - * - * Again even if we can make a decision, we just mark the result and - * the fact that we are done and continue looping. - */ - cond = mbedtls_cf_mpi_uint_lt( Y->p[i - 1], X->p[i - 1] ); - *ret |= cond & ( 1 - done ) & X_is_negative; - done |= cond; - - /* - * If X->p[i - 1] < Y->p[i - 1] then X < Y is true if and only if both - * X and Y are positive. - * - * Again even if we can make a decision, we just mark the result and - * the fact that we are done and continue looping. - */ - cond = mbedtls_cf_mpi_uint_lt( X->p[i - 1], Y->p[i - 1] ); - *ret |= cond & ( 1 - done ) & ( 1 - X_is_negative ); - done |= cond; - } - - return( 0 ); -} - /* * Compare signed values */ diff --git a/library/constant_time.c b/library/constant_time.c index b2203ba9a4..8a9c45399d 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -637,4 +637,70 @@ cleanup: return( ret ); } +/* + * Compare signed values in constant time + */ +int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, const mbedtls_mpi *Y, + unsigned *ret ) +{ + size_t i; + /* The value of any of these variables is either 0 or 1 at all times. */ + unsigned cond, done, X_is_negative, Y_is_negative; + + MPI_VALIDATE_RET( X != NULL ); + MPI_VALIDATE_RET( Y != NULL ); + MPI_VALIDATE_RET( ret != NULL ); + + if( X->n != Y->n ) + return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; + + /* + * Set sign_N to 1 if N >= 0, 0 if N < 0. + * We know that N->s == 1 if N >= 0 and N->s == -1 if N < 0. + */ + X_is_negative = ( X->s & 2 ) >> 1; + Y_is_negative = ( Y->s & 2 ) >> 1; + + /* + * If the signs are different, then the positive operand is the bigger. + * That is if X is negative (X_is_negative == 1), then X < Y is true and it + * is false if X is positive (X_is_negative == 0). + */ + cond = ( X_is_negative ^ Y_is_negative ); + *ret = cond & X_is_negative; + + /* + * This is a constant-time function. We might have the result, but we still + * need to go through the loop. Record if we have the result already. + */ + done = cond; + + for( i = X->n; i > 0; i-- ) + { + /* + * If Y->p[i - 1] < X->p[i - 1] then X < Y is true if and only if both + * X and Y are negative. + * + * Again even if we can make a decision, we just mark the result and + * the fact that we are done and continue looping. + */ + cond = mbedtls_cf_mpi_uint_lt( Y->p[i - 1], X->p[i - 1] ); + *ret |= cond & ( 1 - done ) & X_is_negative; + done |= cond; + + /* + * If X->p[i - 1] < Y->p[i - 1] then X < Y is true if and only if both + * X and Y are positive. + * + * Again even if we can make a decision, we just mark the result and + * the fact that we are done and continue looping. + */ + cond = mbedtls_cf_mpi_uint_lt( X->p[i - 1], Y->p[i - 1] ); + *ret |= cond & ( 1 - done ) & ( 1 - X_is_negative ); + done |= cond; + } + + return( 0 ); +} + #endif /* MBEDTLS_BIGNUM_C */ From 65cefdbfcbfc2aa9d886c77d6a0d925126ba8322 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 15:47:00 +0200 Subject: [PATCH 23/52] Create mbedtls_cf_size_if function Add a constant-time function with size_t parameter for choosing between two integer values, like the ?: ternary operator. Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 6 ++++++ library/constant_time.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/library/constant_time.c b/library/constant_time.c index 8a9c45399d..d841327492 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -295,6 +295,12 @@ unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ) return( ( mask & if1 ) | (~mask & if0 ) ); } +size_t mbedtls_cf_size_if( unsigned cond, size_t if1, size_t if0 ) +{ + size_t mask = mbedtls_cf_size_mask( cond ); + return( ( mask & if1 ) | (~mask & if0 ) ); +} + /** * Select between two sign values in constant-time. * diff --git a/library/constant_time.h b/library/constant_time.h index 782e89a6a8..c7ca490d39 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -59,6 +59,8 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ); +size_t mbedtls_cf_size_if( unsigned cond, size_t if1, size_t if0 ); + int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ); #if defined(MBEDTLS_BIGNUM_C) From bef600f27e0a41dcdb12cb60c076c1bc3cfd41e7 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Sun, 26 Sep 2021 15:20:48 +0200 Subject: [PATCH 24/52] Move the constant-time part of mbedtls_rsa_rsaes_pkcs1_v15_decrypt to a function Tne unpadding part of `mbedtls_rsa_rsaes_pkcs1_v15_decrypt` function is contant-time therefore it moved to a separate function to be prepared for moving to the contant-time module. Signed-off-by: gabor-mezei-arm --- library/rsa.c | 82 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 889b94457d..ff769bb11f 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1458,20 +1458,15 @@ cleanup: #endif /* MBEDTLS_PKCS1_V21 */ #if defined(MBEDTLS_PKCS1_V15) -/* - * Implementation of the PKCS#1 v2.1 RSAES-PKCS1-V1_5-DECRYPT function - */ -int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, - int (*f_rng)(void *, unsigned char *, size_t), - void *p_rng, - size_t *olen, - const unsigned char *input, - unsigned char *output, - size_t output_max_len ) +int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, + size_t *olen, + unsigned char *output, + size_t output_max_len, + unsigned char *buf ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t ilen, i, plaintext_max_size; - unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; + size_t i, plaintext_max_size; + /* The following variables take sensitive values: their value must * not leak into the observable behavior of the function other than * the designated outputs (output, olen, return value). Otherwise @@ -1488,26 +1483,9 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, size_t plaintext_size = 0; unsigned output_too_large; - RSA_VALIDATE_RET( ctx != NULL ); - RSA_VALIDATE_RET( output_max_len == 0 || output != NULL ); - RSA_VALIDATE_RET( input != NULL ); - RSA_VALIDATE_RET( olen != NULL ); - - ilen = ctx->len; - plaintext_max_size = ( output_max_len > ilen - 11 ? - ilen - 11 : - output_max_len ); - - if( ctx->padding != MBEDTLS_RSA_PKCS_V15 ) - return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); - - if( ilen < 16 || ilen > sizeof( buf ) ) - return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); - - ret = mbedtls_rsa_private( ctx, f_rng, p_rng, input, buf ); - - if( ret != 0 ) - goto cleanup; + plaintext_max_size = mbedtls_cf_size_if( output_max_len > ilen - 11, + ilen - 11, + output_max_len ); /* Check and get padding length in constant time and constant * memory trace. The first byte must be 0. */ @@ -1604,6 +1582,46 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, * to the good case limits the risks of leaking the padding validity. */ *olen = plaintext_size; + return( ret ); +} + +/* + * Implementation of the PKCS#1 v2.1 RSAES-PKCS1-V1_5-DECRYPT function + */ +int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng, + size_t *olen, + const unsigned char *input, + unsigned char *output, + size_t output_max_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t ilen; + unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; + + RSA_VALIDATE_RET( ctx != NULL ); + RSA_VALIDATE_RET( output_max_len == 0 || output != NULL ); + RSA_VALIDATE_RET( input != NULL ); + RSA_VALIDATE_RET( olen != NULL ); + + ilen = ctx->len; + + if( ctx->padding != MBEDTLS_RSA_PKCS_V15 ) + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + + if( ilen < 16 || ilen > sizeof( buf ) ) + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + + ret = mbedtls_rsa_private( ctx, f_rng, p_rng, input, buf ); + + if( ret != 0 ) + goto cleanup; + + ret = mbedtls_cf_rsaes_pkcs1_v15_unpadding( ilen, olen, output, + output_max_len, + (unsigned char *) &buf ); + cleanup: mbedtls_platform_zeroize( buf, sizeof( buf ) ); From fdb71183f86fd56e118ecab4b0fa46f97b5dd73d Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 16:11:12 +0200 Subject: [PATCH 25/52] Move mbedtls_cf_rsaes_pkcs1_v15_unpadding function to the constant-time module Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 132 ++++++++++++++++++++++++++++++++++++++++ library/constant_time.h | 10 +++ library/rsa.c | 127 -------------------------------------- 3 files changed, 142 insertions(+), 127 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index d841327492..3784365a6e 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -29,6 +29,7 @@ #include "ssl_misc.h" #endif +#include /* constant-time buffer comparison */ int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ) @@ -710,3 +711,134 @@ int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, const mbedtls_mpi *Y, } #endif /* MBEDTLS_BIGNUM_C */ + +#if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) + +int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, + size_t *olen, + unsigned char *output, + size_t output_max_len, + unsigned char *buf ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t i, plaintext_max_size; + + /* The following variables take sensitive values: their value must + * not leak into the observable behavior of the function other than + * the designated outputs (output, olen, return value). Otherwise + * this would open the execution of the function to + * side-channel-based variants of the Bleichenbacher padding oracle + * attack. Potential side channels include overall timing, memory + * access patterns (especially visible to an adversary who has access + * to a shared memory cache), and branches (especially visible to + * an adversary who has access to a shared code cache or to a shared + * branch predictor). */ + size_t pad_count = 0; + unsigned bad = 0; + unsigned char pad_done = 0; + size_t plaintext_size = 0; + unsigned output_too_large; + + plaintext_max_size = mbedtls_cf_size_if( output_max_len > ilen - 11, + ilen - 11, + output_max_len ); + + /* Check and get padding length in constant time and constant + * memory trace. The first byte must be 0. */ + bad |= buf[0]; + + + /* Decode EME-PKCS1-v1_5 padding: 0x00 || 0x02 || PS || 0x00 + * where PS must be at least 8 nonzero bytes. */ + bad |= buf[1] ^ MBEDTLS_RSA_CRYPT; + + /* Read the whole buffer. Set pad_done to nonzero if we find + * the 0x00 byte and remember the padding length in pad_count. */ + for( i = 2; i < ilen; i++ ) + { + pad_done |= ((buf[i] | (unsigned char)-buf[i]) >> 7) ^ 1; + pad_count += ((pad_done | (unsigned char)-pad_done) >> 7) ^ 1; + } + + + /* If pad_done is still zero, there's no data, only unfinished padding. */ + bad |= mbedtls_cf_uint_if( pad_done, 0, 1 ); + + /* There must be at least 8 bytes of padding. */ + bad |= mbedtls_cf_size_gt( 8, pad_count ); + + /* If the padding is valid, set plaintext_size to the number of + * remaining bytes after stripping the padding. If the padding + * is invalid, avoid leaking this fact through the size of the + * output: use the maximum message size that fits in the output + * buffer. Do it without branches to avoid leaking the padding + * validity through timing. RSA keys are small enough that all the + * size_t values involved fit in unsigned int. */ + plaintext_size = mbedtls_cf_uint_if( + bad, (unsigned) plaintext_max_size, + (unsigned) ( ilen - pad_count - 3 ) ); + + /* Set output_too_large to 0 if the plaintext fits in the output + * buffer and to 1 otherwise. */ + output_too_large = mbedtls_cf_size_gt( plaintext_size, + plaintext_max_size ); + + /* Set ret without branches to avoid timing attacks. Return: + * - INVALID_PADDING if the padding is bad (bad != 0). + * - OUTPUT_TOO_LARGE if the padding is good but the decrypted + * plaintext does not fit in the output buffer. + * - 0 if the padding is correct. */ + ret = - (int) mbedtls_cf_uint_if( + bad, - MBEDTLS_ERR_RSA_INVALID_PADDING, + mbedtls_cf_uint_if( output_too_large, + - MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, + 0 ) ); + + /* If the padding is bad or the plaintext is too large, zero the + * data that we're about to copy to the output buffer. + * We need to copy the same amount of data + * from the same buffer whether the padding is good or not to + * avoid leaking the padding validity through overall timing or + * through memory or cache access patterns. */ + bad = mbedtls_cf_uint_mask( bad | output_too_large ); + for( i = 11; i < ilen; i++ ) + buf[i] &= ~bad; + + /* If the plaintext is too large, truncate it to the buffer size. + * Copy anyway to avoid revealing the length through timing, because + * revealing the length is as bad as revealing the padding validity + * for a Bleichenbacher attack. */ + plaintext_size = mbedtls_cf_uint_if( output_too_large, + (unsigned) plaintext_max_size, + (unsigned) plaintext_size ); + + /* Move the plaintext to the leftmost position where it can start in + * the working buffer, i.e. make it start plaintext_max_size from + * the end of the buffer. Do this with a memory access trace that + * does not depend on the plaintext size. After this move, the + * starting location of the plaintext is no longer sensitive + * information. */ + mbedtls_cf_mem_move_to_left( buf + ilen - plaintext_max_size, + plaintext_max_size, + plaintext_max_size - plaintext_size ); + + /* Finally copy the decrypted plaintext plus trailing zeros into the output + * buffer. If output_max_len is 0, then output may be an invalid pointer + * and the result of memcpy() would be undefined; prevent undefined + * behavior making sure to depend only on output_max_len (the size of the + * user-provided output buffer), which is independent from plaintext + * length, validity of padding, success of the decryption, and other + * secrets. */ + if( output_max_len != 0 ) + memcpy( output, buf + ilen - plaintext_max_size, plaintext_max_size ); + + /* Report the amount of data we copied to the output buffer. In case + * of errors (bad padding or output too large), the value of *olen + * when this function returns is not specified. Making it equivalent + * to the good case limits the risks of leaking the padding validity. */ + *olen = plaintext_size; + + return( ret ); +} + +#endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ diff --git a/library/constant_time.h b/library/constant_time.h index c7ca490d39..9a6db2807e 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -148,3 +148,13 @@ int mbedtls_ssl_cf_hmac( unsigned char *output ); #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ + +#if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) + +int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, + size_t *olen, + unsigned char *output, + size_t output_max_len, + unsigned char *buf ); + +#endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ diff --git a/library/rsa.c b/library/rsa.c index ff769bb11f..f4131fd8fc 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1458,133 +1458,6 @@ cleanup: #endif /* MBEDTLS_PKCS1_V21 */ #if defined(MBEDTLS_PKCS1_V15) -int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, - size_t *olen, - unsigned char *output, - size_t output_max_len, - unsigned char *buf ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t i, plaintext_max_size; - - /* The following variables take sensitive values: their value must - * not leak into the observable behavior of the function other than - * the designated outputs (output, olen, return value). Otherwise - * this would open the execution of the function to - * side-channel-based variants of the Bleichenbacher padding oracle - * attack. Potential side channels include overall timing, memory - * access patterns (especially visible to an adversary who has access - * to a shared memory cache), and branches (especially visible to - * an adversary who has access to a shared code cache or to a shared - * branch predictor). */ - size_t pad_count = 0; - unsigned bad = 0; - unsigned char pad_done = 0; - size_t plaintext_size = 0; - unsigned output_too_large; - - plaintext_max_size = mbedtls_cf_size_if( output_max_len > ilen - 11, - ilen - 11, - output_max_len ); - - /* Check and get padding length in constant time and constant - * memory trace. The first byte must be 0. */ - bad |= buf[0]; - - - /* Decode EME-PKCS1-v1_5 padding: 0x00 || 0x02 || PS || 0x00 - * where PS must be at least 8 nonzero bytes. */ - bad |= buf[1] ^ MBEDTLS_RSA_CRYPT; - - /* Read the whole buffer. Set pad_done to nonzero if we find - * the 0x00 byte and remember the padding length in pad_count. */ - for( i = 2; i < ilen; i++ ) - { - pad_done |= ((buf[i] | (unsigned char)-buf[i]) >> 7) ^ 1; - pad_count += ((pad_done | (unsigned char)-pad_done) >> 7) ^ 1; - } - - - /* If pad_done is still zero, there's no data, only unfinished padding. */ - bad |= mbedtls_cf_uint_if( pad_done, 0, 1 ); - - /* There must be at least 8 bytes of padding. */ - bad |= mbedtls_cf_size_gt( 8, pad_count ); - - /* If the padding is valid, set plaintext_size to the number of - * remaining bytes after stripping the padding. If the padding - * is invalid, avoid leaking this fact through the size of the - * output: use the maximum message size that fits in the output - * buffer. Do it without branches to avoid leaking the padding - * validity through timing. RSA keys are small enough that all the - * size_t values involved fit in unsigned int. */ - plaintext_size = mbedtls_cf_uint_if( - bad, (unsigned) plaintext_max_size, - (unsigned) ( ilen - pad_count - 3 ) ); - - /* Set output_too_large to 0 if the plaintext fits in the output - * buffer and to 1 otherwise. */ - output_too_large = mbedtls_cf_size_gt( plaintext_size, - plaintext_max_size ); - - /* Set ret without branches to avoid timing attacks. Return: - * - INVALID_PADDING if the padding is bad (bad != 0). - * - OUTPUT_TOO_LARGE if the padding is good but the decrypted - * plaintext does not fit in the output buffer. - * - 0 if the padding is correct. */ - ret = - (int) mbedtls_cf_uint_if( - bad, - MBEDTLS_ERR_RSA_INVALID_PADDING, - mbedtls_cf_uint_if( output_too_large, - - MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, - 0 ) ); - - /* If the padding is bad or the plaintext is too large, zero the - * data that we're about to copy to the output buffer. - * We need to copy the same amount of data - * from the same buffer whether the padding is good or not to - * avoid leaking the padding validity through overall timing or - * through memory or cache access patterns. */ - bad = mbedtls_cf_uint_mask( bad | output_too_large ); - for( i = 11; i < ilen; i++ ) - buf[i] &= ~bad; - - /* If the plaintext is too large, truncate it to the buffer size. - * Copy anyway to avoid revealing the length through timing, because - * revealing the length is as bad as revealing the padding validity - * for a Bleichenbacher attack. */ - plaintext_size = mbedtls_cf_uint_if( output_too_large, - (unsigned) plaintext_max_size, - (unsigned) plaintext_size ); - - /* Move the plaintext to the leftmost position where it can start in - * the working buffer, i.e. make it start plaintext_max_size from - * the end of the buffer. Do this with a memory access trace that - * does not depend on the plaintext size. After this move, the - * starting location of the plaintext is no longer sensitive - * information. */ - mbedtls_cf_mem_move_to_left( buf + ilen - plaintext_max_size, - plaintext_max_size, - plaintext_max_size - plaintext_size ); - - /* Finally copy the decrypted plaintext plus trailing zeros into the output - * buffer. If output_max_len is 0, then output may be an invalid pointer - * and the result of memcpy() would be undefined; prevent undefined - * behavior making sure to depend only on output_max_len (the size of the - * user-provided output buffer), which is independent from plaintext - * length, validity of padding, success of the decryption, and other - * secrets. */ - if( output_max_len != 0 ) - memcpy( output, buf + ilen - plaintext_max_size, plaintext_max_size ); - - /* Report the amount of data we copied to the output buffer. In case - * of errors (bad padding or output too large), the value of *olen - * when this function returns is not specified. Making it equivalent - * to the good case limits the risks of leaking the padding validity. */ - *olen = plaintext_size; - - return( ret ); -} - /* * Implementation of the PKCS#1 v2.1 RSAES-PKCS1-V1_5-DECRYPT function */ From 2dcd7686cef78bd13caf72766883cf796649f26f Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 27 Sep 2021 16:29:52 +0200 Subject: [PATCH 26/52] Typo: Unify indentation of function parameters Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 91 ++++++++++++++++++++++++++--------------- library/constant_time.h | 57 ++++++++++++++++++-------- 2 files changed, 97 insertions(+), 51 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 3784365a6e..9783215368 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -32,7 +32,9 @@ #include /* constant-time buffer comparison */ -int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ) +int mbedtls_ssl_safer_memcmp( const void *a, + const void *b, + size_t n ) { size_t i; volatile const unsigned char *A = (volatile const unsigned char *) a; @@ -56,7 +58,8 @@ int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ) * 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, +int mbedtls_constant_time_memcmp( const void *v1, + const void *v2, size_t len ) { const unsigned char *p1 = (const unsigned char*) v1; @@ -71,7 +74,9 @@ int mbedtls_constant_time_memcmp( const void *v1, const void *v2, } /* constant-time buffer comparison */ -unsigned char mbedtls_nist_kw_safer_memcmp( const void *a, const void *b, size_t n ) +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; @@ -91,7 +96,9 @@ unsigned char mbedtls_nist_kw_safer_memcmp( const void *a, const void *b, size_t } /* constant-time buffer comparison */ -int mbedtls_safer_memcmp( const void *a, const void *b, size_t n ) +int mbedtls_safer_memcmp( const void *a, + const void *b, + size_t n ) { size_t i; const unsigned char *A = (const unsigned char *) a; @@ -159,7 +166,8 @@ size_t mbedtls_cf_size_mask( size_t bit ) * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ) +size_t mbedtls_cf_size_mask_lt( size_t x, + size_t y ) { /* This has the most significant bit set if and only if x < y */ const size_t sub = x - y; @@ -184,7 +192,8 @@ size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ) * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ) +size_t mbedtls_cf_size_mask_ge( size_t x, + size_t y ) { return( ~mbedtls_cf_size_mask_lt( x, y ) ); } @@ -200,7 +209,8 @@ size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ) * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) +size_t mbedtls_cf_size_bool_eq( size_t x, + size_t y ) { /* diff = 0 if x == y, non-zero otherwise */ const size_t diff = x ^ y; @@ -235,7 +245,8 @@ size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ) * \return \c 0 if `size <= max`. * \return \c 1 if `size > max`. */ -unsigned mbedtls_cf_size_gt( size_t size, size_t max ) +unsigned mbedtls_cf_size_gt( size_t size, + size_t max ) { /* Return the sign bit (1 for negative) of (max - size). */ return( ( max - size ) >> ( sizeof( size_t ) * 8 - 1 ) ); @@ -251,7 +262,7 @@ unsigned mbedtls_cf_size_gt( size_t size, size_t max ) * \return 1 if \p x is less than \p y, 0 otherwise */ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, - const mbedtls_mpi_uint y ) + const mbedtls_mpi_uint y ) { mbedtls_mpi_uint ret; mbedtls_mpi_uint cond; @@ -290,13 +301,17 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, * \param if0 Value to use if \p cond is zero. * \return \c if1 if \p cond is nonzero, otherwise \c if0. */ -unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ) +unsigned mbedtls_cf_uint_if( unsigned cond, + unsigned if1, + unsigned if0 ) { unsigned mask = mbedtls_cf_uint_mask( cond ); return( ( mask & if1 ) | (~mask & if0 ) ); } -size_t mbedtls_cf_size_if( unsigned cond, size_t if1, size_t if0 ) +size_t mbedtls_cf_size_if( unsigned cond, + size_t if1, + size_t if0 ) { size_t mask = mbedtls_cf_size_mask( cond ); return( ( mask & if1 ) | (~mask & if0 ) ); @@ -314,7 +329,9 @@ size_t mbedtls_cf_size_if( unsigned cond, size_t if1, size_t if0 ) * * \return The selected sign value. */ -int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ) +int mbedtls_cf_cond_select_sign( int a, + int b, + unsigned char second ) { /* In order to avoid questions about what we can reasonnably assume about * the representations of signed integers, move everything to unsigned @@ -385,8 +402,8 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, * \param offset Offset from which to copy \p total - \p offset bytes. */ void mbedtls_cf_mem_move_to_left( void *start, - size_t total, - size_t offset ) + size_t total, + size_t offset ) { volatile unsigned char *buf = start; size_t i, n; @@ -418,9 +435,10 @@ void mbedtls_cf_mem_move_to_left( void *start, * might be translated to branches by some compilers on some platforms. */ void mbedtls_cf_memcpy_if_eq( unsigned char *dst, - const unsigned char *src, - size_t len, - size_t c1, size_t c2 ) + const unsigned char *src, + size_t len, + size_t c1, + size_t c2 ) { /* mask = c1 == c2 ? 0xff : 0x00 */ const size_t equal = mbedtls_cf_size_bool_eq( c1, c2 ); @@ -436,12 +454,12 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dst, * - functionally equivalent to memcpy(dst, src + offset_secret, len) * - but with execution flow independent from the value of offset_secret. */ -void mbedtls_cf_memcpy_offset( - unsigned char *dst, - const unsigned char *src_base, - size_t offset_secret, - size_t offset_min, size_t offset_max, - size_t len ) +void mbedtls_cf_memcpy_offset( unsigned char *dst, + const unsigned char *src_base, + size_t offset_secret, + size_t offset_min, + size_t offset_max, + size_t len ) { size_t offset; @@ -460,12 +478,14 @@ void mbedtls_cf_memcpy_offset( * Only works with MD-5, SHA-1, SHA-256 and SHA-384. * (Otherwise, computation of block_size needs to be adapted.) */ -int mbedtls_cf_hmac( - mbedtls_md_context_t *ctx, - const unsigned char *add_data, size_t add_data_len, - const unsigned char *data, size_t data_len_secret, - size_t min_data_len, size_t max_data_len, - unsigned char *output ) +int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, + const unsigned char *add_data, + size_t add_data_len, + const unsigned char *data, + size_t data_len_secret, + size_t min_data_len, + size_t max_data_len, + unsigned char *output ) { /* * This function breaks the HMAC abstraction and uses the md_clone() @@ -554,7 +574,9 @@ cleanup: * about whether the assignment was made or not. * (Leaking information about the respective sizes of X and Y is ok however.) */ -int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, const mbedtls_mpi *Y, unsigned char assign ) +int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, + const mbedtls_mpi *Y, + unsigned char assign ) { int ret = 0; size_t i; @@ -597,7 +619,9 @@ cleanup: * Here it is not ok to simply swap the pointers, which whould lead to * different memory access patterns when X and Y are used afterwards. */ -int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, mbedtls_mpi *Y, unsigned char swap ) +int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, + mbedtls_mpi *Y, + unsigned char swap ) { int ret, s; size_t i; @@ -647,8 +671,9 @@ cleanup: /* * Compare signed values in constant time */ -int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, const mbedtls_mpi *Y, - unsigned *ret ) +int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, + const mbedtls_mpi *Y, + unsigned *ret ) { size_t i; /* The value of any of these variables is either 0 or 1 at all times. */ diff --git a/library/constant_time.h b/library/constant_time.h index 9a6db2807e..f890a3de8c 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -29,26 +29,38 @@ #include -int mbedtls_ssl_safer_memcmp( const void *a, const void *b, size_t n ); +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 ); +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 ); +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_safer_memcmp( const void *a, + const void *b, + size_t n ); unsigned mbedtls_cf_uint_mask( unsigned value ); size_t mbedtls_cf_size_mask( size_t bit ); -size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); +size_t mbedtls_cf_size_mask_lt( size_t x, + size_t y ); -size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); +size_t mbedtls_cf_size_mask_ge( size_t x, + size_t y ); -size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ); +size_t mbedtls_cf_size_bool_eq( size_t x, + size_t y ); -unsigned mbedtls_cf_size_gt( size_t size, size_t max ); +unsigned mbedtls_cf_size_gt( size_t size, + size_t max ); #if defined(MBEDTLS_BIGNUM_C) @@ -57,11 +69,17 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, #endif /* MBEDTLS_BIGNUM_C */ -unsigned mbedtls_cf_uint_if( unsigned cond, unsigned if1, unsigned if0 ); +unsigned mbedtls_cf_uint_if( unsigned cond, + unsigned if1, + unsigned if0 ); -size_t mbedtls_cf_size_if( unsigned cond, size_t if1, size_t if0 ); +size_t mbedtls_cf_size_if( unsigned cond, + size_t if1, + size_t if0 ); -int mbedtls_cf_cond_select_sign( int a, int b, unsigned char second ); +int mbedtls_cf_cond_select_sign( int a, + int b, + unsigned char second ); #if defined(MBEDTLS_BIGNUM_C) @@ -102,7 +120,8 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dst, void mbedtls_cf_memcpy_offset( unsigned char *dst, const unsigned char *src_base, size_t offset_secret, - size_t offset_min, size_t offset_max, + size_t offset_min, + size_t offset_max, size_t len ); #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) @@ -140,12 +159,14 @@ void mbedtls_cf_memcpy_offset( unsigned char *dst, * \retval MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED * The hardware accelerator failed. */ -int mbedtls_ssl_cf_hmac( - mbedtls_md_context_t *ctx, - const unsigned char *add_data, size_t add_data_len, - const unsigned char *data, size_t data_len_secret, - size_t min_data_len, size_t max_data_len, - unsigned char *output ); +int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, + const unsigned char *add_data, + size_t add_data_len, + const unsigned char *data, + size_t data_len_secret, + size_t min_data_len, + size_t max_data_len, + unsigned char *output ); #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ From 4602564d7a588cc8f2df57d9a8fdddbf06c1ff80 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 19 Jul 2021 15:19:19 +0200 Subject: [PATCH 27/52] Unify memcmp functions Signed-off-by: gabor-mezei-arm --- 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 a53609e4eb..ce5179c5e7 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -1159,7 +1159,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 ); @@ -1181,7 +1181,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 9783215368..2388cab939 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 f890a3de8c..08e831fee5 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 aaed42a18c..b71befd88d 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -399,7 +399,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 ) { @@ -448,7 +448,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 f4131fd8fc..edc8ecccfe 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1887,7 +1887,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; @@ -2159,8 +2159,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 3ef318c963..8fd28cf772 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1381,9 +1381,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 5936d3598b..6ed3f2be33 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -227,7 +227,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 f8f366021a..55be047945 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -1172,7 +1172,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; @@ -1195,8 +1195,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 ); @@ -1406,8 +1406,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 716fa7de15..c4be1970e7 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" ) ); @@ -3673,7 +3673,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 c5ffa4dbbd..d6f038575b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2915,7 +2915,7 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } - 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" ) ); From 87ac5bef9773130459111976f8e80a5787025c13 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Tue, 10 Aug 2021 20:36:09 +0200 Subject: [PATCH 28/52] Unify function parameters Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 83 +++++++++++++++++++++-------------------- library/constant_time.h | 18 ++++----- 2 files changed, 51 insertions(+), 50 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 2388cab939..496843d640 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -178,19 +178,19 @@ size_t mbedtls_cf_size_bool_eq( size_t x, /** Check whether a size is out of bounds, without branches. * - * This is equivalent to `size > max`, but is likely to be compiled to + * This is equivalent to `x > y`, but is likely to be compiled to * to code using bitwise operation rather than a branch. * - * \param size Size to check. - * \param max Maximum desired value for \p size. - * \return \c 0 if `size <= max`. - * \return \c 1 if `size > max`. + * \param x Size to check. + * \param y Maximum desired value for \p size. + * \return \c 0 if `x <= y`. + * \return \c 1 if `x > y`. */ -unsigned mbedtls_cf_size_gt( size_t size, - size_t max ) +unsigned mbedtls_cf_size_gt( size_t x, + size_t y ) { - /* Return the sign bit (1 for negative) of (max - size). */ - return( ( max - size ) >> ( sizeof( size_t ) * 8 - 1 ) ); + /* Return the sign bit (1 for negative) of (y - x). */ + return( ( y - x ) >> ( sizeof( size_t ) * 8 - 1 ) ); } #if defined(MBEDTLS_BIGNUM_C) @@ -234,57 +234,58 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, /** Choose between two integer values, without branches. * - * This is equivalent to `cond ? if1 : if0`, but is likely to be compiled + * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled * to code using bitwise operation rather than a branch. * - * \param cond Condition to test. - * \param if1 Value to use if \p cond is nonzero. - * \param if0 Value to use if \p cond is zero. - * \return \c if1 if \p cond is nonzero, otherwise \c if0. + * \param condition Condition to test. + * \param if1 Value to use if \p condition is nonzero. + * \param if0 Value to use if \p condition is zero. + * \return \c if1 if \p condition is nonzero, otherwise \c if0. */ -unsigned mbedtls_cf_uint_if( unsigned cond, + +unsigned mbedtls_cf_uint_if( unsigned condition, unsigned if1, unsigned if0 ) { - unsigned mask = mbedtls_cf_uint_mask( cond ); + unsigned mask = mbedtls_cf_uint_mask( condition ); return( ( mask & if1 ) | (~mask & if0 ) ); } -size_t mbedtls_cf_size_if( unsigned cond, +size_t mbedtls_cf_size_if( unsigned condition, size_t if1, size_t if0 ) { - size_t mask = mbedtls_cf_size_mask( cond ); + size_t mask = mbedtls_cf_size_mask( condition ); return( ( mask & if1 ) | (~mask & if0 ) ); } /** * Select between two sign values in constant-time. * - * This is functionally equivalent to second ? a : b but uses only bit + * This is functionally equivalent to condition ? if1 : if0 but uses only bit * operations in order to avoid branches. * - * \param[in] a The first sign; must be either +1 or -1. - * \param[in] b The second sign; must be either +1 or -1. - * \param[in] second Must be either 1 (return b) or 0 (return a). + * \param[in] condition Must be either 1 (return \p if1) or 0 (return \pp if0). + * \param[in] if1 The first sign; must be either +1 or -1. + * \param[in] if0 The second sign; must be either +1 or -1. * - * \return The selected sign value. + * \return \c if1 if \p condition is nonzero, otherwise \c if0. */ -int mbedtls_cf_cond_select_sign( int a, - int b, - unsigned char second ) +int mbedtls_cf_cond_select_sign( unsigned char condition, + int if1, + int if0 ) { /* In order to avoid questions about what we can reasonnably assume about * the representations of signed integers, move everything to unsigned * by taking advantage of the fact that a and b are either +1 or -1. */ - unsigned ua = a + 1; - unsigned ub = b + 1; + unsigned uif1 = if1 + 1; + unsigned uif0 = if0 + 1; - /* second was 0 or 1, mask is 0 or 2 as are ua and ub */ - const unsigned mask = second << 1; + /* condition was 0 or 1, mask is 0 or 2 as are ua and ub */ + const unsigned mask = condition << 1; /* select ua or ub */ - unsigned ur = ( ua & ~mask ) | ( ub & mask ); + unsigned ur = ( uif0 & ~mask ) | ( uif1 & mask ); /* ur is now 0 or 2, convert back to -1 or +1 */ return( (int) ur - 1 ); @@ -296,12 +297,12 @@ int mbedtls_cf_cond_select_sign( int a, * Conditionally assign dest = src, without leaking information * about whether the assignment was made or not. * dest and src must be arrays of limbs of size n. - * assign must be 0 or 1. + * condition must be 0 or 1. */ void mbedtls_cf_mpi_uint_cond_assign( size_t n, mbedtls_mpi_uint *dest, const mbedtls_mpi_uint *src, - unsigned char assign ) + unsigned char condition ) { size_t i; @@ -312,8 +313,8 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, #pragma warning( disable : 4146 ) #endif - /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */ - const mbedtls_mpi_uint mask = -assign; + /* all-bits 1 if condition is 1, all-bits 0 if condition is 0 */ + const mbedtls_mpi_uint mask = -condition; #if defined(_MSC_VER) #pragma warning( pop ) @@ -375,7 +376,7 @@ void mbedtls_cf_mem_move_to_left( void *start, * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -void mbedtls_cf_memcpy_if_eq( unsigned char *dst, +void mbedtls_cf_memcpy_if_eq( unsigned char *dest, const unsigned char *src, size_t len, size_t c1, @@ -385,9 +386,9 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dst, const size_t equal = mbedtls_cf_size_bool_eq( c1, c2 ); const unsigned char mask = (unsigned char) mbedtls_cf_size_mask( equal ); - /* dst[i] = c1 == c2 ? src[i] : dst[i] */ + /* dest[i] = c1 == c2 ? src[i] : dest[i] */ for( size_t i = 0; i < len; i++ ) - dst[i] = ( src[i] & mask ) | ( dst[i] & ~mask ); + dest[i] = ( src[i] & mask ) | ( dest[i] & ~mask ); } /* @@ -543,7 +544,7 @@ int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); - X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, assign ); + X->s = mbedtls_cf_cond_select_sign( assign, Y->s, X->s ); mbedtls_cf_mpi_uint_cond_assign( Y->n, X->p, Y->p, assign ); @@ -594,8 +595,8 @@ int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, MBEDTLS_MPI_CHK( mbedtls_mpi_grow( Y, X->n ) ); s = X->s; - X->s = mbedtls_cf_cond_select_sign( X->s, Y->s, swap ); - Y->s = mbedtls_cf_cond_select_sign( Y->s, s, swap ); + X->s = mbedtls_cf_cond_select_sign( swap, Y->s, X->s ); + Y->s = mbedtls_cf_cond_select_sign( swap, s, Y->s ); for( i = 0; i < X->n; i++ ) diff --git a/library/constant_time.h b/library/constant_time.h index 08e831fee5..ed6ec6afa5 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -47,8 +47,8 @@ size_t mbedtls_cf_size_mask_ge( size_t x, size_t mbedtls_cf_size_bool_eq( size_t x, size_t y ); -unsigned mbedtls_cf_size_gt( size_t size, - size_t max ); +unsigned mbedtls_cf_size_gt( size_t x, + size_t y ); #if defined(MBEDTLS_BIGNUM_C) @@ -57,24 +57,24 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, #endif /* MBEDTLS_BIGNUM_C */ -unsigned mbedtls_cf_uint_if( unsigned cond, +unsigned mbedtls_cf_uint_if( unsigned condition, unsigned if1, unsigned if0 ); -size_t mbedtls_cf_size_if( unsigned cond, +size_t mbedtls_cf_size_if( unsigned condition, size_t if1, size_t if0 ); -int mbedtls_cf_cond_select_sign( int a, - int b, - unsigned char second ); +int mbedtls_cf_cond_select_sign( unsigned char condition, + int if1, + int if0 ); #if defined(MBEDTLS_BIGNUM_C) void mbedtls_cf_mpi_uint_cond_assign( size_t n, mbedtls_mpi_uint *dest, const mbedtls_mpi_uint *src, - unsigned char assign ); + unsigned char condition ); #endif /* MBEDTLS_BIGNUM_C */ @@ -82,7 +82,7 @@ void mbedtls_cf_mem_move_to_left( void *start, size_t total, size_t offset ); -void mbedtls_cf_memcpy_if_eq( unsigned char *dst, +void mbedtls_cf_memcpy_if_eq( unsigned char *dest, const unsigned char *src, size_t len, size_t c1, size_t c2 ); From 396438c57b03a96eb6d8eab9a6da3423be2a39a0 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Tue, 10 Aug 2021 20:56:21 +0200 Subject: [PATCH 29/52] Unify mask generation functions Generate all-bits 0 or all bits 1 mask from a value instead of from a bit. Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 10 +++++----- library/constant_time.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 496843d640..a407c798bb 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -72,9 +72,9 @@ unsigned mbedtls_cf_uint_mask( unsigned value ) } /* - * Turn a bit into a mask: - * - if bit == 1, return the all-bits 1 mask, aka (size_t) -1 - * - if bit == 0, return the all-bits 0 mask, aka 0 + * Turn a value into a mask: + * - if value != 0, return the all-bits 1 mask, aka (size_t) -1 + * - if value == 0, return the all-bits 0 mask, aka 0 * * This function can be used to write constant-time code by replacing branches * with bit operations using masks. @@ -82,7 +82,7 @@ unsigned mbedtls_cf_uint_mask( unsigned value ) * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -size_t mbedtls_cf_size_mask( size_t bit ) +size_t mbedtls_cf_size_mask( size_t value ) { /* MSVC has a warning about unary minus on unsigned integer types, * but this is well-defined and precisely what we want to do here. */ @@ -90,7 +90,7 @@ size_t mbedtls_cf_size_mask( size_t bit ) #pragma warning( push ) #pragma warning( disable : 4146 ) #endif - return -bit; + return( - ( ( value | - value ) >> ( sizeof( value ) * 8 - 1 ) ) ); #if defined(_MSC_VER) #pragma warning( pop ) #endif diff --git a/library/constant_time.h b/library/constant_time.h index ed6ec6afa5..80e59dac6e 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -36,7 +36,7 @@ int mbedtls_cf_memcmp( const void *a, unsigned mbedtls_cf_uint_mask( unsigned value ); -size_t mbedtls_cf_size_mask( size_t bit ); +size_t mbedtls_cf_size_mask( size_t value ); size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); From 9cb55698aa557caa441cfa6d115fa0b51a669221 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Wed, 11 Aug 2021 15:07:02 +0200 Subject: [PATCH 30/52] Propagate usage of mask generation functions Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 48 +++++++++++++++++------------------------ library/constant_time.h | 6 ++++++ library/ssl_srv.c | 11 +--------- 3 files changed, 27 insertions(+), 38 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index a407c798bb..76cab097ae 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -96,6 +96,24 @@ size_t mbedtls_cf_size_mask( size_t value ) #endif } +#if defined(MBEDTLS_BIGNUM_C) + +mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ) +{ + /* MSVC has a warning about unary minus on unsigned, but this is + * well-defined and precisely what we want to do here */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif + return( - ( ( value | - value ) >> ( sizeof( value ) * 8 - 1 ) ) ); +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif +} + +#endif /* MBEDTLS_BIGNUM_C */ + /* * Constant-flow mask generation for "less than" comparison: * - if x < y, return all bits 1, that is (size_t) -1 @@ -526,21 +544,8 @@ int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( Y != NULL ); - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - - /* make sure assign is 0 or 1 in a time-constant manner */ - assign = (assign | (unsigned char)-assign) >> (sizeof( assign ) * 8 - 1); /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */ - limb_mask = -assign; - -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif + limb_mask = mbedtls_cf_mpi_uint_mask( assign );; MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); @@ -575,21 +580,8 @@ int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, if( X == Y ) return( 0 ); - /* MSVC has a warning about unary minus on unsigned integer types, - * but this is well-defined and precisely what we want to do here. */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - - /* make sure swap is 0 or 1 in a time-constant manner */ - swap = (swap | (unsigned char)-swap) >> (sizeof( swap ) * 8 - 1); /* all-bits 1 if swap is 1, all-bits 0 if swap is 0 */ - limb_mask = -swap; - -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif + limb_mask = mbedtls_cf_mpi_uint_mask( swap ); MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_grow( Y, X->n ) ); diff --git a/library/constant_time.h b/library/constant_time.h index 80e59dac6e..ce2a9ef57b 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -38,6 +38,12 @@ unsigned mbedtls_cf_uint_mask( unsigned value ); size_t mbedtls_cf_size_mask( size_t value ); +#if defined(MBEDTLS_BIGNUM_C) + +mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); + +#endif /* MBEDTLS_BIGNUM_C */ + size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index c4be1970e7..989cfe07b4 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3581,16 +3581,7 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, diff |= peer_pms[1] ^ ver[1]; /* mask = diff ? 0xff : 0x00 using bit operations to avoid branches */ - /* MSVC has a warning about unary minus on unsigned, but this is - * well-defined and precisely what we want to do here */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - mask = - ( ( diff | - diff ) >> ( sizeof( unsigned int ) * 8 - 1 ) ); -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif + mask = mbedtls_cf_uint_mask( diff ); /* * Protection against Bleichenbacher's attack: invalid PKCS#1 v1.5 padding From b11a56e34c13eb76b160a6481608a598958b78c5 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Wed, 11 Aug 2021 17:28:49 +0200 Subject: [PATCH 31/52] Unify equality checker functions return value The equality checker functions always return 0 or 1 value, thus the type of return value can be the same dispite of the size of the parameters. Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 6 +++--- library/constant_time.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 76cab097ae..4d475ba85d 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -168,8 +168,8 @@ size_t mbedtls_cf_size_mask_ge( size_t x, * This function is implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ -size_t mbedtls_cf_size_bool_eq( size_t x, - size_t y ) +unsigned mbedtls_cf_size_bool_eq( size_t x, + size_t y ) { /* diff = 0 if x == y, non-zero otherwise */ const size_t diff = x ^ y; @@ -189,7 +189,7 @@ size_t mbedtls_cf_size_bool_eq( size_t x, #endif /* diff1 = (x != y) ? 1 : 0 */ - const size_t diff1 = diff_msb >> ( sizeof( diff_msb ) * 8 - 1 ); + const unsigned diff1 = diff_msb >> ( sizeof( diff_msb ) * 8 - 1 ); return( 1 ^ diff1 ); } diff --git a/library/constant_time.h b/library/constant_time.h index ce2a9ef57b..cdc5f853ee 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -50,8 +50,8 @@ size_t mbedtls_cf_size_mask_lt( size_t x, size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); -size_t mbedtls_cf_size_bool_eq( size_t x, - size_t y ); +unsigned mbedtls_cf_size_bool_eq( size_t x, + size_t y ); unsigned mbedtls_cf_size_gt( size_t x, size_t y ); From 90d96cc74192e63758359821b44acdfb68ff59e8 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Wed, 11 Aug 2021 16:40:35 +0200 Subject: [PATCH 32/52] Add documentation for the functions Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 132 ---------------------- library/constant_time.h | 241 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 241 insertions(+), 132 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 4d475ba85d..eedbfa0401 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -52,11 +52,6 @@ int mbedtls_cf_memcmp( const void *a, return( (int)diff ); } -/** Turn zero-or-nonzero into zero-or-all-bits-one, without branches. - * - * \param value The value to analyze. - * \return Zero if \p value is zero, otherwise all-bits-one. - */ unsigned mbedtls_cf_uint_mask( unsigned value ) { /* MSVC has a warning about unary minus on unsigned, but this is @@ -71,17 +66,6 @@ unsigned mbedtls_cf_uint_mask( unsigned value ) #endif } -/* - * Turn a value into a mask: - * - if value != 0, return the all-bits 1 mask, aka (size_t) -1 - * - if value == 0, return the all-bits 0 mask, aka 0 - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ size_t mbedtls_cf_size_mask( size_t value ) { /* MSVC has a warning about unary minus on unsigned integer types, @@ -114,17 +98,6 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ) #endif /* MBEDTLS_BIGNUM_C */ -/* - * Constant-flow mask generation for "less than" comparison: - * - if x < y, return all bits 1, that is (size_t) -1 - * - otherwise, return all bits 0, that is 0 - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ) { @@ -140,34 +113,12 @@ size_t mbedtls_cf_size_mask_lt( size_t x, return( mask ); } -/* - * Constant-flow mask generation for "greater or equal" comparison: - * - if x >= y, return all bits 1, that is (size_t) -1 - * - otherwise, return all bits 0, that is 0 - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ) { return( ~mbedtls_cf_size_mask_lt( x, y ) ); } -/* - * Constant-flow boolean "equal" comparison: - * return x == y - * - * This function can be used to write constant-time code by replacing branches - * with bit operations - it can be used in conjunction with - * mbedtls_ssl_cf_mask_from_bit(). - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ unsigned mbedtls_cf_size_bool_eq( size_t x, size_t y ) { @@ -194,16 +145,6 @@ unsigned mbedtls_cf_size_bool_eq( size_t x, return( 1 ^ diff1 ); } -/** Check whether a size is out of bounds, without branches. - * - * This is equivalent to `x > y`, but is likely to be compiled to - * to code using bitwise operation rather than a branch. - * - * \param x Size to check. - * \param y Maximum desired value for \p size. - * \return \c 0 if `x <= y`. - * \return \c 1 if `x > y`. - */ unsigned mbedtls_cf_size_gt( size_t x, size_t y ) { @@ -213,13 +154,6 @@ unsigned mbedtls_cf_size_gt( size_t x, #if defined(MBEDTLS_BIGNUM_C) -/** Decide if an integer is less than the other, without branches. - * - * \param x First integer. - * \param y Second integer. - * - * \return 1 if \p x is less than \p y, 0 otherwise - */ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, const mbedtls_mpi_uint y ) { @@ -250,17 +184,6 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, #endif /* MBEDTLS_BIGNUM_C */ -/** Choose between two integer values, without branches. - * - * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled - * to code using bitwise operation rather than a branch. - * - * \param condition Condition to test. - * \param if1 Value to use if \p condition is nonzero. - * \param if0 Value to use if \p condition is zero. - * \return \c if1 if \p condition is nonzero, otherwise \c if0. - */ - unsigned mbedtls_cf_uint_if( unsigned condition, unsigned if1, unsigned if0 ) @@ -277,18 +200,6 @@ size_t mbedtls_cf_size_if( unsigned condition, return( ( mask & if1 ) | (~mask & if0 ) ); } -/** - * Select between two sign values in constant-time. - * - * This is functionally equivalent to condition ? if1 : if0 but uses only bit - * operations in order to avoid branches. - * - * \param[in] condition Must be either 1 (return \p if1) or 0 (return \pp if0). - * \param[in] if1 The first sign; must be either +1 or -1. - * \param[in] if0 The second sign; must be either +1 or -1. - * - * \return \c if1 if \p condition is nonzero, otherwise \c if0. - */ int mbedtls_cf_cond_select_sign( unsigned char condition, int if1, int if0 ) @@ -311,12 +222,6 @@ int mbedtls_cf_cond_select_sign( unsigned char condition, #if defined(MBEDTLS_BIGNUM_C) -/* - * Conditionally assign dest = src, without leaking information - * about whether the assignment was made or not. - * dest and src must be arrays of limbs of size n. - * condition must be 0 or 1. - */ void mbedtls_cf_mpi_uint_cond_assign( size_t n, mbedtls_mpi_uint *dest, const mbedtls_mpi_uint *src, @@ -344,23 +249,6 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, #endif /* MBEDTLS_BIGNUM_C */ -/** Shift some data towards the left inside a buffer without leaking - * the length of the data through side channels. - * - * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally - * equivalent to - * ``` - * memmove(start, start + offset, total - offset); - * memset(start + offset, 0, total - offset); - * ``` - * but it strives to use a memory access pattern (and thus total timing) - * that does not depend on \p offset. This timing independence comes at - * the expense of performance. - * - * \param start Pointer to the start of the buffer. - * \param total Total size of the buffer. - * \param offset Offset from which to copy \p total - \p offset bytes. - */ void mbedtls_cf_mem_move_to_left( void *start, size_t total, size_t offset ) @@ -385,15 +273,6 @@ void mbedtls_cf_mem_move_to_left( void *start, } } -/* - * Constant-flow conditional memcpy: - * - if c1 == c2, equivalent to memcpy(dst, src, len), - * - otherwise, a no-op, - * but with execution flow independent of the values of c1 and c2. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - */ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, const unsigned char *src, size_t len, @@ -409,11 +288,6 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, dest[i] = ( src[i] & mask ) | ( dest[i] & ~mask ); } -/* - * Constant-flow memcpy from variable position in buffer. - * - functionally equivalent to memcpy(dst, src + offset_secret, len) - * - but with execution flow independent from the value of offset_secret. - */ void mbedtls_cf_memcpy_offset( unsigned char *dst, const unsigned char *src_base, size_t offset_secret, @@ -432,12 +306,6 @@ void mbedtls_cf_memcpy_offset( unsigned char *dst, #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -/* - * Compute HMAC of variable-length data with constant flow. - * - * Only works with MD-5, SHA-1, SHA-256 and SHA-384. - * (Otherwise, computation of block_size needs to be adapted.) - */ int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, const unsigned char *add_data, size_t add_data_len, diff --git a/library/constant_time.h b/library/constant_time.h index cdc5f853ee..3c7502f231 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -30,53 +30,241 @@ #include +/** Constant-time buffer comparison without branches. + * + * This is equivalent to the standard memncmp function, but is likely to be + * compiled to code using bitwise operation rather than a branch. + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param a Pointer to the first buffer. + * \param b Pointer to the second buffer. + * \param n The number of bytes to compare in the buffer. + * + * \return Zero if the content of the two buffer is the same, + * otherwise non-zero. + */ int mbedtls_cf_memcmp( const void *a, const void *b, size_t n ); +/** Turn a value into a mask: + * - if \p value == 0, return the all-bits 0 mask, aka 0 + * - otherwise, return the all-bits 1 mask, aka (size_t) -1 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param value The value to analyze. + * + * \return Zero if \p value is zero, otherwise all-bits-one. + */ unsigned mbedtls_cf_uint_mask( unsigned value ); +/** Turn a value into a mask: + * - if \p value == 0, return the all-bits 0 mask, aka 0 + * - otherwise, return the all-bits 1 mask, aka (size_t) -1 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param value The value to analyze. + * + * \return Zero if \p value is zero, otherwise all-bits-one. + */ size_t mbedtls_cf_size_mask( size_t value ); #if defined(MBEDTLS_BIGNUM_C) +/** Turn a value into a mask: + * - if \p value == 0, return the all-bits 0 mask, aka 0 + * - otherwise, return the all-bits 1 mask, aka (size_t) -1 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param value The value to analyze. + * + * \return Zero if \p value is zero, otherwise all-bits-one. + */ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); #endif /* MBEDTLS_BIGNUM_C */ +/** Constant-flow mask generation for "less than" comparison: + * - if \p x < \p y, return all-bits 1, that is (size_t) -1 + * - otherwise, return all bits 0, that is 0 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param x The first value to analyze. + * \param y The second value to analyze. + * + * \return All-bits-one if \p x is less than \p y, otherwise zero. + */ size_t mbedtls_cf_size_mask_lt( size_t x, size_t y ); +/** Constant-flow mask generation for "greater or equal" comparison: + * - if \p x >= \p y, return all-bits 1, that is (size_t) -1 + * - otherwise, return all bits 0, that is 0 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param x The first value to analyze. + * \param y The second value to analyze. + * + * \return All-bits-one if \p x is greater or equal than \p y, + * otherwise zero. + */ size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); +/** Constant-flow boolean "equal" comparison: + * return x == y + * + * This is equivalent to \p x == \p y, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param x The first value to analyze. + * \param y The second value to analyze. + * + * \return 1 if \p x equals to \p y, otherwise 0. + */ unsigned mbedtls_cf_size_bool_eq( size_t x, size_t y ); +/** Constant-flow "greater than" comparison: + * return x > y + * + * This is equivalent to \p x > \p y, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param x The first value to analyze. + * \param y The second value to analyze. + * + * \return 1 if \p x greater than \p y, otherwise 0. + */ unsigned mbedtls_cf_size_gt( size_t x, size_t y ); #if defined(MBEDTLS_BIGNUM_C) +/** Decide if an integer is less than the other, without branches. + * + * This is equivalent to \p x < \p y, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param x The first value to analyze. + * \param y The second value to analyze. + * + * \return 1 if \p x is less than \p y, otherwise 0. + */ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, const mbedtls_mpi_uint y ); #endif /* MBEDTLS_BIGNUM_C */ +/** Choose between two integer values without branches. + * + * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param condition Condition to test. + * \param if1 Value to use if \p condition is nonzero. + * \param if0 Value to use if \p condition is zero. + * + * \return \c if1 if \p condition is nonzero, otherwise \c if0. + */ unsigned mbedtls_cf_uint_if( unsigned condition, unsigned if1, unsigned if0 ); +/** Choose between two integer values without branches. + * + * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param condition Condition to test. + * \param if1 Value to use if \p condition is nonzero. + * \param if0 Value to use if \p condition is zero. + * + * \return \c if1 if \p condition is nonzero, otherwise \c if0. + */ size_t mbedtls_cf_size_if( unsigned condition, size_t if1, size_t if0 ); +/** Select between two sign values witout branches. + * + * This is functionally equivalent to `condition ? if1 : if0` but uses only bit + * operations in order to avoid branches. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param condition Condition to test. + * \param if1 The first sign; must be either +1 or -1. + * \param if0 The second sign; must be either +1 or -1. + * + * \return \c if1 if \p condition is nonzero, otherwise \c if0. */ int mbedtls_cf_cond_select_sign( unsigned char condition, int if1, int if0 ); #if defined(MBEDTLS_BIGNUM_C) +/** Conditionally assign a value without branches. + * + * This is equivalent to `if ( condition ) dest = src`, but is likely + * to be compiled to code using bitwise operation rather than a branch. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param n \p dest and \p src must be arrays of limbs of size n. + * \param dest The MPI to conditionally assign to. This must point + * to an initialized MPI. + * \param src The MPI to be assigned from. This must point to an + * initialized MPI. + * \param condition Condition to test, must be 0 or 1. + */ void mbedtls_cf_mpi_uint_cond_assign( size_t n, mbedtls_mpi_uint *dest, const mbedtls_mpi_uint *src, @@ -84,10 +272,41 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, #endif /* MBEDTLS_BIGNUM_C */ + +/** Shift some data towards the left inside a buffer. + * + * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally + * equivalent to + * ``` + * memmove(start, start + offset, total - offset); + * memset(start + offset, 0, total - offset); + * ``` + * but it strives to use a memory access pattern (and thus total timing) + * that does not depend on \p offset. This timing independence comes at + * the expense of performance. + * + * \param start Pointer to the start of the buffer. + * \param total Total size of the buffer. + * \param offset Offset from which to copy \p total - \p offset bytes. + */ void mbedtls_cf_mem_move_to_left( void *start, size_t total, size_t offset ); +/** Conditional memcpy without branches. + * + * This is equivalent to `if ( c1 == c2 ) memcpy(dst, src, len)`, but is likely + * to be compiled to code using bitwise operation rather than a branch. + * + * This function is implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + * + * \param dest The pointer to conditionally copy to. + * \param src The pointer to copy from. + * \param len The number of bytes to copy. + * \param c1 The first value to analyze in the condition. + * \param c2 The second value to analyze in the condition. + */ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, const unsigned char *src, size_t len, @@ -98,6 +317,7 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, * This function copies \p len bytes from \p src_base + \p offset_secret to \p * dst, with a code flow and memory access pattern that does not depend on \p * offset_secret, but only on \p offset_min, \p offset_max and \p len. + * Functionally equivalent to memcpy(dst, src + offset_secret, len). * * \param dst The destination buffer. This must point to a writable * buffer of at least \p len bytes. @@ -166,6 +386,27 @@ int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, #if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) +/** This function performs the unpadding part of a PKCS#1 v1.5 decryption + * operation (RSAES-PKCS1-v1_5-DECRYPT). + * + * \note The output buffer length \c output_max_len should be + * as large as the size \p ctx->len of \p ctx->N, for example, + * 128 Bytes if RSA-1024 is used, to be able to hold an + * arbitrary decrypted message. If it is not large enough to + * hold the decryption of the particular ciphertext provided, + * the function returns #MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE. + * + * \param ilen The length of the ciphertext. + * \param olen The address at which to store the length of + * the plaintext. This must not be \c NULL. + * \param output The buffer used to hold the plaintext. This must + * be a writable buffer of length \p output_max_len Bytes. + * \param output_max_len The length in Bytes of the output buffer \p output. + * \param buf The input buffer for the unpadding operation. + * + * \return \c 0 on success. + * \return An \c MBEDTLS_ERR_RSA_XXX error code on failure. + */ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, size_t *olen, unsigned char *output, From 6565ea0739b1a41fad324b70b12c16e7a206da4b Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Wed, 25 Aug 2021 20:39:07 +0200 Subject: [PATCH 33/52] Add changelog entry Signed-off-by: gabor-mezei-arm --- ChangeLog.d/constant_time_module.txt | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 ChangeLog.d/constant_time_module.txt diff --git a/ChangeLog.d/constant_time_module.txt b/ChangeLog.d/constant_time_module.txt new file mode 100644 index 0000000000..5e76deaa8d --- /dev/null +++ b/ChangeLog.d/constant_time_module.txt @@ -0,0 +1,6 @@ +Changes + * The mbedcrypto library includes new a source code module constant_time.c, + containing various functions meant to resist timing side channel attacks. + This module does not have a separate configuration option, and functions + from this module will be included in the build as required. Currently + the interface of this module is private and may change at any time. From 5b3a32d88311d173e59e5c5c1c5e256b2c4f89f0 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Wed, 29 Sep 2021 10:50:31 +0200 Subject: [PATCH 34/52] Fix missing includes Signed-off-by: gabor-mezei-arm --- library/constant_time.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/constant_time.c b/library/constant_time.c index eedbfa0401..966361278d 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -20,6 +20,7 @@ #include "common.h" #include "constant_time.h" #include "mbedtls/error.h" +#include "mbedtls/platform_util.h" #if defined(MBEDTLS_BIGNUM_C) #include "mbedtls/bignum.h" @@ -29,6 +30,10 @@ #include "ssl_misc.h" #endif +#if defined(MBEDTLS_RSA_C) +#include "mbedtls/rsa.h" +#endif + #include int mbedtls_cf_memcmp( const void *a, From 1e64261da54146e21db4a686d8da13b168a3f1f0 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 16:05:50 +0200 Subject: [PATCH 35/52] Make mbedtls_cf_size_mask_lt function static The mbedtls_cf_size_mask_lt is solely used as an auxiliary function for mbedtls_cf_size_mask_ge. Signed-off-by: Gabor Mezei --- library/constant_time.c | 16 ++++++++++++++-- library/constant_time.h | 18 ------------------ 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 966361278d..e0eadac7c8 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -103,8 +103,20 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ) #endif /* MBEDTLS_BIGNUM_C */ -size_t mbedtls_cf_size_mask_lt( size_t x, - size_t y ) +/** Constant-flow mask generation for "less than" comparison: + * - if \p x < \p y, return all-bits 1, that is (size_t) -1 + * - otherwise, return all bits 0, that is 0 + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * \param x The first value to analyze. + * \param y The second value to analyze. + * + * \return All-bits-one if \p x is less than \p y, otherwise zero. + */ +static size_t mbedtls_cf_size_mask_lt( size_t x, + size_t y ) { /* This has the most significant bit set if and only if x < y */ const size_t sub = x - y; diff --git a/library/constant_time.h b/library/constant_time.h index 3c7502f231..336a151d91 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -104,24 +104,6 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); #endif /* MBEDTLS_BIGNUM_C */ -/** Constant-flow mask generation for "less than" comparison: - * - if \p x < \p y, return all-bits 1, that is (size_t) -1 - * - otherwise, return all bits 0, that is 0 - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * - * \param x The first value to analyze. - * \param y The second value to analyze. - * - * \return All-bits-one if \p x is less than \p y, otherwise zero. - */ -size_t mbedtls_cf_size_mask_lt( size_t x, - size_t y ); - /** Constant-flow mask generation for "greater or equal" comparison: * - if \p x >= \p y, return all-bits 1, that is (size_t) -1 * - otherwise, return all bits 0, that is 0 From eab90bcc36c71f07317ade30f19dad11931f04be Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 16:09:41 +0200 Subject: [PATCH 36/52] Move implementation specific comment This comment is about how the functions are implemented, not about their public interface, so it doesn't belong in the header file. It applies to everything in constant_time.c so moved there. Signed-off-by: Gabor Mezei --- library/constant_time.c | 5 +++++ library/constant_time.h | 38 -------------------------------------- 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index e0eadac7c8..98fdbd757b 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -17,6 +17,11 @@ * limitations under the License. */ + /* + * The following functiona are implemented without using comparison operators, as those + * might be translated to branches by some compilers on some platforms. + */ + #include "common.h" #include "constant_time.h" #include "mbedtls/error.h" diff --git a/library/constant_time.h b/library/constant_time.h index 336a151d91..4810c92ec5 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -38,9 +38,6 @@ * This function can be used to write constant-time code by replacing branches * with bit operations using masks. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param a Pointer to the first buffer. * \param b Pointer to the second buffer. * \param n The number of bytes to compare in the buffer. @@ -59,9 +56,6 @@ int mbedtls_cf_memcmp( const void *a, * This function can be used to write constant-time code by replacing branches * with bit operations using masks. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param value The value to analyze. * * \return Zero if \p value is zero, otherwise all-bits-one. @@ -75,9 +69,6 @@ unsigned mbedtls_cf_uint_mask( unsigned value ); * This function can be used to write constant-time code by replacing branches * with bit operations using masks. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param value The value to analyze. * * \return Zero if \p value is zero, otherwise all-bits-one. @@ -93,9 +84,6 @@ size_t mbedtls_cf_size_mask( size_t value ); * This function can be used to write constant-time code by replacing branches * with bit operations using masks. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param value The value to analyze. * * \return Zero if \p value is zero, otherwise all-bits-one. @@ -111,9 +99,6 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); * This function can be used to write constant-time code by replacing branches * with bit operations using masks. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param x The first value to analyze. * \param y The second value to analyze. * @@ -129,9 +114,6 @@ size_t mbedtls_cf_size_mask_ge( size_t x, * This is equivalent to \p x == \p y, but is likely to be compiled * to code using bitwise operation rather than a branch. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param x The first value to analyze. * \param y The second value to analyze. * @@ -146,9 +128,6 @@ unsigned mbedtls_cf_size_bool_eq( size_t x, * This is equivalent to \p x > \p y, but is likely to be compiled * to code using bitwise operation rather than a branch. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param x The first value to analyze. * \param y The second value to analyze. * @@ -164,9 +143,6 @@ unsigned mbedtls_cf_size_gt( size_t x, * This is equivalent to \p x < \p y, but is likely to be compiled * to code using bitwise operation rather than a branch. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param x The first value to analyze. * \param y The second value to analyze. * @@ -182,9 +158,6 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled * to code using bitwise operation rather than a branch. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param condition Condition to test. * \param if1 Value to use if \p condition is nonzero. * \param if0 Value to use if \p condition is zero. @@ -200,9 +173,6 @@ unsigned mbedtls_cf_uint_if( unsigned condition, * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled * to code using bitwise operation rather than a branch. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param condition Condition to test. * \param if1 Value to use if \p condition is nonzero. * \param if0 Value to use if \p condition is zero. @@ -218,8 +188,6 @@ size_t mbedtls_cf_size_if( unsigned condition, * This is functionally equivalent to `condition ? if1 : if0` but uses only bit * operations in order to avoid branches. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. * * \param condition Condition to test. * \param if1 The first sign; must be either +1 or -1. @@ -237,9 +205,6 @@ int mbedtls_cf_cond_select_sign( unsigned char condition, * This is equivalent to `if ( condition ) dest = src`, but is likely * to be compiled to code using bitwise operation rather than a branch. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param n \p dest and \p src must be arrays of limbs of size n. * \param dest The MPI to conditionally assign to. This must point * to an initialized MPI. @@ -280,9 +245,6 @@ void mbedtls_cf_mem_move_to_left( void *start, * This is equivalent to `if ( c1 == c2 ) memcpy(dst, src, len)`, but is likely * to be compiled to code using bitwise operation rather than a branch. * - * This function is implemented without using comparison operators, as those - * might be translated to branches by some compilers on some platforms. - * * \param dest The pointer to conditionally copy to. * \param src The pointer to copy from. * \param len The number of bytes to copy. From 7013f62ee57125c3454270d46bac05439c71b3a3 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 16:12:45 +0200 Subject: [PATCH 37/52] Use condition for not sensitive data Signed-off-by: Gabor Mezei --- library/constant_time.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 98fdbd757b..2d9e91de92 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -588,9 +588,8 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, size_t plaintext_size = 0; unsigned output_too_large; - plaintext_max_size = mbedtls_cf_size_if( output_max_len > ilen - 11, - ilen - 11, - output_max_len ); + plaintext_max_size = ( output_max_len > ilen - 11 ) ? ilen - 11 + : output_max_len; /* Check and get padding length in constant time and constant * memory trace. The first byte must be 0. */ From 63bbba5c133356d10ff53c8d9c09900163b9566f Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 16:17:57 +0200 Subject: [PATCH 38/52] Rename and reorder function parameters Signed-off-by: Gabor Mezei --- library/constant_time.c | 36 ++++++++++++++++++------------------ library/constant_time.h | 30 +++++++++++++++--------------- library/rsa.c | 5 ++--- 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 2d9e91de92..db4bdb3282 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -310,19 +310,19 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, dest[i] = ( src[i] & mask ) | ( dest[i] & ~mask ); } -void mbedtls_cf_memcpy_offset( unsigned char *dst, - const unsigned char *src_base, - size_t offset_secret, +void mbedtls_cf_memcpy_offset( unsigned char *dest, + const unsigned char *src, + size_t offset, size_t offset_min, size_t offset_max, size_t len ) { - size_t offset; + size_t offsetval; - for( offset = offset_min; offset <= offset_max; offset++ ) + for( offsetval = offset_min; offsetval <= offset_max; offsetval++ ) { - mbedtls_cf_memcpy_if_eq( dst, src_base + offset, len, - offset, offset_secret ); + mbedtls_cf_memcpy_if_eq( dest, src + offsetval, len, + offsetval, offset ); } } @@ -563,11 +563,11 @@ int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, #if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) -int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, - size_t *olen, +int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, + size_t ilen, unsigned char *output, size_t output_max_len, - unsigned char *buf ) + size_t *olen ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t i, plaintext_max_size; @@ -593,18 +593,18 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, /* Check and get padding length in constant time and constant * memory trace. The first byte must be 0. */ - bad |= buf[0]; + bad |= input[0]; /* Decode EME-PKCS1-v1_5 padding: 0x00 || 0x02 || PS || 0x00 - * where PS must be at least 8 nonzero bytes. */ - bad |= buf[1] ^ MBEDTLS_RSA_CRYPT; + * where PS must be at least 8 nonzero bytes. */ + bad |= input[1] ^ MBEDTLS_RSA_CRYPT; /* Read the whole buffer. Set pad_done to nonzero if we find - * the 0x00 byte and remember the padding length in pad_count. */ + * the 0x00 byte and remember the padding length in pad_count. */ for( i = 2; i < ilen; i++ ) { - pad_done |= ((buf[i] | (unsigned char)-buf[i]) >> 7) ^ 1; + pad_done |= ((input[i] | (unsigned char)-input[i]) >> 7) ^ 1; pad_count += ((pad_done | (unsigned char)-pad_done) >> 7) ^ 1; } @@ -650,7 +650,7 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, * through memory or cache access patterns. */ bad = mbedtls_cf_uint_mask( bad | output_too_large ); for( i = 11; i < ilen; i++ ) - buf[i] &= ~bad; + input[i] &= ~bad; /* If the plaintext is too large, truncate it to the buffer size. * Copy anyway to avoid revealing the length through timing, because @@ -666,7 +666,7 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, * does not depend on the plaintext size. After this move, the * starting location of the plaintext is no longer sensitive * information. */ - mbedtls_cf_mem_move_to_left( buf + ilen - plaintext_max_size, + mbedtls_cf_mem_move_to_left( input + ilen - plaintext_max_size, plaintext_max_size, plaintext_max_size - plaintext_size ); @@ -678,7 +678,7 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, * length, validity of padding, success of the decryption, and other * secrets. */ if( output_max_len != 0 ) - memcpy( output, buf + ilen - plaintext_max_size, plaintext_max_size ); + memcpy( output, input + ilen - plaintext_max_size, plaintext_max_size ); /* Report the amount of data we copied to the output buffer. In case * of errors (bad padding or output too large), the value of *olen diff --git a/library/constant_time.h b/library/constant_time.h index 4810c92ec5..9e101ff826 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -263,21 +263,21 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, * offset_secret, but only on \p offset_min, \p offset_max and \p len. * Functionally equivalent to memcpy(dst, src + offset_secret, len). * - * \param dst The destination buffer. This must point to a writable + * \param dest The destination buffer. This must point to a writable * buffer of at least \p len bytes. - * \param src_base The base of the source buffer. This must point to a + * \param src The base of the source buffer. This must point to a * readable buffer of at least \p offset_max + \p len - * bytes. - * \param offset_secret The offset in the source buffer from which to copy. + * bytes. Shouldn't overlap with \p dest. + * \param offset The offset in the source buffer from which to copy. * This must be no less than \p offset_min and no greater * than \p offset_max. - * \param offset_min The minimal value of \p offset_secret. - * \param offset_max The maximal value of \p offset_secret. + * \param offset_min The minimal value of \p offset. + * \param offset_max The maximal value of \p offset. * \param len The number of bytes to copy. */ -void mbedtls_cf_memcpy_offset( unsigned char *dst, - const unsigned char *src_base, - size_t offset_secret, +void mbedtls_cf_memcpy_offset( unsigned char *dest, + const unsigned char *src, + size_t offset, size_t offset_min, size_t offset_max, size_t len ); @@ -340,21 +340,21 @@ int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, * hold the decryption of the particular ciphertext provided, * the function returns #MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE. * + * \param input The input buffer for the unpadding operation. * \param ilen The length of the ciphertext. - * \param olen The address at which to store the length of - * the plaintext. This must not be \c NULL. * \param output The buffer used to hold the plaintext. This must * be a writable buffer of length \p output_max_len Bytes. * \param output_max_len The length in Bytes of the output buffer \p output. - * \param buf The input buffer for the unpadding operation. + * \param olen The address at which to store the length of + * the plaintext. This must not be \c NULL. * * \return \c 0 on success. * \return An \c MBEDTLS_ERR_RSA_XXX error code on failure. */ -int mbedtls_cf_rsaes_pkcs1_v15_unpadding( size_t ilen, - size_t *olen, +int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, + size_t ilen, unsigned char *output, size_t output_max_len, - unsigned char *buf ); + size_t *olen ); #endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ diff --git a/library/rsa.c b/library/rsa.c index edc8ecccfe..6ac974a50f 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1491,9 +1491,8 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, if( ret != 0 ) goto cleanup; - ret = mbedtls_cf_rsaes_pkcs1_v15_unpadding( ilen, olen, output, - output_max_len, - (unsigned char *) &buf ); + ret = mbedtls_cf_rsaes_pkcs1_v15_unpadding( buf, ilen, + output, output_max_len, olen ); cleanup: mbedtls_platform_zeroize( buf, sizeof( buf ) ); From a316fc8eb0e698e7555e7f4128a0e06da5f650fe Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 16:28:27 +0200 Subject: [PATCH 39/52] Update documentation and comments Signed-off-by: Gabor Mezei --- library/constant_time.c | 6 ++-- library/constant_time.h | 64 ++++++++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index db4bdb3282..4fad1b2d0e 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -228,14 +228,14 @@ int mbedtls_cf_cond_select_sign( unsigned char condition, { /* In order to avoid questions about what we can reasonnably assume about * the representations of signed integers, move everything to unsigned - * by taking advantage of the fact that a and b are either +1 or -1. */ + * by taking advantage of the fact that if1 and if0 are either +1 or -1. */ unsigned uif1 = if1 + 1; unsigned uif0 = if0 + 1; - /* condition was 0 or 1, mask is 0 or 2 as are ua and ub */ + /* condition was 0 or 1, mask is 0 or 2 as are uif1 and uif0 */ const unsigned mask = condition << 1; - /* select ua or ub */ + /* select uif1 or uif0 */ unsigned ur = ( uif0 & ~mask ) | ( uif1 & mask ); /* ur is now 0 or 2, convert back to -1 or +1 */ diff --git a/library/constant_time.h b/library/constant_time.h index 9e101ff826..11a07f312a 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -51,7 +51,7 @@ int mbedtls_cf_memcmp( const void *a, /** Turn a value into a mask: * - if \p value == 0, return the all-bits 0 mask, aka 0 - * - otherwise, return the all-bits 1 mask, aka (size_t) -1 + * - otherwise, return the all-bits 1 mask, aka (unsigned) -1 * * This function can be used to write constant-time code by replacing branches * with bit operations using masks. @@ -79,7 +79,7 @@ size_t mbedtls_cf_size_mask( size_t value ); /** Turn a value into a mask: * - if \p value == 0, return the all-bits 0 mask, aka 0 - * - otherwise, return the all-bits 1 mask, aka (size_t) -1 + * - otherwise, return the all-bits 1 mask, aka (mbedtls_mpi_uint) -1 * * This function can be used to write constant-time code by replacing branches * with bit operations using masks. @@ -188,6 +188,8 @@ size_t mbedtls_cf_size_if( unsigned condition, * This is functionally equivalent to `condition ? if1 : if0` but uses only bit * operations in order to avoid branches. * + * \note if1 and if0 must be either 1 or -1, otherwise the result + * is undefined. * * \param condition Condition to test. * \param if1 The first sign; must be either +1 or -1. @@ -246,7 +248,7 @@ void mbedtls_cf_mem_move_to_left( void *start, * to be compiled to code using bitwise operation rather than a branch. * * \param dest The pointer to conditionally copy to. - * \param src The pointer to copy from. + * \param src The pointer to copy from. Shouldn't overlap with \p dest. * \param len The number of bytes to copy. * \param c1 The first value to analyze in the condition. * \param c2 The second value to analyze in the condition. @@ -261,7 +263,7 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, * This function copies \p len bytes from \p src_base + \p offset_secret to \p * dst, with a code flow and memory access pattern that does not depend on \p * offset_secret, but only on \p offset_min, \p offset_max and \p len. - * Functionally equivalent to memcpy(dst, src + offset_secret, len). + * Functionally equivalent to `memcpy(dst, src + offset_secret, len)`. * * \param dest The destination buffer. This must point to a writable * buffer of at least \p len bytes. @@ -298,23 +300,26 @@ void mbedtls_cf_memcpy_offset( unsigned char *dest, * It is reset using mbedtls_md_hmac_reset() after * the computation is complete to prepare for the * next computation. - * \param add_data The additional data prepended to \p data. This - * must point to a readable buffer of \p add_data_len - * bytes. + * \param add_data The first part of the message whose HMAC is being + * calculated. This must point to a readable buffer + * of \p add_data_len bytes. * \param add_data_len The length of \p add_data in bytes. - * \param data The data appended to \p add_data. This must point - * to a readable buffer of \p max_data_len bytes. + * \param data The buffer containing the second part of the + * message. This must point to a readable buffer + * of \p max_data_len bytes. * \param data_len_secret The length of the data to process in \p data. * This must be no less than \p min_data_len and no * greater than \p max_data_len. - * \param min_data_len The minimal length of \p data in bytes. - * \param max_data_len The maximal length of \p data in bytes. + * \param min_data_len The minimal length of the second part of the + * message, read from /p data. + * \param max_data_len The maximal length of the second part of the + * message, read from /p data. * \param output The HMAC will be written here. This must point to * a writable buffer of sufficient size to hold the * HMAC value. * * \retval 0 on success. - * \retval MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED + * \retval #MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED * The hardware accelerator failed. */ int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, @@ -331,25 +336,30 @@ int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, #if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) /** This function performs the unpadding part of a PKCS#1 v1.5 decryption - * operation (RSAES-PKCS1-v1_5-DECRYPT). + * operation (EME-PKCS1-v1_5 decoding). * - * \note The output buffer length \c output_max_len should be - * as large as the size \p ctx->len of \p ctx->N, for example, - * 128 Bytes if RSA-1024 is used, to be able to hold an - * arbitrary decrypted message. If it is not large enough to - * hold the decryption of the particular ciphertext provided, - * the function returns #MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE. + * \note The return value from this function is a sensitive value + * (this is unusual). #MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE shouldn't happen + * in a well-written application, but 0 vs #MBEDTLS_ERR_RSA_INVALID_PADDING + * is often a situation that an attacker can provoke and leaking which + * one is the result is precisely the information the attacker wants. * - * \param input The input buffer for the unpadding operation. - * \param ilen The length of the ciphertext. - * \param output The buffer used to hold the plaintext. This must - * be a writable buffer of length \p output_max_len Bytes. - * \param output_max_len The length in Bytes of the output buffer \p output. + * \param input The input buffer which is the payload inside PKCS#1v1.5 + * encryption padding, called the "encoded message EM" + * by the terminology. + * \param ilen The length of the payload in the \p input buffer. + * \param output The buffer for the payload, called "message M" by the + * PKCS#1 terminology. This must be a writable buffer of + * length \p output_max_len bytes. * \param olen The address at which to store the length of - * the plaintext. This must not be \c NULL. + * the payload. This must not be \c NULL. + * \param output_max_len The length in bytes of the output buffer \p output. * - * \return \c 0 on success. - * \return An \c MBEDTLS_ERR_RSA_XXX error code on failure. + * \return \c 0 on success. + * \return #MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE + * The output buffer is too small for the unpadded payload. + * \return #MBEDTLS_ERR_RSA_INVALID_PADDING + * The input doesn't contain properly formatted padding. */ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, size_t ilen, From a2d0f90c5a9f94bd28e58b92c8c4914e1be6218d Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 16:35:23 +0200 Subject: [PATCH 40/52] Make functions static These functions are only used as an auxiliary function for constant-time functions. Signed-off-by: Gabor Mezei --- library/constant_time.c | 74 +++++++++++++++++++++++++++++++++++------ library/constant_time.h | 65 ------------------------------------ 2 files changed, 63 insertions(+), 76 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 4fad1b2d0e..6e8a7c2f57 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -167,8 +167,19 @@ unsigned mbedtls_cf_size_bool_eq( size_t x, return( 1 ^ diff1 ); } -unsigned mbedtls_cf_size_gt( size_t x, - size_t y ) +/** Constant-flow "greater than" comparison: + * return x > y + * + * This is equivalent to \p x > \p y, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * \param x The first value to analyze. + * \param y The second value to analyze. + * + * \return 1 if \p x greater than \p y, otherwise 0. + */ +static unsigned mbedtls_cf_size_gt( size_t x, + size_t y ) { /* Return the sign bit (1 for negative) of (y - x). */ return( ( y - x ) >> ( sizeof( size_t ) * 8 - 1 ) ); @@ -214,17 +225,42 @@ unsigned mbedtls_cf_uint_if( unsigned condition, return( ( mask & if1 ) | (~mask & if0 ) ); } -size_t mbedtls_cf_size_if( unsigned condition, - size_t if1, - size_t if0 ) +/** Choose between two integer values without branches. + * + * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * \param condition Condition to test. + * \param if1 Value to use if \p condition is nonzero. + * \param if0 Value to use if \p condition is zero. + * + * \return \c if1 if \p condition is nonzero, otherwise \c if0. + */ +static size_t mbedtls_cf_size_if( unsigned condition, + size_t if1, + size_t if0 ) { size_t mask = mbedtls_cf_size_mask( condition ); return( ( mask & if1 ) | (~mask & if0 ) ); } -int mbedtls_cf_cond_select_sign( unsigned char condition, - int if1, - int if0 ) +/** Select between two sign values witout branches. + * + * This is functionally equivalent to `condition ? if1 : if0` but uses only bit + * operations in order to avoid branches. + * + * \note if1 and if0 must be either 1 or -1, otherwise the result + * is undefined. + * + * \param condition Condition to test. + * \param if1 The first sign; must be either +1 or -1. + * \param if0 The second sign; must be either +1 or -1. + * + * \return \c if1 if \p condition is nonzero, otherwise \c if0. + * */ +static int mbedtls_cf_cond_select_sign( unsigned char condition, + int if1, + int if0 ) { /* In order to avoid questions about what we can reasonnably assume about * the representations of signed integers, move everything to unsigned @@ -271,9 +307,25 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, #endif /* MBEDTLS_BIGNUM_C */ -void mbedtls_cf_mem_move_to_left( void *start, - size_t total, - size_t offset ) +/** Shift some data towards the left inside a buffer. + * + * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally + * equivalent to + * ``` + * memmove(start, start + offset, total - offset); + * memset(start + offset, 0, total - offset); + * ``` + * but it strives to use a memory access pattern (and thus total timing) + * that does not depend on \p offset. This timing independence comes at + * the expense of performance. + * + * \param start Pointer to the start of the buffer. + * \param total Total size of the buffer. + * \param offset Offset from which to copy \p total - \p offset bytes. + */ +static void mbedtls_cf_mem_move_to_left( void *start, + size_t total, + size_t offset ) { volatile unsigned char *buf = start; size_t i, n; diff --git a/library/constant_time.h b/library/constant_time.h index 11a07f312a..6dfac8b3fb 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -122,19 +122,6 @@ size_t mbedtls_cf_size_mask_ge( size_t x, unsigned mbedtls_cf_size_bool_eq( size_t x, size_t y ); -/** Constant-flow "greater than" comparison: - * return x > y - * - * This is equivalent to \p x > \p y, but is likely to be compiled - * to code using bitwise operation rather than a branch. - * - * \param x The first value to analyze. - * \param y The second value to analyze. - * - * \return 1 if \p x greater than \p y, otherwise 0. - */ -unsigned mbedtls_cf_size_gt( size_t x, - size_t y ); #if defined(MBEDTLS_BIGNUM_C) @@ -168,38 +155,6 @@ unsigned mbedtls_cf_uint_if( unsigned condition, unsigned if1, unsigned if0 ); -/** Choose between two integer values without branches. - * - * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled - * to code using bitwise operation rather than a branch. - * - * \param condition Condition to test. - * \param if1 Value to use if \p condition is nonzero. - * \param if0 Value to use if \p condition is zero. - * - * \return \c if1 if \p condition is nonzero, otherwise \c if0. - */ -size_t mbedtls_cf_size_if( unsigned condition, - size_t if1, - size_t if0 ); - -/** Select between two sign values witout branches. - * - * This is functionally equivalent to `condition ? if1 : if0` but uses only bit - * operations in order to avoid branches. - * - * \note if1 and if0 must be either 1 or -1, otherwise the result - * is undefined. - * - * \param condition Condition to test. - * \param if1 The first sign; must be either +1 or -1. - * \param if0 The second sign; must be either +1 or -1. - * - * \return \c if1 if \p condition is nonzero, otherwise \c if0. */ -int mbedtls_cf_cond_select_sign( unsigned char condition, - int if1, - int if0 ); - #if defined(MBEDTLS_BIGNUM_C) /** Conditionally assign a value without branches. @@ -222,26 +177,6 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, #endif /* MBEDTLS_BIGNUM_C */ -/** Shift some data towards the left inside a buffer. - * - * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally - * equivalent to - * ``` - * memmove(start, start + offset, total - offset); - * memset(start + offset, 0, total - offset); - * ``` - * but it strives to use a memory access pattern (and thus total timing) - * that does not depend on \p offset. This timing independence comes at - * the expense of performance. - * - * \param start Pointer to the start of the buffer. - * \param total Total size of the buffer. - * \param offset Offset from which to copy \p total - \p offset bytes. - */ -void mbedtls_cf_mem_move_to_left( void *start, - size_t total, - size_t offset ); - /** Conditional memcpy without branches. * * This is equivalent to `if ( c1 == c2 ) memcpy(dst, src, len)`, but is likely From 949455892fc0ea7966db9dc8b0084ccd203c0a69 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 17:02:29 +0200 Subject: [PATCH 41/52] Remove unused function Signed-off-by: Gabor Mezei --- library/constant_time.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index 6e8a7c2f57..a8d992679e 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -225,24 +225,6 @@ unsigned mbedtls_cf_uint_if( unsigned condition, return( ( mask & if1 ) | (~mask & if0 ) ); } -/** Choose between two integer values without branches. - * - * This is equivalent to `condition ? if1 : if0`, but is likely to be compiled - * to code using bitwise operation rather than a branch. - * - * \param condition Condition to test. - * \param if1 Value to use if \p condition is nonzero. - * \param if0 Value to use if \p condition is zero. - * - * \return \c if1 if \p condition is nonzero, otherwise \c if0. - */ -static size_t mbedtls_cf_size_if( unsigned condition, - size_t if1, - size_t if0 ) -{ - size_t mask = mbedtls_cf_size_mask( condition ); - return( ( mask & if1 ) | (~mask & if0 ) ); -} /** Select between two sign values witout branches. * From e212379810a2b46b6d0baaacd8f44b01ed2f2a38 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 18 Oct 2021 17:05:06 +0200 Subject: [PATCH 42/52] Bind functions' availability for config options Signed-off-by: Gabor Mezei --- library/constant_time.c | 19 +++++++++++++++---- library/constant_time.h | 8 +++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index a8d992679e..5bd74b6f80 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -108,6 +108,8 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ) #endif /* MBEDTLS_BIGNUM_C */ +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) + /** Constant-flow mask generation for "less than" comparison: * - if \p x < \p y, return all-bits 1, that is (size_t) -1 * - otherwise, return all bits 0, that is 0 @@ -141,6 +143,8 @@ size_t mbedtls_cf_size_mask_ge( size_t x, return( ~mbedtls_cf_size_mask_lt( x, y ) ); } +#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ + unsigned mbedtls_cf_size_bool_eq( size_t x, size_t y ) { @@ -167,6 +171,8 @@ unsigned mbedtls_cf_size_bool_eq( size_t x, return( 1 ^ diff1 ); } +#if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) + /** Constant-flow "greater than" comparison: * return x > y * @@ -185,6 +191,8 @@ static unsigned mbedtls_cf_size_gt( size_t x, return( ( y - x ) >> ( sizeof( size_t ) * 8 - 1 ) ); } +#endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ + #if defined(MBEDTLS_BIGNUM_C) unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, @@ -225,6 +233,7 @@ unsigned mbedtls_cf_uint_if( unsigned condition, return( ( mask & if1 ) | (~mask & if0 ) ); } +#if defined(MBEDTLS_BIGNUM_C) /** Select between two sign values witout branches. * @@ -260,8 +269,6 @@ static int mbedtls_cf_cond_select_sign( unsigned char condition, return( (int) ur - 1 ); } -#if defined(MBEDTLS_BIGNUM_C) - void mbedtls_cf_mpi_uint_cond_assign( size_t n, mbedtls_mpi_uint *dest, const mbedtls_mpi_uint *src, @@ -289,6 +296,8 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, #endif /* MBEDTLS_BIGNUM_C */ +#if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) + /** Shift some data towards the left inside a buffer. * * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally @@ -329,6 +338,10 @@ static void mbedtls_cf_mem_move_to_left( void *start, } } +#endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ + +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) + void mbedtls_cf_memcpy_if_eq( unsigned char *dest, const unsigned char *src, size_t len, @@ -360,8 +373,6 @@ void mbedtls_cf_memcpy_offset( unsigned char *dest, } } -#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) - int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, const unsigned char *add_data, size_t add_data_len, diff --git a/library/constant_time.h b/library/constant_time.h index 6dfac8b3fb..a055c6ae60 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -92,6 +92,8 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); #endif /* MBEDTLS_BIGNUM_C */ +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) + /** Constant-flow mask generation for "greater or equal" comparison: * - if \p x >= \p y, return all-bits 1, that is (size_t) -1 * - otherwise, return all bits 0, that is 0 @@ -108,6 +110,8 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); size_t mbedtls_cf_size_mask_ge( size_t x, size_t y ); +#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ + /** Constant-flow boolean "equal" comparison: * return x == y * @@ -122,7 +126,6 @@ size_t mbedtls_cf_size_mask_ge( size_t x, unsigned mbedtls_cf_size_bool_eq( size_t x, size_t y ); - #if defined(MBEDTLS_BIGNUM_C) /** Decide if an integer is less than the other, without branches. @@ -176,6 +179,7 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, #endif /* MBEDTLS_BIGNUM_C */ +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) /** Conditional memcpy without branches. * @@ -219,8 +223,6 @@ void mbedtls_cf_memcpy_offset( unsigned char *dest, size_t offset_max, size_t len ); -#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) - /** Compute the HMAC of variable-length data with constant flow. * * This function computes the HMAC of the concatenation of \p add_data and \p From 291df7bbaba646d88ca578a6ef563218debaff52 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Tue, 19 Oct 2021 11:27:17 +0200 Subject: [PATCH 43/52] Add macro guard for header file Signed-off-by: Gabor Mezei --- library/constant_time.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/constant_time.h b/library/constant_time.h index a055c6ae60..010cfad9db 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -17,6 +17,9 @@ * limitations under the License. */ +#ifndef MBEDTLS_CONSTANT_TIME_INTERNAL_H +#define MBEDTLS_CONSTANT_TIME_INTERNAL_H + #include "common.h" #if defined(MBEDTLS_BIGNUM_C) @@ -305,3 +308,5 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, size_t *olen ); #endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ + +#endif /* MBEDTLS_CONSTANT_TIME_INTERNAL_H */ From 765862c4f37938e7e5ec97044b301820f2b1a3de Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Tue, 19 Oct 2021 12:22:25 +0200 Subject: [PATCH 44/52] Move mbedtls_cf_memcmp to a new public header Signed-off-by: Gabor Mezei --- include/mbedtls/constant_time.h | 47 +++++++++++++++++++++++++++++++++ library/cipher.c | 2 +- library/constant_time.c | 1 + library/constant_time.h | 19 ------------- library/nist_kw.c | 2 +- library/rsa.c | 1 + library/ssl_cli.c | 2 +- library/ssl_cookie.c | 2 +- library/ssl_msg.c | 1 + library/ssl_srv.c | 1 + library/ssl_tls.c | 2 +- 11 files changed, 56 insertions(+), 24 deletions(-) create mode 100644 include/mbedtls/constant_time.h diff --git a/include/mbedtls/constant_time.h b/include/mbedtls/constant_time.h new file mode 100644 index 0000000000..69df954d4b --- /dev/null +++ b/include/mbedtls/constant_time.h @@ -0,0 +1,47 @@ +/** + * Constant-time functions + * + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef MBEDTLS_CONSTANT_TIME_H +#define MBEDTLS_CONSTANT_TIME_H + +#include "common.h" + +#include + + +/** Constant-time buffer comparison without branches. + * + * This is equivalent to the standard memncmp function, but is likely to be + * compiled to code using bitwise operation rather than a branch. + * + * This function can be used to write constant-time code by replacing branches + * with bit operations using masks. + * + * \param a Pointer to the first buffer. + * \param b Pointer to the second buffer. + * \param n The number of bytes to compare in the buffer. + * + * \return Zero if the content of the two buffer is the same, + * otherwise non-zero. + */ +int mbedtls_cf_memcmp( const void *a, + const void *b, + size_t n ); + +#endif /* MBEDTLS_CONSTANT_TIME_H */ diff --git a/library/cipher.c b/library/cipher.c index ce5179c5e7..b48fd6d8a8 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -29,7 +29,7 @@ #include "cipher_wrap.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" -#include "constant_time.h" +#include "mbedtls/constant_time.h" #include #include diff --git a/library/constant_time.c b/library/constant_time.c index 5bd74b6f80..aff30eae86 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -24,6 +24,7 @@ #include "common.h" #include "constant_time.h" +#include "mbedtls/constant_time.h" #include "mbedtls/error.h" #include "mbedtls/platform_util.h" diff --git a/library/constant_time.h b/library/constant_time.h index 010cfad9db..598b0eb0a1 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -33,25 +33,6 @@ #include -/** Constant-time buffer comparison without branches. - * - * This is equivalent to the standard memncmp function, but is likely to be - * compiled to code using bitwise operation rather than a branch. - * - * This function can be used to write constant-time code by replacing branches - * with bit operations using masks. - * - * \param a Pointer to the first buffer. - * \param b Pointer to the second buffer. - * \param n The number of bytes to compare in the buffer. - * - * \return Zero if the content of the two buffer is the same, - * otherwise non-zero. - */ -int mbedtls_cf_memcmp( const void *a, - const void *b, - size_t n ); - /** Turn a value into a mask: * - if \p value == 0, return the all-bits 0 mask, aka 0 * - otherwise, return the all-bits 1 mask, aka (unsigned) -1 diff --git a/library/nist_kw.c b/library/nist_kw.c index b71befd88d..5795441319 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -34,7 +34,7 @@ #include "mbedtls/nist_kw.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" -#include "constant_time.h" +#include "mbedtls/constant_time.h" #include #include diff --git a/library/rsa.c b/library/rsa.c index 6ac974a50f..856a04bf4a 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -45,6 +45,7 @@ #include "mbedtls/platform_util.h" #include "mbedtls/error.h" #include "constant_time.h" +#include "mbedtls/constant_time.h" #include diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 8fd28cf772..ec5e824cf5 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -33,7 +33,7 @@ #include "ssl_misc.h" #include "mbedtls/debug.h" #include "mbedtls/error.h" -#include "constant_time.h" +#include "mbedtls/constant_time.h" #if defined(MBEDTLS_USE_PSA_CRYPTO) #include "mbedtls/psa_util.h" diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c index 6ed3f2be33..cb89c94323 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -36,7 +36,7 @@ #include "ssl_misc.h" #include "mbedtls/error.h" #include "mbedtls/platform_util.h" -#include "constant_time.h" +#include "mbedtls/constant_time.h" #include diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 55be047945..923f2b57ea 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -41,6 +41,7 @@ #include "mbedtls/platform_util.h" #include "mbedtls/version.h" #include "constant_time.h" +#include "mbedtls/constant_time.h" #include diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 989cfe07b4..0066744530 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -35,6 +35,7 @@ #include "mbedtls/error.h" #include "mbedtls/platform_util.h" #include "constant_time.h" +#include "mbedtls/constant_time.h" #include diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d6f038575b..1a1543ea28 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -39,7 +39,7 @@ #include "mbedtls/error.h" #include "mbedtls/platform_util.h" #include "mbedtls/version.h" -#include "constant_time.h" +#include "mbedtls/constant_time.h" #include From 6a426c9f9fff3e099fecb068e5f6a1d5d3dd8b62 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 20 Oct 2021 11:17:43 +0200 Subject: [PATCH 45/52] Bind functions' availability for config options Signed-off-by: Gabor Mezei --- library/constant_time.c | 4 ++++ library/constant_time.h | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/library/constant_time.c b/library/constant_time.c index aff30eae86..20b7f4106f 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -77,6 +77,8 @@ unsigned mbedtls_cf_uint_mask( unsigned value ) #endif } +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) + size_t mbedtls_cf_size_mask( size_t value ) { /* MSVC has a warning about unary minus on unsigned integer types, @@ -91,6 +93,8 @@ size_t mbedtls_cf_size_mask( size_t value ) #endif } +#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ + #if defined(MBEDTLS_BIGNUM_C) mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ) diff --git a/library/constant_time.h b/library/constant_time.h index 598b0eb0a1..5988f52071 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -46,6 +46,8 @@ */ unsigned mbedtls_cf_uint_mask( unsigned value ); +#if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) + /** Turn a value into a mask: * - if \p value == 0, return the all-bits 0 mask, aka 0 * - otherwise, return the all-bits 1 mask, aka (size_t) -1 @@ -59,6 +61,8 @@ unsigned mbedtls_cf_uint_mask( unsigned value ); */ size_t mbedtls_cf_size_mask( size_t value ); +#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ + #if defined(MBEDTLS_BIGNUM_C) /** Turn a value into a mask: From 116cd6a6b4409723999fa85cd2a79de84be76027 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 20 Oct 2021 11:18:37 +0200 Subject: [PATCH 46/52] Fix documentation Signed-off-by: Gabor Mezei --- library/constant_time.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/constant_time.h b/library/constant_time.h index 5988f52071..163d65a7e8 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -236,9 +236,9 @@ void mbedtls_cf_memcpy_offset( unsigned char *dest, * This must be no less than \p min_data_len and no * greater than \p max_data_len. * \param min_data_len The minimal length of the second part of the - * message, read from /p data. + * message, read from \p data. * \param max_data_len The maximal length of the second part of the - * message, read from /p data. + * message, read from \p data. * \param output The HMAC will be written here. This must point to * a writable buffer of sufficient size to hold the * HMAC value. From 53dd04c13ba04e7abd2b0d535997a3e7f5bf7385 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 20 Oct 2021 11:19:16 +0200 Subject: [PATCH 47/52] Remove unneeded include Signed-off-by: Gabor Mezei --- include/mbedtls/constant_time.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/mbedtls/constant_time.h b/include/mbedtls/constant_time.h index 69df954d4b..bd7758fff9 100644 --- a/include/mbedtls/constant_time.h +++ b/include/mbedtls/constant_time.h @@ -20,8 +20,6 @@ #ifndef MBEDTLS_CONSTANT_TIME_H #define MBEDTLS_CONSTANT_TIME_H -#include "common.h" - #include From 90437e37620bb550ce6fbd47565a6f521682513a Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 20 Oct 2021 11:59:27 +0200 Subject: [PATCH 48/52] Rename constant-time functions to have mbedtls_ct prefix Rename functions to better suite with the module name. Signed-off-by: Gabor Mezei --- include/mbedtls/constant_time.h | 2 +- library/bignum.c | 4 +- library/cipher.c | 4 +- library/constant_time.c | 90 ++++++++++++++-------------- library/constant_time.h | 24 ++++---- library/nist_kw.c | 4 +- library/rsa.c | 6 +- library/ssl_cli.c | 4 +- library/ssl_cookie.c | 2 +- library/ssl_msg.c | 24 ++++---- library/ssl_srv.c | 6 +- library/ssl_tls.c | 2 +- tests/suites/test_suite_ssl.function | 6 +- 13 files changed, 89 insertions(+), 89 deletions(-) diff --git a/include/mbedtls/constant_time.h b/include/mbedtls/constant_time.h index bd7758fff9..b7f9a861d1 100644 --- a/include/mbedtls/constant_time.h +++ b/include/mbedtls/constant_time.h @@ -38,7 +38,7 @@ * \return Zero if the content of the two buffer is the same, * otherwise non-zero. */ -int mbedtls_cf_memcmp( const void *a, +int mbedtls_ct_memcmp( const void *a, const void *b, size_t n ); diff --git a/library/bignum.c b/library/bignum.c index ee5f27d706..4ddda58f7a 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1951,7 +1951,7 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi * so d[n] == 1 and we want to set A to the result of the subtraction * which is d - (2^biL)^n, i.e. the n least significant limbs of d. * This exactly corresponds to a conditional assignment. */ - mbedtls_cf_mpi_uint_cond_assign( n, A->p, d, (unsigned char) d[n] ); + mbedtls_ct_mpi_uint_cond_assign( n, A->p, d, (unsigned char) d[n] ); } /* @@ -1993,7 +1993,7 @@ static int mpi_select( mbedtls_mpi *R, const mbedtls_mpi *T, size_t T_size, size for( size_t i = 0; i < T_size; i++ ) { MBEDTLS_MPI_CHK( mbedtls_mpi_safe_cond_assign( R, &T[i], - (unsigned char) mbedtls_cf_size_bool_eq( i, idx ) ) ); + (unsigned char) mbedtls_ct_size_bool_eq( i, idx ) ) ); } cleanup: diff --git a/library/cipher.c b/library/cipher.c index b48fd6d8a8..5078d36339 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -1159,7 +1159,7 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, } /* Check the tag in "constant-time" */ - if( mbedtls_cf_memcmp( tag, check_tag, tag_len ) != 0 ) + if( mbedtls_ct_memcmp( tag, check_tag, tag_len ) != 0 ) return( MBEDTLS_ERR_CIPHER_AUTH_FAILED ); return( 0 ); @@ -1181,7 +1181,7 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, } /* Check the tag in "constant-time" */ - if( mbedtls_cf_memcmp( tag, check_tag, tag_len ) != 0 ) + if( mbedtls_ct_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 20b7f4106f..84ac15c57b 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -42,7 +42,7 @@ #include -int mbedtls_cf_memcmp( const void *a, +int mbedtls_ct_memcmp( const void *a, const void *b, size_t n ) { @@ -63,7 +63,7 @@ int mbedtls_cf_memcmp( const void *a, return( (int)diff ); } -unsigned mbedtls_cf_uint_mask( unsigned value ) +unsigned mbedtls_ct_uint_mask( unsigned value ) { /* MSVC has a warning about unary minus on unsigned, but this is * well-defined and precisely what we want to do here */ @@ -79,7 +79,7 @@ unsigned mbedtls_cf_uint_mask( unsigned value ) #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -size_t mbedtls_cf_size_mask( size_t value ) +size_t mbedtls_ct_size_mask( size_t value ) { /* MSVC has a warning about unary minus on unsigned integer types, * but this is well-defined and precisely what we want to do here. */ @@ -97,7 +97,7 @@ size_t mbedtls_cf_size_mask( size_t value ) #if defined(MBEDTLS_BIGNUM_C) -mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ) +mbedtls_mpi_uint mbedtls_ct_mpi_uint_mask( mbedtls_mpi_uint value ) { /* MSVC has a warning about unary minus on unsigned, but this is * well-defined and precisely what we want to do here */ @@ -127,7 +127,7 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ) * * \return All-bits-one if \p x is less than \p y, otherwise zero. */ -static size_t mbedtls_cf_size_mask_lt( size_t x, +static size_t mbedtls_ct_size_mask_lt( size_t x, size_t y ) { /* This has the most significant bit set if and only if x < y */ @@ -137,20 +137,20 @@ static size_t mbedtls_cf_size_mask_lt( size_t x, const size_t sub1 = sub >> ( sizeof( sub ) * 8 - 1 ); /* mask = (x < y) ? 0xff... : 0x00... */ - const size_t mask = mbedtls_cf_size_mask( sub1 ); + const size_t mask = mbedtls_ct_size_mask( sub1 ); return( mask ); } -size_t mbedtls_cf_size_mask_ge( size_t x, +size_t mbedtls_ct_size_mask_ge( size_t x, size_t y ) { - return( ~mbedtls_cf_size_mask_lt( x, y ) ); + return( ~mbedtls_ct_size_mask_lt( x, y ) ); } #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ -unsigned mbedtls_cf_size_bool_eq( size_t x, +unsigned mbedtls_ct_size_bool_eq( size_t x, size_t y ) { /* diff = 0 if x == y, non-zero otherwise */ @@ -189,7 +189,7 @@ unsigned mbedtls_cf_size_bool_eq( size_t x, * * \return 1 if \p x greater than \p y, otherwise 0. */ -static unsigned mbedtls_cf_size_gt( size_t x, +static unsigned mbedtls_ct_size_gt( size_t x, size_t y ) { /* Return the sign bit (1 for negative) of (y - x). */ @@ -200,7 +200,7 @@ static unsigned mbedtls_cf_size_gt( size_t x, #if defined(MBEDTLS_BIGNUM_C) -unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, +unsigned mbedtls_ct_mpi_uint_lt( const mbedtls_mpi_uint x, const mbedtls_mpi_uint y ) { mbedtls_mpi_uint ret; @@ -230,11 +230,11 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, #endif /* MBEDTLS_BIGNUM_C */ -unsigned mbedtls_cf_uint_if( unsigned condition, +unsigned mbedtls_ct_uint_if( unsigned condition, unsigned if1, unsigned if0 ) { - unsigned mask = mbedtls_cf_uint_mask( condition ); + unsigned mask = mbedtls_ct_uint_mask( condition ); return( ( mask & if1 ) | (~mask & if0 ) ); } @@ -254,7 +254,7 @@ unsigned mbedtls_cf_uint_if( unsigned condition, * * \return \c if1 if \p condition is nonzero, otherwise \c if0. * */ -static int mbedtls_cf_cond_select_sign( unsigned char condition, +static int mbedtls_ct_cond_select_sign( unsigned char condition, int if1, int if0 ) { @@ -274,7 +274,7 @@ static int mbedtls_cf_cond_select_sign( unsigned char condition, return( (int) ur - 1 ); } -void mbedtls_cf_mpi_uint_cond_assign( size_t n, +void mbedtls_ct_mpi_uint_cond_assign( size_t n, mbedtls_mpi_uint *dest, const mbedtls_mpi_uint *src, unsigned char condition ) @@ -305,7 +305,7 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, /** Shift some data towards the left inside a buffer. * - * `mbedtls_cf_mem_move_to_left(start, total, offset)` is functionally + * `mbedtls_ct_mem_move_to_left(start, total, offset)` is functionally * equivalent to * ``` * memmove(start, start + offset, total - offset); @@ -319,7 +319,7 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, * \param total Total size of the buffer. * \param offset Offset from which to copy \p total - \p offset bytes. */ -static void mbedtls_cf_mem_move_to_left( void *start, +static void mbedtls_ct_mem_move_to_left( void *start, size_t total, size_t offset ) { @@ -329,7 +329,7 @@ static void mbedtls_cf_mem_move_to_left( void *start, return; for( i = 0; i < total; i++ ) { - unsigned no_op = mbedtls_cf_size_gt( total - offset, i ); + unsigned no_op = mbedtls_ct_size_gt( total - offset, i ); /* The first `total - offset` passes are a no-op. The last * `offset` passes shift the data one byte to the left and * zero out the last byte. */ @@ -337,9 +337,9 @@ static void mbedtls_cf_mem_move_to_left( void *start, { unsigned char current = buf[n]; unsigned char next = buf[n+1]; - buf[n] = mbedtls_cf_uint_if( no_op, current, next ); + buf[n] = mbedtls_ct_uint_if( no_op, current, next ); } - buf[total-1] = mbedtls_cf_uint_if( no_op, buf[total-1], 0 ); + buf[total-1] = mbedtls_ct_uint_if( no_op, buf[total-1], 0 ); } } @@ -347,22 +347,22 @@ static void mbedtls_cf_mem_move_to_left( void *start, #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) -void mbedtls_cf_memcpy_if_eq( unsigned char *dest, +void mbedtls_ct_memcpy_if_eq( unsigned char *dest, const unsigned char *src, size_t len, size_t c1, size_t c2 ) { /* mask = c1 == c2 ? 0xff : 0x00 */ - const size_t equal = mbedtls_cf_size_bool_eq( c1, c2 ); - const unsigned char mask = (unsigned char) mbedtls_cf_size_mask( equal ); + const size_t equal = mbedtls_ct_size_bool_eq( c1, c2 ); + const unsigned char mask = (unsigned char) mbedtls_ct_size_mask( equal ); /* dest[i] = c1 == c2 ? src[i] : dest[i] */ for( size_t i = 0; i < len; i++ ) dest[i] = ( src[i] & mask ) | ( dest[i] & ~mask ); } -void mbedtls_cf_memcpy_offset( unsigned char *dest, +void mbedtls_ct_memcpy_offset( unsigned char *dest, const unsigned char *src, size_t offset, size_t offset_min, @@ -373,12 +373,12 @@ void mbedtls_cf_memcpy_offset( unsigned char *dest, for( offsetval = offset_min; offsetval <= offset_max; offsetval++ ) { - mbedtls_cf_memcpy_if_eq( dest, src + offsetval, len, + mbedtls_ct_memcpy_if_eq( dest, src + offsetval, len, offsetval, offset ); } } -int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, +int mbedtls_ct_hmac( mbedtls_md_context_t *ctx, const unsigned char *add_data, size_t add_data_len, const unsigned char *data, @@ -436,7 +436,7 @@ int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, MD_CHK( mbedtls_md_clone( &aux, ctx ) ); MD_CHK( mbedtls_md_finish( &aux, aux_out ) ); /* Keep only the correct inner_hash in the output buffer */ - mbedtls_cf_memcpy_if_eq( output, aux_out, hash_size, + mbedtls_ct_memcpy_if_eq( output, aux_out, hash_size, offset, data_len_secret ); if( offset < max_data_len ) @@ -485,13 +485,13 @@ int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, MPI_VALIDATE_RET( Y != NULL ); /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */ - limb_mask = mbedtls_cf_mpi_uint_mask( assign );; + limb_mask = mbedtls_ct_mpi_uint_mask( assign );; MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); - X->s = mbedtls_cf_cond_select_sign( assign, Y->s, X->s ); + X->s = mbedtls_ct_cond_select_sign( assign, Y->s, X->s ); - mbedtls_cf_mpi_uint_cond_assign( Y->n, X->p, Y->p, assign ); + mbedtls_ct_mpi_uint_cond_assign( Y->n, X->p, Y->p, assign ); for( i = Y->n; i < X->n; i++ ) X->p[i] &= ~limb_mask; @@ -521,14 +521,14 @@ int mbedtls_mpi_safe_cond_swap( mbedtls_mpi *X, return( 0 ); /* all-bits 1 if swap is 1, all-bits 0 if swap is 0 */ - limb_mask = mbedtls_cf_mpi_uint_mask( swap ); + limb_mask = mbedtls_ct_mpi_uint_mask( swap ); MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_grow( Y, X->n ) ); s = X->s; - X->s = mbedtls_cf_cond_select_sign( swap, Y->s, X->s ); - Y->s = mbedtls_cf_cond_select_sign( swap, s, Y->s ); + X->s = mbedtls_ct_cond_select_sign( swap, Y->s, X->s ); + Y->s = mbedtls_ct_cond_select_sign( swap, s, Y->s ); for( i = 0; i < X->n; i++ ) @@ -590,7 +590,7 @@ int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, * Again even if we can make a decision, we just mark the result and * the fact that we are done and continue looping. */ - cond = mbedtls_cf_mpi_uint_lt( Y->p[i - 1], X->p[i - 1] ); + cond = mbedtls_ct_mpi_uint_lt( Y->p[i - 1], X->p[i - 1] ); *ret |= cond & ( 1 - done ) & X_is_negative; done |= cond; @@ -601,7 +601,7 @@ int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, * Again even if we can make a decision, we just mark the result and * the fact that we are done and continue looping. */ - cond = mbedtls_cf_mpi_uint_lt( X->p[i - 1], Y->p[i - 1] ); + cond = mbedtls_ct_mpi_uint_lt( X->p[i - 1], Y->p[i - 1] ); *ret |= cond & ( 1 - done ) & ( 1 - X_is_negative ); done |= cond; } @@ -613,7 +613,7 @@ int mbedtls_mpi_lt_mpi_ct( const mbedtls_mpi *X, #if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) -int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, +int mbedtls_ct_rsaes_pkcs1_v15_unpadding( unsigned char *input, size_t ilen, unsigned char *output, size_t output_max_len, @@ -660,10 +660,10 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, /* If pad_done is still zero, there's no data, only unfinished padding. */ - bad |= mbedtls_cf_uint_if( pad_done, 0, 1 ); + bad |= mbedtls_ct_uint_if( pad_done, 0, 1 ); /* There must be at least 8 bytes of padding. */ - bad |= mbedtls_cf_size_gt( 8, pad_count ); + bad |= mbedtls_ct_size_gt( 8, pad_count ); /* If the padding is valid, set plaintext_size to the number of * remaining bytes after stripping the padding. If the padding @@ -672,13 +672,13 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, * buffer. Do it without branches to avoid leaking the padding * validity through timing. RSA keys are small enough that all the * size_t values involved fit in unsigned int. */ - plaintext_size = mbedtls_cf_uint_if( + plaintext_size = mbedtls_ct_uint_if( bad, (unsigned) plaintext_max_size, (unsigned) ( ilen - pad_count - 3 ) ); /* Set output_too_large to 0 if the plaintext fits in the output * buffer and to 1 otherwise. */ - output_too_large = mbedtls_cf_size_gt( plaintext_size, + output_too_large = mbedtls_ct_size_gt( plaintext_size, plaintext_max_size ); /* Set ret without branches to avoid timing attacks. Return: @@ -686,9 +686,9 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, * - OUTPUT_TOO_LARGE if the padding is good but the decrypted * plaintext does not fit in the output buffer. * - 0 if the padding is correct. */ - ret = - (int) mbedtls_cf_uint_if( + ret = - (int) mbedtls_ct_uint_if( bad, - MBEDTLS_ERR_RSA_INVALID_PADDING, - mbedtls_cf_uint_if( output_too_large, + mbedtls_ct_uint_if( output_too_large, - MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, 0 ) ); @@ -698,7 +698,7 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, * from the same buffer whether the padding is good or not to * avoid leaking the padding validity through overall timing or * through memory or cache access patterns. */ - bad = mbedtls_cf_uint_mask( bad | output_too_large ); + bad = mbedtls_ct_uint_mask( bad | output_too_large ); for( i = 11; i < ilen; i++ ) input[i] &= ~bad; @@ -706,7 +706,7 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, * Copy anyway to avoid revealing the length through timing, because * revealing the length is as bad as revealing the padding validity * for a Bleichenbacher attack. */ - plaintext_size = mbedtls_cf_uint_if( output_too_large, + plaintext_size = mbedtls_ct_uint_if( output_too_large, (unsigned) plaintext_max_size, (unsigned) plaintext_size ); @@ -716,7 +716,7 @@ int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, * does not depend on the plaintext size. After this move, the * starting location of the plaintext is no longer sensitive * information. */ - mbedtls_cf_mem_move_to_left( input + ilen - plaintext_max_size, + mbedtls_ct_mem_move_to_left( input + ilen - plaintext_max_size, plaintext_max_size, plaintext_max_size - plaintext_size ); diff --git a/library/constant_time.h b/library/constant_time.h index 163d65a7e8..bb1f3536a9 100644 --- a/library/constant_time.h +++ b/library/constant_time.h @@ -44,7 +44,7 @@ * * \return Zero if \p value is zero, otherwise all-bits-one. */ -unsigned mbedtls_cf_uint_mask( unsigned value ); +unsigned mbedtls_ct_uint_mask( unsigned value ); #if defined(MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC) @@ -59,7 +59,7 @@ unsigned mbedtls_cf_uint_mask( unsigned value ); * * \return Zero if \p value is zero, otherwise all-bits-one. */ -size_t mbedtls_cf_size_mask( size_t value ); +size_t mbedtls_ct_size_mask( size_t value ); #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ @@ -76,7 +76,7 @@ size_t mbedtls_cf_size_mask( size_t value ); * * \return Zero if \p value is zero, otherwise all-bits-one. */ -mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); +mbedtls_mpi_uint mbedtls_ct_mpi_uint_mask( mbedtls_mpi_uint value ); #endif /* MBEDTLS_BIGNUM_C */ @@ -95,7 +95,7 @@ mbedtls_mpi_uint mbedtls_cf_mpi_uint_mask( mbedtls_mpi_uint value ); * \return All-bits-one if \p x is greater or equal than \p y, * otherwise zero. */ -size_t mbedtls_cf_size_mask_ge( size_t x, +size_t mbedtls_ct_size_mask_ge( size_t x, size_t y ); #endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */ @@ -111,7 +111,7 @@ size_t mbedtls_cf_size_mask_ge( size_t x, * * \return 1 if \p x equals to \p y, otherwise 0. */ -unsigned mbedtls_cf_size_bool_eq( size_t x, +unsigned mbedtls_ct_size_bool_eq( size_t x, size_t y ); #if defined(MBEDTLS_BIGNUM_C) @@ -126,7 +126,7 @@ unsigned mbedtls_cf_size_bool_eq( size_t x, * * \return 1 if \p x is less than \p y, otherwise 0. */ -unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, +unsigned mbedtls_ct_mpi_uint_lt( const mbedtls_mpi_uint x, const mbedtls_mpi_uint y ); #endif /* MBEDTLS_BIGNUM_C */ @@ -142,7 +142,7 @@ unsigned mbedtls_cf_mpi_uint_lt( const mbedtls_mpi_uint x, * * \return \c if1 if \p condition is nonzero, otherwise \c if0. */ -unsigned mbedtls_cf_uint_if( unsigned condition, +unsigned mbedtls_ct_uint_if( unsigned condition, unsigned if1, unsigned if0 ); @@ -160,7 +160,7 @@ unsigned mbedtls_cf_uint_if( unsigned condition, * initialized MPI. * \param condition Condition to test, must be 0 or 1. */ -void mbedtls_cf_mpi_uint_cond_assign( size_t n, +void mbedtls_ct_mpi_uint_cond_assign( size_t n, mbedtls_mpi_uint *dest, const mbedtls_mpi_uint *src, unsigned char condition ); @@ -180,7 +180,7 @@ void mbedtls_cf_mpi_uint_cond_assign( size_t n, * \param c1 The first value to analyze in the condition. * \param c2 The second value to analyze in the condition. */ -void mbedtls_cf_memcpy_if_eq( unsigned char *dest, +void mbedtls_ct_memcpy_if_eq( unsigned char *dest, const unsigned char *src, size_t len, size_t c1, size_t c2 ); @@ -204,7 +204,7 @@ void mbedtls_cf_memcpy_if_eq( unsigned char *dest, * \param offset_max The maximal value of \p offset. * \param len The number of bytes to copy. */ -void mbedtls_cf_memcpy_offset( unsigned char *dest, +void mbedtls_ct_memcpy_offset( unsigned char *dest, const unsigned char *src, size_t offset, size_t offset_min, @@ -247,7 +247,7 @@ void mbedtls_cf_memcpy_offset( unsigned char *dest, * \retval #MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED * The hardware accelerator failed. */ -int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, +int mbedtls_ct_hmac( mbedtls_md_context_t *ctx, const unsigned char *add_data, size_t add_data_len, const unsigned char *data, @@ -286,7 +286,7 @@ int mbedtls_cf_hmac( mbedtls_md_context_t *ctx, * \return #MBEDTLS_ERR_RSA_INVALID_PADDING * The input doesn't contain properly formatted padding. */ -int mbedtls_cf_rsaes_pkcs1_v15_unpadding( unsigned char *input, +int mbedtls_ct_rsaes_pkcs1_v15_unpadding( unsigned char *input, size_t ilen, unsigned char *output, size_t output_max_len, diff --git a/library/nist_kw.c b/library/nist_kw.c index 5795441319..b9f45531c0 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -399,7 +399,7 @@ int mbedtls_nist_kw_unwrap( mbedtls_nist_kw_context *ctx, goto cleanup; /* Check ICV in "constant-time" */ - diff = mbedtls_cf_memcmp( NIST_KW_ICV1, A, KW_SEMIBLOCK_LENGTH ); + diff = mbedtls_ct_memcmp( NIST_KW_ICV1, A, KW_SEMIBLOCK_LENGTH ); if( diff != 0 ) { @@ -448,7 +448,7 @@ int mbedtls_nist_kw_unwrap( mbedtls_nist_kw_context *ctx, } /* Check ICV in "constant-time" */ - diff = mbedtls_cf_memcmp( NIST_KW_ICV2, A, KW_SEMIBLOCK_LENGTH / 2 ); + diff = mbedtls_ct_memcmp( NIST_KW_ICV2, A, KW_SEMIBLOCK_LENGTH / 2 ); if( diff != 0 ) { diff --git a/library/rsa.c b/library/rsa.c index 856a04bf4a..d839d45978 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1492,7 +1492,7 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, if( ret != 0 ) goto cleanup; - ret = mbedtls_cf_rsaes_pkcs1_v15_unpadding( buf, ilen, + ret = mbedtls_ct_rsaes_pkcs1_v15_unpadding( buf, ilen, output, output_max_len, olen ); cleanup: @@ -1887,7 +1887,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_cf_memcmp( verif, sig, ctx->len ) != 0 ) + if( mbedtls_ct_memcmp( verif, sig, ctx->len ) != 0 ) { ret = MBEDTLS_ERR_RSA_PRIVATE_FAILED; goto cleanup; @@ -2159,7 +2159,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_verify( mbedtls_rsa_context *ctx, * Compare */ - if( ( ret = mbedtls_cf_memcmp( encoded, encoded_expected, + if( ( ret = mbedtls_ct_memcmp( encoded, encoded_expected, sig_len ) ) != 0 ) { ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; diff --git a/library/ssl_cli.c b/library/ssl_cli.c index ec5e824cf5..d14b90e278 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1381,9 +1381,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_cf_memcmp( buf + 1, + mbedtls_ct_memcmp( buf + 1, ssl->own_verify_data, ssl->verify_data_len ) != 0 || - mbedtls_cf_memcmp( buf + 1 + ssl->verify_data_len, + mbedtls_ct_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 cb89c94323..dd46971c98 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -227,7 +227,7 @@ int mbedtls_ssl_cookie_check( void *p_ctx, if( ret != 0 ) return( ret ); - if( mbedtls_cf_memcmp( cookie + 4, ref_hmac, sizeof( ref_hmac ) ) != 0 ) + if( mbedtls_ct_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 923f2b57ea..c14634fa36 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -1173,7 +1173,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_cf_memcmp() below. + * mbedtls_ct_memcmp() below. * * Further, we still know that data_len > minlen */ rec->data_len -= transform->maclen; @@ -1196,7 +1196,7 @@ 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_cf_memcmp( data + rec->data_len, mac_expect, + if( mbedtls_ct_memcmp( data + rec->data_len, mac_expect, transform->maclen ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); @@ -1258,7 +1258,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, if( auth_done == 1 ) { - const size_t mask = mbedtls_cf_size_mask_ge( + const size_t mask = mbedtls_ct_size_mask_ge( rec->data_len, padlen + 1 ); correct &= mask; @@ -1278,7 +1278,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, } #endif - const size_t mask = mbedtls_cf_size_mask_ge( + const size_t mask = mbedtls_ct_size_mask_ge( rec->data_len, transform->maclen + padlen + 1 ); correct &= mask; @@ -1312,18 +1312,18 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, /* pad_count += (idx >= padding_idx) && * (check[idx] == padlen - 1); */ - const size_t mask = mbedtls_cf_size_mask_ge( idx, padding_idx ); - const size_t equal = mbedtls_cf_size_bool_eq( check[idx], + const size_t mask = mbedtls_ct_size_mask_ge( idx, padding_idx ); + const size_t equal = mbedtls_ct_size_bool_eq( check[idx], padlen - 1 ); pad_count += mask & equal; } - correct &= mbedtls_cf_size_bool_eq( pad_count, padlen ); + correct &= mbedtls_ct_size_bool_eq( pad_count, padlen ); #if defined(MBEDTLS_SSL_DEBUG_ALL) if( padlen > 0 && correct == 0 ) MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad padding byte detected" ) ); #endif - padlen &= mbedtls_cf_size_mask( correct ); + padlen &= mbedtls_ct_size_mask( correct ); #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ @@ -1386,17 +1386,17 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, const size_t max_len = rec->data_len + padlen; const size_t min_len = ( max_len > 256 ) ? max_len - 256 : 0; - ret = mbedtls_cf_hmac( &transform->md_ctx_dec, + ret = mbedtls_ct_hmac( &transform->md_ctx_dec, add_data, add_data_len, data, rec->data_len, min_len, max_len, mac_expect ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cf_hmac", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ct_hmac", ret ); return( ret ); } - mbedtls_cf_memcpy_offset( mac_peer, data, + mbedtls_ct_memcpy_offset( mac_peer, data, rec->data_len, min_len, max_len, transform->maclen ); @@ -1407,7 +1407,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, MBEDTLS_SSL_DEBUG_BUF( 4, "message mac", mac_peer, transform->maclen ); #endif - if( mbedtls_cf_memcmp( mac_peer, mac_expect, + if( mbedtls_ct_memcmp( mac_peer, mac_expect, transform->maclen ) != 0 ) { #if defined(MBEDTLS_SSL_DEBUG_ALL) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 0066744530..ab5cdea2bb 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -198,7 +198,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_cf_memcmp( buf + 1, ssl->peer_verify_data, + mbedtls_ct_memcmp( buf + 1, ssl->peer_verify_data, ssl->verify_data_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching renegotiation info" ) ); @@ -3582,7 +3582,7 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, diff |= peer_pms[1] ^ ver[1]; /* mask = diff ? 0xff : 0x00 using bit operations to avoid branches */ - mask = mbedtls_cf_uint_mask( diff ); + mask = mbedtls_ct_uint_mask( diff ); /* * Protection against Bleichenbacher's attack: invalid PKCS#1 v1.5 padding @@ -3665,7 +3665,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_cf_memcmp( ssl->conf->psk_identity, *p, n ) != 0 ) + mbedtls_ct_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 1a1543ea28..61785294a7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2915,7 +2915,7 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } - if( mbedtls_cf_memcmp( ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ), + if( mbedtls_ct_memcmp( ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ), buf, hash_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 5623a1c2d6..6b3658fcf1 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -4555,7 +4555,7 @@ void resize_buffers_renegotiate_mfl( int mfl, int legacy_renegotiation, void ssl_cf_hmac( int hash ) { /* - * Test the function mbedtls_cf_hmac() against a reference + * Test the function mbedtls_ct_hmac() against a reference * implementation. */ mbedtls_md_context_t ctx, ref_ctx; @@ -4614,7 +4614,7 @@ void ssl_cf_hmac( int hash ) /* Get the function's result */ TEST_CF_SECRET( &in_len, sizeof( in_len ) ); - TEST_EQUAL( 0, mbedtls_cf_hmac( &ctx, add_data, sizeof( add_data ), + TEST_EQUAL( 0, mbedtls_ct_hmac( &ctx, add_data, sizeof( add_data ), data, in_len, min_in_len, max_in_len, out ) ); @@ -4664,7 +4664,7 @@ void ssl_cf_memcpy_offset( int offset_min, int offset_max, int len ) mbedtls_test_set_step( (int) secret ); TEST_CF_SECRET( &secret, sizeof( secret ) ); - mbedtls_cf_memcpy_offset( dst, src, secret, + mbedtls_ct_memcpy_offset( dst, src, secret, offset_min, offset_max, len ); TEST_CF_PUBLIC( &secret, sizeof( secret ) ); TEST_CF_PUBLIC( dst, len ); From 22c9a6fccc3a569387c82c3d6cde4446c62c2af9 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 20 Oct 2021 12:09:35 +0200 Subject: [PATCH 49/52] Rename internal header constant_time.h to constant_time_internal.h Signed-off-by: Gabor Mezei --- library/bignum.c | 2 +- library/constant_time.c | 2 +- library/{constant_time.h => constant_time_internal.h} | 0 library/rsa.c | 2 +- library/ssl_msg.c | 2 +- library/ssl_srv.c | 2 +- tests/suites/test_suite_ssl.function | 2 +- 7 files changed, 6 insertions(+), 6 deletions(-) rename library/{constant_time.h => constant_time_internal.h} (100%) diff --git a/library/bignum.c b/library/bignum.c index 4ddda58f7a..fdf8c9a42c 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -41,7 +41,7 @@ #include "bn_mul.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" -#include "constant_time.h" +#include "constant_time_internal.h" #include diff --git a/library/constant_time.c b/library/constant_time.c index 84ac15c57b..f71f7b942c 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -23,7 +23,7 @@ */ #include "common.h" -#include "constant_time.h" +#include "constant_time_internal.h" #include "mbedtls/constant_time.h" #include "mbedtls/error.h" #include "mbedtls/platform_util.h" diff --git a/library/constant_time.h b/library/constant_time_internal.h similarity index 100% rename from library/constant_time.h rename to library/constant_time_internal.h diff --git a/library/rsa.c b/library/rsa.c index d839d45978..e3ec0568e7 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -44,7 +44,7 @@ #include "mbedtls/oid.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" -#include "constant_time.h" +#include "constant_time_internal.h" #include "mbedtls/constant_time.h" #include diff --git a/library/ssl_msg.c b/library/ssl_msg.c index c14634fa36..299be32782 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -40,7 +40,7 @@ #include "mbedtls/error.h" #include "mbedtls/platform_util.h" #include "mbedtls/version.h" -#include "constant_time.h" +#include "constant_time_internal.h" #include "mbedtls/constant_time.h" #include diff --git a/library/ssl_srv.c b/library/ssl_srv.c index ab5cdea2bb..2d2514e21d 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -34,7 +34,7 @@ #include "mbedtls/debug.h" #include "mbedtls/error.h" #include "mbedtls/platform_util.h" -#include "constant_time.h" +#include "constant_time_internal.h" #include "mbedtls/constant_time.h" #include diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 6b3658fcf1..8ac2af1819 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -10,7 +10,7 @@ #include -#include +#include #include From 642eeb28796e2226644ad51dd05096683fe8a7bf Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 3 Nov 2021 16:13:32 +0100 Subject: [PATCH 50/52] Fix documentation and comments Signed-off-by: Gabor Mezei --- include/mbedtls/constant_time.h | 2 +- library/constant_time.c | 6 +++--- library/constant_time_internal.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/constant_time.h b/include/mbedtls/constant_time.h index b7f9a861d1..c5de57a01f 100644 --- a/include/mbedtls/constant_time.h +++ b/include/mbedtls/constant_time.h @@ -25,7 +25,7 @@ /** Constant-time buffer comparison without branches. * - * This is equivalent to the standard memncmp function, but is likely to be + * This is equivalent to the standard memcmp function, but is likely to be * compiled to code using bitwise operation rather than a branch. * * This function can be used to write constant-time code by replacing branches diff --git a/library/constant_time.c b/library/constant_time.c index f71f7b942c..9bb275cf5c 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -18,7 +18,7 @@ */ /* - * The following functiona are implemented without using comparison operators, as those + * The following functions are implemented without using comparison operators, as those * might be translated to branches by some compilers on some platforms. */ @@ -240,7 +240,7 @@ unsigned mbedtls_ct_uint_if( unsigned condition, #if defined(MBEDTLS_BIGNUM_C) -/** Select between two sign values witout branches. +/** Select between two sign values without branches. * * This is functionally equivalent to `condition ? if1 : if0` but uses only bit * operations in order to avoid branches. @@ -258,7 +258,7 @@ static int mbedtls_ct_cond_select_sign( unsigned char condition, int if1, int if0 ) { - /* In order to avoid questions about what we can reasonnably assume about + /* In order to avoid questions about what we can reasonably assume about * the representations of signed integers, move everything to unsigned * by taking advantage of the fact that if1 and if0 are either +1 or -1. */ unsigned uif1 = if1 + 1; diff --git a/library/constant_time_internal.h b/library/constant_time_internal.h index bb1f3536a9..ac18b56a9a 100644 --- a/library/constant_time_internal.h +++ b/library/constant_time_internal.h @@ -171,7 +171,7 @@ void mbedtls_ct_mpi_uint_cond_assign( size_t n, /** Conditional memcpy without branches. * - * This is equivalent to `if ( c1 == c2 ) memcpy(dst, src, len)`, but is likely + * This is equivalent to `if ( c1 == c2 ) memcpy(dest, src, len)`, but is likely * to be compiled to code using bitwise operation rather than a branch. * * \param dest The pointer to conditionally copy to. From 77390dc8ecd07dbf83ef29f15f06d87cd7b7af02 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 3 Nov 2021 17:12:56 +0100 Subject: [PATCH 51/52] Update changelog with the new public API Signed-off-by: Gabor Mezei --- ChangeLog.d/constant_time_module.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/constant_time_module.txt b/ChangeLog.d/constant_time_module.txt index 5e76deaa8d..ebb0b7fb96 100644 --- a/ChangeLog.d/constant_time_module.txt +++ b/ChangeLog.d/constant_time_module.txt @@ -1,6 +1,10 @@ Changes - * The mbedcrypto library includes new a source code module constant_time.c, + * The mbedcrypto library includes a new source code module constant_time.c, containing various functions meant to resist timing side channel attacks. This module does not have a separate configuration option, and functions from this module will be included in the build as required. Currently - the interface of this module is private and may change at any time. + most of the interface of this module is private and may change at any + time. + +Features + * Add new API mbedtls_ct_memcmp for constant time buffer comparison. From 685472bfb64d05bc0f6f9f866b671074c6789430 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 24 Nov 2021 11:17:36 +0100 Subject: [PATCH 52/52] Update function name Signed-off-by: Gabor Mezei --- library/ssl_tls13_generic.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 12ef4d58c8..53810b81ad 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -29,6 +29,7 @@ #include "mbedtls/debug.h" #include "mbedtls/oid.h" #include "mbedtls/platform.h" +#include "mbedtls/constant_time.h" #include #include "ssl_misc.h" @@ -903,9 +904,9 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, expected_verify_data_len ); /* Semantic validation */ - if( mbedtls_ssl_safer_memcmp( buf, - expected_verify_data, - expected_verify_data_len ) != 0 ) + if( mbedtls_ct_memcmp( buf, + expected_verify_data, + expected_verify_data_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) );