From 07a057756c63e6b550ad15bdda299dca3fdd962d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 9 Jul 2025 10:42:28 +0200 Subject: [PATCH 01/16] bignum_core: Add mbedtls_mpi_core_gcd_modinv_odd() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a direct translation of sict_mi2() from https://github.com/mpg/cryptohack/blob/main/ct-pres.py which was presented in the book club's special session. This commit only includes two test cases which is very little. Most of the test cases will be generated by Python modules that belong to the framework. However we can't have the framework generate those before we have the corresponding test function in the consuming branches. So, extended tests are coming as a 2nd step, after the test function has been merged. (The test cases in .misc should stay, as they can be convenient when working on the test function.) Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 185 ++++++++++++++++++ library/bignum_core.h | 34 ++++ tests/suites/test_suite_bignum_core.function | 91 +++++++++ tests/suites/test_suite_bignum_core.misc.data | 6 + 4 files changed, 316 insertions(+) diff --git a/library/bignum_core.c b/library/bignum_core.c index 88582c2d38..1e80a5bf5d 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -1019,4 +1019,189 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, mbedtls_mpi_core_montmul(X, A, &Rinv, 1, N, AN_limbs, mm, T); } +/* + * Compute X = A - B mod N. + * Both A and B must be in [0, N) and so will the output. + */ +static void mpi_core_sub_mod(mbedtls_mpi_uint *X, + const mbedtls_mpi_uint *A, + const mbedtls_mpi_uint *B, + const mbedtls_mpi_uint *N, + size_t limbs) +{ + mbedtls_mpi_uint c = mbedtls_mpi_core_sub(X, A, B, limbs); + (void) mbedtls_mpi_core_add_if(X, N, limbs, (unsigned) c); +} + +/* + * Divide X by 2 mod N in place, assuming N is odd. + * The input must be in [0, N) and so will the output. + */ +static void mpi_core_div2_mod_odd(mbedtls_mpi_uint *X, + const mbedtls_mpi_uint *N, + size_t limbs) +{ + /* If X is odd, add N to make it even before shifting. */ + unsigned odd = (unsigned) X[0] & 1; + mbedtls_mpi_uint c = mbedtls_mpi_core_add_if(X, N, limbs, odd); + mbedtls_mpi_core_shift_r(X, limbs, 1); + X[limbs - 1] |= c << (biL - 1); +} + +/* + * Constant-time GCD and modular inversion - odd modulus. + * + * Pre-conditions: see public documentation. + * + * See https://www.jstage.jst.go.jp/article/transinf/E106.D/9/E106.D_2022ICP0009/_pdf + * This is an adaptation of Alg 7 / Alg 8: + * - Alg 7 is readable but not constant-time, Alg 8 is constant-time but not + * readable (and uses signed arithmetic). We mostly follow Alg 7 and make it + * constant-time by using our usual primitives (conditional assign, + * conditional swap) rather than re-inventing them. We only take a few + * notations from Alg 8 for temporaries. + * - Compared to both, we skip the trick with pre_comm: I think this trick + * complicates things for no benefit (see comment on the big I != NULL block + * below for details). + */ +void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, + mbedtls_mpi_uint *I, + const mbedtls_mpi_uint *A, + size_t A_limbs, + const mbedtls_mpi_uint *N, + size_t N_limbs, + mbedtls_mpi_uint *T) +{ + /* Note: N is called p in the paper, but doesn't need to be prime, only odd. + */ + + /* GCD and modinv, names common to Alg 7 and Alg 8 */ + mbedtls_mpi_uint *u = T + 0 * N_limbs; + mbedtls_mpi_uint *v = G; + + /* GCD and modinv, my name (t1, t2 from Alg 7) */ + mbedtls_mpi_uint *d = T + 1 * N_limbs; + + /* GCD and modinv, names from Alg 8 (note: t1, t2 from Alg 7 are d above) */ + mbedtls_mpi_uint *t1 = T + 2 * N_limbs; + mbedtls_mpi_uint *t2 = T + 3 * N_limbs; + + /* modinv only, names common to Alg 7 and Alg 8 */ + mbedtls_mpi_uint *q = I; + mbedtls_mpi_uint *r = I != NULL ? T + 4 * N_limbs : NULL; + + /* + * Initial values: + * u, v = A, N + * q, r = 0, 1 + */ + memcpy(u, A, A_limbs * ciL); + memset((char *) u + A_limbs * ciL, 0, (N_limbs - A_limbs) * ciL); + + memcpy(v, N, N_limbs * ciL); + + if (I != NULL) { + memset(q, 0, N_limbs * ciL); + + memset(r, 0, N_limbs * ciL); + r[0] = 1; + } + + /* + * At each step, out of u, v, v - u we keep one, shift another, and discard + * the third, then update (u, v) with the ordered result. + * Then we mirror those actions with q, r, r - q mod N. + * + * Loop invariants: + * u <= v (on entry: A <= N) + * GCD(u, v) == GCD(A, N) (on entry: trivial) + * v = A * q mod N (on entry: N = A * 0 mod N) + * u = A * r mod N (on entry: A = A * 1 mod N) + * q, r in [0, N) (on entry: 0, 1) + * + * On exit: + * u = 0 + * v = GCD(A, N) = A * q mod N + * if v == 1 then 1 = A * q mod N ie q is A's inverse mod N + * r = 0 + * + * The exit state is a fixed point of the loop's body. + * Alg 7 and Alg 8 use 2 * bitlen(N) iterations but Theorem 2 (above in the + * paper) says bitlen(A) + bitlen(N) is actually enough. + */ + for (size_t i = 0; i < (A_limbs + N_limbs) * biL; i++) { + /* s, z in Alg 8 - use meaningful names instead */ + mbedtls_ct_condition_t u_odd = mbedtls_ct_bool(u[0] & 1); + mbedtls_ct_condition_t v_odd = mbedtls_ct_bool(v[0] & 1); + + /* Other conditions that will be useful below */ + mbedtls_ct_condition_t u_odd_v_odd = mbedtls_ct_bool_and(u_odd, v_odd); + mbedtls_ct_condition_t v_even = mbedtls_ct_bool_not(v_odd); + mbedtls_ct_condition_t u_odd_v_even = mbedtls_ct_bool_and(u_odd, v_even); + + /* This is called t1 in Alg 7 (no name in Alg 8). + * We know that u <= v so there is no carry */ + (void) mbedtls_mpi_core_sub(d, v, u, N_limbs); + + /* t1 (the thing that's kept) can be d (default) or u (if t2 is d) */ + memcpy(t1, d, N_limbs * ciL); + mbedtls_mpi_core_cond_assign(t1, u, N_limbs, u_odd_v_odd); + + /* t2 (the thing that's shifted) can be u (if even), or v (if even), + * or d (which is even if both u and v were odd) */ + memcpy(t2, u, N_limbs * ciL); + mbedtls_mpi_core_cond_assign(t2, v, N_limbs, u_odd_v_even); + mbedtls_mpi_core_cond_assign(t2, d, N_limbs, u_odd_v_odd); + + mbedtls_mpi_core_shift_r(t2, N_limbs, 1); // t2 is even + + /* Update u, v and re-order them if needed */ + memcpy(u, t1, N_limbs * ciL); + memcpy(v, t2, N_limbs * ciL); + mbedtls_ct_condition_t swap = mbedtls_mpi_core_lt_ct(v, u, N_limbs); + mbedtls_mpi_core_cond_swap(u, v, N_limbs, swap); + + /* Now, if modinv was requested, do the same with q, r, but: + * - decisions still based on u and v (their initial values); + * - operations are now mod N; + * - we re-use t1, t2 for what the paper calls t3, t4 in Alg 8. + * + * Here we slightly diverge from the paper and instead do the obvious + * thing that preserves the invariants involving q and r: mirror + * operations on u and v, ie also divide by 2 here (mod N). + * + * The paper uses a trick where it replaces division by 2 with + * multiplication by 2 here, and compensates in the end by doing a + * final multiplication, which is probably intended as an optimisation. + * + * However I believe it's not actually an optimisation, since + * constant-time modular multiplication by 2 (left-shift + conditional + * subtract) is just as costly as constant-time modular division by 2 + * (conditional add + right-shift). So, skip it and keep things simple. + */ + if (I != NULL) { + /* This is called t2 in Alg 7 (no name in Alg 8). */ + mpi_core_sub_mod(d, q, r, N, N_limbs); + + /* t3 (the thing that's kept) */ + memcpy(t1, d, N_limbs * ciL); + mbedtls_mpi_core_cond_assign(t1, r, N_limbs, u_odd_v_odd); + + /* t4 (the thing that's shifted) */ + memcpy(t2, r, N_limbs * ciL); + mbedtls_mpi_core_cond_assign(t2, q, N_limbs, u_odd_v_even); + mbedtls_mpi_core_cond_assign(t2, d, N_limbs, u_odd_v_odd); + + mpi_core_div2_mod_odd(t2, N, N_limbs); + + /* Update and possibly swap */ + memcpy(r, t1, N_limbs * ciL); + memcpy(q, t2, N_limbs * ciL); + mbedtls_mpi_core_cond_swap(r, q, N_limbs, swap); + } + } + + /* G and I already hold the correct values by virtue of being aliased */ +} + #endif /* MBEDTLS_BIGNUM_C */ diff --git a/library/bignum_core.h b/library/bignum_core.h index 264ee63550..29e05cd3c9 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -822,4 +822,38 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, mbedtls_mpi_uint mm, mbedtls_mpi_uint *T); +/** Compute GCD(A, N) and optionally the inverse of A mod N if it exists. + * + * Requires N to be odd, and 0 <= A <= N. + * + * None of the parameters may alias or overlap another. + * + * \param[out] G The GCD of \p A and \p N. + * Must have the same number of limbs as \p N. + * \param[out] I The inverse of \p A modulo \p N if it exists (that is, + * if \p G above is 1 on exit); indeterminate otherwise. + * This must either be NULL (to only compute the GCD), + * or have the same number of limbs as \p N. + * \param[in] A The 1st operand of GCD and number to invert. + * This value must be less than or equal to \p N. + * \param A_limbs The number of limbs of \p A. + * Must be less than or equal to \p N_limbs. + * \param[in] N The 2nd operand of GCD and modulus for inversion. + * Must be odd or the results are indeterminate. + * \param N_limbs The number of limbs of \p N. + * \param[in,out] T Temporary storage of size at least 5 * N_limbs limbs, + * or 4 * N_limbs if \p I is NULL (GCD only). + * Its initial content is unused and + * its final content is indeterminate. + * It must not alias or otherwise overlap any of the + * other parameters. + */ +void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, + mbedtls_mpi_uint *I, + const mbedtls_mpi_uint *A, + size_t A_limbs, + const mbedtls_mpi_uint *N, + size_t N_limbs, + mbedtls_mpi_uint *T); + #endif /* MBEDTLS_BIGNUM_CORE_H */ diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index c2b44bccdd..81c2242997 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1356,3 +1356,94 @@ exit: mbedtls_free(X); } /* END_CASE */ + +/* BEGIN_CASE */ +void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, + char *input_exp_G, char *input_exp_I) +{ + mbedtls_mpi_uint *A = NULL; + size_t A_limbs = 0; + mbedtls_mpi_uint *N = NULL; + size_t N_limbs = 0; + mbedtls_mpi_uint *exp_G = NULL; + size_t exp_G_limbs = 0; + mbedtls_mpi_uint *exp_I = NULL; + size_t exp_I_limbs = 0; + mbedtls_mpi_uint *G = NULL, *I = NULL, *T = NULL; + + /* Read test parameters into MPI structures */ + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&A, &A_limbs, input_A)); + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&N, &N_limbs, input_N)); + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&exp_G, &exp_G_limbs, input_exp_G)); + const unsigned char got_I = strlen(input_exp_I) != 0; + if (got_I) { + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&exp_I, &exp_I_limbs, input_exp_I)); + } + + /* The function under test wants this */ + TEST_EQUAL(N[0] & 1, 1); + TEST_LE_U(A_limbs, N_limbs); + if (A_limbs == N_limbs) { + TEST_EQUAL(mbedtls_mpi_core_lt_ct(N, A, N_limbs), MBEDTLS_CT_FALSE); + } + /* Other things we want from test data, for our convenience */ + TEST_EQUAL(exp_G_limbs, N_limbs); + if (got_I) { + TEST_EQUAL(exp_I_limbs, N_limbs); + } + + const size_t limbs = N_limbs; + const size_t bytes = limbs * sizeof(mbedtls_mpi_uint); + + TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); + + /* + * Test GCD only (I == NULL) + */ + TEST_CALLOC(G, N_limbs); + memset(G, 'G', bytes); + + TEST_CALLOC(T, 4 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, N, N_limbs, T); + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + + mbedtls_free(G); + G = NULL; + mbedtls_free(T); + T = NULL; + + /* + * Test GCD + modinv + */ + TEST_CALLOC(G, N_limbs); + memset(G, 'G', bytes); + + TEST_CALLOC(I, N_limbs); + memset(I, 'I', bytes); + + TEST_CALLOC(T, 5 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, I, A, A_limbs, N, N_limbs, T); + + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + if (got_I) { + TEST_CF_PUBLIC(I, bytes); + TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + } + +exit: + mbedtls_free(A); + mbedtls_free(N); + mbedtls_free(exp_G); + mbedtls_free(exp_I); + mbedtls_free(G); + mbedtls_free(I); + mbedtls_free(T); +} +/* END_CASE */ diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index ba86029977..118ca07e65 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 + +GCD-modinv random 80-bit, non-trivial GCD -> no inverse +mpi_core_gcd_modinv_odd:"e4518a1900fce698fa3":"1a84113636607520200d":"00000000000000000003":"" + +GCD-modinv random 80-bit, trivial GCD -> inverse +mpi_core_gcd_modinv_odd:"7f2405d6de7db80a7bc":"1a84113636607520200d":"00000000000000000001":"15f158844a59cd7a3ed2" From de5eeb5ce9aa3efd4b0bdd5e003736fd0ba5cdf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jul 2025 12:12:36 +0200 Subject: [PATCH 02/16] Relax and test aliasing rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is consistent with the general rules documented at the top of the file: - when computing GCD(A, N), there is no modular arithmetic, so the output can alias any of the inputs; - when computing a modular inverse, N is the modulus, so it can't be aliased by any of the outputs (we'll use it for modular operations over the entire course of the function's execution). But since this function has two modes of operations with different aliasing rules (G can alias N only if I == NULL), I think it should really be stated explicitly. Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 10 ++- library/bignum_core.h | 4 +- tests/suites/test_suite_bignum_core.function | 91 ++++++++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 1e80a5bf5d..e020da100e 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -1094,11 +1094,19 @@ void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, * Initial values: * u, v = A, N * q, r = 0, 1 + * + * We only write to G (aka v) after reading from inputs (A and N), which + * allows aliasing, except with N when I != NULL, as then we'll be operating + * mod N on q and r later - see the public documentation. + * + * Also avoid possible UB with memcpy when src == dst. */ memcpy(u, A, A_limbs * ciL); memset((char *) u + A_limbs * ciL, 0, (N_limbs - A_limbs) * ciL); - memcpy(v, N, N_limbs * ciL); + if (v != N) { + memcpy(v, N, N_limbs * ciL); + } if (I != NULL) { memset(q, 0, N_limbs * ciL); diff --git a/library/bignum_core.h b/library/bignum_core.h index 29e05cd3c9..1589c34640 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -826,7 +826,9 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, * * Requires N to be odd, and 0 <= A <= N. * - * None of the parameters may alias or overlap another. + * When I == NULL (computing only the GCD), G may alias A or N. + * When I != NULL (computing the modular inverse), G or I may alias A + * but none of them may alias N (the modulus). * * \param[out] G The GCD of \p A and \p N. * Must have the same number of limbs as \p N. diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 81c2242997..93782c1331 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1416,6 +1416,38 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_free(T); T = NULL; + /* GCD only, G aliased to N */ + TEST_CALLOC(G, N_limbs); + memcpy(G, N, bytes); + + TEST_CALLOC(T, 4 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, G, N_limbs, T); + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + + mbedtls_free(G); + G = NULL; + mbedtls_free(T); + T = NULL; + + /* GCD only, G aliased to A (hence A_limbs = N_limbs) */ + TEST_CALLOC(G, N_limbs); + memcpy(G, A, A_limbs * sizeof(mbedtls_mpi_uint)); + + TEST_CALLOC(T, 4 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, N_limbs, N, N_limbs, T); + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + + mbedtls_free(G); + G = NULL; + mbedtls_free(T); + T = NULL; + /* * Test GCD + modinv */ @@ -1437,6 +1469,65 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); } + mbedtls_free(G); + G = NULL; + mbedtls_free(I); + I = NULL; + mbedtls_free(T); + T = NULL; + + /* GCD + modinv, G aliased to A */ + TEST_CALLOC(G, N_limbs); + memcpy(G, A, A_limbs * sizeof(mbedtls_mpi_uint)); + + TEST_CALLOC(I, N_limbs); + memset(I, 'I', bytes); + + TEST_CALLOC(T, 5 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, I, G, N_limbs, N, N_limbs, T); + + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + if (got_I) { + TEST_CF_PUBLIC(I, bytes); + TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + } + + mbedtls_free(G); + G = NULL; + mbedtls_free(I); + I = NULL; + mbedtls_free(T); + T = NULL; + + /* GCD + modinv, I aliased to A */ + TEST_CALLOC(G, N_limbs); + memset(G, 'G', bytes); + + TEST_CALLOC(I, N_limbs); + memcpy(I, A, A_limbs * sizeof(mbedtls_mpi_uint)); + + TEST_CALLOC(T, 5 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, I, I, N_limbs, N, N_limbs, T); + + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + if (got_I) { + TEST_CF_PUBLIC(I, bytes); + TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + } + + mbedtls_free(G); + G = NULL; + mbedtls_free(I); + I = NULL; + mbedtls_free(T); + T = NULL; + exit: mbedtls_free(A); mbedtls_free(N); From d0527406c04a10d8fbd95bd4ffeaf86562b3c1af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jul 2025 10:41:48 +0200 Subject: [PATCH 03/16] Relax number-of-limbs requirement on test data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also more precisely enforce requirement that A <= N. Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_bignum_core.function | 47 ++++++++++++------- tests/suites/test_suite_bignum_core.misc.data | 4 +- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 93782c1331..1a67192908 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -148,6 +148,26 @@ exit: return ret; } +/* + * Return -1 if A < B, +1 if A > B and 0 if A == B + */ +static int mpi_core_cmp(const mbedtls_mpi_uint *A, size_t A_limbs, + const mbedtls_mpi_uint *B, size_t B_limbs) +{ + const mbedtls_mpi AA = { + .p = (mbedtls_mpi_uint *) A, + .s = 1, + .n = (unsigned short) A_limbs, + }; + const mbedtls_mpi BB = { + .p = (mbedtls_mpi_uint *) B, + .s = 1, + .n = (unsigned short) B_limbs, + }; + + return mbedtls_mpi_cmp_mpi(&AA, &BB); +} + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -1383,14 +1403,7 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, /* The function under test wants this */ TEST_EQUAL(N[0] & 1, 1); TEST_LE_U(A_limbs, N_limbs); - if (A_limbs == N_limbs) { - TEST_EQUAL(mbedtls_mpi_core_lt_ct(N, A, N_limbs), MBEDTLS_CT_FALSE); - } - /* Other things we want from test data, for our convenience */ - TEST_EQUAL(exp_G_limbs, N_limbs); - if (got_I) { - TEST_EQUAL(exp_I_limbs, N_limbs); - } + TEST_ASSERT(mpi_core_cmp(A, A_limbs, N, N_limbs) <= 0); const size_t limbs = N_limbs; const size_t bytes = limbs * sizeof(mbedtls_mpi_uint); @@ -1409,7 +1422,7 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, N, N_limbs, T); TEST_CF_PUBLIC(G, bytes); - TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); mbedtls_free(G); G = NULL; @@ -1425,7 +1438,7 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, G, N_limbs, T); TEST_CF_PUBLIC(G, bytes); - TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); mbedtls_free(G); G = NULL; @@ -1441,7 +1454,7 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, N_limbs, N, N_limbs, T); TEST_CF_PUBLIC(G, bytes); - TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); mbedtls_free(G); G = NULL; @@ -1463,10 +1476,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_mpi_core_gcd_modinv_odd(G, I, A, A_limbs, N, N_limbs, T); TEST_CF_PUBLIC(G, bytes); - TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); if (got_I) { TEST_CF_PUBLIC(I, bytes); - TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + TEST_EQUAL(mpi_core_cmp(I, N_limbs, exp_I, exp_I_limbs), 0); } mbedtls_free(G); @@ -1489,10 +1502,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_mpi_core_gcd_modinv_odd(G, I, G, N_limbs, N, N_limbs, T); TEST_CF_PUBLIC(G, bytes); - TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); if (got_I) { TEST_CF_PUBLIC(I, bytes); - TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + TEST_EQUAL(mpi_core_cmp(I, N_limbs, exp_I, exp_I_limbs), 0); } mbedtls_free(G); @@ -1515,10 +1528,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_mpi_core_gcd_modinv_odd(G, I, I, N_limbs, N, N_limbs, T); TEST_CF_PUBLIC(G, bytes); - TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); if (got_I) { TEST_CF_PUBLIC(I, bytes); - TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + TEST_EQUAL(mpi_core_cmp(I, N_limbs, exp_I, exp_I_limbs), 0); } mbedtls_free(G); diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index 118ca07e65..67164e2f90 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -525,7 +525,7 @@ CLZ: 100000 0: skip overly long input mpi_core_clz:100000:0 GCD-modinv random 80-bit, non-trivial GCD -> no inverse -mpi_core_gcd_modinv_odd:"e4518a1900fce698fa3":"1a84113636607520200d":"00000000000000000003":"" +mpi_core_gcd_modinv_odd:"e4518a1900fce698fa3":"1a84113636607520200d":"3":"" GCD-modinv random 80-bit, trivial GCD -> inverse -mpi_core_gcd_modinv_odd:"7f2405d6de7db80a7bc":"1a84113636607520200d":"00000000000000000001":"15f158844a59cd7a3ed2" +mpi_core_gcd_modinv_odd:"7f2405d6de7db80a7bc":"1a84113636607520200d":"1":"15f158844a59cd7a3ed2" From 04ac5d8d35b3dd5753d816f2f4504e0f92d043a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jul 2025 10:55:24 +0200 Subject: [PATCH 04/16] Reduce clutter & improve readbility in test func MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_bignum_core.function | 65 +++++++------------- 1 file changed, 22 insertions(+), 43 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 1a67192908..b82ba19e33 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -154,6 +154,9 @@ exit: static int mpi_core_cmp(const mbedtls_mpi_uint *A, size_t A_limbs, const mbedtls_mpi_uint *B, size_t B_limbs) { + TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(B, B_limbs * sizeof(mbedtls_mpi_uint)); + const mbedtls_mpi AA = { .p = (mbedtls_mpi_uint *) A, .s = 1, @@ -1411,6 +1414,14 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); +#define FREE_G_I_T \ + mbedtls_free(G); \ + G = NULL; \ + mbedtls_free(I); \ + I = NULL; \ + mbedtls_free(T); \ + T = NULL + /* * Test GCD only (I == NULL) */ @@ -1421,13 +1432,9 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, memset(T, 'T', bytes); mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, N, N_limbs, T); - TEST_CF_PUBLIC(G, bytes); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); - mbedtls_free(G); - G = NULL; - mbedtls_free(T); - T = NULL; + FREE_G_I_T; /* GCD only, G aliased to N */ TEST_CALLOC(G, N_limbs); @@ -1436,14 +1443,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_CALLOC(T, 4 * N_limbs); memset(T, 'T', bytes); - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, G, N_limbs, T); - TEST_CF_PUBLIC(G, bytes); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, /* N */ G, N_limbs, T); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); - mbedtls_free(G); - G = NULL; - mbedtls_free(T); - T = NULL; + FREE_G_I_T; /* GCD only, G aliased to A (hence A_limbs = N_limbs) */ TEST_CALLOC(G, N_limbs); @@ -1452,14 +1455,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_CALLOC(T, 4 * N_limbs); memset(T, 'T', bytes); - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, N_limbs, N, N_limbs, T); - TEST_CF_PUBLIC(G, bytes); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, /* A */ G, N_limbs, N, N_limbs, T); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); - mbedtls_free(G); - G = NULL; - mbedtls_free(T); - T = NULL; + FREE_G_I_T; /* * Test GCD + modinv @@ -1475,19 +1474,12 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_mpi_core_gcd_modinv_odd(G, I, A, A_limbs, N, N_limbs, T); - TEST_CF_PUBLIC(G, bytes); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); if (got_I) { - TEST_CF_PUBLIC(I, bytes); TEST_EQUAL(mpi_core_cmp(I, N_limbs, exp_I, exp_I_limbs), 0); } - mbedtls_free(G); - G = NULL; - mbedtls_free(I); - I = NULL; - mbedtls_free(T); - T = NULL; + FREE_G_I_T; /* GCD + modinv, G aliased to A */ TEST_CALLOC(G, N_limbs); @@ -1499,21 +1491,14 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_CALLOC(T, 5 * N_limbs); memset(T, 'T', bytes); - mbedtls_mpi_core_gcd_modinv_odd(G, I, G, N_limbs, N, N_limbs, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, /* A */ G, N_limbs, N, N_limbs, T); - TEST_CF_PUBLIC(G, bytes); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); if (got_I) { - TEST_CF_PUBLIC(I, bytes); TEST_EQUAL(mpi_core_cmp(I, N_limbs, exp_I, exp_I_limbs), 0); } - mbedtls_free(G); - G = NULL; - mbedtls_free(I); - I = NULL; - mbedtls_free(T); - T = NULL; + FREE_G_I_T; /* GCD + modinv, I aliased to A */ TEST_CALLOC(G, N_limbs); @@ -1525,22 +1510,16 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_CALLOC(T, 5 * N_limbs); memset(T, 'T', bytes); - mbedtls_mpi_core_gcd_modinv_odd(G, I, I, N_limbs, N, N_limbs, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, /* A */ I, N_limbs, N, N_limbs, T); - TEST_CF_PUBLIC(G, bytes); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); if (got_I) { - TEST_CF_PUBLIC(I, bytes); TEST_EQUAL(mpi_core_cmp(I, N_limbs, exp_I, exp_I_limbs), 0); } - mbedtls_free(G); - G = NULL; - mbedtls_free(I); - I = NULL; - mbedtls_free(T); - T = NULL; + FREE_G_I_T; +#undef FREE_G_I_T exit: mbedtls_free(A); mbedtls_free(N); From fb2001faf541e55bf370aae091f2274ff5fa82fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jul 2025 12:20:33 +0200 Subject: [PATCH 05/16] Make sure the whole temporary array is non-zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_bignum_core.function | 27 ++++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index b82ba19e33..61660c4851 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1408,8 +1408,7 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_LE_U(A_limbs, N_limbs); TEST_ASSERT(mpi_core_cmp(A, A_limbs, N, N_limbs) <= 0); - const size_t limbs = N_limbs; - const size_t bytes = limbs * sizeof(mbedtls_mpi_uint); + const size_t N_bytes = N_limbs * sizeof(mbedtls_mpi_uint); TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); @@ -1426,10 +1425,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, * Test GCD only (I == NULL) */ TEST_CALLOC(G, N_limbs); - memset(G, 'G', bytes); + memset(G, 'G', N_bytes); TEST_CALLOC(T, 4 * N_limbs); - memset(T, 'T', bytes); + memset(T, 'T', 4 * N_bytes); mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, N, N_limbs, T); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); @@ -1438,10 +1437,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, /* GCD only, G aliased to N */ TEST_CALLOC(G, N_limbs); - memcpy(G, N, bytes); + memcpy(G, N, N_bytes); TEST_CALLOC(T, 4 * N_limbs); - memset(T, 'T', bytes); + memset(T, 'T', 4 * N_bytes); mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, /* N */ G, N_limbs, T); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); @@ -1453,7 +1452,7 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, memcpy(G, A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CALLOC(T, 4 * N_limbs); - memset(T, 'T', bytes); + memset(T, 'T', 4 * N_bytes); mbedtls_mpi_core_gcd_modinv_odd(G, NULL, /* A */ G, N_limbs, N, N_limbs, T); TEST_EQUAL(mpi_core_cmp(G, N_limbs, exp_G, exp_G_limbs), 0); @@ -1464,13 +1463,13 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, * Test GCD + modinv */ TEST_CALLOC(G, N_limbs); - memset(G, 'G', bytes); + memset(G, 'G', N_bytes); TEST_CALLOC(I, N_limbs); - memset(I, 'I', bytes); + memset(I, 'I', N_bytes); TEST_CALLOC(T, 5 * N_limbs); - memset(T, 'T', bytes); + memset(T, 'T', 5 * N_bytes); mbedtls_mpi_core_gcd_modinv_odd(G, I, A, A_limbs, N, N_limbs, T); @@ -1486,10 +1485,10 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, memcpy(G, A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CALLOC(I, N_limbs); - memset(I, 'I', bytes); + memset(I, 'I', N_bytes); TEST_CALLOC(T, 5 * N_limbs); - memset(T, 'T', bytes); + memset(T, 'T', 5 * N_bytes); mbedtls_mpi_core_gcd_modinv_odd(G, I, /* A */ G, N_limbs, N, N_limbs, T); @@ -1502,13 +1501,13 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, /* GCD + modinv, I aliased to A */ TEST_CALLOC(G, N_limbs); - memset(G, 'G', bytes); + memset(G, 'G', N_bytes); TEST_CALLOC(I, N_limbs); memcpy(I, A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CALLOC(T, 5 * N_limbs); - memset(T, 'T', bytes); + memset(T, 'T', 5 * N_bytes); mbedtls_mpi_core_gcd_modinv_odd(G, I, /* A */ I, N_limbs, N, N_limbs, T); From 7fba4668264bfbf8fd3bc8e4bfc7e48d7d6525c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 17 Jul 2025 09:17:12 +0200 Subject: [PATCH 06/16] Unit-test mpi_core_div2_mod_odd() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function has specific code to handle carries and it's not clear how to exercises that code through the modinv function, so well, that's what unit tests are for. Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 10 +++--- library/bignum_core_invasive.h | 19 +++++++++-- tests/suites/test_suite_bignum_core.function | 34 +++++++++++++++++++ tests/suites/test_suite_bignum_core.misc.data | 12 +++++++ 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index e020da100e..3490f7d885 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -18,6 +18,7 @@ #include "mbedtls/platform.h" #include "bignum_core.h" +#include "bignum_core_invasive.h" #include "bn_mul.h" #include "constant_time_internal.h" @@ -1037,9 +1038,10 @@ static void mpi_core_sub_mod(mbedtls_mpi_uint *X, * Divide X by 2 mod N in place, assuming N is odd. * The input must be in [0, N) and so will the output. */ -static void mpi_core_div2_mod_odd(mbedtls_mpi_uint *X, - const mbedtls_mpi_uint *N, - size_t limbs) +MBEDTLS_STATIC_TESTABLE +void mbedtls_mpi_core_div2_mod_odd(mbedtls_mpi_uint *X, + const mbedtls_mpi_uint *N, + size_t limbs) { /* If X is odd, add N to make it even before shifting. */ unsigned odd = (unsigned) X[0] & 1; @@ -1200,7 +1202,7 @@ void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, mbedtls_mpi_core_cond_assign(t2, q, N_limbs, u_odd_v_even); mbedtls_mpi_core_cond_assign(t2, d, N_limbs, u_odd_v_odd); - mpi_core_div2_mod_odd(t2, N, N_limbs); + mbedtls_mpi_core_div2_mod_odd(t2, N, N_limbs); /* Update and possibly swap */ memcpy(r, t1, N_limbs * ciL); diff --git a/library/bignum_core_invasive.h b/library/bignum_core_invasive.h index 167099dc91..a9d447f792 100644 --- a/library/bignum_core_invasive.h +++ b/library/bignum_core_invasive.h @@ -13,11 +13,26 @@ #include "bignum_core.h" -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) +#if defined(MBEDTLS_TEST_HOOKS) + +#if !defined(MBEDTLS_THREADING_C) extern void (*mbedtls_safe_codepath_hook)(void); extern void (*mbedtls_unsafe_codepath_hook)(void); -#endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ +#endif /* !MBEDTLS_THREADING_C */ + +/** Divide X by 2 mod N in place, assuming N is odd. + * + * \param[in,out] X The value to divide by 2 mod \p N. + * \param[in] N The modulus. Must be odd. + * \param[in] limbs The number of limbs in \p X and \p N. + */ +MBEDTLS_STATIC_TESTABLE +void mbedtls_mpi_core_div2_mod_odd(mbedtls_mpi_uint *X, + const mbedtls_mpi_uint *N, + size_t limbs); + +#endif /* MBEDTLS_TEST_HOOKS */ #endif /* MBEDTLS_BIGNUM_CORE_INVASIVE_H */ diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 61660c4851..cad9c1c0dc 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -2,6 +2,7 @@ #include "mbedtls/bignum.h" #include "mbedtls/entropy.h" #include "bignum_core.h" +#include "bignum_core_invasive.h" #include "constant_time_internal.h" #include "test/constant_flow.h" #include "test/bignum_codepath_check.h" @@ -1529,3 +1530,36 @@ exit: mbedtls_free(T); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS */ +void mpi_core_div2_mod_odd(char *input_X, char *input_N, char *input_exp_X) +{ + mbedtls_mpi_uint *X = NULL; + size_t X_limbs = 0; + mbedtls_mpi_uint *N = NULL; + size_t N_limbs = 0; + mbedtls_mpi_uint *exp_X = NULL; + size_t exp_X_limbs = 0; + + /* Read test parameters into MPI structures */ + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&X, &X_limbs, input_X)); + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&N, &N_limbs, input_N)); + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&exp_X, &exp_X_limbs, input_exp_X)); + + /* The function under test requires this */ + TEST_EQUAL(X_limbs, N_limbs); + + TEST_CF_SECRET(X, X_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); + + mbedtls_mpi_core_div2_mod_odd(X, N, N_limbs); + + TEST_CF_PUBLIC(X, X_limbs * sizeof(mbedtls_mpi_uint)); + TEST_EQUAL(0, mpi_core_cmp(X, X_limbs, exp_X, exp_X_limbs)); + +exit: + mbedtls_free(X); + mbedtls_free(N); + mbedtls_free(exp_X); +} +/* END_CASE */ diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index 67164e2f90..d4cf00f893 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -529,3 +529,15 @@ mpi_core_gcd_modinv_odd:"e4518a1900fce698fa3":"1a84113636607520200d":"3":"" GCD-modinv random 80-bit, trivial GCD -> inverse mpi_core_gcd_modinv_odd:"7f2405d6de7db80a7bc":"1a84113636607520200d":"1":"15f158844a59cd7a3ed2" + +Div2 mod odd: even value +mpi_core_div2_mod_odd:"4":"7":"2" + +Div2 mod odd: odd value, no carry +mpi_core_div2_mod_odd:"5":"7":"6" + +Div2 mod odd: odd value with carry +mpi_core_div2_mod_odd:"8000000000000001":"8000000000000003":"8000000000000002" + +Div2 mod odd: even value with top bit set +mpi_core_div2_mod_odd:"8000000000000002":"8000000000000003":"4000000000000001" From 5972096114127759abce1c8450b29129cba97814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 17 Jul 2025 09:39:55 +0200 Subject: [PATCH 07/16] Forbid uninteresting edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A == N (as pointers) will not happen in pratice: in our context, it would mean we know at compile time that A == N (as values), and we wouldn't be calling this function if we knew that already. N == 1 when I != NULL is also not going to happen: we don't care about operations mod 1. Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/bignum_core.h b/library/bignum_core.h index 1589c34640..b8e0807170 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -825,7 +825,9 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, /** Compute GCD(A, N) and optionally the inverse of A mod N if it exists. * * Requires N to be odd, and 0 <= A <= N. + * When I != NULL, N (the modulus) must not be 1. * + * A and N may not alias each other. * When I == NULL (computing only the GCD), G may alias A or N. * When I != NULL (computing the modular inverse), G or I may alias A * but none of them may alias N (the modulus). From 0d25cd965d89a5be0d5dd5aa68bce9a92d3a8c93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 17 Jul 2025 10:10:56 +0200 Subject: [PATCH 08/16] Add test case exercising (almost) max iterations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this data, the loop only settles to its final state u == 0 and v == GCD(A, N) at the last iteration. However it already has v == GCD(A, N) at the previous iteration. Concretely, this means that if in mbedtls_mpi_core_gcd_modinv_odd() we change the main loop as follows - for (size_t i = 0; i < (A_limbs + N_limbs) * biL; i++) { + for (size_t i = 0; i < (A_limbs + N_limbs) * biL - 2; i++) { then this test case would fail. Ideally we'd like a test case that would fail with -1 above but I've not been able to find one and I have no idea whether that's possible. Experimentally I've systematically tried small values (8 bit) and noticed the case A = 2^n and N significantly larger then A is promising, so I explored that further. Clearly we want A and N's bitlength to be a multiple of biL because the bound in the paper is with bitlenths while we use limbs * biL. Anyway, I ended up with the following Python script. import secrets import math bil = 64 def bitlimbs(x): return (x.bit_length() + bil - 1) // bil * bil def sict_gcd(p, a): assert p >= a >= 0 assert p & 1 != 0 or a & 1 != 0 u, v = a, p for i in range(2 * p.bit_length()): s, z = u & 1, v & 1 t1 = (s ^ z) * v + (2 * s * z - 1) * u t2 = (s * v + (2 - 2 * s - z) * u) >> 1 if t2 >= t1: u, v = t1, t2 else: u, v = t2, t1 if u == 0: # v == 1 ideally, but can't get it return bitlimbs(a) + bitlimbs(p) - (i + 1) return 0 a = 2 ** (bil - 1) m = 1000 while m != 0: n = secrets.randbits(2 * bil) | 1 d = sict_gcd(n, a) if d < m: m = d print(d) g = math.gcd(a, n) i = pow(a, -1, n) print(f'mpi_core_gcd_modinv_odd:"{a:x}":"{n:x}":"{g:x}":"{i:x}"') Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_bignum_core.misc.data | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index d4cf00f893..5d3c6753ce 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -530,6 +530,11 @@ mpi_core_gcd_modinv_odd:"e4518a1900fce698fa3":"1a84113636607520200d":"3":"" GCD-modinv random 80-bit, trivial GCD -> inverse mpi_core_gcd_modinv_odd:"7f2405d6de7db80a7bc":"1a84113636607520200d":"1":"15f158844a59cd7a3ed2" +# This data results in the gcd-modinv loop converging to its final state +# only in the last iteration. See python script in commit message. +GCD-modinv (almost) max iterations +mpi_core_gcd_modinv_odd:"8000000000000000":"b26eb5721a2cb24c36acb4550b176671":"1":"77e1dd63583a6b3c8deefe7737862c89" + Div2 mod odd: even value mpi_core_div2_mod_odd:"4":"7":"2" From ed711e142080c18645da5cddbdb1a184d345a429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 22 Jul 2025 09:00:52 +0200 Subject: [PATCH 09/16] Clarify preconditions and impact if not met MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/library/bignum_core.h b/library/bignum_core.h index b8e0807170..cd78e723f1 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -824,14 +824,16 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, /** Compute GCD(A, N) and optionally the inverse of A mod N if it exists. * - * Requires N to be odd, and 0 <= A <= N. - * When I != NULL, N (the modulus) must not be 1. + * Requires N to be odd, 0 <= A <= N and A_limbs <= N_limbs. + * When I != NULL, N (the modulus) must be greater than 1. * * A and N may not alias each other. * When I == NULL (computing only the GCD), G may alias A or N. * When I != NULL (computing the modular inverse), G or I may alias A * but none of them may alias N (the modulus). * + * If any precondition is not met, output values are unspecified. + * * \param[out] G The GCD of \p A and \p N. * Must have the same number of limbs as \p N. * \param[out] I The inverse of \p A modulo \p N if it exists (that is, @@ -843,7 +845,8 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, * \param A_limbs The number of limbs of \p A. * Must be less than or equal to \p N_limbs. * \param[in] N The 2nd operand of GCD and modulus for inversion. - * Must be odd or the results are indeterminate. + * This value must be odd. + * If I != NULL this value must be greater than 1. * \param N_limbs The number of limbs of \p N. * \param[in,out] T Temporary storage of size at least 5 * N_limbs limbs, * or 4 * N_limbs if \p I is NULL (GCD only). From dbda87236900e02be9173f79be38357f6d70b5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 22 Jul 2025 09:21:53 +0200 Subject: [PATCH 10/16] Expand comment about adaptations from the paper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 3490f7d885..2ec4a8fca5 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -1060,8 +1060,17 @@ void mbedtls_mpi_core_div2_mod_odd(mbedtls_mpi_uint *X, * - Alg 7 is readable but not constant-time, Alg 8 is constant-time but not * readable (and uses signed arithmetic). We mostly follow Alg 7 and make it * constant-time by using our usual primitives (conditional assign, - * conditional swap) rather than re-inventing them. We only take a few - * notations from Alg 8 for temporaries. + * conditional swap) rather than re-creating them. See the comments in the + * body of the paper (around tables 2) about how to make Alg 7 constant-time. + * - Both Alg 7 and Alg 8 have temporaries called t1, t2 which have different + * meanings; we use the meaning from Alg 8 (see declarations below). + * - Compared to both, we re-order operations, grouping those related to + * the inverse together. This saves temporaries (we can re-use d, t1, t2 from + * the GCD part as they are no longer used) and improves readability + * considering we make computation of the inverse optional. + * - Compared to Alg 7, we use an explicit conditional swap at the end, which is + * closer to the use of the sort array in Alg 8 (or the max.min function in + * Alg 6 and earlier). * - Compared to both, we skip the trick with pre_comm: I think this trick * complicates things for no benefit (see comment on the big I != NULL block * below for details). From 9361550c4599682a099f7b5256d3ed7ea7b42cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 23 Jul 2025 13:21:07 +0200 Subject: [PATCH 11/16] Tune comment about paper vs our code again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 56 +++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 2ec4a8fca5..49a966fb3f 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -1056,24 +1056,41 @@ void mbedtls_mpi_core_div2_mod_odd(mbedtls_mpi_uint *X, * Pre-conditions: see public documentation. * * See https://www.jstage.jst.go.jp/article/transinf/E106.D/9/E106.D_2022ICP0009/_pdf - * This is an adaptation of Alg 7 / Alg 8: - * - Alg 7 is readable but not constant-time, Alg 8 is constant-time but not - * readable (and uses signed arithmetic). We mostly follow Alg 7 and make it - * constant-time by using our usual primitives (conditional assign, - * conditional swap) rather than re-creating them. See the comments in the - * body of the paper (around tables 2) about how to make Alg 7 constant-time. - * - Both Alg 7 and Alg 8 have temporaries called t1, t2 which have different - * meanings; we use the meaning from Alg 8 (see declarations below). - * - Compared to both, we re-order operations, grouping those related to - * the inverse together. This saves temporaries (we can re-use d, t1, t2 from - * the GCD part as they are no longer used) and improves readability - * considering we make computation of the inverse optional. - * - Compared to Alg 7, we use an explicit conditional swap at the end, which is - * closer to the use of the sort array in Alg 8 (or the max.min function in - * Alg 6 and earlier). - * - Compared to both, we skip the trick with pre_comm: I think this trick - * complicates things for no benefit (see comment on the big I != NULL block - * below for details). + * This is a minor rewrite + adaptation from Algorithm 8: SICT-MI (p. 1403). + * + * u, v = A, N # N is called p in the paper but doesn't have to be prime + * q, r = 0, 1 + * repeat bits(A_limbs + N_limbs) times: + * d = v - u + * t1 = (u and v both odd) ? u : d + * t2 = (u and v both odd) ? d : (u odd) ? v : u + * t2 >>= 1 + * s = t1 <= t2 + * u, v = (s) ? t1, t2 : t2, t1 + * + * d = r - q mod N + * t1 = (u and v both odd) ? q : d # t3 in the paper + * t2 = (u and v both odd) ? d : (u odd) ? r : q # t4 in the paper + * t2 /= 2 mod N + * q, r = (s) ? t1, t2 : t2, t1 + * return v, q + * + * The ternary operators in the above pseudo-code need to be realised in a + * constant-time fashion. For t1-t4, The paper does it by using a mixture of bit + * and (signed) arithmetic operations which is hardly readable; we'll use + * conditional assign instead. We'll use conditional swap for the final update. + * + * Note: for insight about the complicated expressions for t1-t4 in Alg 8, it is + * useful to look at Alg 7 (warning: different meanings for t1, t2, they're our + * d above), table 2 and the surrounding text. + * + * The other minor rewrite compared to Alg 8 in the paper is we re-ordered + * operations, grouping things related to the inverse, which facilitates making + * its computation optional, and requires fewer temporaries. + * + * The only actual change from Alg 8 is dropping the trick with pre_comm, + * which I think complicates things for no benefit. + * See the comment on the big I != NULL block below for details. */ void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, mbedtls_mpi_uint *I, @@ -1083,9 +1100,6 @@ void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, size_t N_limbs, mbedtls_mpi_uint *T) { - /* Note: N is called p in the paper, but doesn't need to be prime, only odd. - */ - /* GCD and modinv, names common to Alg 7 and Alg 8 */ mbedtls_mpi_uint *u = T + 0 * N_limbs; mbedtls_mpi_uint *v = G; From efd242a0e5e79759a98e99c1959987f8b5fa890a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 24 Jul 2025 11:10:59 +0200 Subject: [PATCH 12/16] Gracefully handle A_limbs > N_limbs and test it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 7 +- library/bignum_core.h | 2 +- tests/suites/test_suite_bignum_core.function | 104 ++++++++++++++++++ tests/suites/test_suite_bignum_core.misc.data | 3 + 4 files changed, 113 insertions(+), 3 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 49a966fb3f..8ac65e2202 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -1123,12 +1123,15 @@ void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, * We only write to G (aka v) after reading from inputs (A and N), which * allows aliasing, except with N when I != NULL, as then we'll be operating * mod N on q and r later - see the public documentation. - * - * Also avoid possible UB with memcpy when src == dst. */ + if (A_limbs > N_limbs) { + /* Violating this precondition should not result in memory errors. */ + A_limbs = N_limbs; + } memcpy(u, A, A_limbs * ciL); memset((char *) u + A_limbs * ciL, 0, (N_limbs - A_limbs) * ciL); + /* Avoid possible UB with memcpy when src == dst. */ if (v != N) { memcpy(v, N, N_limbs * ciL); } diff --git a/library/bignum_core.h b/library/bignum_core.h index cd78e723f1..f044b33f93 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -832,7 +832,7 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, * When I != NULL (computing the modular inverse), G or I may alias A * but none of them may alias N (the modulus). * - * If any precondition is not met, output values are unspecified. + * If any of the above preconditions is not met, output values are unspecified. * * \param[out] G The GCD of \p A and \p N. * Must have the same number of limbs as \p N. diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index cad9c1c0dc..8ae1931cbc 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1531,6 +1531,110 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mpi_core_gcd_modinv_odd_preconditions() +{ + /* + * The purpose of this test function is to ensure that the function doesn't + * crash (but just outputs garbage) when preconditions are not met. + */ + + mbedtls_mpi_uint two_limbs[2]; + mbedtls_mpi_uint one_limb[1]; + mbedtls_mpi_uint *G = NULL, *I = NULL, *T = NULL; + + /* Large enough for all calls below */ + TEST_CALLOC(G, 2); + TEST_CALLOC(I, 2); + TEST_CALLOC(T, 5 * 2); + + /* + * Input values + */ + + /* N is not odd */ + two_limbs[0] = 2; // N = 2^n + 2 + two_limbs[1] = 1; + one_limb[0] = 42; // A = 42 + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, T); + + /* A > N */ + two_limbs[0] = 3; // N = 3 + two_limbs[1] = 0; + one_limb[0] = 42; // A = 42 + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, T); + + /* A_limbs > N_limbs (but A <= N) */ + one_limb[0] = 5; // N = 5 + two_limbs[0] = 3; // A = 3 + two_limbs[1] = 0; + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, one_limb, 1, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, T); + + /* A_limbs > N_limbs (and A > N) */ + one_limb[0] = 5; // N = 5 + two_limbs[0] = 7; // A = 7 + two_limbs[1] = 0; + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, one_limb, 1, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, T); + + /* I != NULL but N is 1 */ + two_limbs[0] = 1; // N = 1 + two_limbs[1] = 0; + one_limb[0] = 1; // A = 1 + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, T); + + /* + * Aliasing + */ + + /* Now use valid values for remaining tests */ + two_limbs[0] = 1; // N = 2^n + 1 + two_limbs[1] = 1; + one_limb[0] = 42; // A = 42 + + /* A aliased to N */ + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, two_limbs, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, two_limbs, 2, T); + + /* G aliased to A and N */ + memcpy(G, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, 2, G, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, G, 2, G, 2, T); + + /* I != NULL, G aliased to N */ + memcpy(G, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, G, 2, T); + + /* I != NULL, I aliased to N */ + memcpy(I, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, I, 2, T); + + /* I aliased to A and N */ + memcpy(I, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); + mbedtls_mpi_core_gcd_modinv_odd(G, I, I, 2, I, 2, T); + + /* + * Not specific to this function + */ + + /* A_limbs = 0 */ + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 0, two_limbs, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 0, two_limbs, 2, T); + + /* A_limbs = N_limbs = 0 */ + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 0, two_limbs, 0, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 0, two_limbs, 0, T); + +exit: + mbedtls_free(G); + mbedtls_free(I); + mbedtls_free(T); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS */ void mpi_core_div2_mod_odd(char *input_X, char *input_N, char *input_exp_X) { diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index 5d3c6753ce..e2ba5a61f7 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -535,6 +535,9 @@ mpi_core_gcd_modinv_odd:"7f2405d6de7db80a7bc":"1a84113636607520200d":"1":"15f158 GCD-modinv (almost) max iterations mpi_core_gcd_modinv_odd:"8000000000000000":"b26eb5721a2cb24c36acb4550b176671":"1":"77e1dd63583a6b3c8deefe7737862c89" +GCD-modinv preconditions not met +mpi_core_gcd_modinv_odd_preconditions: + Div2 mod odd: even value mpi_core_div2_mod_odd:"4":"7":"2" From ec35382a510bb9417407068f1b80a32d9ff25423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 24 Jul 2025 12:22:16 +0200 Subject: [PATCH 13/16] Try again to clarify connection with the paper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 47 ++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 8ac65e2202..8091694615 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -1056,39 +1056,36 @@ void mbedtls_mpi_core_div2_mod_odd(mbedtls_mpi_uint *X, * Pre-conditions: see public documentation. * * See https://www.jstage.jst.go.jp/article/transinf/E106.D/9/E106.D_2022ICP0009/_pdf - * This is a minor rewrite + adaptation from Algorithm 8: SICT-MI (p. 1403). + * + * The paper gives two computationally equivalent algorithms: Alg 7 (readable) + * and Alg 8 (constant-time). We use a third version that's hopefully both: * * u, v = A, N # N is called p in the paper but doesn't have to be prime * q, r = 0, 1 * repeat bits(A_limbs + N_limbs) times: - * d = v - u - * t1 = (u and v both odd) ? u : d - * t2 = (u and v both odd) ? d : (u odd) ? v : u + * d = v - u # t1 in Alg 7 + * t1 = (u and v both odd) ? u : d # t1 in Alg 8 + * t2 = (u and v both odd) ? d : (u odd) ? v : u # t2 in Alg 8 * t2 >>= 1 - * s = t1 <= t2 - * u, v = (s) ? t1, t2 : t2, t1 + * swap = t1 > t2 # similar to s, z in Alg 8 + * u, v = (swap) ? t2, t1 : t1, t2 * - * d = r - q mod N - * t1 = (u and v both odd) ? q : d # t3 in the paper - * t2 = (u and v both odd) ? d : (u odd) ? r : q # t4 in the paper - * t2 /= 2 mod N - * q, r = (s) ? t1, t2 : t2, t1 - * return v, q + * d = r - q mod N # t2 in Alg 7 + * t1 = (u and v both odd) ? q : d # t3 in Alg 8 + * t2 = (u and v both odd) ? d : (u odd) ? r : q # t4 Alg 8 + * t2 /= 2 mod N # see below (pre_com) + * q, r = (swap) ? t2, t1 : t1, t2 + * return v, q # v: GCD, see Alg 6; q: no mult by pre_com, see below * * The ternary operators in the above pseudo-code need to be realised in a - * constant-time fashion. For t1-t4, The paper does it by using a mixture of bit - * and (signed) arithmetic operations which is hardly readable; we'll use - * conditional assign instead. We'll use conditional swap for the final update. + * constant-time fashion. We use conditional assign for t1, t2 and conditional + * swap for the final update. (Note: the similarity between branches of Alg 7 + * are highlighted in tables 2 and 3 and the surrounding text.) * - * Note: for insight about the complicated expressions for t1-t4 in Alg 8, it is - * useful to look at Alg 7 (warning: different meanings for t1, t2, they're our - * d above), table 2 and the surrounding text. + * Also, we re-order operations, grouping things related to the inverse, which + * facilitates making its computation optional, and requires fewer temporaries. * - * The other minor rewrite compared to Alg 8 in the paper is we re-ordered - * operations, grouping things related to the inverse, which facilitates making - * its computation optional, and requires fewer temporaries. - * - * The only actual change from Alg 8 is dropping the trick with pre_comm, + * The only actual change from the paper is dropping the trick with pre_com, * which I think complicates things for no benefit. * See the comment on the big I != NULL block below for details. */ @@ -1207,8 +1204,8 @@ void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, * operations on u and v, ie also divide by 2 here (mod N). * * The paper uses a trick where it replaces division by 2 with - * multiplication by 2 here, and compensates in the end by doing a - * final multiplication, which is probably intended as an optimisation. + * multiplication by 2 here, and compensates in the end by multiplying + * by pre_com, which is probably intended as an optimisation. * * However I believe it's not actually an optimisation, since * constant-time modular multiplication by 2 (left-shift + conditional From 0904a742358d8aac71cec1add593305dd7e0e43d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 25 Jul 2025 09:33:20 +0200 Subject: [PATCH 14/16] Remove tests for 0 limbs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That rule is common to the whole module and not a likely mistake to make. Also, the test was not really precise as G, I, T were oversized. Better remove it than give a false sense of security. Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_bignum_core.function | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 8ae1931cbc..55693dec27 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1616,18 +1616,6 @@ void mpi_core_gcd_modinv_odd_preconditions() memcpy(I, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); mbedtls_mpi_core_gcd_modinv_odd(G, I, I, 2, I, 2, T); - /* - * Not specific to this function - */ - - /* A_limbs = 0 */ - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 0, two_limbs, 2, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 0, two_limbs, 2, T); - - /* A_limbs = N_limbs = 0 */ - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 0, two_limbs, 0, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 0, two_limbs, 0, T); - exit: mbedtls_free(G); mbedtls_free(I); From be8983d394f687f073b830a04418a9b254bf1009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 25 Jul 2025 09:46:52 +0200 Subject: [PATCH 15/16] Use precise sizes for temporaries in test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_bignum_core.function | 57 +++++++++++--------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 55693dec27..70f761d46f 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1539,14 +1539,16 @@ void mpi_core_gcd_modinv_odd_preconditions() * crash (but just outputs garbage) when preconditions are not met. */ - mbedtls_mpi_uint two_limbs[2]; mbedtls_mpi_uint one_limb[1]; - mbedtls_mpi_uint *G = NULL, *I = NULL, *T = NULL; + mbedtls_mpi_uint two_limbs[2]; + mbedtls_mpi_uint three_limbs[3]; + mbedtls_mpi_uint *G = NULL, *I = NULL, *TG = NULL, *TI = NULL; - /* Large enough for all calls below */ + /* We'll always use a two-limbs N */ TEST_CALLOC(G, 2); TEST_CALLOC(I, 2); - TEST_CALLOC(T, 5 * 2); + TEST_CALLOC(TG, 4 * 2); // For I == NULL + TEST_CALLOC(TI, 5 * 2); // For I != NULL /* * Input values @@ -1556,35 +1558,39 @@ void mpi_core_gcd_modinv_odd_preconditions() two_limbs[0] = 2; // N = 2^n + 2 two_limbs[1] = 1; one_limb[0] = 42; // A = 42 - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, TI); /* A > N */ two_limbs[0] = 3; // N = 3 two_limbs[1] = 0; one_limb[0] = 42; // A = 42 - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, TI); /* A_limbs > N_limbs (but A <= N) */ - one_limb[0] = 5; // N = 5 - two_limbs[0] = 3; // A = 3 + two_limbs[0] = 3; // N = 3 two_limbs[1] = 0; - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, one_limb, 1, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, T); + three_limbs[0] = 1; // A = 1 + three_limbs[1] = 0; + three_limbs[2] = 0; + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, three_limbs, 3, two_limbs, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, three_limbs, 3, two_limbs, 2, TI); /* A_limbs > N_limbs (and A > N) */ - one_limb[0] = 5; // N = 5 - two_limbs[0] = 7; // A = 7 + two_limbs[0] = 3; // N = 3 two_limbs[1] = 0; - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, one_limb, 1, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, T); + three_limbs[0] = 0; // A = 2^2n + three_limbs[1] = 0; + three_limbs[2] = 1; + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, three_limbs, 3, two_limbs, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, three_limbs, 3, two_limbs, 2, TI); /* I != NULL but N is 1 */ two_limbs[0] = 1; // N = 1 two_limbs[1] = 0; one_limb[0] = 1; // A = 1 - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, TI); /* * Aliasing @@ -1596,30 +1602,31 @@ void mpi_core_gcd_modinv_odd_preconditions() one_limb[0] = 42; // A = 42 /* A aliased to N */ - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, two_limbs, 2, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, two_limbs, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, two_limbs, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, two_limbs, 2, TI); /* G aliased to A and N */ memcpy(G, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, 2, G, 2, T); - mbedtls_mpi_core_gcd_modinv_odd(G, I, G, 2, G, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, 2, G, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, G, 2, G, 2, TI); /* I != NULL, G aliased to N */ memcpy(G, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, G, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, G, 2, TI); /* I != NULL, I aliased to N */ memcpy(I, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, I, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, I, 2, TI); /* I aliased to A and N */ memcpy(I, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); - mbedtls_mpi_core_gcd_modinv_odd(G, I, I, 2, I, 2, T); + mbedtls_mpi_core_gcd_modinv_odd(G, I, I, 2, I, 2, TI); exit: mbedtls_free(G); mbedtls_free(I); - mbedtls_free(T); + mbedtls_free(TG); + mbedtls_free(TI); } /* END_CASE */ From eb346801263ba4612b5e29633bd55fe18943ca65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 25 Jul 2025 09:49:30 +0200 Subject: [PATCH 16/16] Use more meaningful names in test function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_bignum_core.function | 84 ++++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 70f761d46f..759801906f 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1539,9 +1539,9 @@ void mpi_core_gcd_modinv_odd_preconditions() * crash (but just outputs garbage) when preconditions are not met. */ - mbedtls_mpi_uint one_limb[1]; - mbedtls_mpi_uint two_limbs[2]; - mbedtls_mpi_uint three_limbs[3]; + mbedtls_mpi_uint A[1]; + mbedtls_mpi_uint N[2]; + mbedtls_mpi_uint AAA[3]; mbedtls_mpi_uint *G = NULL, *I = NULL, *TG = NULL, *TI = NULL; /* We'll always use a two-limbs N */ @@ -1555,71 +1555,71 @@ void mpi_core_gcd_modinv_odd_preconditions() */ /* N is not odd */ - two_limbs[0] = 2; // N = 2^n + 2 - two_limbs[1] = 1; - one_limb[0] = 42; // A = 42 - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, TG); - mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, TI); + N[0] = 2; // N = 2^n + 2 + N[1] = 1; + A[0] = 42; // A = 42 + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, 1, N, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, A, 1, N, 2, TI); /* A > N */ - two_limbs[0] = 3; // N = 3 - two_limbs[1] = 0; - one_limb[0] = 42; // A = 42 - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, one_limb, 1, two_limbs, 2, TG); - mbedtls_mpi_core_gcd_modinv_odd(G, I, one_limb, 1, two_limbs, 2, TI); + N[0] = 3; // N = 3 + N[1] = 0; + A[0] = 42; // A = 42 + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, 1, N, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, A, 1, N, 2, TI); /* A_limbs > N_limbs (but A <= N) */ - two_limbs[0] = 3; // N = 3 - two_limbs[1] = 0; - three_limbs[0] = 1; // A = 1 - three_limbs[1] = 0; - three_limbs[2] = 0; - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, three_limbs, 3, two_limbs, 2, TG); - mbedtls_mpi_core_gcd_modinv_odd(G, I, three_limbs, 3, two_limbs, 2, TI); + N[0] = 3; // N = 3 + N[1] = 0; + AAA[0] = 1; // A = 1 + AAA[1] = 0; + AAA[2] = 0; + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, AAA, 3, N, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, AAA, 3, N, 2, TI); /* A_limbs > N_limbs (and A > N) */ - two_limbs[0] = 3; // N = 3 - two_limbs[1] = 0; - three_limbs[0] = 0; // A = 2^2n - three_limbs[1] = 0; - three_limbs[2] = 1; - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, three_limbs, 3, two_limbs, 2, TG); - mbedtls_mpi_core_gcd_modinv_odd(G, I, three_limbs, 3, two_limbs, 2, TI); + N[0] = 3; // N = 3 + N[1] = 0; + AAA[0] = 0; // A = 2^2n + AAA[1] = 0; + AAA[2] = 1; + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, AAA, 3, N, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, AAA, 3, N, 2, TI); /* I != NULL but N is 1 */ - two_limbs[0] = 1; // N = 1 - two_limbs[1] = 0; - one_limb[0] = 1; // A = 1 - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, one_limb, 1, TI); + N[0] = 1; // N = 1 + N[1] = 0; + A[0] = 1; // A = 1 + mbedtls_mpi_core_gcd_modinv_odd(G, I, N, 2, A, 1, TI); /* * Aliasing */ /* Now use valid values for remaining tests */ - two_limbs[0] = 1; // N = 2^n + 1 - two_limbs[1] = 1; - one_limb[0] = 42; // A = 42 + N[0] = 1; // N = 2^n + 1 + N[1] = 1; + A[0] = 42; // A = 42 /* A aliased to N */ - mbedtls_mpi_core_gcd_modinv_odd(G, NULL, two_limbs, 2, two_limbs, 2, TG); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, two_limbs, 2, TI); + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, N, 2, N, 2, TG); + mbedtls_mpi_core_gcd_modinv_odd(G, I, N, 2, N, 2, TI); /* G aliased to A and N */ - memcpy(G, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); + memcpy(G, N, 2 * sizeof(mbedtls_mpi_uint)); mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, 2, G, 2, TG); mbedtls_mpi_core_gcd_modinv_odd(G, I, G, 2, G, 2, TI); /* I != NULL, G aliased to N */ - memcpy(G, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, G, 2, TI); + memcpy(G, N, 2 * sizeof(mbedtls_mpi_uint)); + mbedtls_mpi_core_gcd_modinv_odd(G, I, A, 1, G, 2, TI); /* I != NULL, I aliased to N */ - memcpy(I, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); - mbedtls_mpi_core_gcd_modinv_odd(G, I, two_limbs, 2, I, 2, TI); + memcpy(I, N, 2 * sizeof(mbedtls_mpi_uint)); + mbedtls_mpi_core_gcd_modinv_odd(G, I, A, 1, I, 2, TI); /* I aliased to A and N */ - memcpy(I, two_limbs, 2 * sizeof(mbedtls_mpi_uint)); + memcpy(I, N, 2 * sizeof(mbedtls_mpi_uint)); mbedtls_mpi_core_gcd_modinv_odd(G, I, I, 2, I, 2, TI); exit: