diff --git a/library/constant_time.c b/library/constant_time.c index f2cdddf96..6523ccf15 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -30,6 +30,8 @@ #include "mbedtls/error.h" #include "mbedtls/platform_util.h" +#include "../tests/include/test/constant_flow.h" + #include #if defined(MBEDTLS_USE_PSA_CRYPTO) && defined(MBEDTLS_SSL_SOME_SUITES_USE_MAC) @@ -127,6 +129,20 @@ int mbedtls_ct_memcmp(const void *a, void mbedtls_ct_memmove_left(void *start, size_t total, size_t offset) { + /* In case of inlining, ensure that code generated is independent of the value of offset + * (e.g., if the compiler knows that offset == 0, it might be able to optimise this function + * to a no-op). */ + size_t hidden_offset = mbedtls_ct_compiler_opaque(offset); + + /* During this loop, j will take every value from [0..total) exactly once, + * regardless of the value of hidden_offset (it only changes the initial + * value for j). + * + * For this reason, when testing, it is safe to mark hidden_offset as non-secret. + * This prevents the const-flow checkers from generating a false-positive. + */ + TEST_CF_PUBLIC(&hidden_offset, sizeof(hidden_offset)); + /* Iterate over the array, reading each byte once and writing each byte once. */ for (size_t i = 0; i < total; i++) { /* Each iteration, read one byte, and write it to start[i]. @@ -138,9 +154,8 @@ void mbedtls_ct_memmove_left(void *start, size_t total, size_t offset) * If the source address is out of range, mask it to zero. */ - // The address that we will read from - // TODO: if offset is marked as secret, this upsets Memsan. - size_t j = i + offset; + // The offset that we will read from (if in range) + size_t j = i + hidden_offset; // Is the address off the end of the array? mbedtls_ct_condition_t not_dummy = mbedtls_ct_bool_lt(j, total); diff --git a/tests/suites/test_suite_constant_time.function b/tests/suites/test_suite_constant_time.function index dbcc9f759..d8a1fccbe 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -329,11 +329,10 @@ void mbedtls_ct_memmove_left(int len, int offset) buf_expected[i] = buf[i]; } - //Note: Marking o as secret causes false positives from Memsan - //TEST_CF_SECRET(&o, sizeof(o)); + TEST_CF_SECRET(&o, sizeof(o)); TEST_CF_SECRET(buf, l); mbedtls_ct_memmove_left(buf, l, o); - //TEST_CF_PUBLIC(&o, sizeof(o)); + TEST_CF_PUBLIC(&o, sizeof(o)); TEST_CF_PUBLIC(buf, l); if (l > 0) {