From f08284769d770ea9ed2840c4d66d455b6877435a 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 +++ programs/ssl/query_config.c | 8 ++++++++ scripts/config.pl | 1 + tests/scripts/all.sh | 22 ++++++++++++++++++++++ tests/suites/helpers.function | 32 +++++++++++++++++++++++++++++++- 6 files changed, 82 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 751fa0a72..a00eec5e4 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -565,6 +565,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 de14a2ff6..cbf38dc2c 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -280,6 +280,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/programs/ssl/query_config.c b/programs/ssl/query_config.c index 7c8fbd8cd..798c9178f 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -747,6 +747,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN */ +#if defined(MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND) + if( strcmp( "MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND ); + return( 0 ); + } +#endif /* MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND */ + #if defined(MBEDTLS_TEST_NULL_ENTROPY) if( strcmp( "MBEDTLS_TEST_NULL_ENTROPY", config ) == 0 ) { diff --git a/scripts/config.pl b/scripts/config.pl index 1cf143506..e5cc69756 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -127,6 +127,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 8317cabc4..83299c97f 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1047,6 +1047,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 0b3d44b66..4f46ea967 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -46,6 +46,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 @@ -55,7 +76,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)