diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h index 31f294f70..a52f9f5db 100644 --- a/include/mbedtls/error.h +++ b/include/mbedtls/error.h @@ -86,7 +86,7 @@ * CHACHA20 3 0x0051-0x0055 * POLY1305 3 0x0057-0x005B * CHACHAPOLY 2 0x0054-0x0056 - * PLATFORM 1 0x0070-0x0072 + * PLATFORM 3 0x0070-0x0072 0x0071-0x0071 * * High-level module nr (3 bits - 0x0...-0x7...) * Name ID Nr of Errors diff --git a/include/mbedtls/platform.h b/include/mbedtls/platform.h index 89fe8a7b1..16f9b8a3c 100644 --- a/include/mbedtls/platform.h +++ b/include/mbedtls/platform.h @@ -45,6 +45,7 @@ #define MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED -0x0070 /**< Hardware accelerator failed */ #define MBEDTLS_ERR_PLATFORM_FEATURE_UNSUPPORTED -0x0072 /**< The requested feature is not supported by the platform */ +#define MBEDTLS_ERR_PLATFORM_FAULT_DETECTED -0x0071 /**< A hardware fault was detected in a critical path. As a security precaution this should be treated as a potential physical attack */ #ifdef __cplusplus extern "C" { diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 586f0d9ee..e20f1c32e 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -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 ); +/** + * \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) /** * \brief Platform-specific implementation of gmtime_r() diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 2da74b3c0..0d1d9ec98 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -83,6 +83,13 @@ extern "C" { #endif +/* Return values for functions, chosen with large Hamming distances between + * them (especially to SUCESS) to mitigate the impact of fault injection + * attacks flipping a low number of bits. */ +#define UECC_SUCCESS 0 +#define UECC_FAILURE 0x75555555 +#define UECC_ATTACK_DETECTED 0x7aaaaaaa + /* Word size (4 bytes considering 32-bits architectures) */ #define uECC_WORD_SIZE 4 @@ -415,7 +422,7 @@ uECC_word_t uECC_vli_sub(uECC_word_t *result, const uECC_word_t *left, * @param left IN -- left term in comparison * @param right IN -- right term in comparison * @param num_words IN -- number of words - * @return Returns 0 if left == right, 1 otherwise. + * @return Returns 0 if left == right, non-zero otherwise. */ uECC_word_t uECC_vli_equal(const uECC_word_t *left, const uECC_word_t *right); diff --git a/include/tinycrypt/ecc_dsa.h b/include/tinycrypt/ecc_dsa.h index e54a77e85..55b9d43f3 100644 --- a/include/tinycrypt/ecc_dsa.h +++ b/include/tinycrypt/ecc_dsa.h @@ -123,8 +123,8 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, /** * @brief Verify an ECDSA signature. - * @return returns TC_SUCCESS (1) if the signature is valid - * returns TC_FAIL (0) if the signature is invalid. + * @return returns UECC_SUCCESS if the signature is valid + * returns UECC_FAILURE if the signature is invalid. * * @param p_public_key IN -- The signer's public key. * @param p_message_hash IN -- The hash of the signed data. diff --git a/library/error.c b/library/error.c index c993524fe..74c9d0b39 100644 --- a/library/error.c +++ b/library/error.c @@ -841,6 +841,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) mbedtls_snprintf( buf, buflen, "PLATFORM - Hardware accelerator failed" ); if( use_ret == -(MBEDTLS_ERR_PLATFORM_FEATURE_UNSUPPORTED) ) mbedtls_snprintf( buf, buflen, "PLATFORM - The requested feature is not supported by the platform" ); + if( use_ret == -(MBEDTLS_ERR_PLATFORM_FAULT_DETECTED) ) + mbedtls_snprintf( buf, buflen, "PLATFORM - A hardware fault was detected in a critical path. As a security precaution this should be treated as a potential physical attack" ); #endif /* MBEDTLS_PLATFORM_C */ #if defined(MBEDTLS_POLY1305_C) diff --git a/library/pk.c b/library/pk.c index 29123eb05..9eddb61ab 100644 --- a/library/pk.c +++ b/library/pk.c @@ -577,6 +577,7 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *sig, size_t sig_len ) { int ret; + volatile int ret_fi; uint8_t signature[2*NUM_ECC_BYTES]; unsigned char *p; const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); @@ -589,12 +590,22 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, if( ret != 0 ) return( ret ); - ret = uECC_verify( keypair->public_key, hash, - (unsigned) hash_len, signature, uecc_curve ); - if( ret == 0 ) - return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + ret_fi = uECC_verify( keypair->public_key, hash, + (unsigned) hash_len, signature, uecc_curve ); - return( 0 ); + if( ret_fi == UECC_ATTACK_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + + if( ret_fi == UECC_SUCCESS ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ret_fi == UECC_SUCCESS ) + return( 0 ); + else + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } + + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); } /* diff --git a/library/platform_util.c b/library/platform_util.c index 1a0fefae6..97dfe73ef 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -168,6 +168,15 @@ uint32_t mbedtls_platform_random_in_range( size_t num ) #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) #include #if !defined(_WIN32) && (defined(unix) || \ diff --git a/tests/suites/test_suite_tinycrypt.data b/tests/suites/test_suite_tinycrypt.data index ac2a8e23e..2c4d54b5a 100644 --- a/tests/suites/test_suite_tinycrypt.data +++ b/tests/suites/test_suite_tinycrypt.data @@ -8,4 +8,4 @@ ECDH primitive rfc 5903 p256 ecdh_primitive_testvec:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"DAD0B65394221CF9B051E1FECA5787D098DFE637FC90B9EF945D0C3772581180":"5271A0461CDB8252D61F1C456FA3E59AB1F45B33ACCF5F58389E0577B8990BB3":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D12DFB5289C8D4F81208B70270398C342296970A0BCCB74C736FC7554494BF63":"56FBF3CA366CC23E8157854C13C58D6AAC23F046ADA30F8353E74F33039872AB":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE" ECDSA primitive rfc 4754 p256 -ecdsa_primitive_testvec:"2442A5CC0ECD015FA3CA31DC8E2BBC70BF42D60CBCA20085E0822CB04235E970":"6FC98BD7E50211A4A27102FA3549DF79EBCB4BF246B80945CDDFE7D509BBFD7D":"BA7816BF8F01CFEA414140DE5DAE2223B00361A396177A9CB410FF61F20015AD":"CB28E0999B9C7715FD0A80D8E47A77079716CBBF917DD72E97566EA1C066957C":"86FA3BB4E26CAD5BF90B7F81899256CE7594BB1EA0C89212748BFF3B3D5B0315":1 +ecdsa_primitive_testvec:"2442A5CC0ECD015FA3CA31DC8E2BBC70BF42D60CBCA20085E0822CB04235E970":"6FC98BD7E50211A4A27102FA3549DF79EBCB4BF246B80945CDDFE7D509BBFD7D":"BA7816BF8F01CFEA414140DE5DAE2223B00361A396177A9CB410FF61F20015AD":"CB28E0999B9C7715FD0A80D8E47A77079716CBBF917DD72E97566EA1C066957C":"86FA3BB4E26CAD5BF90B7F81899256CE7594BB1EA0C89212748BFF3B3D5B0315" diff --git a/tests/suites/test_suite_tinycrypt.function b/tests/suites/test_suite_tinycrypt.function index 24b331d80..664cd0862 100644 --- a/tests/suites/test_suite_tinycrypt.function +++ b/tests/suites/test_suite_tinycrypt.function @@ -55,7 +55,7 @@ void test_ecdsa() TEST_ASSERT( uECC_sign( private, hash, sizeof( hash ), sig, curve ) != 0 ); - TEST_ASSERT( uECC_verify( public, hash, sizeof( hash ), sig, curve ) != 0 ); + TEST_ASSERT( uECC_verify( public, hash, sizeof( hash ), sig, curve ) == UECC_SUCCESS ); } /* END_CASE */ @@ -88,8 +88,7 @@ void ecdh_primitive_testvec( data_t * private1, data_t * xA_str, /* BEGIN_CASE depends_on:MBEDTLS_USE_TINYCRYPT */ void ecdsa_primitive_testvec( data_t * xQ_str, data_t * yQ_str, - data_t * hash, data_t * r_str, data_t * s_str, - int result ) + data_t * hash, data_t * r_str, data_t * s_str ) { const struct uECC_Curve_t * curve = uECC_secp256r1(); uint8_t pub_bytes[2*NUM_ECC_BYTES] = {0}; @@ -101,7 +100,7 @@ void ecdsa_primitive_testvec( data_t * xQ_str, data_t * yQ_str, memcpy( sig_bytes + NUM_ECC_BYTES, s_str->x, r_str->len ); TEST_ASSERT( uECC_verify( pub_bytes, hash->x, hash->len, - sig_bytes, curve ) == result ); + sig_bytes, curve ) == UECC_SUCCESS ); // Alter the signature and check the verification fails for( int i = 0; i < 2*NUM_ECC_BYTES; i++ ) @@ -109,7 +108,7 @@ void ecdsa_primitive_testvec( data_t * xQ_str, data_t * yQ_str, uint8_t temp = sig_bytes[i]; sig_bytes[i] = ( sig_bytes[i] + 1 ) % 256; TEST_ASSERT( uECC_verify( pub_bytes, hash->x, hash->len, - sig_bytes, curve ) == 0 ); + sig_bytes, curve ) == UECC_FAILURE ); sig_bytes[i] = temp; } diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index d01c67617..92906fd76 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -185,7 +185,7 @@ uECC_word_t uECC_vli_equal(const uECC_word_t *left, const uECC_word_t *right) for (i = NUM_ECC_WORDS - 1; i >= 0; --i) { diff |= (left[i] ^ right[i]); } - return !(diff == 0); + return diff; } uECC_word_t cond_set(uECC_word_t p_true, uECC_word_t p_false, unsigned int cond) diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 04b1bfabd..6c171c3a9 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -67,6 +67,7 @@ #if defined(MBEDTLS_USE_TINYCRYPT) #include #include +#include "mbedtls/platform_util.h" #if default_RNG_defined static uECC_RNG_Function g_rng_function = &default_CSPRNG; @@ -214,6 +215,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, const uECC_word_t *point; bitcount_t num_bits; bitcount_t i; + volatile uECC_word_t diff; uECC_word_t _public[NUM_ECC_WORDS * 2]; uECC_word_t r[NUM_ECC_WORDS], s[NUM_ECC_WORDS]; @@ -235,13 +237,13 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, /* r, s must not be 0. */ if (uECC_vli_isZero(r) || uECC_vli_isZero(s)) { - return 0; + return UECC_FAILURE; } /* r, s must be < n. */ if (uECC_vli_cmp_unsafe(curve->n, r) != 1 || uECC_vli_cmp_unsafe(curve->n, s) != 1) { - return 0; + return UECC_FAILURE; } /* Calculate u1 and u2. */ @@ -301,7 +303,18 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, } /* Accept only if v == r. */ - return (int)(uECC_vli_equal(rx, r) == 0); + diff = uECC_vli_equal(rx, r); + if (diff == 0) { + mbedtls_platform_enforce_volatile_reads(); + if (diff == 0) { + return UECC_SUCCESS; + } + else { + return UECC_ATTACK_DETECTED; + } + } + + return UECC_FAILURE; } #else typedef int mbedtls_dummy_tinycrypt_def;