From d26a3d6da72e12e93d3384939bcc4241e47bcc99 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 11 Sep 2023 18:25:16 +0100 Subject: [PATCH 01/14] Eliminate duplicate ct memcmp Signed-off-by: Dave Rodgman --- library/ccm.c | 8 ++------ library/chachapoly.c | 6 ++---- library/gcm.c | 6 ++---- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/library/ccm.c b/library/ccm.c index cd689c806..7b297754c 100644 --- a/library/ccm.c +++ b/library/ccm.c @@ -33,6 +33,7 @@ #include "mbedtls/ccm.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "mbedtls/constant_time.h" #include @@ -533,13 +534,8 @@ static int mbedtls_ccm_compare_tags(const unsigned char *tag1, const unsigned char *tag2, size_t tag_len) { - unsigned char i; - int diff; - /* Check tag in "constant-time" */ - for (diff = 0, i = 0; i < tag_len; i++) { - diff |= tag1[i] ^ tag2[i]; - } + int diff = mbedtls_ct_memcmp(tag1, tag2, tag_len); if (diff != 0) { return MBEDTLS_ERR_CCM_AUTH_FAILED; diff --git a/library/chachapoly.c b/library/chachapoly.c index 0124d7570..aebc646aa 100644 --- a/library/chachapoly.c +++ b/library/chachapoly.c @@ -25,6 +25,7 @@ #include "mbedtls/chachapoly.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "mbedtls/constant_time.h" #include @@ -310,7 +311,6 @@ int mbedtls_chachapoly_auth_decrypt(mbedtls_chachapoly_context *ctx, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char check_tag[16]; - size_t i; int diff; if ((ret = chachapoly_crypt_and_tag(ctx, @@ -320,9 +320,7 @@ int mbedtls_chachapoly_auth_decrypt(mbedtls_chachapoly_context *ctx, } /* Check tag in "constant-time" */ - for (diff = 0, i = 0; i < sizeof(check_tag); i++) { - diff |= tag[i] ^ check_tag[i]; - } + diff = mbedtls_ct_memcmp(tag, check_tag, sizeof(check_tag)); if (diff != 0) { mbedtls_platform_zeroize(output, length); diff --git a/library/gcm.c b/library/gcm.c index 786290f2f..b773529a4 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -35,6 +35,7 @@ #include "mbedtls/platform.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "mbedtls/constant_time.h" #include @@ -601,7 +602,6 @@ int mbedtls_gcm_auth_decrypt(mbedtls_gcm_context *ctx, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char check_tag[16]; - size_t i; int diff; if ((ret = mbedtls_gcm_crypt_and_tag(ctx, MBEDTLS_GCM_DECRYPT, length, @@ -611,9 +611,7 @@ int mbedtls_gcm_auth_decrypt(mbedtls_gcm_context *ctx, } /* Check tag in "constant-time" */ - for (diff = 0, i = 0; i < tag_len; i++) { - diff |= tag[i] ^ check_tag[i]; - } + diff = mbedtls_ct_memcmp(tag, check_tag, tag_len); if (diff != 0) { mbedtls_platform_zeroize(output, length); From 9c14007ac332294dcf8c276a715457ab9dd8f210 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 18 Sep 2023 18:20:27 +0100 Subject: [PATCH 02/14] Add mbedtls_ct_memcmp_partial Signed-off-by: Dave Rodgman --- library/constant_time.c | 28 ++++++++++++++++++++++++++++ library/constant_time_internal.h | 31 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/library/constant_time.c b/library/constant_time.c index 55e7f9435..fffc02f64 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -141,6 +141,34 @@ int mbedtls_ct_memcmp(const void *a, #endif } +#if defined(MBEDTLS_NIST_KW_C) + +int mbedtls_ct_memcmp_partial(const void *a, + const void *b, + size_t n, + size_t skip_head, + size_t skip_tail) +{ + unsigned int diff = 0; + + volatile const unsigned char *A = (volatile const unsigned char *) a; + volatile const unsigned char *B = (volatile const unsigned char *) b; + + size_t valid_end = n - skip_tail; + + for (size_t i = 0; i < n; i++) { + unsigned char x = A[i], y = B[i]; + int d = x ^ y; + mbedtls_ct_condition_t valid = mbedtls_ct_bool_and(mbedtls_ct_uint_ge(i, skip_head), + mbedtls_ct_uint_lt(i, valid_end)); + diff |= mbedtls_ct_uint_if_else_0(valid, d); + } + + return (int) ((diff & 0xffff) | (diff >> 16)); +} + +#endif + #if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) && !defined(MBEDTLS_RSA_ALT) void mbedtls_ct_memmove_left(void *start, size_t total, size_t offset) diff --git a/library/constant_time_internal.h b/library/constant_time_internal.h index 44b74aec6..323edef45 100644 --- a/library/constant_time_internal.h +++ b/library/constant_time_internal.h @@ -492,6 +492,37 @@ void mbedtls_ct_memcpy_offset(unsigned char *dest, size_t n); */ +#if defined(MBEDTLS_NIST_KW_C) + +/** Constant-time buffer comparison without branches. + * + * Similar to mbedtls_ct_memcmp, except that the result only depends on part of + * the input data - differences in the head or tail are ignored. Functionally equivalent to: + * + * memcmp(a + skip_head, b + skip_head, size - skip_head - skip_tail) + * + * Time taken depends on \p n, but not on \p skip_head or \p skip_tail . + * + * Behaviour is undefined if ( \p skip_head + \p skip_tail) > \p n. + * + * \param a Secret. Pointer to the first buffer, containing at least \p n bytes. May not be NULL. + * \param b Secret. Pointer to the second buffer, containing at least \p n bytes. May not be NULL. + * \param n The number of bytes to examine (total size of the buffers). + * \param skip_head Secret. The number of bytes to treat as non-significant at the start of the buffer. + * These bytes will still be read. + * \param skip_tail Secret. The number of bytes to treat as non-significant at the end of the buffer. + * These bytes will still be read. + * + * \return Zero if the contents of the two buffers are the same, otherwise non-zero. + */ +int mbedtls_ct_memcmp_partial(const void *a, + const void *b, + size_t n, + size_t skip_head, + size_t skip_tail); + +#endif + /* Include the implementation of static inline functions above. */ #include "constant_time_impl.h" From 771ac65b0ca5d2388b9c8ab7d06e87c26be395f3 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 18 Sep 2023 18:20:55 +0100 Subject: [PATCH 03/14] Add tests for mbedtls_ct_memcmp_partial Signed-off-by: Dave Rodgman --- tests/suites/test_suite_constant_time.data | 66 +++++++++++++++++++ .../suites/test_suite_constant_time.function | 38 +++++++++++ 2 files changed, 104 insertions(+) diff --git a/tests/suites/test_suite_constant_time.data b/tests/suites/test_suite_constant_time.data index 785bdec29..02b62f7c2 100644 --- a/tests/suites/test_suite_constant_time.data +++ b/tests/suites/test_suite_constant_time.data @@ -702,3 +702,69 @@ mbedtls_ct_memmove_left:16:15 mbedtls_ct_memmove_left 16 16 mbedtls_ct_memmove_left:16:16 + +mbedtls_ct_memcmp_partial -1 0 0 0 +mbedtls_ct_memcmp_partial:-1:0:0:0:0 + +mbedtls_ct_memcmp_partial 0 1 0 0 +mbedtls_ct_memcmp_partial:0:1:0:0:1 + +mbedtls_ct_memcmp_partial 0 1 1 0 +mbedtls_ct_memcmp_partial:0:1:1:0:0 + +mbedtls_ct_memcmp_partial 0 1 0 1 +mbedtls_ct_memcmp_partial:0:1:0:1:0 + +mbedtls_ct_memcmp_partial -1 1 0 0 +mbedtls_ct_memcmp_partial:-1:1:0:0:0 + +mbedtls_ct_memcmp_partial 0 2 0 1 +mbedtls_ct_memcmp_partial:0:2:0:1:1 + +mbedtls_ct_memcmp_partial 0 2 1 0 +mbedtls_ct_memcmp_partial:0:2:1:0:0 + +mbedtls_ct_memcmp_partial 0 16 4 4 +mbedtls_ct_memcmp_partial:0:16:4:4:0 + +mbedtls_ct_memcmp_partial 2 16 4 4 +mbedtls_ct_memcmp_partial:2:16:4:4:0 + +mbedtls_ct_memcmp_partial 3 16 4 4 +mbedtls_ct_memcmp_partial:3:16:4:4:0 + +mbedtls_ct_memcmp_partial 4 16 4 4 +mbedtls_ct_memcmp_partial:4:16:4:4:1 + +mbedtls_ct_memcmp_partial 7 16 4 4 +mbedtls_ct_memcmp_partial:7:16:4:4:1 + +mbedtls_ct_memcmp_partial 11 16 4 4 +mbedtls_ct_memcmp_partial:11:16:4:4:1 + +mbedtls_ct_memcmp_partial 12 16 4 4 +mbedtls_ct_memcmp_partial:12:16:4:4:0 + +mbedtls_ct_memcmp_partial 15 16 4 4 +mbedtls_ct_memcmp_partial:15:16:4:4:0 + +mbedtls_ct_memcmp_partial 15 16 4 0 +mbedtls_ct_memcmp_partial:15:16:4:0:1 + +mbedtls_ct_memcmp_partial 15 16 0 4 +mbedtls_ct_memcmp_partial:15:16:0:4:0 + +mbedtls_ct_memcmp_partial 0 16 0 0 +mbedtls_ct_memcmp_partial:0:16:0:0:1 + +mbedtls_ct_memcmp_partial 15 16 0 0 +mbedtls_ct_memcmp_partial:15:16:0:0:1 + +mbedtls_ct_memcmp_partial -1 16 0 0 +mbedtls_ct_memcmp_partial:-1:16:0:0:0 + +mbedtls_ct_memcmp_partial -1 16 12 4 +mbedtls_ct_memcmp_partial:-1:16:12:4:0 + +mbedtls_ct_memcmp_partial -1 16 8 8 +mbedtls_ct_memcmp_partial:-1:16:8:8:0 diff --git a/tests/suites/test_suite_constant_time.function b/tests/suites/test_suite_constant_time.function index 5dbba063e..d80610f70 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -259,6 +259,44 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_NIST_KW_C */ +void mbedtls_ct_memcmp_partial(int diff, int size, int skip_head, int skip_tail, int expected) +{ + uint8_t *a = NULL, *b = NULL; + TEST_CALLOC(a, size + 1); // + 1 to avoid NULL result from TEST_CALLOC(0) + TEST_CALLOC(b, size + 1); + + TEST_ASSERT((skip_head + skip_tail) <= size); + + /* Construct data that matches, except for specified byte (if in range). */ + for (int i = 0; i < size; i++) { + a[i] = i & 0xff; + if (i != diff) { + b[i] = a[i]; + } else { + b[i] = (i + 1) & 0xff; + } + } + + int reference = memcmp(a + skip_head, b + skip_head, size - skip_head - skip_tail); + + TEST_CF_SECRET(a, size); + TEST_CF_SECRET(b, size); + + int actual = mbedtls_ct_memcmp_partial(a, b, size, skip_head, skip_tail); + + TEST_CF_PUBLIC(a, size); + TEST_CF_PUBLIC(b, size); + TEST_CF_PUBLIC(&actual, sizeof(actual)); + + TEST_EQUAL(!!reference, !!actual); + TEST_EQUAL(!!expected, !!actual); +exit: + mbedtls_free(a); + mbedtls_free(b); +} +/* END_CASE */ + /* BEGIN_CASE */ void mbedtls_ct_memcpy_if(int eq, int size, int offset) { From d337bd9bfe1396f88019baf826e8866ca7a4700f Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 18 Sep 2023 18:22:27 +0100 Subject: [PATCH 04/14] Improve const-timeness of mbedtls_nist_kw_unwrap Signed-off-by: Dave Rodgman --- library/nist_kw.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/library/nist_kw.c b/library/nist_kw.c index fbd7221a4..15b5bc0f9 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -35,6 +35,7 @@ #include "mbedtls/platform_util.h" #include "mbedtls/error.h" #include "mbedtls/constant_time.h" +#include "constant_time_internal.h" #include #include @@ -333,9 +334,9 @@ int mbedtls_nist_kw_unwrap(mbedtls_nist_kw_context *ctx, unsigned char *output, size_t *out_len, size_t out_size) { int ret = 0; - size_t i, olen; + size_t olen; unsigned char A[KW_SEMIBLOCK_LENGTH]; - unsigned char diff, bad_padding = 0; + int diff; *out_len = 0; if (out_size < in_len - KW_SEMIBLOCK_LENGTH) { @@ -426,13 +427,10 @@ int mbedtls_nist_kw_unwrap(mbedtls_nist_kw_context *ctx, } /* Check padding in "constant-time" */ - for (diff = 0, i = 0; i < KW_SEMIBLOCK_LENGTH; i++) { - if (i >= KW_SEMIBLOCK_LENGTH - padlen) { - diff |= output[*out_len - KW_SEMIBLOCK_LENGTH + i]; - } else { - bad_padding |= output[*out_len - KW_SEMIBLOCK_LENGTH + i]; - } - } + const uint8_t zero[KW_SEMIBLOCK_LENGTH] = { 0 }; + diff = mbedtls_ct_memcmp_partial( + &output[*out_len - KW_SEMIBLOCK_LENGTH], zero, + KW_SEMIBLOCK_LENGTH, KW_SEMIBLOCK_LENGTH - padlen, 0); if (diff != 0) { ret = MBEDTLS_ERR_CIPHER_AUTH_FAILED; @@ -454,7 +452,6 @@ cleanup: *out_len = 0; } - mbedtls_platform_zeroize(&bad_padding, sizeof(bad_padding)); mbedtls_platform_zeroize(&diff, sizeof(diff)); mbedtls_platform_zeroize(A, sizeof(A)); From 66d6ac92e633186e16910c6ee3d037cec1fe4b86 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 18 Sep 2023 18:35:03 +0100 Subject: [PATCH 05/14] Use mbedtls_ct_memcmp in mbedtls_rsa_rsaes_oaep_decrypt Signed-off-by: Dave Rodgman --- library/rsa.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index d0782f53c..02626b377 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1541,7 +1541,8 @@ int mbedtls_rsa_rsaes_oaep_decrypt(mbedtls_rsa_context *ctx, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t ilen, i, pad_len; - unsigned char *p, bad, pad_done; + unsigned char *p, pad_done; + int bad; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; unsigned char lhash[MBEDTLS_MD_MAX_SIZE]; unsigned int hlen; @@ -1608,9 +1609,8 @@ int mbedtls_rsa_rsaes_oaep_decrypt(mbedtls_rsa_context *ctx, p += hlen; /* Skip seed */ /* Check lHash */ - for (i = 0; i < hlen; i++) { - bad |= lhash[i] ^ *p++; - } + bad |= mbedtls_ct_memcmp(lhash, p, hlen); + p += hlen; /* Get zero-padding len, but always read till end of buffer * (minus one, for the 01 byte) */ From c2630fac52e2d951994a35ab581334915fcd355d Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 14:13:41 +0100 Subject: [PATCH 06/14] Simplify mbedtls_ct_memcmp_partial Signed-off-by: Dave Rodgman --- library/constant_time.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/constant_time.c b/library/constant_time.c index fffc02f64..8b41aed19 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -158,13 +158,15 @@ int mbedtls_ct_memcmp_partial(const void *a, for (size_t i = 0; i < n; i++) { unsigned char x = A[i], y = B[i]; - int d = x ^ y; + unsigned int d = x ^ y; mbedtls_ct_condition_t valid = mbedtls_ct_bool_and(mbedtls_ct_uint_ge(i, skip_head), mbedtls_ct_uint_lt(i, valid_end)); diff |= mbedtls_ct_uint_if_else_0(valid, d); } - return (int) ((diff & 0xffff) | (diff >> 16)); + /* Since we go byte-by-byte, the only bits set will be in the bottom 8 bits, so the + * cast from uint to int is safe. */ + return (int) diff; } #endif From 51c15309f296570ebf931ebbdee7bc68c1f58ee6 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 17:09:13 +0100 Subject: [PATCH 07/14] Make padlen check const-time Signed-off-by: Dave Rodgman --- library/nist_kw.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/nist_kw.c b/library/nist_kw.c index 15b5bc0f9..3de1b6ade 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -421,10 +421,9 @@ int mbedtls_nist_kw_unwrap(mbedtls_nist_kw_context *ctx, * larger than 8, because of the type wrap around. */ padlen = in_len - KW_SEMIBLOCK_LENGTH - Plen; - if (padlen > 7) { - padlen &= 7; - ret = MBEDTLS_ERR_CIPHER_AUTH_FAILED; - } + ret = -((int) mbedtls_ct_uint_if_else_0(mbedtls_ct_uint_gt(padlen, 7), + -MBEDTLS_ERR_CIPHER_AUTH_FAILED)); + padlen &= 7; /* Check padding in "constant-time" */ const uint8_t zero[KW_SEMIBLOCK_LENGTH] = { 0 }; From ba600b2fd92979d13b21d2e87035d75defefc74a Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 17:26:13 +0100 Subject: [PATCH 08/14] Remove expected param from mbedtls_ct_memcmp_partial test Signed-off-by: Dave Rodgman --- tests/suites/test_suite_constant_time.data | 44 +++++++++---------- .../suites/test_suite_constant_time.function | 3 +- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/tests/suites/test_suite_constant_time.data b/tests/suites/test_suite_constant_time.data index 02b62f7c2..82ee869e4 100644 --- a/tests/suites/test_suite_constant_time.data +++ b/tests/suites/test_suite_constant_time.data @@ -704,67 +704,67 @@ mbedtls_ct_memmove_left 16 16 mbedtls_ct_memmove_left:16:16 mbedtls_ct_memcmp_partial -1 0 0 0 -mbedtls_ct_memcmp_partial:-1:0:0:0:0 +mbedtls_ct_memcmp_partial:-1:0:0:0 mbedtls_ct_memcmp_partial 0 1 0 0 -mbedtls_ct_memcmp_partial:0:1:0:0:1 +mbedtls_ct_memcmp_partial:0:1:0:0 mbedtls_ct_memcmp_partial 0 1 1 0 -mbedtls_ct_memcmp_partial:0:1:1:0:0 +mbedtls_ct_memcmp_partial:0:1:1:0 mbedtls_ct_memcmp_partial 0 1 0 1 -mbedtls_ct_memcmp_partial:0:1:0:1:0 +mbedtls_ct_memcmp_partial:0:1:0:1 mbedtls_ct_memcmp_partial -1 1 0 0 -mbedtls_ct_memcmp_partial:-1:1:0:0:0 +mbedtls_ct_memcmp_partial:-1:1:0:0 mbedtls_ct_memcmp_partial 0 2 0 1 -mbedtls_ct_memcmp_partial:0:2:0:1:1 +mbedtls_ct_memcmp_partial:0:2:0:1 mbedtls_ct_memcmp_partial 0 2 1 0 -mbedtls_ct_memcmp_partial:0:2:1:0:0 +mbedtls_ct_memcmp_partial:0:2:1:0 mbedtls_ct_memcmp_partial 0 16 4 4 -mbedtls_ct_memcmp_partial:0:16:4:4:0 +mbedtls_ct_memcmp_partial:0:16:4:4 mbedtls_ct_memcmp_partial 2 16 4 4 -mbedtls_ct_memcmp_partial:2:16:4:4:0 +mbedtls_ct_memcmp_partial:2:16:4:4 mbedtls_ct_memcmp_partial 3 16 4 4 -mbedtls_ct_memcmp_partial:3:16:4:4:0 +mbedtls_ct_memcmp_partial:3:16:4:4 mbedtls_ct_memcmp_partial 4 16 4 4 -mbedtls_ct_memcmp_partial:4:16:4:4:1 +mbedtls_ct_memcmp_partial:4:16:4:4 mbedtls_ct_memcmp_partial 7 16 4 4 -mbedtls_ct_memcmp_partial:7:16:4:4:1 +mbedtls_ct_memcmp_partial:7:16:4:4 mbedtls_ct_memcmp_partial 11 16 4 4 -mbedtls_ct_memcmp_partial:11:16:4:4:1 +mbedtls_ct_memcmp_partial:11:16:4:4 mbedtls_ct_memcmp_partial 12 16 4 4 -mbedtls_ct_memcmp_partial:12:16:4:4:0 +mbedtls_ct_memcmp_partial:12:16:4:4 mbedtls_ct_memcmp_partial 15 16 4 4 -mbedtls_ct_memcmp_partial:15:16:4:4:0 +mbedtls_ct_memcmp_partial:15:16:4:4 mbedtls_ct_memcmp_partial 15 16 4 0 -mbedtls_ct_memcmp_partial:15:16:4:0:1 +mbedtls_ct_memcmp_partial:15:16:4:0 mbedtls_ct_memcmp_partial 15 16 0 4 -mbedtls_ct_memcmp_partial:15:16:0:4:0 +mbedtls_ct_memcmp_partial:15:16:0:4 mbedtls_ct_memcmp_partial 0 16 0 0 -mbedtls_ct_memcmp_partial:0:16:0:0:1 +mbedtls_ct_memcmp_partial:0:16:0:0 mbedtls_ct_memcmp_partial 15 16 0 0 -mbedtls_ct_memcmp_partial:15:16:0:0:1 +mbedtls_ct_memcmp_partial:15:16:0:0 mbedtls_ct_memcmp_partial -1 16 0 0 -mbedtls_ct_memcmp_partial:-1:16:0:0:0 +mbedtls_ct_memcmp_partial:-1:16:0:0 mbedtls_ct_memcmp_partial -1 16 12 4 -mbedtls_ct_memcmp_partial:-1:16:12:4:0 +mbedtls_ct_memcmp_partial:-1:16:12:4 mbedtls_ct_memcmp_partial -1 16 8 8 -mbedtls_ct_memcmp_partial:-1:16:8:8:0 +mbedtls_ct_memcmp_partial:-1:16:8:8 diff --git a/tests/suites/test_suite_constant_time.function b/tests/suites/test_suite_constant_time.function index d80610f70..baf77f3ca 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -260,7 +260,7 @@ exit: /* END_CASE */ /* BEGIN_CASE depends_on:MBEDTLS_NIST_KW_C */ -void mbedtls_ct_memcmp_partial(int diff, int size, int skip_head, int skip_tail, int expected) +void mbedtls_ct_memcmp_partial(int diff, int size, int skip_head, int skip_tail) { uint8_t *a = NULL, *b = NULL; TEST_CALLOC(a, size + 1); // + 1 to avoid NULL result from TEST_CALLOC(0) @@ -290,7 +290,6 @@ void mbedtls_ct_memcmp_partial(int diff, int size, int skip_head, int skip_tail, TEST_CF_PUBLIC(&actual, sizeof(actual)); TEST_EQUAL(!!reference, !!actual); - TEST_EQUAL(!!expected, !!actual); exit: mbedtls_free(a); mbedtls_free(b); From a328635305fdb0530da7026bf6cbbca5f0753aa6 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 17:34:39 +0100 Subject: [PATCH 09/14] Introduce TEST_CALLOC_NONNULL Signed-off-by: Dave Rodgman --- tests/include/test/macros.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/include/test/macros.h b/tests/include/test/macros.h index 7edc991ad..c107ccfca 100644 --- a/tests/include/test/macros.h +++ b/tests/include/test/macros.h @@ -143,6 +143,32 @@ } \ } while (0) +/** Allocate memory dynamically and fail the test case if this fails. + * The allocated memory will be filled with zeros. + * + * You must set \p pointer to \c NULL before calling this macro and + * put `mbedtls_free(pointer)` in the test's cleanup code. + * + * If \p item_count is zero, the resulting \p pointer will not be \c NULL. + * + * This macro expands to an instruction, not an expression. + * It may jump to the \c exit label. + * + * \param pointer An lvalue where the address of the allocated buffer + * will be stored. + * This expression may be evaluated multiple times. + * \param item_count Number of elements to allocate. + * This expression may be evaluated multiple times. + * + */ +#define TEST_CALLOC_NONNULL(pointer, item_count) \ + do { \ + TEST_ASSERT((pointer) == NULL); \ + (pointer) = mbedtls_calloc(sizeof(*(pointer)), \ + (item_count)); \ + TEST_ASSERT((pointer) != NULL); \ + } while (0) + /* For backwards compatibility */ #define ASSERT_ALLOC(pointer, item_count) TEST_CALLOC(pointer, item_count) From 28bc1ab923addc5b0af4731ad1155701b0e08a4a Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 17:34:57 +0100 Subject: [PATCH 10/14] Use exact bounds for allocations in mbedtls_ct_memcmp_partial test Signed-off-by: Dave Rodgman --- tests/suites/test_suite_constant_time.function | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_constant_time.function b/tests/suites/test_suite_constant_time.function index baf77f3ca..9df9432eb 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -263,8 +263,9 @@ exit: void mbedtls_ct_memcmp_partial(int diff, int size, int skip_head, int skip_tail) { uint8_t *a = NULL, *b = NULL; - TEST_CALLOC(a, size + 1); // + 1 to avoid NULL result from TEST_CALLOC(0) - TEST_CALLOC(b, size + 1); + + TEST_CALLOC_NONNULL(a, size); + TEST_CALLOC_NONNULL(b, size); TEST_ASSERT((skip_head + skip_tail) <= size); From 2c9f86b3b6a4d4ed80b50d04d941230b3a78c4ce Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 17:48:13 +0100 Subject: [PATCH 11/14] Add docs for mbedtls_ct_memcmp_partial test Signed-off-by: Dave Rodgman --- tests/suites/test_suite_constant_time.function | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/suites/test_suite_constant_time.function b/tests/suites/test_suite_constant_time.function index 9df9432eb..262f841bc 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -260,6 +260,17 @@ exit: /* END_CASE */ /* BEGIN_CASE depends_on:MBEDTLS_NIST_KW_C */ + +/** + * Generate two arrays of the given size, and test mbedtls_ct_memcmp_partial + * over them. The arrays will be identical, except that one byte may be specified + * to be different. + * + * \p diff Index of byte that differs (if out of range, the arrays will match). + * \p size Size of arrays to compare + * \p skip_head Leading bytes to skip, as per mbedtls_ct_memcmp_partial + * \p skip_tail Trailing bytes to skip, as per mbedtls_ct_memcmp_partial + */ void mbedtls_ct_memcmp_partial(int diff, int size, int skip_head, int skip_tail) { uint8_t *a = NULL, *b = NULL; From 6568f60358b95dc132f85efcf49ca070c940d149 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 17:48:24 +0100 Subject: [PATCH 12/14] Simplify mbedtls_ct_memcmp_partial test Signed-off-by: Dave Rodgman --- tests/suites/test_suite_constant_time.function | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_constant_time.function b/tests/suites/test_suite_constant_time.function index 262f841bc..087ccdfd3 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -283,10 +283,10 @@ void mbedtls_ct_memcmp_partial(int diff, int size, int skip_head, int skip_tail) /* Construct data that matches, except for specified byte (if in range). */ for (int i = 0; i < size; i++) { a[i] = i & 0xff; - if (i != diff) { - b[i] = a[i]; - } else { - b[i] = (i + 1) & 0xff; + b[i] = a[i]; + if (i == diff) { + // modify the specified byte + b[i] ^= 1; } } From 986006e5674af3aa2c88fe04db9b376b6d3338e8 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 18:30:25 +0100 Subject: [PATCH 13/14] Make TEST_CALLOC_NONNULL more robust Signed-off-by: Dave Rodgman --- tests/include/test/macros.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/include/test/macros.h b/tests/include/test/macros.h index c107ccfca..3bfbe3333 100644 --- a/tests/include/test/macros.h +++ b/tests/include/test/macros.h @@ -160,12 +160,18 @@ * \param item_count Number of elements to allocate. * This expression may be evaluated multiple times. * + * Note: if passing size 0, mbedtls_calloc may return NULL. In this case, + * we reattempt to allocate with the smallest possible buffer to assure a + * non-NULL pointer. */ #define TEST_CALLOC_NONNULL(pointer, item_count) \ do { \ TEST_ASSERT((pointer) == NULL); \ (pointer) = mbedtls_calloc(sizeof(*(pointer)), \ (item_count)); \ + if (((pointer) == NULL) && ((item_count) == 0)) { \ + (pointer) = mbedtls_calloc(1, 1); \ + } \ TEST_ASSERT((pointer) != NULL); \ } while (0) From 814d096420b3db3fec651911c4973065a1f5b912 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 19 Sep 2023 19:45:54 +0100 Subject: [PATCH 14/14] Fix error in handling of return value from mbedtls_nist_kw_unwrap Signed-off-by: Dave Rodgman --- library/nist_kw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/nist_kw.c b/library/nist_kw.c index 3de1b6ade..d73e82fe4 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -421,8 +421,8 @@ int mbedtls_nist_kw_unwrap(mbedtls_nist_kw_context *ctx, * larger than 8, because of the type wrap around. */ padlen = in_len - KW_SEMIBLOCK_LENGTH - Plen; - ret = -((int) mbedtls_ct_uint_if_else_0(mbedtls_ct_uint_gt(padlen, 7), - -MBEDTLS_ERR_CIPHER_AUTH_FAILED)); + ret = -((int) mbedtls_ct_uint_if(mbedtls_ct_uint_gt(padlen, 7), + -MBEDTLS_ERR_CIPHER_AUTH_FAILED, -ret)); padlen &= 7; /* Check padding in "constant-time" */