From 426c2d4a388d087cea397016f4fcd64b7355106f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 25 Aug 2020 11:26:37 +0200 Subject: [PATCH] Add an option to test constant-flow with valgrind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the new component in all.sh fails because mbedtls_ssl_cf_memcpy_offset() is not actually constant flow - this is on purpose to be able to verify that the new test works. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/config.h | 17 +++++++++++++++++ library/version_features.c | 3 +++ scripts/config.pl | 1 + tests/scripts/all.sh | 22 ++++++++++++++++++++++ tests/suites/helpers.function | 32 +++++++++++++++++++++++++++++++- 5 files changed, 74 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 82d8d8b7f..8f3dba97d 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -458,6 +458,23 @@ */ //#define MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN +/** + * \def MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND + * + * Enable testing of the constant-flow nature of some sensitive functions with + * valgrind's memcheck tool. This causes some existing tests to also test + * non-functional properties of the code under test. + * + * This setting requires valgrind headers for building, and is only useful for + * testing if the tests suites are run with valgrind's memcheck. + * + * \warning This macro is only used for extended testing; it is not considered + * part of the library's API, so it may change or disappear at any time. + * + * Uncomment to enable testing of the constant-flow nature of selected code. + */ +//#define MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND + /** * \def MBEDTLS_TEST_NULL_ENTROPY * diff --git a/library/version_features.c b/library/version_features.c index dbe6ba6ad..a3e3df4f9 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -256,6 +256,9 @@ static const char *features[] = { #if defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) "MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN", #endif /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN */ +#if defined(MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND) + "MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND", +#endif /* MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND */ #if defined(MBEDTLS_TEST_NULL_ENTROPY) "MBEDTLS_TEST_NULL_ENTROPY", #endif /* MBEDTLS_TEST_NULL_ENTROPY */ diff --git a/scripts/config.pl b/scripts/config.pl index 2dadc87c0..80b6cd88c 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -125,6 +125,7 @@ MBEDTLS_REMOVE_ARC4_CIPHERSUITES MBEDTLS_RSA_NO_CRT MBEDTLS_SSL_HW_RECORD_ACCEL MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN +MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND MBEDTLS_TEST_NULL_ENTROPY MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION MBEDTLS_ZLIB_SUPPORT diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 81396945b..50f98eb29 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -979,6 +979,28 @@ component_test_memsan_constant_flow () { make test } +component_test_valgrind_constant_flow () { + # This tests both (1) everything that valgrind's memcheck usually checks + # (heap buffer overflows, use of uninitialized memory, use-after-free, + # etc.) and (2) branches or memory access depending on secret values, + # which will be reported as uninitialized memory. To distinguish between + # secret and actually uninitialized: + # - unset MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND - does the failure persist? + # - or alternatively, build with debug info and manually run the offending + # test suite with valgrind --track-origins=yes, then check if the origin + # was TEST_CF_SECRET() or something else. + msg "build: cmake release GCC, full config with constant flow testing" + scripts/config.pl full + scripts/config.pl set MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND + cmake -D CMAKE_BUILD_TYPE:String=Release . + make + + # this only shows a summary of the results (how many of each type) + # details are left in Testing//DynamicAnalysis.xml + msg "test: main suites (valgrind + constant flow)" + make memcheck +} + component_test_default_no_deprecated () { # Test that removing the deprecated features from the default # configuration leaves something consistent. diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 3309173d3..c1c76ec09 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -38,6 +38,27 @@ typedef UINT32 uint32_t; #include #endif +/* + * Define the two macros + * + * #define TEST_CF_SECRET(ptr, size) + * #define TEST_CF_PUBLIC(ptr, size) + * + * that can be used in tests to mark a memory area as secret (no branch or + * memory access should depend on it) or public (default, only needs to be + * marked explicitly when it was derived from secret data). + * + * Arguments: + * - ptr: a pointer to the memory area to be marked + * - size: the size in bytes of the memory area + * + * Implementation: + * The basic idea is that of ctgrind : we can + * re-use tools that were designed for checking use of uninitialized memory. + * This file contains two implementations: one based on MemorySanitizer, the + * other on valgrind's memcheck. If none of them is enabled, dummy macros that + * do nothing are defined for convenience. + */ #if defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) #include @@ -47,7 +68,16 @@ typedef UINT32 uint32_t; #define TEST_CF_PUBLIC __msan_unpoison // void __msan_unpoison(const volatile void *a, size_t size); -#else /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN */ +#elif defined(MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND) +#include + +#define TEST_CF_SECRET VALGRIND_MAKE_MEM_UNDEFINED +// VALGRIND_MAKE_MEM_UNDEFINED(_qzz_addr, _qzz_len) +#define TEST_CF_PUBLIC VALGRIND_MAKE_MEM_DEFINED +// VALGRIND_MAKE_MEM_DEFINED(_qzz_addr, _qzz_len) + +#else /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN || + MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND */ #define TEST_CF_SECRET(ptr, size) #define TEST_CF_PUBLIC(ptr, size)