Merge pull request #741 from mpg/cf-varpos-copy-2.16-restricted

[backport 2.16] Constant-flow copy of HMAC from variable position
This commit is contained in:
Janos Follath 2020-08-25 14:35:45 +01:00 committed by GitHub
commit 5b2e60dc36
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 185 additions and 33 deletions

View file

@ -554,9 +554,10 @@
*
* Enable testing of the constant-flow nature of some sensitive functions with
* clang's MemorySanitizer. This causes some existing tests to also test
* non-functional properties of the code under test.
* this non-functional property of the code under test.
*
* This setting requires compiling with clang -fsanitize=memory.
* This setting requires compiling with clang -fsanitize=memory. The test
* suites can then be run normally.
*
* \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.
@ -565,6 +566,25 @@
*/
//#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
* this non-functional property 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. This can be
* done for an individual test suite with 'valgrind ./test_suite_xxx', or when
* using CMake, this can be done for all test suites with 'make 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
*

View file

@ -900,6 +900,30 @@ int mbedtls_ssl_cf_hmac(
const unsigned char *data, size_t data_len_secret,
size_t min_data_len, size_t max_data_len,
unsigned char *output );
/** \brief Copy data from a secret position with constant flow.
*
* This function copies \p len bytes from \p src_base + \p offset_secret to \p
* dst, with a code flow and memory access pattern that does not depend on \p
* offset_secret, but only on \p offset_min, \p offset_max and \p len.
*
* \param dst The destination buffer. This must point to a writable
* buffer of at least \p len bytes.
* \param src_base The base of the source buffer. This must point to a
* readable buffer of at least \p offset_max + \p len
* bytes.
* \param offset_secret The offset in the source buffer from which to copy.
* This must be no less than \p offset_min and no greater
* than \p offset_max.
* \param offset_min The minimal value of \p offset_secret.
* \param offset_max The maximal value of \p offset_secret.
* \param len The number of bytes to copy.
*/
void mbedtls_ssl_cf_memcpy_offset( unsigned char *dst,
const unsigned char *src_base,
size_t offset_secret,
size_t offset_min, size_t offset_max,
size_t len );
#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
#ifdef __cplusplus

View file

@ -1435,27 +1435,6 @@ static void ssl_mac( mbedtls_md_context_t *md_ctx,
#define SSL_SOME_MODES_USE_MAC
#endif
/* The function below is only used in the Lucky 13 counter-measure in
* ssl_decrypt_buf(). These are the defines that guard the call site. */
#if defined(SSL_SOME_MODES_USE_MAC) && \
( defined(MBEDTLS_SSL_PROTO_TLS1) || \
defined(MBEDTLS_SSL_PROTO_TLS1_1) || \
defined(MBEDTLS_SSL_PROTO_TLS1_2) )
/* This function makes sure every byte in the memory region is accessed
* (in ascending addresses order) */
static void ssl_read_memory( const unsigned char *p, size_t len )
{
unsigned char acc = 0;
volatile unsigned char force;
for( ; len != 0; p++, len-- )
acc ^= *p;
force = acc;
(void) force;
}
#endif /* SSL_SOME_MODES_USE_MAC && ( TLS1 || TLS1_1 || TLS1_2 ) */
/*
* Encryption/decryption functions
*/
@ -1931,6 +1910,26 @@ cleanup:
mbedtls_md_free( &aux );
return( ret );
}
/*
* Constant-flow memcpy from variable position in buffer.
* - functionally equivalent to memcpy(dst, src + offset_secret, len)
* - but with execution flow independent from the value of offset_secret.
*/
void mbedtls_ssl_cf_memcpy_offset( unsigned char *dst,
const unsigned char *src_base,
size_t offset_secret,
size_t offset_min, size_t offset_max,
size_t len )
{
size_t offset;
for( offset = offset_min; offset <= offset_max; offset++ )
{
mbedtls_ssl_cf_memcpy_if_eq( dst, src_base + offset, len,
offset, offset_secret );
}
}
#endif /* MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
@ -2319,6 +2318,7 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
if( auth_done == 0 )
{
unsigned char mac_expect[MBEDTLS_SSL_MAC_ADD];
unsigned char mac_peer[MBEDTLS_SSL_MAC_ADD];
ssl->in_msglen -= ssl->transform_in->maclen;
@ -2333,6 +2333,8 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
ssl->in_msg, ssl->in_msglen,
ssl->in_ctr, ssl->in_msgtype,
mac_expect );
memcpy( mac_peer, ssl->in_msg + ssl->in_msglen,
ssl->transform_in->maclen );
}
else
#endif /* MBEDTLS_SSL_PROTO_SSL3 */
@ -2371,12 +2373,10 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
return( ret );
}
/* Make sure we access all the memory that could contain the MAC,
* before we check it in the next code block. This makes the
* synchronisation requirements for just-in-time Prime+Probe
* attacks much tighter and hopefully impractical. */
ssl_read_memory( ssl->in_msg + min_len,
max_len - min_len + ssl->transform_in->maclen );
mbedtls_ssl_cf_memcpy_offset( mac_peer, ssl->in_msg,
ssl->in_msglen,
min_len, max_len,
ssl->transform_in->maclen );
}
else
#endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \
@ -2388,11 +2388,10 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
#if defined(MBEDTLS_SSL_DEBUG_ALL)
MBEDTLS_SSL_DEBUG_BUF( 4, "expected mac", mac_expect, ssl->transform_in->maclen );
MBEDTLS_SSL_DEBUG_BUF( 4, "message mac", ssl->in_msg + ssl->in_msglen,
ssl->transform_in->maclen );
MBEDTLS_SSL_DEBUG_BUF( 4, "message mac", mac_peer, ssl->transform_in->maclen );
#endif
if( mbedtls_ssl_safer_memcmp( ssl->in_msg + ssl->in_msglen, mac_expect,
if( mbedtls_ssl_safer_memcmp( mac_peer, mac_expect,
ssl->transform_in->maclen ) != 0 )
{
#if defined(MBEDTLS_SSL_DEBUG_ALL)

View file

@ -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 */

View file

@ -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 )
{

View file

@ -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

View file

@ -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/<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.

View file

@ -46,6 +46,27 @@ typedef UINT32 uint32_t;
#include <strings.h>
#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 <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>
@ -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 <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)

View file

@ -73,3 +73,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 355-bytes plaintext and untruncated SHA-384
Constant-flow memcpy from offset: large
ssl_cf_memcpy_offset:100:339:48

View file

@ -144,3 +144,36 @@ exit:
mbedtls_free( out );
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_SSL_SOME_SUITES_USE_TLS_CBC */
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;
dst = mbedtls_calloc( 1, len );
TEST_ASSERT( dst != NULL );
src = mbedtls_calloc( 1, src_len );
TEST_ASSERT( src != NULL );
/* Fill src in a way that we can detect if we copied the right bytes */
rnd_std_rand( NULL, src, src_len );
for( secret = offset_min; secret <= (size_t) offset_max; 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 );
TEST_ASSERT( memcmp( dst, src + secret, len ) == 0 );
}
exit:
mbedtls_free( dst );
mbedtls_free( src );
}
/* END_CASE */