From 3235165e070803b0459a37fa7469dd3e918fc77a Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 8 May 2024 17:36:09 +0000 Subject: [PATCH 1/4] Change mpi_core_check_sub to be constant time Signed-off-by: Waleed Elmelegy --- library/bignum_core.c | 5 ++-- tests/suites/test_suite_bignum_core.function | 30 +++++++++++++++++++ tests/suites/test_suite_bignum_core.misc.data | 6 ++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 1a3e0b9b6..ee3d704a1 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -449,9 +449,10 @@ mbedtls_mpi_uint mbedtls_mpi_core_sub(mbedtls_mpi_uint *X, mbedtls_mpi_uint c = 0; for (size_t i = 0; i < limbs; i++) { - mbedtls_mpi_uint z = (A[i] < c); + mbedtls_mpi_uint z = mbedtls_ct_mpi_uint_if(mbedtls_ct_uint_lt(A[i], c), + 1, 0); mbedtls_mpi_uint t = A[i] - c; - c = (t < B[i]) + z; + c = mbedtls_ct_mpi_uint_if(mbedtls_ct_uint_lt(t, B[i]), 1, 0) + z; X[i] = t - B[i]; } diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index db84d6238..61eeaf545 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1317,3 +1317,33 @@ exit: mbedtls_free(X); } /* END_CASE */ + +/* BEGIN_CASE */ +void mpi_core_check_sub_ct(char *input_A, char *input_B, int exp_ret) +{ + mbedtls_mpi_uint *A = NULL; + mbedtls_mpi_uint *B = NULL; + mbedtls_mpi_uint *X = NULL; + size_t A_limbs, B_limbs; + int ret; + + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&A, &A_limbs, input_A)); + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&B, &B_limbs, input_B)); + + TEST_EQUAL(A_limbs, B_limbs); + + size_t limbs = A_limbs; + TEST_CALLOC(X, limbs); + + TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(B, B_limbs * sizeof(mbedtls_mpi_uint)); + + ret = mbedtls_mpi_core_sub(X, A, B, limbs); + TEST_EQUAL(ret, exp_ret); + +exit: + mbedtls_free(A); + mbedtls_free(B); + mbedtls_free(X); +} +/* END_CASE */ diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index ba8602997..ccf375052 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -523,3 +523,9 @@ mpi_core_clz:64:0 CLZ: 100000 0: skip overly long input mpi_core_clz:100000:0 + +Constant time Subtraction +mpi_core_check_sub_ct:"1234567890abcdef0":"10000000000000000":0 + +Constant time Subtraction #2 +mpi_core_check_sub_ct:"10000000000000000":"1234567890abcdef0":1 From e27738308cfb45d34e9a91335e65b12d7d6dde2e Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 21 May 2024 16:05:52 +0000 Subject: [PATCH 2/4] Merge mbedtls_mpi_core_sub() constant time testing and functional testing Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_bignum_core.function | 53 ++++++++----------- tests/suites/test_suite_bignum_core.misc.data | 6 --- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 61eeaf545..1bb1ab06a 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -660,31 +660,54 @@ void mpi_core_sub(char *input_A, char *input_B, memcpy(b, B.p, B.n * sizeof(mbedtls_mpi_uint)); memcpy(x, X.p, X.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(a, bytes); + TEST_CF_SECRET(b, bytes); + /* 1a) r = a - b => we should get the correct carry */ TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, b, limbs)); + TEST_CF_PUBLIC(a, bytes); + TEST_CF_PUBLIC(b, bytes); + TEST_CF_PUBLIC(r, bytes); + /* 1b) r = a - b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); /* 2 and 3 test "r may be aliased to a or b" */ /* 2a) r = a; r -= b => we should get the correct carry (use r to avoid clobbering a) */ memcpy(r, a, bytes); + + TEST_CF_SECRET(r, bytes); + TEST_CF_SECRET(b, bytes); + TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, r, b, limbs)); + TEST_CF_PUBLIC(r, bytes); + TEST_CF_PUBLIC(b, bytes); + /* 2b) r -= b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); /* 3a) r = b; r = a - r => we should get the correct carry (use r to avoid clobbering b) */ memcpy(r, b, bytes); + + TEST_CF_SECRET(r, bytes); + TEST_CF_SECRET(a, bytes); + TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, r, limbs)); + TEST_CF_PUBLIC(r, bytes); + TEST_CF_PUBLIC(a, bytes); + /* 3b) r = a - b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); /* 4 tests "r may be aliased to [...] both" */ if (A.n == B.n && memcmp(A.p, B.p, bytes) == 0) { memcpy(r, b, bytes); + TEST_CF_SECRET(r, bytes); TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, r, r, limbs)); + TEST_CF_PUBLIC(r, bytes); TEST_MEMORY_COMPARE(r, bytes, x, bytes); } @@ -1317,33 +1340,3 @@ exit: mbedtls_free(X); } /* END_CASE */ - -/* BEGIN_CASE */ -void mpi_core_check_sub_ct(char *input_A, char *input_B, int exp_ret) -{ - mbedtls_mpi_uint *A = NULL; - mbedtls_mpi_uint *B = NULL; - mbedtls_mpi_uint *X = NULL; - size_t A_limbs, B_limbs; - int ret; - - TEST_EQUAL(0, mbedtls_test_read_mpi_core(&A, &A_limbs, input_A)); - TEST_EQUAL(0, mbedtls_test_read_mpi_core(&B, &B_limbs, input_B)); - - TEST_EQUAL(A_limbs, B_limbs); - - size_t limbs = A_limbs; - TEST_CALLOC(X, limbs); - - TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); - TEST_CF_SECRET(B, B_limbs * sizeof(mbedtls_mpi_uint)); - - ret = mbedtls_mpi_core_sub(X, A, B, limbs); - TEST_EQUAL(ret, exp_ret); - -exit: - mbedtls_free(A); - mbedtls_free(B); - mbedtls_free(X); -} -/* END_CASE */ diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index ccf375052..ba8602997 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -523,9 +523,3 @@ mpi_core_clz:64:0 CLZ: 100000 0: skip overly long input mpi_core_clz:100000:0 - -Constant time Subtraction -mpi_core_check_sub_ct:"1234567890abcdef0":"10000000000000000":0 - -Constant time Subtraction #2 -mpi_core_check_sub_ct:"10000000000000000":"1234567890abcdef0":1 From a9fe03ea4ed1314d78e0d4833a585cae498931df Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 21 May 2024 16:17:06 +0000 Subject: [PATCH 3/4] Add comment to mbedtls_mpi_core_sub() to indicate it is costant time Signed-off-by: Waleed Elmelegy --- library/bignum_core.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/bignum_core.h b/library/bignum_core.h index 92c8d47db..bebd81032 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -376,6 +376,9 @@ mbedtls_mpi_uint mbedtls_mpi_core_add_if(mbedtls_mpi_uint *X, * \p X may be aliased to \p A or \p B, or even both, but may not overlap * either otherwise. * + * This function operates in constant time with respect to the values + * of \p X and \p A and \p B. + * * \param[out] X The result of the subtraction. * \param[in] A Little-endian presentation of left operand. * \param[in] B Little-endian presentation of right operand. From 473dea26a6f2660edb528161e99a0d06103fd4af Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 28 May 2024 11:15:21 +0000 Subject: [PATCH 4/4] Remove unnecessary testing and documentation Signed-off-by: Waleed Elmelegy --- library/bignum_core.h | 2 +- tests/suites/test_suite_bignum_core.function | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/library/bignum_core.h b/library/bignum_core.h index bebd81032..b32d60736 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -377,7 +377,7 @@ mbedtls_mpi_uint mbedtls_mpi_core_add_if(mbedtls_mpi_uint *X, * either otherwise. * * This function operates in constant time with respect to the values - * of \p X and \p A and \p B. + * of \p A and \p B. * * \param[out] X The result of the subtraction. * \param[in] A Little-endian presentation of left operand. diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 1bb1ab06a..bd1dfa687 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -666,8 +666,6 @@ void mpi_core_sub(char *input_A, char *input_B, /* 1a) r = a - b => we should get the correct carry */ TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, b, limbs)); - TEST_CF_PUBLIC(a, bytes); - TEST_CF_PUBLIC(b, bytes); TEST_CF_PUBLIC(r, bytes); /* 1b) r = a - b => we should get the correct result */ @@ -678,12 +676,10 @@ void mpi_core_sub(char *input_A, char *input_B, memcpy(r, a, bytes); TEST_CF_SECRET(r, bytes); - TEST_CF_SECRET(b, bytes); TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, r, b, limbs)); TEST_CF_PUBLIC(r, bytes); - TEST_CF_PUBLIC(b, bytes); /* 2b) r -= b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); @@ -692,12 +688,10 @@ void mpi_core_sub(char *input_A, char *input_B, memcpy(r, b, bytes); TEST_CF_SECRET(r, bytes); - TEST_CF_SECRET(a, bytes); TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, r, limbs)); TEST_CF_PUBLIC(r, bytes); - TEST_CF_PUBLIC(a, bytes); /* 3b) r = a - b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes);