Merge pull request #1059 from daverodgman/ct_memcmp_fix
Constant time memcmp check for 16-bit int
diff --git a/library/constant_time.c b/library/constant_time.c
index d3c69cf..55e7f94 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 <stdint.h>
#include <limits.h>
#include "common.h"
@@ -120,7 +121,24 @@
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 1b0b964..785bdec 100644
--- a/tests/suites/test_suite_constant_time.data
+++ b/tests/suites/test_suite_constant_time.data
@@ -91,6 +91,9 @@
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 0e2cfdc..5dbba06 100644
--- a/tests/suites/test_suite_constant_time.function
+++ b/tests/suites/test_suite_constant_time.function
@@ -173,6 +173,49 @@
/* 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)
{
uint8_t *a = NULL, *b = NULL;