diff --git a/library/constant_time.c b/library/constant_time.c index d3c69cfa8..55e7f9435 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -22,6 +22,7 @@ * might be translated to branches by some compilers on some platforms. */ +#include #include #include "common.h" @@ -120,7 +121,24 @@ int mbedtls_ct_memcmp(const void *a, diff |= x ^ y; } - return (int) diff; + +#if (INT_MAX < INT32_MAX) + /* We don't support int smaller than 32-bits, but if someone tried to build + * with this configuration, there is a risk that, for differing data, the + * only bits set in diff are in the top 16-bits, and would be lost by a + * simple cast from uint32 to int. + * This would have significant security implications, so protect against it. */ +#error "mbedtls_ct_memcmp() requires minimum 32-bit ints" +#else + /* The bit-twiddling ensures that when we cast uint32_t to int, we are casting + * a value that is in the range 0..INT_MAX - a value larger than this would + * result in implementation defined behaviour. + * + * This ensures that the value returned by the function is non-zero iff + * diff is non-zero. + */ + return (int) ((diff & 0xffff) | (diff >> 16)); +#endif } #if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) diff --git a/tests/suites/test_suite_constant_time.data b/tests/suites/test_suite_constant_time.data index 1b0b964da..785bdec29 100644 --- a/tests/suites/test_suite_constant_time.data +++ b/tests/suites/test_suite_constant_time.data @@ -91,6 +91,9 @@ mbedtls_ct_memcmp:-1:17:2 mbedtls_ct_memcmp len 17 offset 3 mbedtls_ct_memcmp:-1:17:3 +mbedtls_ct_memcmp_single_bit_diff +mbedtls_ct_memcmp_single_bit_diff: + mbedtls_ct_memcpy_if len 1 offset 0 mbedtls_ct_memcpy_if:1:1:0 diff --git a/tests/suites/test_suite_constant_time.function b/tests/suites/test_suite_constant_time.function index 0e2cfdc0c..5dbba063e 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -172,6 +172,49 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mbedtls_ct_memcmp_single_bit_diff() +{ + uint8_t *a = NULL, *b = NULL; + size_t size = 32; + TEST_CALLOC(a, size); + TEST_CALLOC(b, size); + + TEST_CF_SECRET(a, size); + TEST_CF_SECRET(b, size); + int result = mbedtls_ct_memcmp(a, b, size); + TEST_CF_PUBLIC(a, size); + TEST_CF_PUBLIC(b, size); + TEST_CF_PUBLIC(&result, sizeof(result)); + + TEST_EQUAL(result, 0); + + for (size_t offset = 0; offset < size; offset++) { + for (size_t bit_offset = 0; bit_offset < 8; bit_offset++) { + /* Set a single bit to be different at given offset, to test that we + detect single-bit differences */ + a[offset] = 1 << bit_offset; + + TEST_CF_SECRET(a, size); + TEST_CF_SECRET(b, size); + result = mbedtls_ct_memcmp(a, b, size); + TEST_CF_PUBLIC(a, size); + TEST_CF_PUBLIC(b, size); + TEST_CF_PUBLIC(&result, sizeof(result)); + + TEST_ASSERT(result != 0); + + a[offset] = 0; + } + } + + +exit: + mbedtls_free(a); + mbedtls_free(b); +} +/* END_CASE */ + /* BEGIN_CASE */ void mbedtls_ct_memcmp(int same, int size, int offset) {