From 4c63b98e94477feee2f2f60c102b183c17d02e71 Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Mon, 2 Dec 2019 15:01:41 +0200 Subject: [PATCH 1/9] Add random delay function to platform_utils Add delay function to platform_utils. The function will delay program execution by incrementing local variable randomised number of times. --- include/mbedtls/platform_util.h | 15 +++++++++++++++ library/platform_util.c | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index e20f1c32e..d6ee886de 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -238,6 +238,21 @@ int mbedtls_platform_memcmp( const void *buf1, const void *buf2, size_t num ); */ uint32_t mbedtls_platform_random_in_range( size_t num ); +/** + * \brief Random delay function. + * + * Function implements random delay by incrementing local variable + * randomized number of times. + * + * \note Currently the function is dependent of hardware providing an + * rng with MBEDTLS_ENTROPY_HARDWARE_ALT. + * + * \param num Max-value for the local variable increments. + * + * \return In success number of increments, -1 in case of errors. + */ +int mbedtls_platform_random_delay( size_t num ); + /** * \brief This function does nothing, but can be inserted between * successive reads to a volatile local variable to prevent diff --git a/library/platform_util.c b/library/platform_util.c index 16867aad6..5a6fc206e 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -165,6 +165,29 @@ uint32_t mbedtls_platform_random_in_range( size_t num ) #endif } +int mbedtls_platform_random_delay( size_t max_rand ) +{ +#if !defined(MBEDTLS_ENTROPY_HARDWARE_ALT) + (void) max_rand; + return -1; +#else + size_t random_number; + volatile size_t i = 0; + if( max_rand == 0 || max_rand > INT_MAX ) + { + return -1; + } + + random_number = mbedtls_platform_random_in_range( max_rand ); + + do { + i++; + } while ( i < random_number ); + + return (int) i; +#endif /* !MBEDTLS_ENTROPY_HARDWARE_ALT */ +} + /* Some compilers (armcc 5 for example) optimize away successive reads from a * volatile local variable (which we use as a counter-measure to fault * injection attacks), unless there is a call to an external function between From b47b10583890dc2881335e3dc07968926c43bba7 Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Thu, 5 Dec 2019 17:32:05 +0200 Subject: [PATCH 2/9] Follow Mbed TLS coding style --- library/platform_util.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index 5a6fc206e..6384d29e7 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -175,16 +175,17 @@ int mbedtls_platform_random_delay( size_t max_rand ) volatile size_t i = 0; if( max_rand == 0 || max_rand > INT_MAX ) { - return -1; + return( -1 ); } random_number = mbedtls_platform_random_in_range( max_rand ); - do { + do + { i++; - } while ( i < random_number ); + } while( i < random_number ); - return (int) i; + return( (int)i ); #endif /* !MBEDTLS_ENTROPY_HARDWARE_ALT */ } From 9a506e7424941fcd822389444b52b9db5d5ed7e5 Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Mon, 9 Dec 2019 10:54:03 +0200 Subject: [PATCH 3/9] Update comments of mbedtls_platform_random_delay --- include/mbedtls/platform_util.h | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index d6ee886de..96c96a374 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -241,15 +241,21 @@ uint32_t mbedtls_platform_random_in_range( size_t num ); /** * \brief Random delay function. * - * Function implements random delay by incrementing local variable - * randomized number of times. + * Function implements a random delay by incrementing a local + * variable randomized number of times (busy-looping). + * + * Duration of the delay is random as number of variable increments + * is randomized. * * \note Currently the function is dependent of hardware providing an * rng with MBEDTLS_ENTROPY_HARDWARE_ALT. * - * \param num Max-value for the local variable increments. + * \param num Max-value for the number of local variable increments, must be + * less than INT_MAX. Total number of variable increment will be + * randomized between 1 and num. * - * \return In success number of increments, -1 in case of errors. + * \return In success number of increments made. + * \return Negative value in case of errors. */ int mbedtls_platform_random_delay( size_t num ); From 0490485be55395fb5a485cdca9b6c9b20be8d09a Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Mon, 9 Dec 2019 14:39:51 +0200 Subject: [PATCH 4/9] Add random delay to enforce_volatile_reads Add a random delay to mbedtls_platform_enforce_volatile_reads() as a countermeasure to fault injection attacks. --- include/mbedtls/platform_util.h | 9 ++++++--- library/platform_util.c | 7 ++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 96c96a374..3bad5598a 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -260,9 +260,12 @@ uint32_t mbedtls_platform_random_in_range( size_t num ); int mbedtls_platform_random_delay( size_t num ); /** - * \brief This function does nothing, but can be inserted between - * successive reads to a volatile local variable to prevent - * compilers from optimizing them away. + * \brief This function can be inserted between successive reads to a + * volatile local variable to prevent compilers from optimizing + * them away. In addition, this function will spent a small random + * time in a busy loop as a counter-measure to fault injection + * attack. + * */ void mbedtls_platform_enforce_volatile_reads( void ); diff --git a/library/platform_util.c b/library/platform_util.c index 6384d29e7..acb0ee651 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -189,13 +189,10 @@ int mbedtls_platform_random_delay( size_t max_rand ) #endif /* !MBEDTLS_ENTROPY_HARDWARE_ALT */ } -/* Some compilers (armcc 5 for example) optimize away successive reads from a - * volatile local variable (which we use as a counter-measure to fault - * injection attacks), unless there is a call to an external function between - * them. This functions doesn't need to do anything, it just needs to be - * in another compilation unit. So here's a function that does nothing. */ void mbedtls_platform_enforce_volatile_reads( void ) { + // Add a small random delay as a counter-measure to fault injection attack. + mbedtls_platform_random_delay( 50 ); } #if defined(MBEDTLS_HAVE_TIME_DATE) && !defined(MBEDTLS_PLATFORM_GMTIME_R_ALT) From dbf2b43ceb9e6274905c67f51f8507f5dc0e0987 Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Mon, 30 Dec 2019 12:55:30 +0200 Subject: [PATCH 5/9] Add more variation to random delay countermeasure Add more variation to the random delay function by xor:ing two variables. It is not enough to increment just a counter to create a delay as it will be visible as uniform delay that can be easily removed from the trace by analysis. --- library/platform_util.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index acb0ee651..691af7100 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -171,19 +171,28 @@ int mbedtls_platform_random_delay( size_t max_rand ) (void) max_rand; return -1; #else - size_t random_number; + size_t rn_1, rn_2, rn_3; volatile size_t i = 0; + uint8_t shift; if( max_rand == 0 || max_rand > INT_MAX ) { return( -1 ); } - random_number = mbedtls_platform_random_in_range( max_rand ); + rn_1 = mbedtls_platform_random_in_range( max_rand ); + rn_2 = mbedtls_platform_random_in_range( 0xffffffff ) + 1; + rn_3 = mbedtls_platform_random_in_range( 0xffffffff ) + 1; do { i++; - } while( i < random_number ); + shift = rn_2 & 0x07; + if ( i % 2 ) + rn_2 = (uint32_t)( rn_2 >> shift | rn_2 << ( 32 - shift ) ); + else + rn_3 = (uint32_t)( rn_3 << shift | rn_3 >> ( 32 - shift ) ); + rn_2 ^= rn_3; + } while( i < rn_1 || rn_2 == 0 || rn_3 == 0 ); return( (int)i ); #endif /* !MBEDTLS_ENTROPY_HARDWARE_ALT */ From e91f0dc905dde1177140de4452601883dca84d63 Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Mon, 30 Dec 2019 17:32:58 +0200 Subject: [PATCH 6/9] Replace mbedtls_platform_enforce_volatile_reads Replace function mbedtls_platform_enforce_volatile_reads() with mbedtls_platform_random_delay(). --- include/mbedtls/platform_util.h | 10 ---------- library/pk.c | 2 +- library/platform_util.c | 6 ------ library/x509_crt.c | 6 +++--- tinycrypt/ecc.c | 14 +++++++------- tinycrypt/ecc_dsa.c | 2 +- 6 files changed, 12 insertions(+), 28 deletions(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 3bad5598a..27989d68d 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -259,16 +259,6 @@ uint32_t mbedtls_platform_random_in_range( size_t num ); */ int mbedtls_platform_random_delay( size_t num ); -/** - * \brief This function can be inserted between successive reads to a - * volatile local variable to prevent compilers from optimizing - * them away. In addition, this function will spent a small random - * time in a busy loop as a counter-measure to fault injection - * attack. - * - */ -void mbedtls_platform_enforce_volatile_reads( void ); - #if defined(MBEDTLS_HAVE_TIME_DATE) /** * \brief Platform-specific implementation of gmtime_r() diff --git a/library/pk.c b/library/pk.c index 27276a829..1e991c5a0 100644 --- a/library/pk.c +++ b/library/pk.c @@ -597,7 +597,7 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, if( ret_fi == UECC_SUCCESS ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 50 ); if( ret_fi == UECC_SUCCESS ) return( 0 ); else diff --git a/library/platform_util.c b/library/platform_util.c index 691af7100..6c5bd3e2c 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -198,12 +198,6 @@ int mbedtls_platform_random_delay( size_t max_rand ) #endif /* !MBEDTLS_ENTROPY_HARDWARE_ALT */ } -void mbedtls_platform_enforce_volatile_reads( void ) -{ - // Add a small random delay as a counter-measure to fault injection attack. - mbedtls_platform_random_delay( 50 ); -} - #if defined(MBEDTLS_HAVE_TIME_DATE) && !defined(MBEDTLS_PLATFORM_GMTIME_R_ALT) #include #if !defined(_WIN32) && (defined(unix) || \ diff --git a/library/x509_crt.c b/library/x509_crt.c index fd3fa1a04..e624c6da1 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -3043,7 +3043,7 @@ check_signature: if( ret_fi == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 50 ); if( ret_fi == 0 ) signature_is_good = X509_SIGNATURE_IS_GOOD; } @@ -3549,7 +3549,7 @@ find_parent: if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD ) *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA; - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 50 ); if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD ) *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA; @@ -3861,7 +3861,7 @@ exit: flags_fi = *flags; if( flags_fi == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 50 ); if( flags_fi == 0 ) return( 0 ); } diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index a0333b049..d0519b45e 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -168,7 +168,7 @@ static int uECC_check_curve_integrity(void) } /* i should be 32 */ - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 10 ); diff |= (unsigned char) i ^ 32; return diff; @@ -296,7 +296,7 @@ uECC_word_t uECC_vli_equal(const uECC_word_t *left, const uECC_word_t *right) } /* i should be -1 now */ - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 10 ); diff |= i ^ -1; return diff; @@ -1046,7 +1046,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, if (problem != 0) { return UECC_FAULT_DETECTED; } - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 50 ); if (problem != 0) { return UECC_FAULT_DETECTED; } @@ -1058,7 +1058,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, /* invalid input, can happen without fault */ return UECC_FAILURE; } - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 50 ); if (problem != 0) { /* failure on second check means fault, though */ return UECC_FAULT_DETECTED; @@ -1088,7 +1088,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, r = UECC_FAULT_DETECTED; goto clear_and_out; } - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 10 ); if (problem != 0) { r = UECC_FAULT_DETECTED; goto clear_and_out; @@ -1101,7 +1101,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, r = UECC_FAULT_DETECTED; goto clear_and_out; } - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 10 ); if (problem != 0) { r = UECC_FAULT_DETECTED; goto clear_and_out; @@ -1198,7 +1198,7 @@ int uECC_valid_point(const uECC_word_t *point) /* Make sure that y^2 == x^3 + ax + b */ diff = uECC_vli_equal(tmp1, tmp2); if (diff == 0) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 10 ); if (diff == 0) { return 0; } diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index ca071f814..c19c73e9c 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -299,7 +299,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, /* Accept only if v == r. */ diff = uECC_vli_equal(rx, r); if (diff == 0) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay( 50 ); if (diff == 0) { return UECC_SUCCESS; } From 7195571681ec4793f04758ee0ec55de418d2bb69 Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Fri, 3 Jan 2020 14:26:20 +0200 Subject: [PATCH 7/9] Replace mbedtls_platform_enforce_volatile_reads 2 Replace remaining mbedtls_platform_enforce_volatile_reads() with mbedtls_platform_random_delay(). --- library/entropy.c | 4 ++-- library/pk.c | 2 +- library/ssl_cli.c | 8 ++++---- library/ssl_srv.c | 4 ++-- library/ssl_tls.c | 24 ++++++++++++------------ 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index b4d1f2921..8d42dd78e 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -273,7 +273,7 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) volatile int strong_fi = ctx->source[i].strong; if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) have_one_strong_fi = MBEDTLS_ENTROPY_SOURCE_STRONG; @@ -305,7 +305,7 @@ cleanup: if( have_one_strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( have_one_strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { return( ret ); diff --git a/library/pk.c b/library/pk.c index 1e991c5a0..252c78905 100644 --- a/library/pk.c +++ b/library/pk.c @@ -1553,7 +1553,7 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, if( verify_ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( verify_ret == 0 ) { return( verify_ret ); diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 479554d78..88d609b07 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -717,7 +717,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 28 ); if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_SET; @@ -2369,7 +2369,7 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2442,7 +2442,7 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -3071,7 +3071,7 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 92d1da016..2ba1c19f4 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4064,7 +4064,7 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, if( pmscounter == ssl->handshake->pmslen ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( pmscounter == ssl->handshake->pmslen ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -4651,7 +4651,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 19bdc9079..d86960e10 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1929,7 +1929,7 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) ssl ); if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->key_derivation_done = MBEDTLS_SSL_FI_FLAG_SET; @@ -2011,7 +2011,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2054,7 +2054,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2085,7 +2085,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ); if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2114,7 +2114,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -7346,7 +7346,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, if( verify_ret == 0 ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( verify_ret == 0 ) { flow_counter++; @@ -7436,7 +7436,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, ( verify_ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || verify_ret == MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ) ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && ( verify_ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || verify_ret == MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ) ) @@ -7502,7 +7502,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, flow_counter == 4 ) #endif { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( verify_ret == 0 && #if defined(MBEDTLS_ECP_C) || defined(MBEDTLS_USE_TINYCRYPT) flow_counter == 5 ) @@ -7989,7 +7989,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) 1 ) #endif { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( authmode == MBEDTLS_SSL_VERIFY_NONE || authmode == MBEDTLS_SSL_VERIFY_OPTIONAL || #if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) @@ -8010,7 +8010,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { /* When doing session resume, no premaster or peer authentication */ @@ -8027,7 +8027,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) if( ssl->handshake->peer_authenticated == MBEDTLS_SSL_FI_FLAG_SET ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ssl->handshake->peer_authenticated == MBEDTLS_SSL_FI_FLAG_SET ) { ret = 0; @@ -8048,7 +8048,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) ssl->handshake->key_derivation_done == MBEDTLS_SSL_FI_FLAG_SET && ssl->handshake->premaster_generated == MBEDTLS_SSL_FI_FLAG_SET ) { - mbedtls_platform_enforce_volatile_reads(); + mbedtls_platform_random_delay(50); if( ssl->handshake->hello_random_set == MBEDTLS_SSL_FI_FLAG_SET && ssl->handshake->key_derivation_done == MBEDTLS_SSL_FI_FLAG_SET && ssl->handshake->premaster_generated == MBEDTLS_SSL_FI_FLAG_SET ) From ac6d2269393f699a303e858913586b731630c6bf Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Thu, 9 Jan 2020 10:11:20 +0200 Subject: [PATCH 8/9] Update signature of mbedtls_platform_random_delay Skip parameter and return value from mbedtls_platform_random_delay to make it more resistant for FI attacks. --- include/mbedtls/platform_util.h | 9 +-------- library/entropy.c | 4 ++-- library/pk.c | 4 ++-- library/platform_util.c | 16 +++++++--------- library/ssl_cli.c | 8 ++++---- library/ssl_srv.c | 4 ++-- library/ssl_tls.c | 24 ++++++++++++------------ library/x509_crt.c | 6 +++--- tinycrypt/ecc.c | 14 +++++++------- tinycrypt/ecc_dsa.c | 2 +- 10 files changed, 41 insertions(+), 50 deletions(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 27989d68d..fa9f3269c 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -249,15 +249,8 @@ uint32_t mbedtls_platform_random_in_range( size_t num ); * * \note Currently the function is dependent of hardware providing an * rng with MBEDTLS_ENTROPY_HARDWARE_ALT. - * - * \param num Max-value for the number of local variable increments, must be - * less than INT_MAX. Total number of variable increment will be - * randomized between 1 and num. - * - * \return In success number of increments made. - * \return Negative value in case of errors. */ -int mbedtls_platform_random_delay( size_t num ); +void mbedtls_platform_random_delay( void ); #if defined(MBEDTLS_HAVE_TIME_DATE) /** diff --git a/library/entropy.c b/library/entropy.c index 8d42dd78e..6656ee86b 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -273,7 +273,7 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) volatile int strong_fi = ctx->source[i].strong; if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) have_one_strong_fi = MBEDTLS_ENTROPY_SOURCE_STRONG; @@ -305,7 +305,7 @@ cleanup: if( have_one_strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( have_one_strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { return( ret ); diff --git a/library/pk.c b/library/pk.c index 252c78905..caa5e17d0 100644 --- a/library/pk.c +++ b/library/pk.c @@ -597,7 +597,7 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, if( ret_fi == UECC_SUCCESS ) { - mbedtls_platform_random_delay( 50 ); + mbedtls_platform_random_delay(); if( ret_fi == UECC_SUCCESS ) return( 0 ); else @@ -1553,7 +1553,7 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, if( verify_ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( verify_ret == 0 ) { return( verify_ret ); diff --git a/library/platform_util.c b/library/platform_util.c index 6c5bd3e2c..c615e34d2 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -45,6 +45,9 @@ #include #include +/* Max number of loops for mbedtls_platform_random_delay */ +#define MBEDTLS_MAX_RAND_DELAY 100 + #if !defined(MBEDTLS_PLATFORM_ZEROIZE_ALT) /* * This implementation should never be optimized out by the compiler @@ -165,21 +168,16 @@ uint32_t mbedtls_platform_random_in_range( size_t num ) #endif } -int mbedtls_platform_random_delay( size_t max_rand ) +void mbedtls_platform_random_delay( void ) { #if !defined(MBEDTLS_ENTROPY_HARDWARE_ALT) - (void) max_rand; - return -1; + return; #else size_t rn_1, rn_2, rn_3; volatile size_t i = 0; uint8_t shift; - if( max_rand == 0 || max_rand > INT_MAX ) - { - return( -1 ); - } - rn_1 = mbedtls_platform_random_in_range( max_rand ); + rn_1 = mbedtls_platform_random_in_range( MBEDTLS_MAX_RAND_DELAY ); rn_2 = mbedtls_platform_random_in_range( 0xffffffff ) + 1; rn_3 = mbedtls_platform_random_in_range( 0xffffffff ) + 1; @@ -194,7 +192,7 @@ int mbedtls_platform_random_delay( size_t max_rand ) rn_2 ^= rn_3; } while( i < rn_1 || rn_2 == 0 || rn_3 == 0 ); - return( (int)i ); + return; #endif /* !MBEDTLS_ENTROPY_HARDWARE_ALT */ } diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 88d609b07..3c5992328 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -717,7 +717,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 28 ); if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_SET; @@ -2369,7 +2369,7 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2442,7 +2442,7 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -3071,7 +3071,7 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 2ba1c19f4..bab8f00d7 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4064,7 +4064,7 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, if( pmscounter == ssl->handshake->pmslen ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( pmscounter == ssl->handshake->pmslen ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -4651,7 +4651,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d86960e10..03bfd1192 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1929,7 +1929,7 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) ssl ); if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->key_derivation_done = MBEDTLS_SSL_FI_FLAG_SET; @@ -2011,7 +2011,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2054,7 +2054,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2085,7 +2085,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ); if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -2114,7 +2114,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ret == 0 ) { ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; @@ -7346,7 +7346,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, if( verify_ret == 0 ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( verify_ret == 0 ) { flow_counter++; @@ -7436,7 +7436,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, ( verify_ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || verify_ret == MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ) ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && ( verify_ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || verify_ret == MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ) ) @@ -7502,7 +7502,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, flow_counter == 4 ) #endif { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( verify_ret == 0 && #if defined(MBEDTLS_ECP_C) || defined(MBEDTLS_USE_TINYCRYPT) flow_counter == 5 ) @@ -7989,7 +7989,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) 1 ) #endif { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( authmode == MBEDTLS_SSL_VERIFY_NONE || authmode == MBEDTLS_SSL_VERIFY_OPTIONAL || #if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) @@ -8010,7 +8010,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { /* When doing session resume, no premaster or peer authentication */ @@ -8027,7 +8027,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) if( ssl->handshake->peer_authenticated == MBEDTLS_SSL_FI_FLAG_SET ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ssl->handshake->peer_authenticated == MBEDTLS_SSL_FI_FLAG_SET ) { ret = 0; @@ -8048,7 +8048,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) ssl->handshake->key_derivation_done == MBEDTLS_SSL_FI_FLAG_SET && ssl->handshake->premaster_generated == MBEDTLS_SSL_FI_FLAG_SET ) { - mbedtls_platform_random_delay(50); + mbedtls_platform_random_delay(); if( ssl->handshake->hello_random_set == MBEDTLS_SSL_FI_FLAG_SET && ssl->handshake->key_derivation_done == MBEDTLS_SSL_FI_FLAG_SET && ssl->handshake->premaster_generated == MBEDTLS_SSL_FI_FLAG_SET ) diff --git a/library/x509_crt.c b/library/x509_crt.c index e624c6da1..af8f1d67f 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -3043,7 +3043,7 @@ check_signature: if( ret_fi == 0 ) { - mbedtls_platform_random_delay( 50 ); + mbedtls_platform_random_delay(); if( ret_fi == 0 ) signature_is_good = X509_SIGNATURE_IS_GOOD; } @@ -3549,7 +3549,7 @@ find_parent: if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD ) *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA; - mbedtls_platform_random_delay( 50 ); + mbedtls_platform_random_delay(); if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD ) *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA; @@ -3861,7 +3861,7 @@ exit: flags_fi = *flags; if( flags_fi == 0 ) { - mbedtls_platform_random_delay( 50 ); + mbedtls_platform_random_delay(); if( flags_fi == 0 ) return( 0 ); } diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index d0519b45e..7694ad7d8 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -168,7 +168,7 @@ static int uECC_check_curve_integrity(void) } /* i should be 32 */ - mbedtls_platform_random_delay( 10 ); + mbedtls_platform_random_delay(); diff |= (unsigned char) i ^ 32; return diff; @@ -296,7 +296,7 @@ uECC_word_t uECC_vli_equal(const uECC_word_t *left, const uECC_word_t *right) } /* i should be -1 now */ - mbedtls_platform_random_delay( 10 ); + mbedtls_platform_random_delay(); diff |= i ^ -1; return diff; @@ -1046,7 +1046,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, if (problem != 0) { return UECC_FAULT_DETECTED; } - mbedtls_platform_random_delay( 50 ); + mbedtls_platform_random_delay(); if (problem != 0) { return UECC_FAULT_DETECTED; } @@ -1058,7 +1058,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, /* invalid input, can happen without fault */ return UECC_FAILURE; } - mbedtls_platform_random_delay( 50 ); + mbedtls_platform_random_delay(); if (problem != 0) { /* failure on second check means fault, though */ return UECC_FAULT_DETECTED; @@ -1088,7 +1088,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, r = UECC_FAULT_DETECTED; goto clear_and_out; } - mbedtls_platform_random_delay( 10 ); + mbedtls_platform_random_delay(); if (problem != 0) { r = UECC_FAULT_DETECTED; goto clear_and_out; @@ -1101,7 +1101,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, r = UECC_FAULT_DETECTED; goto clear_and_out; } - mbedtls_platform_random_delay( 10 ); + mbedtls_platform_random_delay(); if (problem != 0) { r = UECC_FAULT_DETECTED; goto clear_and_out; @@ -1198,7 +1198,7 @@ int uECC_valid_point(const uECC_word_t *point) /* Make sure that y^2 == x^3 + ax + b */ diff = uECC_vli_equal(tmp1, tmp2); if (diff == 0) { - mbedtls_platform_random_delay( 10 ); + mbedtls_platform_random_delay(); if (diff == 0) { return 0; } diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index c19c73e9c..3db8536d5 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -299,7 +299,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, /* Accept only if v == r. */ diff = uECC_vli_equal(rx, r); if (diff == 0) { - mbedtls_platform_random_delay( 50 ); + mbedtls_platform_random_delay(); if (diff == 0) { return UECC_SUCCESS; } From b148651e49ee1a26bd685ea650c0e7d3cc2722ef Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Thu, 9 Jan 2020 11:11:23 +0200 Subject: [PATCH 9/9] Rename macro MBEDTLS_MAX_RAND_DELAY MBEDTLS_MAX_RAND_DELAY renamed to MAX_RAND_DELAY to get CI passing. --- library/platform_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index c615e34d2..2c22b3c64 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -46,7 +46,7 @@ #include /* Max number of loops for mbedtls_platform_random_delay */ -#define MBEDTLS_MAX_RAND_DELAY 100 +#define MAX_RAND_DELAY 100 #if !defined(MBEDTLS_PLATFORM_ZEROIZE_ALT) /* @@ -177,7 +177,7 @@ void mbedtls_platform_random_delay( void ) volatile size_t i = 0; uint8_t shift; - rn_1 = mbedtls_platform_random_in_range( MBEDTLS_MAX_RAND_DELAY ); + rn_1 = mbedtls_platform_random_in_range( MAX_RAND_DELAY ); rn_2 = mbedtls_platform_random_in_range( 0xffffffff ) + 1; rn_3 = mbedtls_platform_random_in_range( 0xffffffff ) + 1;