Merge remote-tracking branch 'public/pr/2940' into baremetal
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 746b38a..39808f9 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -185,7 +185,10 @@
add_subdirectory(library)
add_subdirectory(include)
-add_subdirectory(tinycrypt)
+
+if(USE_TINYCRYPT)
+ add_subdirectory(tinycrypt)
+endif()
if(ENABLE_PROGRAMS)
add_subdirectory(programs)
diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h
index 31f294f..a52f9f5 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/hmac_drbg.h b/include/mbedtls/hmac_drbg.h
index ed03854..f38337a 100644
--- a/include/mbedtls/hmac_drbg.h
+++ b/include/mbedtls/hmac_drbg.h
@@ -70,8 +70,8 @@
/* \} name SECTION: Module settings */
-#define MBEDTLS_HMAC_DRBG_PR_OFF 0 /**< No prediction resistance */
-#define MBEDTLS_HMAC_DRBG_PR_ON 1 /**< Prediction resistance enabled */
+#define MBEDTLS_HMAC_DRBG_PR_OFF 0x55555555 /**< No prediction resistance */
+#define MBEDTLS_HMAC_DRBG_PR_ON 0x2AAAAAAA /**< Prediction resistance enabled */
#ifdef __cplusplus
extern "C" {
@@ -202,7 +202,8 @@
* \param add_len Length of additional data, or 0
*
* \return \c 0 on success, or an error from the underlying
- * hash calculation.
+ * hash calculation or
+ * MBEDTLS_ERR_PLATFORM_FAULT_DETECTED.
*
* \note Additional data is optional, pass NULL and 0 as second
* third argument if no additional data is being used.
@@ -237,7 +238,8 @@
* \return 0 if successful, or
* MBEDTLS_ERR_HMAC_DRBG_ENTROPY_SOURCE_FAILED, or
* MBEDTLS_ERR_HMAC_DRBG_REQUEST_TOO_BIG, or
- * MBEDTLS_ERR_HMAC_DRBG_INPUT_TOO_BIG.
+ * MBEDTLS_ERR_HMAC_DRBG_INPUT_TOO_BIG, or
+ * MBEDTLS_ERR_PLATFORM_FAULT_DETECTED.
*/
int mbedtls_hmac_drbg_random_with_add( void *p_rng,
unsigned char *output, size_t output_len,
@@ -255,7 +257,9 @@
*
* \return 0 if successful, or
* MBEDTLS_ERR_HMAC_DRBG_ENTROPY_SOURCE_FAILED, or
- * MBEDTLS_ERR_HMAC_DRBG_REQUEST_TOO_BIG
+ * MBEDTLS_ERR_HMAC_DRBG_REQUEST_TOO_BIG,
+ * MBEDTLS_ERR_HMAC_DRBG_INPUT_TOO_BIG, or
+ * MBEDTLS_ERR_PLATFORM_FAULT_DETECTED.
*/
int mbedtls_hmac_drbg_random( void *p_rng, unsigned char *output, size_t out_len );
diff --git a/include/mbedtls/platform.h b/include/mbedtls/platform.h
index 89fe8a7..ec1df15 100644
--- a/include/mbedtls/platform.h
+++ b/include/mbedtls/platform.h
@@ -39,13 +39,16 @@
#include MBEDTLS_CONFIG_FILE
#endif
+#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 */
+
+#if defined(MBEDTLS_PLATFORM_C)
+
#if defined(MBEDTLS_HAVE_TIME)
#include "platform_time.h"
#endif
-#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 */
-
#ifdef __cplusplus
extern "C" {
#endif
@@ -364,4 +367,6 @@
}
#endif
+#endif /* MBEDTLS_PLATFORM_C */
+
#endif /* platform.h */
diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h
index 586f0d9..e20f1c3 100644
--- a/include/mbedtls/platform_util.h
+++ b/include/mbedtls/platform_util.h
@@ -238,6 +238,13 @@
*/
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/mbedtls/x509.h b/include/mbedtls/x509.h
index c31847d..b2ad182 100644
--- a/include/mbedtls/x509.h
+++ b/include/mbedtls/x509.h
@@ -85,6 +85,8 @@
* \{
*/
/* Reminder: update x509_crt_verify_strings[] in library/x509_crt.c */
+/* Reminder: update X509_BADCERT_FI_EXTRA in library/x509_crt.c if using more
+ * that 24 bits */
#define MBEDTLS_X509_BADCERT_EXPIRED 0x01 /**< The certificate validity has expired. */
#define MBEDTLS_X509_BADCERT_REVOKED 0x02 /**< The certificate has been revoked (is on a CRL). */
#define MBEDTLS_X509_BADCERT_CN_MISMATCH 0x04 /**< The certificate Common Name (CN) does not match with the expected CN. */
diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h
index 2da74b3..df8f1df 100644
--- a/include/tinycrypt/ecc.h
+++ b/include/tinycrypt/ecc.h
@@ -73,7 +73,6 @@
*
*/
-#if defined(MBEDTLS_USE_TINYCRYPT)
#ifndef __TC_UECC_H__
#define __TC_UECC_H__
@@ -83,6 +82,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 +421,7 @@
* @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);
@@ -528,4 +534,3 @@
#endif
#endif /* __TC_UECC_H__ */
-#endif /* MBEDTLS_USE_TINYCRYPT */
diff --git a/include/tinycrypt/ecc_dh.h b/include/tinycrypt/ecc_dh.h
index a2edb01..d87393d 100644
--- a/include/tinycrypt/ecc_dh.h
+++ b/include/tinycrypt/ecc_dh.h
@@ -71,7 +71,6 @@
* Security: The curve NIST p-256 provides approximately 128 bits of security.
*/
-#if defined(MBEDTLS_USE_TINYCRYPT)
#ifndef __TC_ECC_DH_H__
#define __TC_ECC_DH_H__
@@ -135,4 +134,3 @@
#endif
#endif /* __TC_ECC_DH_H__ */
-#endif /* MBEDTLS_USE_TINYCRYPT */
diff --git a/include/tinycrypt/ecc_dsa.h b/include/tinycrypt/ecc_dsa.h
index e54a77e..f744319 100644
--- a/include/tinycrypt/ecc_dsa.h
+++ b/include/tinycrypt/ecc_dsa.h
@@ -80,7 +80,6 @@
* the signer's public key and the signature values (r and s).
*/
-#if defined(MBEDTLS_USE_TINYCRYPT)
#ifndef __TC_ECC_DSA_H__
#define __TC_ECC_DSA_H__
@@ -123,8 +122,8 @@
/**
* @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.
@@ -143,4 +142,3 @@
#endif
#endif /* __TC_ECC_DSA_H__ */
-#endif /* MBEDTLS_USE_TINYCRYPT */
diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt
index 89f7275..0156856 100644
--- a/library/CMakeLists.txt
+++ b/library/CMakeLists.txt
@@ -1,4 +1,5 @@
option(USE_STATIC_MBEDTLS_LIBRARY "Build mbed TLS static library." ON)
+option(USE_TINYCRYPT "Include TinyCrypt." ON)
option(USE_SHARED_MBEDTLS_LIBRARY "Build mbed TLS shared library." OFF)
option(LINK_WITH_PTHREAD "Explicitly link mbed TLS library to pthread." OFF)
@@ -123,7 +124,9 @@
set(libs ${libs} pthread)
endif()
-set(libs ${libs} tinycrypt)
+if(USE_TINYCRYPT)
+ set(libs ${libs} tinycrypt)
+endif()
if (NOT USE_STATIC_MBEDTLS_LIBRARY AND NOT USE_SHARED_MBEDTLS_LIBRARY)
message(FATAL_ERROR "Need to choose static or shared mbedtls build!")
diff --git a/library/Makefile b/library/Makefile
index 4154c6a..96a9d60 100644
--- a/library/Makefile
+++ b/library/Makefile
@@ -99,8 +99,7 @@
ripemd160.o rsa_internal.o rsa.o \
sha1.o sha256.o sha512.o \
threading.o timing.o version.o \
- version_features.o xtea.o \
- ecc.o ecc_dh.o ecc_dsa.o
+ version_features.o xtea.o
OBJS_X509= certs.o pkcs11.o x509.o
@@ -110,6 +109,17 @@
ssl_srv.o ssl_ticket.o \
ssl_tls.o
+# Default to always build TinyCrypt
+ifndef TINYCRYPT_BUILD
+TINYCRYPT_BUILD=1
+endif
+
+ifeq ($(TINYCRYPT_BUILD),1)
+# Add TinyCrypt to the targets and Makefile path
+VPATH = ../tinycrypt
+OBJS_CRYPTO += ecc.o ecc_dh.o ecc_dsa.o
+endif
+
.SILENT:
.PHONY: all static shared clean
diff --git a/library/error.c b/library/error.c
index c993524..74c9d0b 100644
--- a/library/error.c
+++ b/library/error.c
@@ -841,6 +841,8 @@
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/hmac_drbg.c b/library/hmac_drbg.c
index d0de5de..82703b0 100644
--- a/library/hmac_drbg.c
+++ b/library/hmac_drbg.c
@@ -34,6 +34,7 @@
#if defined(MBEDTLS_HMAC_DRBG_C)
#include "mbedtls/hmac_drbg.h"
+#include "mbedtls/platform.h"
#include "mbedtls/platform_util.h"
#include <string.h>
@@ -51,6 +52,9 @@
#endif /* MBEDTLS_SELF_TEST */
#endif /* MBEDTLS_PLATFORM_C */
+#define HMAC_NONCE_YES 0x4AAAAAAA
+#define HMAC_NONCE_NO 0x75555555
+
/*
* HMAC_DRBG context initialization
*/
@@ -74,42 +78,76 @@
mbedtls_md_get_handle( &ctx->md_ctx ) );
unsigned char rounds = ( additional != NULL && add_len != 0 ) ? 2 : 1;
unsigned char sep[1];
+ volatile unsigned int flow_counter = 0;
unsigned char K[MBEDTLS_MD_MAX_SIZE];
- int ret;
+ int ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED;
for( sep[0] = 0; sep[0] < rounds; sep[0]++ )
{
/* Step 1 or 4 */
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_reset( &ctx->md_ctx ) ) != 0 )
goto exit;
+
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_update( &ctx->md_ctx,
ctx->V, md_len ) ) != 0 )
goto exit;
+
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_update( &ctx->md_ctx,
sep, 1 ) ) != 0 )
goto exit;
+
if( rounds == 2 )
{
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_update( &ctx->md_ctx,
additional, add_len ) ) != 0 )
goto exit;
}
+
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_finish( &ctx->md_ctx, K ) ) != 0 )
goto exit;
/* Step 2 or 5 */
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_starts( &ctx->md_ctx, K, md_len ) ) != 0 )
goto exit;
+
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_update( &ctx->md_ctx,
ctx->V, md_len ) ) != 0 )
goto exit;
+
+ flow_counter++;
if( ( ret = mbedtls_md_hmac_finish( &ctx->md_ctx, ctx->V ) ) != 0 )
goto exit;
+ flow_counter++;
}
exit:
+
mbedtls_platform_zeroize( K, sizeof( K ) );
- return( ret );
+ /* Check for possible attack.
+ * Counters needs to have correct values when returning success
+ */
+ if ( ret != 0 )
+ return( ret ); // error case, return immediately
+
+ if ( ( ( flow_counter == 8 ) && ( sep[0] == 1 ) ) ||
+ ( ( flow_counter == 18 ) && ( sep[0] == 2 ) ) )
+ {
+ flow_counter = flow_counter - sep[0];
+ // Double check flow_counter
+ if ( ( flow_counter == 7 ) || ( flow_counter == 16 ) )
+ {
+ return ret; // success, return 0 from ret
+ }
+ }
+
+ return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED );
}
#if !defined(MBEDTLS_DEPRECATED_REMOVED)
@@ -125,10 +163,10 @@
* Simplified HMAC_DRBG initialisation (for use with deterministic ECDSA)
*/
int mbedtls_hmac_drbg_seed_buf( mbedtls_hmac_drbg_context *ctx,
- mbedtls_md_handle_t md_info,
- const unsigned char *data, size_t data_len )
+ mbedtls_md_handle_t md_info,
+ const unsigned char *data, size_t data_len )
{
- int ret;
+ int ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED;
if( ( ret = mbedtls_md_setup( &ctx->md_ctx, md_info, 1 ) ) != 0 )
return( ret );
@@ -146,7 +184,7 @@
if( ( ret = mbedtls_hmac_drbg_update_ret( ctx, data, data_len ) ) != 0 )
return( ret );
- return( 0 );
+ return( ret );
}
/*
@@ -160,22 +198,19 @@
{
unsigned char seed[MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT];
size_t seedlen = 0;
+ size_t total_entropy_len;
int ret;
+ if( use_nonce == HMAC_NONCE_NO )
+ total_entropy_len = ctx->entropy_len;
+ else
+ total_entropy_len = ctx->entropy_len * 3 / 2;
+
+ /* III. Check input length */
+ if( len > MBEDTLS_HMAC_DRBG_MAX_INPUT ||
+ total_entropy_len + len > MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT )
{
- size_t total_entropy_len;
-
- if( use_nonce == 0 )
- total_entropy_len = ctx->entropy_len;
- else
- total_entropy_len = ctx->entropy_len * 3 / 2;
-
- /* III. Check input length */
- if( len > MBEDTLS_HMAC_DRBG_MAX_INPUT ||
- total_entropy_len + len > MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT )
- {
- return( MBEDTLS_ERR_HMAC_DRBG_INPUT_TOO_BIG );
- }
+ return( MBEDTLS_ERR_HMAC_DRBG_INPUT_TOO_BIG );
}
memset( seed, 0, MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT );
@@ -190,7 +225,7 @@
/* For initial seeding, allow adding of nonce generated
* from the entropy source. See Sect 8.6.7 in SP800-90A. */
- if( use_nonce )
+ if( use_nonce == HMAC_NONCE_YES )
{
/* Note: We don't merge the two calls to f_entropy() in order
* to avoid requesting too much entropy from f_entropy()
@@ -225,9 +260,20 @@
ctx->reseed_counter = 1;
exit:
+
/* 4. Done */
mbedtls_platform_zeroize( seed, seedlen );
- return( ret );
+
+ if ( ret != 0 )
+ return ret;
+
+ if ( ret == 0 && ctx->reseed_counter == 1 )
+ {
+ /* All ok, return 0 from ret */
+ return ret;
+ }
+
+ return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED );
}
/*
@@ -236,8 +282,7 @@
int mbedtls_hmac_drbg_reseed( mbedtls_hmac_drbg_context *ctx,
const unsigned char *additional, size_t len )
{
- return( hmac_drbg_reseed_core( ctx, additional, len,
- 0 /* no nonce */ ) );
+ return( hmac_drbg_reseed_core( ctx, additional, len, HMAC_NONCE_NO ) );
}
/*
@@ -286,13 +331,12 @@
md_size <= 28 ? 24 : /* 224-bits hash -> 192 bits */
32; /* better (256+) -> 256 bits */
- if( ( ret = hmac_drbg_reseed_core( ctx, custom, len,
- 1 /* add nonce */ ) ) != 0 )
+ if( ( ret = hmac_drbg_reseed_core( ctx, custom, len, HMAC_NONCE_YES ) ) != 0 )
{
return( ret );
}
- return( 0 );
+ return( ret );
}
/*
@@ -329,6 +373,8 @@
const unsigned char *additional, size_t add_len )
{
int ret;
+ volatile unsigned char *output_fi = output;
+ volatile size_t out_len_fi = out_len;
mbedtls_hmac_drbg_context *ctx = (mbedtls_hmac_drbg_context *) p_rng;
size_t md_len = mbedtls_md_get_size(
mbedtls_md_get_handle( &ctx->md_ctx ) );
@@ -390,7 +436,21 @@
exit:
/* 8. Done */
- return( ret );
+
+ if ( ret != 0 )
+ return ret;
+
+ /*
+ * Check doubled variables and illegal conditions in case of possible
+ * attack.
+ */
+ if ( ( out_len_fi == out_len ) && ( output_fi == output) &&
+ ( left == 0 ) )
+ {
+ return ret; // Success, return 0
+ }
+
+ return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED );
}
/*
diff --git a/library/md.c b/library/md.c
index d9d6509..4f03acc 100644
--- a/library/md.c
+++ b/library/md.c
@@ -32,6 +32,7 @@
#if defined(MBEDTLS_MD_C)
#include "mbedtls/md.h"
+#include "mbedtls/platform.h"
#include "mbedtls/platform_util.h"
#if defined(MBEDTLS_PLATFORM_C)
@@ -525,7 +526,7 @@
int ret;
unsigned char sum[MBEDTLS_MD_MAX_SIZE];
unsigned char *ipad, *opad;
- size_t i;
+ size_t i = 0;
mbedtls_md_handle_t md_info;
@@ -575,16 +576,27 @@
if( ( ret = mbedtls_md_info_starts( md_info, ctx->md_ctx ) ) != 0 )
goto cleanup;
+ i++; // Use i as flow control
+
if( ( ret = mbedtls_md_info_update( md_info, ctx->md_ctx, ipad,
mbedtls_md_info_block_size( md_info ) ) ) != 0 )
{
goto cleanup;
}
+ i++; // Use i as flow control now
+
cleanup:
mbedtls_platform_zeroize( sum, sizeof( sum ) );
- return( ret );
+ if ( ret != 0 )
+ return ret;
+
+ /* Check possible fault injection */
+ if ( ( i - 2 ) == keylen )
+ return ret; // success, return 0 from ret
+
+ return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED );
}
int mbedtls_md_hmac_update( mbedtls_md_context_t *ctx,
@@ -653,7 +665,7 @@
if( ( ret = mbedtls_md_info_finish( md_info, ctx->md_ctx, output ) ) != 0 )
return( ret );
- return( 0 );
+ return( ret );
}
int mbedtls_md_hmac_reset( mbedtls_md_context_t *ctx )
diff --git a/library/pk.c b/library/pk.c
index 29123eb..9eddb61 100644
--- a/library/pk.c
+++ b/library/pk.c
@@ -577,6 +577,7 @@
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 @@
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 1a0fefa..97dfe73 100644
--- a/library/platform_util.c
+++ b/library/platform_util.c
@@ -168,6 +168,15 @@
#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 <time.h>
#if !defined(_WIN32) && (defined(unix) || \
diff --git a/library/x509_crt.c b/library/x509_crt.c
index e537983..fd3fa1a 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -43,6 +43,7 @@
#include "mbedtls/x509_internal.h"
#include "mbedtls/oid.h"
#include "mbedtls/platform_util.h"
+#include "mbedtls/platform.h"
#include <string.h>
@@ -2884,6 +2885,10 @@
return( 0 );
}
+/* This value is different enough from 0 that it's hard for an active physical
+ * attacker to reach it just by flipping a few bits. */
+#define X509_SIGNATURE_IS_GOOD 0x7f5a5a5a
+
/*
* Find a suitable parent for child in candidates, or return NULL.
*
@@ -2915,7 +2920,8 @@
* - [in] child: certificate for which we're looking for a parent
* - [in] candidates: chained list of potential parents
* - [out] r_parent: parent found (or NULL)
- * - [out] r_signature_is_good: 1 if child signature by parent is valid, or 0
+ * - [out] r_signature_is_good: set to X509_SIGNATURE_IS_GOOD if
+ * child signature by parent is valid, or to 0
* - [in] top: 1 if candidates consists of trusted roots, ie we're at the top
* of the chain, 0 otherwise
* - [in] path_cnt: number of intermediates seen so far
@@ -2938,8 +2944,9 @@
mbedtls_x509_crt_restart_ctx *rs_ctx )
{
int ret;
+ volatile int ret_fi = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED;
mbedtls_x509_crt *parent_crt;
- int signature_is_good;
+ int signature_is_good = 0;
#if defined(MBEDTLS_HAVE_TIME_DATE)
mbedtls_x509_crt *fallback_parent;
@@ -3018,10 +3025,10 @@
continue;
/* Signature */
- ret = x509_crt_check_signature( child_sig, parent_crt, rs_ctx );
+ ret_fi = x509_crt_check_signature( child_sig, parent_crt, rs_ctx );
#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)
- if( rs_ctx != NULL && ret == MBEDTLS_ERR_ECP_IN_PROGRESS )
+ if( rs_ctx != NULL && ret_fi == MBEDTLS_ERR_ECP_IN_PROGRESS )
{
/* save state */
rs_ctx->parent = parent_crt;
@@ -3030,13 +3037,17 @@
rs_ctx->fallback_signature_is_good = fallback_signature_is_good;
#endif /* MBEDTLS_HAVE_TIME_DATE */
- return( ret );
+ return( ret_fi );
}
-#else
- (void) ret;
#endif
- signature_is_good = ret == 0;
+ if( ret_fi == 0 )
+ {
+ mbedtls_platform_enforce_volatile_reads();
+ if( ret_fi == 0 )
+ signature_is_good = X509_SIGNATURE_IS_GOOD;
+ }
+
if( top && ! signature_is_good )
continue;
@@ -3318,6 +3329,23 @@
#endif /* MBEDTLS_X509_REMOVE_VERIFY_CALLBACK */
/*
+ * This is used in addition to the flag for a specific issue, to ensure that
+ * it is not possible for an active physical attacker to entirely clear the
+ * flags just by flipping a single bit. Take advantage of the fact that all
+ * values defined in include/mbedtls/x509.h so far are 24-bit or less, so the
+ * top byte is free.
+ *
+ * Currently this protection is not compatible with the vrfy callback (as it
+ * can observ and modify flags freely), so it's only enabled when the callback
+ * is disabled.
+ */
+#if defined(MBEDTLS_X509_REMOVE_VERIFY_CALLBACK)
+#define X509_BADCERT_FI_EXTRA 0xff000000u
+#else
+#define X509_BADCERT_FI_EXTRA 0u
+#endif
+
+/*
* Build and verify a certificate chain
*
* Given a peer-provided list of certificates EE, C1, ..., Cn and
@@ -3374,6 +3402,7 @@
int parent_is_trusted;
int child_is_trusted;
int signature_is_good;
+ volatile int signature_is_good_fi;
unsigned self_cnt;
#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)
@@ -3422,9 +3451,9 @@
#if !defined(MBEDTLS_X509_CRT_REMOVE_TIME)
/* Check time-validity (all certificates) */
if( mbedtls_x509_time_is_past( &child->valid_to ) )
- *flags |= MBEDTLS_X509_BADCERT_EXPIRED;
+ *flags |= MBEDTLS_X509_BADCERT_EXPIRED | X509_BADCERT_FI_EXTRA;
if( mbedtls_x509_time_is_future( &child->valid_from ) )
- *flags |= MBEDTLS_X509_BADCERT_FUTURE;
+ *flags |= MBEDTLS_X509_BADCERT_FUTURE | X509_BADCERT_FI_EXTRA;
#endif /* !MBEDTLS_X509_CRT_REMOVE_TIME */
/* Stop here for trusted roots (but not for trusted EE certs) */
@@ -3444,10 +3473,10 @@
/* Check signature algorithm: MD & PK algs */
if( x509_profile_check_md_alg( profile, child->sig_md ) != 0 )
- *flags |= MBEDTLS_X509_BADCERT_BAD_MD;
+ *flags |= MBEDTLS_X509_BADCERT_BAD_MD | X509_BADCERT_FI_EXTRA;
if( x509_profile_check_pk_alg( profile, child->sig_pk ) != 0 )
- *flags |= MBEDTLS_X509_BADCERT_BAD_PK;
+ *flags |= MBEDTLS_X509_BADCERT_BAD_PK | X509_BADCERT_FI_EXTRA;
/* Special case: EE certs that are locally trusted */
if( x509_crt_verify_chain_len( ver_chain ) == 1 && self_issued &&
@@ -3495,7 +3524,7 @@
/* No parent? We're done here */
if( parent_crt == NULL )
{
- *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
+ *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA;
return( 0 );
}
@@ -3516,8 +3545,13 @@
}
/* signature was checked while searching parent */
- if( ! signature_is_good )
- *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
+ signature_is_good_fi = signature_is_good;
+ if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD )
+ *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA;
+
+ mbedtls_platform_enforce_volatile_reads();
+ if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD )
+ *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA;
{
mbedtls_pk_context *parent_pk;
@@ -3527,7 +3561,7 @@
/* check size of signing key */
if( x509_profile_check_key( profile, parent_pk ) != 0 )
- *flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
+ *flags |= MBEDTLS_X509_BADCERT_BAD_KEY | X509_BADCERT_FI_EXTRA;
mbedtls_x509_crt_pk_release( parent_crt );
}
@@ -3658,7 +3692,7 @@
if( ret != 0 )
ret = MBEDTLS_ERR_X509_FATAL_ERROR;
- *flags |= MBEDTLS_X509_BADCERT_CN_MISMATCH;
+ *flags |= MBEDTLS_X509_BADCERT_CN_MISMATCH | X509_BADCERT_FI_EXTRA;
return( ret );
}
#endif /* !MBEDTLS_X509_REMOVE_HOSTNAME_VERIFICATION */
@@ -3747,6 +3781,7 @@
int ret;
mbedtls_x509_crt_verify_chain ver_chain;
uint32_t ee_flags;
+ volatile uint32_t flags_fi = (uint32_t) -1;
*flags = 0;
ee_flags = 0;
@@ -3780,10 +3815,10 @@
pk_type = mbedtls_pk_get_type( pk );
if( x509_profile_check_pk_alg( profile, pk_type ) != 0 )
- ee_flags |= MBEDTLS_X509_BADCERT_BAD_PK;
+ ee_flags |= MBEDTLS_X509_BADCERT_BAD_PK | X509_BADCERT_FI_EXTRA;
if( x509_profile_check_key( profile, pk ) != 0 )
- ee_flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
+ ee_flags |= MBEDTLS_X509_BADCERT_BAD_KEY | X509_BADCERT_FI_EXTRA;
mbedtls_x509_crt_pk_release( crt );
}
@@ -3823,10 +3858,19 @@
return( ret );
}
- if( *flags != 0 )
- return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED );
+ flags_fi = *flags;
+ if( flags_fi == 0 )
+ {
+ mbedtls_platform_enforce_volatile_reads();
+ if( flags_fi == 0 )
+ return( 0 );
+ }
- return( 0 );
+ /* Preserve the API by removing internal extra bits - from now on the
+ * fact that flags is non-zero is also redundantly encoded by the
+ * non-zero return value from this function. */
+ *flags &= ~ X509_BADCERT_FI_EXTRA;
+ return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED );
}
/*
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 02f96a9..42ef32d 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -1491,7 +1491,7 @@
msg "build: arm-none-eabi-gcc MBEDTLS_NO_64BIT_MULTIPLICATION, make" # ~ 10s
scripts/config.pl baremetal
scripts/config.pl set MBEDTLS_NO_64BIT_MULTIPLICATION
- make CC=arm-none-eabi-gcc AR=arm-none-eabi-ar LD=arm-none-eabi-ld CFLAGS='-Werror -O1 -march=armv6-m -mthumb' lib
+ make TINYCRYPT_BUILD=0 CC=arm-none-eabi-gcc AR=arm-none-eabi-ar LD=arm-none-eabi-ld CFLAGS='-Werror -O1 -march=armv6-m -mthumb' lib
echo "Checking that software 64-bit multiplication is not required"
if_build_succeeded not grep __aeabi_lmul library/*.o
}
diff --git a/tests/scripts/list-symbols.sh b/tests/scripts/list-symbols.sh
index 930722c..12b3281 100755
--- a/tests/scripts/list-symbols.sh
+++ b/tests/scripts/list-symbols.sh
@@ -16,7 +16,7 @@
scripts/config.pl full
make clean
make_ret=
-CFLAGS=-fno-asynchronous-unwind-tables make lib \
+CFLAGS=-fno-asynchronous-unwind-tables TINYCRYPT_BUILD=0 make lib \
>list-symbols.make.log 2>&1 ||
{
make_ret=$?
diff --git a/tests/suites/test_suite_tinycrypt.data b/tests/suites/test_suite_tinycrypt.data
index ac2a8e2..2c4d54b 100644
--- a/tests/suites/test_suite_tinycrypt.data
+++ b/tests/suites/test_suite_tinycrypt.data
@@ -8,4 +8,4 @@
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 24b331d..664cd08 100644
--- a/tests/suites/test_suite_tinycrypt.function
+++ b/tests/suites/test_suite_tinycrypt.function
@@ -55,7 +55,7 @@
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 @@
/* 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 @@
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 @@
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 d01c676..38b11d9 100644
--- a/tinycrypt/ecc.c
+++ b/tinycrypt/ecc.c
@@ -63,7 +63,6 @@
#include MBEDTLS_CONFIG_FILE
#endif
-#if defined(MBEDTLS_USE_TINYCRYPT)
#include <tinycrypt/ecc.h>
#include "mbedtls/platform_util.h"
#include <string.h>
@@ -185,7 +184,7 @@
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)
@@ -1114,7 +1113,4 @@
curve->num_bytes, curve->num_bytes, _public + curve->num_words);
return 1;
}
-#else
-typedef int mbedtls_dummy_tinycrypt_def;
-#endif /* MBEDTLS_USE_TINYCRYPT */
diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c
index f9c0a5e..3ab7b15 100644
--- a/tinycrypt/ecc_dh.c
+++ b/tinycrypt/ecc_dh.c
@@ -66,7 +66,6 @@
#include MBEDTLS_CONFIG_FILE
#endif
-#if defined(MBEDTLS_USE_TINYCRYPT)
#include <tinycrypt/ecc.h>
#include <tinycrypt/ecc_dh.h>
#include <string.h>
@@ -188,6 +187,3 @@
return r;
}
-#else
-typedef int mbedtls_dummy_tinycrypt_def;
-#endif /* MBEDTLS_USE_TINYCRYPT */
diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c
index 04b1bfa..67b33a4 100644
--- a/tinycrypt/ecc_dsa.c
+++ b/tinycrypt/ecc_dsa.c
@@ -64,9 +64,9 @@
#include MBEDTLS_CONFIG_FILE
#endif
-#if defined(MBEDTLS_USE_TINYCRYPT)
#include <tinycrypt/ecc.h>
#include <tinycrypt/ecc_dsa.h>
+#include "mbedtls/platform_util.h"
#if default_RNG_defined
static uECC_RNG_Function g_rng_function = &default_CSPRNG;
@@ -214,6 +214,7 @@
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 +236,13 @@
/* 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,8 +302,16 @@
}
/* 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;
-#endif /* MBEDTLS_USE_TINYCRYPT */