From 73afa37507dbb6ec76fbed2e21a9305fbb1d6c09 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
<manuel.pegourie-gonnard@arm.com>
Date: Wed, 19 Aug 2020 10:27:38 +0200
Subject: [PATCH] Add 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 <manuel.pegourie-gonnard@arm.com>
---
include/mbedtls/config.h | 17 ++++++++++++++
library/version_features.c | 3 +++
programs/test/query_config.c | 8 +++++++
scripts/config.py | 1 +
tests/include/test/constant_flow.h | 36 ++++++++++++++++++++++++++++--
tests/scripts/all.sh | 22 ++++++++++++++++++
6 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index 5eceea1bc..6337258dc 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -1924,6 +1924,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_HOOKS
*
diff --git a/library/version_features.c b/library/version_features.c
index 2470d8d1d..16719a6eb 100644
--- a/library/version_features.c
+++ b/library/version_features.c
@@ -560,6 +560,9 @@ static const char * const 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_HOOKS)
"MBEDTLS_TEST_HOOKS",
#endif /* MBEDTLS_TEST_HOOKS */
diff --git a/programs/test/query_config.c b/programs/test/query_config.c
index 23dc51512..c242ab511 100644
--- a/programs/test/query_config.c
+++ b/programs/test/query_config.c
@@ -1546,6 +1546,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_HOOKS)
if( strcmp( "MBEDTLS_TEST_HOOKS", config ) == 0 )
{
diff --git a/scripts/config.py b/scripts/config.py
index 793e9dfa7..37a2475e6 100755
--- a/scripts/config.py
+++ b/scripts/config.py
@@ -195,6 +195,7 @@ EXCLUDE_FROM_FULL = frozenset([
'MBEDTLS_SHA512_NO_SHA384', # removes a feature
'MBEDTLS_SSL_HW_RECORD_ACCEL', # build dependency (hook functions)
'MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN', # build dependency (clang+memsan)
+ 'MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND', # build dependency (valgrind headers)
'MBEDTLS_TEST_NULL_ENTROPY', # removes a feature
'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', # influences the use of X.509 in TLS
'MBEDTLS_ZLIB_SUPPORT', # build dependency (libz)
diff --git a/tests/include/test/constant_flow.h b/tests/include/test/constant_flow.h
index 98bee7e48..9307a2513 100644
--- a/tests/include/test/constant_flow.h
+++ b/tests/include/test/constant_flow.h
@@ -32,6 +32,28 @@
#include MBEDTLS_CONFIG_FILE
#endif
+/*
+ * This file defines 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 <https://github.com/agl/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 <sanitizer/msan_interface.h>
@@ -41,11 +63,21 @@
#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 <valgrind/memcheck.h>
+
+#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)
-#endif /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN */
+#endif /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN ||
+ MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND */
#endif /* TEST_CONSTANT_FLOW_H */
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 6cc0cf806..caeb2fe79 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -1111,6 +1111,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.py full
+ scripts/config.py 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/<date>/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.