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/constant_time.c b/library/constant_time.c index 55e7f9435..8b41aed19 100644 --- a/library/constant_time.c +++ b/library/constant_time.c @@ -141,6 +141,36 @@ 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]; + 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); + } + + /* 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 + #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" 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); diff --git a/library/nist_kw.c b/library/nist_kw.c index fbd7221a4..d73e82fe4 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) { @@ -420,19 +421,15 @@ 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(mbedtls_ct_uint_gt(padlen, 7), + -MBEDTLS_ERR_CIPHER_AUTH_FAILED, -ret)); + padlen &= 7; /* 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 +451,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)); 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) */ diff --git a/tests/include/test/macros.h b/tests/include/test/macros.h index 7edc991ad..3bfbe3333 100644 --- a/tests/include/test/macros.h +++ b/tests/include/test/macros.h @@ -143,6 +143,38 @@ } \ } 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. + * + * 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) + /* For backwards compatibility */ #define ASSERT_ALLOC(pointer, item_count) TEST_CALLOC(pointer, item_count) diff --git a/tests/suites/test_suite_constant_time.data b/tests/suites/test_suite_constant_time.data index 785bdec29..82ee869e4 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 + +mbedtls_ct_memcmp_partial 0 1 0 0 +mbedtls_ct_memcmp_partial:0:1:0:0 + +mbedtls_ct_memcmp_partial 0 1 1 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 + +mbedtls_ct_memcmp_partial -1 1 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 + +mbedtls_ct_memcmp_partial 0 2 1 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 + +mbedtls_ct_memcmp_partial 2 16 4 4 +mbedtls_ct_memcmp_partial:2:16:4:4 + +mbedtls_ct_memcmp_partial 3 16 4 4 +mbedtls_ct_memcmp_partial:3:16:4:4 + +mbedtls_ct_memcmp_partial 4 16 4 4 +mbedtls_ct_memcmp_partial:4:16:4:4 + +mbedtls_ct_memcmp_partial 7 16 4 4 +mbedtls_ct_memcmp_partial:7:16:4:4 + +mbedtls_ct_memcmp_partial 11 16 4 4 +mbedtls_ct_memcmp_partial:11:16:4:4 + +mbedtls_ct_memcmp_partial 12 16 4 4 +mbedtls_ct_memcmp_partial:12:16:4:4 + +mbedtls_ct_memcmp_partial 15 16 4 4 +mbedtls_ct_memcmp_partial:15:16:4:4 + +mbedtls_ct_memcmp_partial 15 16 4 0 +mbedtls_ct_memcmp_partial:15:16:4:0 + +mbedtls_ct_memcmp_partial 15 16 0 4 +mbedtls_ct_memcmp_partial:15:16:0:4 + +mbedtls_ct_memcmp_partial 0 16 0 0 +mbedtls_ct_memcmp_partial:0:16:0:0 + +mbedtls_ct_memcmp_partial 15 16 0 0 +mbedtls_ct_memcmp_partial:15:16:0:0 + +mbedtls_ct_memcmp_partial -1 16 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 + +mbedtls_ct_memcmp_partial -1 16 8 8 +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 5dbba063e..087ccdfd3 100644 --- a/tests/suites/test_suite_constant_time.function +++ b/tests/suites/test_suite_constant_time.function @@ -259,6 +259,55 @@ 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; + + TEST_CALLOC_NONNULL(a, size); + TEST_CALLOC_NONNULL(b, size); + + 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; + b[i] = a[i]; + if (i == diff) { + // modify the specified byte + b[i] ^= 1; + } + } + + 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); +exit: + mbedtls_free(a); + mbedtls_free(b); +} +/* END_CASE */ + /* BEGIN_CASE */ void mbedtls_ct_memcpy_if(int eq, int size, int offset) {