Force some compilers to respect volatile reads

Inspection of the generated assembly showed that before this commit, armcc 5
was optimizing away the successive reads to the volatile local variable that's
used for double-checks. Inspection also reveals that inserting a call to an
external function is enough to prevent it from doing that.

The tested versions of ARM-GCC, Clang and Armcc 6 (aka armclang) all keep the
double read, with our without a call to an external function in the middle.

The inserted function can also be changed to insert a random delay if
desired in the future, as it is appropriately places between the reads.
This commit is contained in:
Manuel Pégourié-Gonnard 2019-11-08 10:21:00 +01:00
parent ca7b5ab5ef
commit 72a8c9e7dc
4 changed files with 19 additions and 0 deletions

View File

@ -238,6 +238,13 @@ int mbedtls_platform_memcmp( const void *buf1, const void *buf2, size_t num );
*/ */
uint32_t mbedtls_platform_random_in_range( size_t num ); uint32_t mbedtls_platform_random_in_range( 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.
*/
void mbedtls_platform_enforce_volatile_reads( void );
#if defined(MBEDTLS_HAVE_TIME_DATE) #if defined(MBEDTLS_HAVE_TIME_DATE)
/** /**
* \brief Platform-specific implementation of gmtime_r() * \brief Platform-specific implementation of gmtime_r()

View File

@ -598,6 +598,7 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg,
if( ret_fi == UECC_SUCCESS ) if( ret_fi == UECC_SUCCESS )
{ {
mbedtls_platform_enforce_volatile_reads();
if( ret_fi == UECC_SUCCESS ) if( ret_fi == UECC_SUCCESS )
return( 0 ); return( 0 );
else else

View File

@ -168,6 +168,15 @@ uint32_t mbedtls_platform_random_in_range( size_t num )
#endif #endif
} }
/* 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 )
{
}
#if defined(MBEDTLS_HAVE_TIME_DATE) && !defined(MBEDTLS_PLATFORM_GMTIME_R_ALT) #if defined(MBEDTLS_HAVE_TIME_DATE) && !defined(MBEDTLS_PLATFORM_GMTIME_R_ALT)
#include <time.h> #include <time.h>
#if !defined(_WIN32) && (defined(unix) || \ #if !defined(_WIN32) && (defined(unix) || \

View File

@ -67,6 +67,7 @@
#if defined(MBEDTLS_USE_TINYCRYPT) #if defined(MBEDTLS_USE_TINYCRYPT)
#include <tinycrypt/ecc.h> #include <tinycrypt/ecc.h>
#include <tinycrypt/ecc_dsa.h> #include <tinycrypt/ecc_dsa.h>
#include "mbedtls/platform_util.h"
#if default_RNG_defined #if default_RNG_defined
static uECC_RNG_Function g_rng_function = &default_CSPRNG; static uECC_RNG_Function g_rng_function = &default_CSPRNG;
@ -304,6 +305,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash,
/* Accept only if v == r. */ /* Accept only if v == r. */
diff = uECC_vli_equal(rx, r); diff = uECC_vli_equal(rx, r);
if (diff == 0) { if (diff == 0) {
mbedtls_platform_enforce_volatile_reads();
if (diff == 0) { if (diff == 0) {
return UECC_SUCCESS; return UECC_SUCCESS;
} }