Merge pull request #736 from mpg/cf-varpos-copy-dev-restricted

Constant-flow copy of HMAC from variable position

tests/include/test/constant_flow.h

@@ -30,6 +30,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>
 
@@ -39,11 +61,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 */

tests/scripts/all.sh

@@ -1109,6 +1109,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.

tests/suites/test_suite_ssl.data

@@ -10545,3 +10545,15 @@ ssl_cf_hmac:MBEDTLS_MD_SHA256
 Constant-flow HMAC: SHA384
 depends_on:MBEDTLS_SHA512_C:!MBEDTLS_SHA512_NO_SHA384
 ssl_cf_hmac:MBEDTLS_MD_SHA384
+
+# these are the numbers we'd get with an empty plaintext and truncated HMAC
+Constant-flow memcpy from offset: small
+ssl_cf_memcpy_offset:0:5:10
+
+# we could get this with 255-bytes plaintext and untruncated SHA-256
+Constant-flow memcpy from offset: medium
+ssl_cf_memcpy_offset:0:255:32
+
+# we could get this with 255-bytes plaintext and untruncated SHA-384
+Constant-flow memcpy from offset: large
+ssl_cf_memcpy_offset:100:339:48

tests/suites/test_suite_ssl.function

@@ -4361,3 +4361,36 @@ exit:
     mbedtls_free( out );
 }
 /* END_CASE */
+
+/* BEGIN_CASE depends_on:MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC:MBEDTLS_TEST_HOOKS */
+void ssl_cf_memcpy_offset( int offset_min, int offset_max, int len )
+{
+    unsigned char *dst = NULL;
+    unsigned char *src = NULL;
+    size_t src_len = offset_max + len;
+    size_t secret;
+
+    ASSERT_ALLOC( dst, len );
+    ASSERT_ALLOC( src, src_len );
+
+    /* Fill src in a way that we can detect if we copied the right bytes */
+    mbedtls_test_rnd_std_rand( NULL, src, src_len );
+
+    for( secret = offset_min; secret <= (size_t) offset_max; secret++ )
+    {
+        test_set_step( (int) secret );
+
+        TEST_CF_SECRET( &secret, sizeof( secret ) );
+        mbedtls_ssl_cf_memcpy_offset( dst, src, secret,
+                                      offset_min, offset_max, len );
+        TEST_CF_PUBLIC( &secret, sizeof( secret ) );
+        TEST_CF_PUBLIC( dst, len );
+
+        ASSERT_COMPARE( dst, len, src + secret, len );
+    }
+
+exit:
+    mbedtls_free( dst );
+    mbedtls_free( src );
+}
+/* END_CASE */

tests/suites/test_suite_x509write.function

@@ -5,11 +5,25 @@
 #include "mbedtls/pem.h"
 #include "mbedtls/oid.h"
 #include "mbedtls/rsa.h"
-#if defined(MBEDTLS_USE_PSA_CRYPTO)
+
+/* These are the same depends as the test function x509_crs_check_opaque(),
+ * the only function using PSA here. Using a weaker condition would result in
+ * warnings about the static functions defined in psa_crypto_helpers.h being
+ * unused. */
+#if defined(MBEDTLS_USE_PSA_CRYPTO) && \
+    defined(MBEDTLS_PEM_WRITE_C) && \
+    defined(MBEDTLS_X509_CSR_WRITE_C)
 #include "psa/crypto.h"
 #include "mbedtls/psa_util.h"
-#endif
-
+#include "test/psa_crypto_helpers.h"
+#define PSA_INIT( ) PSA_ASSERT( psa_crypto_init( ) )
+#else
+/* Define empty macros so that we can use them in the preamble and teardown
+ * of every test function that uses PSA conditionally based on
+ * MBEDTLS_USE_PSA_CRYPTO. */
+#define PSA_INIT( ) ( (void) 0 )
+#define PSA_DONE( ) ( (void) 0 )
+#endif /* MBEDTLS_USE_PSA_CRYPTO && MBEDTLS_PEM_WRITE_C && MBEDTLS_X509_CSR_WRITE_C */
 
 #if defined(MBEDTLS_RSA_C)
 int mbedtls_rsa_decrypt_func( void *ctx, int mode, size_t *olen,
@@ -147,7 +161,7 @@ void x509_csr_check_opaque( char *key_file, int md_type, int key_usage,
                                  int cert_type )
 {
     mbedtls_pk_context key;
-    psa_key_handle_t slot;
+    psa_key_handle_t slot = 0;
     psa_algorithm_t md_alg_psa;
     mbedtls_x509write_csr req;
     unsigned char buf[4096];
@@ -156,7 +170,7 @@ void x509_csr_check_opaque( char *key_file, int md_type, int key_usage,
     const char *subject_name = "C=NL,O=PolarSSL,CN=PolarSSL Server 1";
     mbedtls_test_rnd_pseudo_info rnd_info;
 
-    psa_crypto_init();
+    PSA_INIT( );
     memset( &rnd_info, 0x2a, sizeof( mbedtls_test_rnd_pseudo_info ) );
 
     md_alg_psa = mbedtls_psa_translate_md( (mbedtls_md_type_t) md_type );
@@ -184,9 +198,12 @@ void x509_csr_check_opaque( char *key_file, int md_type, int key_usage,
     buf[pem_len] = '\0';
     TEST_ASSERT( x509_crt_verifycsr( buf, pem_len + 1 ) == 0 );
 
+
 exit:
     mbedtls_x509write_csr_free( &req );
     mbedtls_pk_free( &key );
+    psa_destroy_key( slot );
+    PSA_DONE( );
 }
 /* END_CASE */