Merge remote-tracking branch 'public/pr/2934' 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/platform.h b/include/mbedtls/platform.h
index 82d5e33..ec1df15 100644
--- a/include/mbedtls/platform.h
+++ b/include/mbedtls/platform.h
@@ -39,14 +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 */
-#define MBEDTLS_ERR_PLATFORM_FAULT_DETECTED -0x0071 /**< A fault was detected in a critical path, likely indicative of an active physical attack */
-
#ifdef __cplusplus
extern "C" {
#endif
@@ -365,4 +367,6 @@
}
#endif
+#endif /* MBEDTLS_PLATFORM_C */
+
#endif /* platform.h */
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 0d1d9ec..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__
@@ -535,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 55b9d43..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__
@@ -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 77c7133..74c9d0b 100644
--- a/library/error.c
+++ b/library/error.c
@@ -842,7 +842,7 @@
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 fault was detected in a critical path, likely indicative of an active physical attack" );
+ 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/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/tinycrypt/ecc.c b/tinycrypt/ecc.c
index 92906fd..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>
@@ -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 6c171c3..67b33a4 100644
--- a/tinycrypt/ecc_dsa.c
+++ b/tinycrypt/ecc_dsa.c
@@ -64,7 +64,6 @@
#include MBEDTLS_CONFIG_FILE
#endif
-#if defined(MBEDTLS_USE_TINYCRYPT)
#include <tinycrypt/ecc.h>
#include <tinycrypt/ecc_dsa.h>
#include "mbedtls/platform_util.h"
@@ -316,6 +315,3 @@
return UECC_FAILURE;
}
-#else
-typedef int mbedtls_dummy_tinycrypt_def;
-#endif /* MBEDTLS_USE_TINYCRYPT */