From bd7ede3f330d452288de2c170eeb2f60229a58cf Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Mon, 4 Aug 2025 11:33:48 +0100 Subject: [PATCH 01/11] bignum: add mpi wrapper for gcd_modinv Signed-off-by: Felix Conway --- library/bignum.c | 60 +++++++++++++++++++++++++++++++++++++++ library/bignum_internal.h | 26 +++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/library/bignum.c b/library/bignum.c index 424490951d..a9423598dc 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1743,6 +1743,66 @@ int mbedtls_mpi_exp_mod_unsafe(mbedtls_mpi *X, const mbedtls_mpi *A, return mbedtls_mpi_exp_mod_optionally_safe(X, A, E, MBEDTLS_MPI_IS_PUBLIC, N, prec_RR); } +/* Constant-time GCD and/or modinv with odd modulus and A <= N */ +int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, + mbedtls_mpi *I, + const mbedtls_mpi *A, + const mbedtls_mpi *N) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + mbedtls_mpi local_g; + mbedtls_mpi_uint *T = NULL; + const size_t T_factor = I != NULL ? 5 : 4; + + /* Check requirements on A and N */ + if (mbedtls_mpi_cmp_int(A, 0) < 0 || + mbedtls_mpi_cmp_mpi(A, N) > 0 || + mbedtls_mpi_get_bit(N, 0) != 1 || + (I != NULL && mbedtls_mpi_cmp_int(N, 1) == 0)) { + return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; + } + + /* Check aliasing requirements */ + if (A == N || (I != NULL && (I == N || G == N))) { + return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; + } + + mbedtls_mpi_init(&local_g); + + if (G == NULL) { + G = &local_g; + } + + MBEDTLS_MPI_CHK(mbedtls_mpi_grow(G, N->n)); + G->s = 1; + if (I != NULL) { + MBEDTLS_MPI_CHK(mbedtls_mpi_grow(I, N->n)); + I->s = 1; + } + + T = mbedtls_calloc(sizeof(mbedtls_mpi_uint) * N->n, T_factor); + if (T == NULL) { + ret = MBEDTLS_ERR_MPI_ALLOC_FAILED; + goto cleanup; + } + + mbedtls_mpi_uint *Ip = I != NULL ? I->p : NULL; + size_t An = A->n <= N->n ? A->n : N->n; + mbedtls_mpi_core_gcd_modinv_odd(G->p, Ip, A->p, An, N->p, N->n, T); + + if (G->n > N->n) { + memset(G->p + N->n, 0, ciL * (G->n - N->n)); + } + if (I != NULL && I->n > N->n) { + memset(I->p + N->n, 0, ciL * (I->n - N->n)); + } + +cleanup: + mbedtls_mpi_free(&local_g); + mbedtls_free(T); + return ret; +} + /* * Greatest common divisor: G = gcd(A, B) (HAC 14.54) */ diff --git a/library/bignum_internal.h b/library/bignum_internal.h index aceaf55ea2..e5657f5b8f 100644 --- a/library/bignum_internal.h +++ b/library/bignum_internal.h @@ -47,4 +47,30 @@ int mbedtls_mpi_exp_mod_unsafe(mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi *E, const mbedtls_mpi *N, mbedtls_mpi *prec_RR); +/** + * \brief Compute GCD(A, N) and/or A^-1 mod N if it exists, + * in constant time. + * + * \warning Requires N to be odd, and 0 <= A <= N. + * + * \note G and I must not alias each other but may alias A or N. + * + * \param[out] G The GCD of \p A and \p N. + * This may be NULL, to only compute I. + * \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 may be NULL, to only compute G. + * \param[in] A The 1st operand of GCD and number to invert. + * This value must be less than or equal to \p N. + * \param[in] N The 2nd operand of GCD and modulus for inversion. + * Must be odd or the results are indeterminate. + * + * \return \c 0 if successful. + * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if a memory allocation failed. + */ +int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, + mbedtls_mpi *I, + const mbedtls_mpi *A, + const mbedtls_mpi *N); + #endif /* bignum_internal.h */ From 54a94c15988aa0ef7bdf24dc6f86a35c9b39ee70 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Mon, 4 Aug 2025 11:34:19 +0100 Subject: [PATCH 02/11] Adjust mpi_gcd_modinv_odd docs and precondition checking Signed-off-by: Felix Conway --- library/bignum.c | 2 +- library/bignum_internal.h | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index a9423598dc..b09a851d8b 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1763,7 +1763,7 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, } /* Check aliasing requirements */ - if (A == N || (I != NULL && (I == N || G == N))) { + if (A == N || G == I || (I != NULL && (I == N || G == N))) { return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; } diff --git a/library/bignum_internal.h b/library/bignum_internal.h index e5657f5b8f..ee2220a25f 100644 --- a/library/bignum_internal.h +++ b/library/bignum_internal.h @@ -51,9 +51,14 @@ int mbedtls_mpi_exp_mod_unsafe(mbedtls_mpi *X, const mbedtls_mpi *A, * \brief Compute GCD(A, N) and/or A^-1 mod N if it exists, * in constant time. * - * \warning Requires N to be odd, and 0 <= A <= N. + * \warning Requires N to be odd, and 0 <= A <= N, and N > 1 if + * I != NULL. * - * \note G and I must not alias each other but may alias A or N. + * \note G and I must not alias each other. + * A and N must not alias each other. + * When I == NULL (computing only the GCD), G can alias A or N. + * When I != NULL (computing the modular inverse), G or I can + * alias A, but neither of them can alias N (the modulus). * * \param[out] G The GCD of \p A and \p N. * This may be NULL, to only compute I. @@ -67,6 +72,8 @@ int mbedtls_mpi_exp_mod_unsafe(mbedtls_mpi *X, const mbedtls_mpi *A, * * \return \c 0 if successful. * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if a memory allocation failed. + * \return #MBEDTLS_ERR_MPI_BAD_INPUT_DATA if preconditions were not + * met. */ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, mbedtls_mpi *I, From 38ec046c4bd035584be58bc1ca4644da4b413d29 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Mon, 4 Aug 2025 11:34:45 +0100 Subject: [PATCH 03/11] Add mpi_gcd_modinv_odd test functions Signed-off-by: Felix Conway --- tests/suites/test_suite_bignum.function | 165 ++++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/tests/suites/test_suite_bignum.function b/tests/suites/test_suite_bignum.function index 36f1476d76..b8af87fb34 100644 --- a/tests/suites/test_suite_bignum.function +++ b/tests/suites/test_suite_bignum.function @@ -1149,6 +1149,171 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, + char *result_G, char *result_I, + int return_code) +{ + int has_inverse = strcmp(result_I, "no_inverse") ? 1 : 0; + mbedtls_mpi G, I, A, N, exp_G, exp_I; + int res; + mbedtls_mpi_init(&G); mbedtls_mpi_init(&I); mbedtls_mpi_init(&A); mbedtls_mpi_init(&N); + mbedtls_mpi_init(&exp_G); mbedtls_mpi_init(&exp_I); + TEST_EQUAL(mbedtls_test_read_mpi(&A, input_A), 0); + TEST_EQUAL(mbedtls_test_read_mpi(&N, input_N), 0); + TEST_EQUAL(mbedtls_test_read_mpi(&exp_G, result_G), 0); + if (has_inverse) { + TEST_EQUAL(mbedtls_test_read_mpi(&exp_I, result_I), 0); + } + + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + /* If there is no inverse then the value returned in I will be + * indeterminate, and so not useful or possible to test. */ + if (has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } + } + + /* Test pointer aliasing where &G == &A. */ + TEST_EQUAL(mbedtls_test_read_mpi(&G, input_A), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, /* A */ &G, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + /* If there is no inverse then the value returned in I will be + * indeterminate, and so not useful or possible to test. */ + if (has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } + } + + /* Test pointer aliasing where &G == &N. This should fail. */ + TEST_EQUAL(mbedtls_test_read_mpi(&G, input_N), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, /* N */ &G); + TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + + /* Test pointer aliasing where &I == &A. */ + TEST_EQUAL(mbedtls_test_read_mpi(&I, input_A), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, /* A */ &I, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + /* If there is no inverse then the value returned in I will be + * indeterminate, and so not useful or possible to test. */ + if (has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } + } + + /* Test pointer aliasing where &I == &N. This should fail. */ + TEST_EQUAL(mbedtls_test_read_mpi(&I, input_N), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, /* N */ &I); + TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + + /* Test pointer aliasing where &A == &N. This should fail. */ + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, /* N */ &A); + TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void mpi_gcd_modinv_odd_only_gcd(char *input_A, char *input_N, + char *result_G, int return_code) +{ + mbedtls_mpi G, A, N, exp_G; + int res; + mbedtls_mpi_init(&G); mbedtls_mpi_init(&A); mbedtls_mpi_init(&N); + mbedtls_mpi_init(&exp_G); + TEST_EQUAL(mbedtls_test_read_mpi(&A, input_A), 0); + TEST_EQUAL(mbedtls_test_read_mpi(&N, input_N), 0); + TEST_EQUAL(mbedtls_test_read_mpi(&exp_G, result_G), 0); + + res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + } + + /* Test pointer aliasing where &G == &A. */ + TEST_EQUAL(mbedtls_test_read_mpi(&G, input_A), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, /* A */ &G, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + } + + /* Test pointer aliasing where &G == &N. */ + TEST_EQUAL(mbedtls_test_read_mpi(&G, input_N), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, /* N */ &G); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + } + + /* Test pointer aliasing where &A == &N. This should fail. */ + res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, /* N */ &A); + TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void mpi_gcd_modinv_odd_only_modinv(char *input_A, char *input_N, + char *result_I, int return_code) +{ + int has_inverse = strcmp(result_I, "no_inverse") ? 1 : 0; + mbedtls_mpi I, A, N, exp_I; + int res; + mbedtls_mpi_init(&I); mbedtls_mpi_init(&A); mbedtls_mpi_init(&N); + mbedtls_mpi_init(&exp_I); + TEST_EQUAL(mbedtls_test_read_mpi(&A, input_A), 0); + TEST_EQUAL(mbedtls_test_read_mpi(&N, input_N), 0); + if (has_inverse) { + TEST_EQUAL(mbedtls_test_read_mpi(&exp_I, result_I), 0); + } + + res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, &N); + TEST_EQUAL(res, return_code); + /* If there is no inverse then the value returned in I will be + * indeterminate, and so not useful or possible to test. */ + if (res == 0 && has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } + + /* Test pointer aliasing where &I == &A. */ + TEST_EQUAL(mbedtls_test_read_mpi(&I, input_A), 0); + res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, /* A */ &I, &N); + TEST_EQUAL(res, return_code); + /* If there is no inverse then the value returned in I will be + * indeterminate, and so not useful or possible to test. */ + if (res == 0 && has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } + + /* Test pointer aliasing where &I == &N. This should fail. */ + TEST_EQUAL(mbedtls_test_read_mpi(&I, input_N), 0); + res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, /* N */ &I); + TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + + /* Test pointer aliasing where &A == &N. This should fail. */ + res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, /* N */ &A); + TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_GENPRIME */ void mpi_is_prime(char *input_X, int div_result) { From 45835d1bf20e6fcbfbfd141031ce9875fe15e151 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Mon, 4 Aug 2025 11:35:15 +0100 Subject: [PATCH 04/11] Add handful of manual gcd_modinv_odd test cases Signed-off-by: Felix Conway --- tests/suites/test_suite_bignum.misc.data | 54 ++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/suites/test_suite_bignum.misc.data b/tests/suites/test_suite_bignum.misc.data index 2e3ff1ecc0..6afd7f62b4 100644 --- a/tests/suites/test_suite_bignum.misc.data +++ b/tests/suites/test_suite_bignum.misc.data @@ -1531,6 +1531,60 @@ mpi_inv_mod:"00":"11":"":MBEDTLS_ERR_MPI_NOT_ACCEPTABLE Test mbedtls_mpi_inv_mod #1 mpi_inv_mod:"aa4df5cb14b4c31237f98bd1faf527c283c2d0f3eec89718664ba33f9762907c":"fffbbd660b94412ae61ead9c2906a344116e316a256fd387874c6c675b1d587d":"8d6a5c1d7adeae3e94b9bcd2c47e0d46e778bc8804a2cc25c02d775dc3d05b0c":0 +GCD-modinv wrapper: working, A < N +mpi_gcd_modinv_odd_both:"54a":"3999":"1":"30b5":0 + +GCD-modinv wrapper: no mod inverse, A = N +mpi_gcd_modinv_odd_both:"365":"365":"365":"no_inverse":0 + +GCD-modinv wrapper: no mod inverse, A < N +mpi_gcd_modinv_odd_both:"5a":"b9":"5":"no_inverse":0 + +GCD-modinv wrapper: bad inputs, A > N +mpi_gcd_modinv_odd_both:"3999":"54a":"":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper: bad inputs, A < 0 +mpi_gcd_modinv_odd_both:"-5":"54a":"":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper: bad inputs, N even +mpi_gcd_modinv_odd_both:"89":"540":"":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper only gcd: working, A < N +mpi_gcd_modinv_odd_only_gcd:"1de3":"31d9":"7":0 + +GCD-modinv wrapper only gcd: working, A = N +mpi_gcd_modinv_odd_only_gcd:"365":"365":"365":0 + +GCD-modinv wrapper only gcd: working, no mod inverse, A < N +mpi_gcd_modinv_odd_only_gcd:"19e":"a47f":"9":0 + +GCD-modinv wrapper only gcd: bad inputs, A > N +mpi_gcd_modinv_odd_only_gcd:"319d":"1de3":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper only gcd: bad inputs, A < 0 +mpi_gcd_modinv_odd_only_gcd:"-628ef":"991827f":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper only gcd: bad inputs, N even +mpi_gcd_modinv_odd_only_gcd:"319d":"24":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper only modinv: working, A < N +mpi_gcd_modinv_odd_only_modinv:"28c":"26f9":"84f":0 + +GCD-modinv wrapper only modinv: no mod inverse, A = N +mpi_gcd_modinv_odd_only_modinv:"365":"365":"no_inverse":0 + +GCD-modinv wrapper only modinv: no mod inverse, A < N +mpi_gcd_modinv_odd_only_modinv:"19e":"a47f":"no_inverse":0 + +GCD-modinv wrapper only modinv: bad inputs, A > N +mpi_gcd_modinv_odd_only_modinv:"26f9":"28c":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper only modinv: bad inputs, A < 0 +mpi_gcd_modinv_odd_only_modinv:"-992f":"1000002":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +GCD-modinv wrapper only modinv: bad inputs, N even +mpi_gcd_modinv_odd_only_modinv:"28c":"26f0":"":MBEDTLS_ERR_MPI_BAD_INPUT_DATA + Base test mbedtls_mpi_is_prime #1 depends_on:MBEDTLS_GENPRIME mpi_is_prime:"0":MBEDTLS_ERR_MPI_NOT_ACCEPTABLE From fae58c4a0ce33935e9ecfb1bb74850921f9f46ef Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Mon, 4 Aug 2025 13:05:34 +0100 Subject: [PATCH 05/11] Fix memory leak Signed-off-by: Felix Conway --- tests/suites/test_suite_bignum.function | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/suites/test_suite_bignum.function b/tests/suites/test_suite_bignum.function index b8af87fb34..7454fb809d 100644 --- a/tests/suites/test_suite_bignum.function +++ b/tests/suites/test_suite_bignum.function @@ -1222,6 +1222,10 @@ void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, /* Test pointer aliasing where &A == &N. This should fail. */ res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + +exit: + mbedtls_mpi_free(&G); mbedtls_mpi_free(&I); mbedtls_mpi_free(&A); mbedtls_mpi_free(&N); + mbedtls_mpi_free(&exp_G); mbedtls_mpi_free(&exp_I); } /* END_CASE */ @@ -1265,6 +1269,10 @@ void mpi_gcd_modinv_odd_only_gcd(char *input_A, char *input_N, /* Test pointer aliasing where &A == &N. This should fail. */ res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + +exit: + mbedtls_mpi_free(&G); mbedtls_mpi_free(&A); mbedtls_mpi_free(&N); + mbedtls_mpi_free(&exp_G); } /* END_CASE */ @@ -1311,6 +1319,10 @@ void mpi_gcd_modinv_odd_only_modinv(char *input_A, char *input_N, /* Test pointer aliasing where &A == &N. This should fail. */ res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + +exit: + mbedtls_mpi_free(&I); mbedtls_mpi_free(&A); mbedtls_mpi_free(&N); + mbedtls_mpi_free(&exp_I); } /* END_CASE */ From f4df43b6c4c63a987431978e9d9eba4233a2bbf2 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Mon, 4 Aug 2025 17:00:10 +0100 Subject: [PATCH 06/11] Fix gcd_invmod_odd wrapper when A is 0 (null) Signed-off-by: Felix Conway --- library/bignum.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index b09a851d8b..2f2ce6a41e 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1751,6 +1751,7 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_mpi local_g; + mbedtls_mpi local_a; mbedtls_mpi_uint *T = NULL; const size_t T_factor = I != NULL ? 5 : 4; @@ -1767,6 +1768,16 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; } + mbedtls_mpi_init(&local_a); + /* If A is 0 (null), then A->p will be null, which is an issue when A->p is + * passed to mbedtls_mpi_core_gcd_modinv_odd below, so set A to 0 (1 limb) + * in this case. */ + if (A->n == 0 && A->p == NULL) { + mbedtls_mpi_read_string(&local_a, 16, "00"); + } else { + mbedtls_mpi_copy(&local_a, A); + } + mbedtls_mpi_init(&local_g); if (G == NULL) { @@ -1787,8 +1798,8 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, } mbedtls_mpi_uint *Ip = I != NULL ? I->p : NULL; - size_t An = A->n <= N->n ? A->n : N->n; - mbedtls_mpi_core_gcd_modinv_odd(G->p, Ip, A->p, An, N->p, N->n, T); + size_t An = local_a.n <= N->n ? local_a.n : N->n; + mbedtls_mpi_core_gcd_modinv_odd(G->p, Ip, local_a.p, An, N->p, N->n, T); if (G->n > N->n) { memset(G->p + N->n, 0, ciL * (G->n - N->n)); @@ -1799,6 +1810,7 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, cleanup: mbedtls_mpi_free(&local_g); + mbedtls_mpi_free(&local_a); mbedtls_free(T); return ret; } From d9c4c9c44167a431bfe571ff531c64413eb20b28 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Tue, 5 Aug 2025 14:33:32 +0100 Subject: [PATCH 07/11] Update mpi_gcd_invmod_odd() related comments/documentation Signed-off-by: Felix Conway --- library/bignum.c | 2 +- library/bignum_internal.h | 12 ++++++------ tests/suites/test_suite_bignum.function | 14 ++++---------- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 2f2ce6a41e..e141cda740 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1764,7 +1764,7 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, } /* Check aliasing requirements */ - if (A == N || G == I || (I != NULL && (I == N || G == N))) { + if (A == N || (I != NULL && (I == N || G == N))) { return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; } diff --git a/library/bignum_internal.h b/library/bignum_internal.h index ee2220a25f..f3f6fcbc8d 100644 --- a/library/bignum_internal.h +++ b/library/bignum_internal.h @@ -48,14 +48,14 @@ int mbedtls_mpi_exp_mod_unsafe(mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi *prec_RR); /** - * \brief Compute GCD(A, N) and/or A^-1 mod N if it exists, - * in constant time. + * \brief A wrapper around a constant time function to compute + * GCD(A, N) and/or A^-1 mod N if it exists. * - * \warning Requires N to be odd, and 0 <= A <= N, and N > 1 if - * I != NULL. + * \warning Requires N to be odd, and 0 <= A <= N. Additionally, if + * I != NULL, requires N > 1. + * The wrapper part of this function is not constant time. * - * \note G and I must not alias each other. - * A and N must not alias each other. + * \note A and N must not alias each other. * When I == NULL (computing only the GCD), G can alias A or N. * When I != NULL (computing the modular inverse), G or I can * alias A, but neither of them can alias N (the modulus). diff --git a/tests/suites/test_suite_bignum.function b/tests/suites/test_suite_bignum.function index 7454fb809d..2a9d878fd0 100644 --- a/tests/suites/test_suite_bignum.function +++ b/tests/suites/test_suite_bignum.function @@ -1162,6 +1162,8 @@ void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, TEST_EQUAL(mbedtls_test_read_mpi(&A, input_A), 0); TEST_EQUAL(mbedtls_test_read_mpi(&N, input_N), 0); TEST_EQUAL(mbedtls_test_read_mpi(&exp_G, result_G), 0); + /* If there is no inverse then the value returned in I will be + * indeterminate, and so not useful or possible to test. */ if (has_inverse) { TEST_EQUAL(mbedtls_test_read_mpi(&exp_I, result_I), 0); } @@ -1171,8 +1173,6 @@ void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, if (res == 0) { TEST_ASSERT(sign_is_valid(&G)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); - /* If there is no inverse then the value returned in I will be - * indeterminate, and so not useful or possible to test. */ if (has_inverse) { TEST_ASSERT(sign_is_valid(&I)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); @@ -1186,8 +1186,6 @@ void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, if (res == 0) { TEST_ASSERT(sign_is_valid(&G)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); - /* If there is no inverse then the value returned in I will be - * indeterminate, and so not useful or possible to test. */ if (has_inverse) { TEST_ASSERT(sign_is_valid(&I)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); @@ -1206,8 +1204,6 @@ void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, if (res == 0) { TEST_ASSERT(sign_is_valid(&G)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); - /* If there is no inverse then the value returned in I will be - * indeterminate, and so not useful or possible to test. */ if (has_inverse) { TEST_ASSERT(sign_is_valid(&I)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); @@ -1287,14 +1283,14 @@ void mpi_gcd_modinv_odd_only_modinv(char *input_A, char *input_N, mbedtls_mpi_init(&exp_I); TEST_EQUAL(mbedtls_test_read_mpi(&A, input_A), 0); TEST_EQUAL(mbedtls_test_read_mpi(&N, input_N), 0); + /* If there is no inverse then the value returned in I will be + * indeterminate, and so not useful or possible to test. */ if (has_inverse) { TEST_EQUAL(mbedtls_test_read_mpi(&exp_I, result_I), 0); } res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, &N); TEST_EQUAL(res, return_code); - /* If there is no inverse then the value returned in I will be - * indeterminate, and so not useful or possible to test. */ if (res == 0 && has_inverse) { TEST_ASSERT(sign_is_valid(&I)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); @@ -1304,8 +1300,6 @@ void mpi_gcd_modinv_odd_only_modinv(char *input_A, char *input_N, TEST_EQUAL(mbedtls_test_read_mpi(&I, input_A), 0); res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, /* A */ &I, &N); TEST_EQUAL(res, return_code); - /* If there is no inverse then the value returned in I will be - * indeterminate, and so not useful or possible to test. */ if (res == 0 && has_inverse) { TEST_ASSERT(sign_is_valid(&I)); TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); From eefdfe99a47d8d7b8a8c67f8f1e621d06e6042b2 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Tue, 5 Aug 2025 14:35:53 +0100 Subject: [PATCH 08/11] Change A=0 (null) handling in mpi_gcd_invmod_odd() Signed-off-by: Felix Conway --- library/bignum.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index e141cda740..53ff95eadd 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1751,9 +1751,9 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_mpi local_g; - mbedtls_mpi local_a; mbedtls_mpi_uint *T = NULL; const size_t T_factor = I != NULL ? 5 : 4; + const mbedtls_mpi_uint zero = 0; /* Check requirements on A and N */ if (mbedtls_mpi_cmp_int(A, 0) < 0 || @@ -1768,16 +1768,6 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, return MBEDTLS_ERR_MPI_BAD_INPUT_DATA; } - mbedtls_mpi_init(&local_a); - /* If A is 0 (null), then A->p will be null, which is an issue when A->p is - * passed to mbedtls_mpi_core_gcd_modinv_odd below, so set A to 0 (1 limb) - * in this case. */ - if (A->n == 0 && A->p == NULL) { - mbedtls_mpi_read_string(&local_a, 16, "00"); - } else { - mbedtls_mpi_copy(&local_a, A); - } - mbedtls_mpi_init(&local_g); if (G == NULL) { @@ -1797,9 +1787,15 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, goto cleanup; } + /* We have to handle G and I carefully as they could be aliased + * to A or N. */ mbedtls_mpi_uint *Ip = I != NULL ? I->p : NULL; - size_t An = local_a.n <= N->n ? local_a.n : N->n; - mbedtls_mpi_core_gcd_modinv_odd(G->p, Ip, local_a.p, An, N->p, N->n, T); + /* If A is 0 (null), then A->p would be null, which would be an issue if + * A->p was passed to mbedtls_mpi_core_gcd_modinv_odd below. */ + const mbedtls_mpi_uint *Ap = A->p != NULL ? A->p : &zero; + size_t An = A->p == NULL ? 0 : A->n; + An = A->n <= N->n ? A->n : N->n; + mbedtls_mpi_core_gcd_modinv_odd(G->p, Ip, Ap, An, N->p, N->n, T); if (G->n > N->n) { memset(G->p + N->n, 0, ciL * (G->n - N->n)); @@ -1810,7 +1806,6 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, cleanup: mbedtls_mpi_free(&local_g); - mbedtls_mpi_free(&local_a); mbedtls_free(T); return ret; } From 49a2bc475036cc697aea8d00aa23a81231eef4c9 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Tue, 5 Aug 2025 14:38:20 +0100 Subject: [PATCH 09/11] Add gcd_invmod_odd() tests where G/I are initialized to large numbers Signed-off-by: Felix Conway --- tests/suites/test_suite_bignum.function | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/suites/test_suite_bignum.function b/tests/suites/test_suite_bignum.function index 2a9d878fd0..9019053258 100644 --- a/tests/suites/test_suite_bignum.function +++ b/tests/suites/test_suite_bignum.function @@ -1219,6 +1219,20 @@ void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + /* Test G & I initialized to large number with non-zero limbs. */ + TEST_EQUAL(mbedtls_test_read_mpi(&G, "c7420eb50e52ce18795a1020896787ce18dcd6b0"), 0); + TEST_EQUAL(mbedtls_test_read_mpi(&I, "5702c227ee207cca33eb7a6151531b50541a47ef"), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + if (has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } + } + exit: mbedtls_mpi_free(&G); mbedtls_mpi_free(&I); mbedtls_mpi_free(&A); mbedtls_mpi_free(&N); mbedtls_mpi_free(&exp_G); mbedtls_mpi_free(&exp_I); @@ -1266,6 +1280,15 @@ void mpi_gcd_modinv_odd_only_gcd(char *input_A, char *input_N, res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + /* Test G initialized to large number with non-zero limbs. */ + TEST_EQUAL(mbedtls_test_read_mpi(&G, "8d81032aaa52fb4d29831c7ed183fffee9baf169"), 0); + res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + } + exit: mbedtls_mpi_free(&G); mbedtls_mpi_free(&A); mbedtls_mpi_free(&N); mbedtls_mpi_free(&exp_G); @@ -1314,6 +1337,15 @@ void mpi_gcd_modinv_odd_only_modinv(char *input_A, char *input_N, res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); + /* Test I initialized to large number with non-zero limbs. */ + TEST_EQUAL(mbedtls_test_read_mpi(&I, "bc0ccc030cb2d8b31e40e08fac727d2f4a8c9c1d"), 0); + res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0 && has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } + exit: mbedtls_mpi_free(&I); mbedtls_mpi_free(&A); mbedtls_mpi_free(&N); mbedtls_mpi_free(&exp_I); From a1c95e378a24a4253cbccb4b58d5e6564906a6f7 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Wed, 6 Aug 2025 09:54:11 +0100 Subject: [PATCH 10/11] Adjust mpi_gcd_modinv_odd() internals Signed-off-by: Felix Conway --- library/bignum.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 53ff95eadd..4f7ca16cda 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1774,11 +1774,11 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, G = &local_g; } + /* We can't modify the values of G or I before use in the main function, + * as they could be aliased to A or N. */ MBEDTLS_MPI_CHK(mbedtls_mpi_grow(G, N->n)); - G->s = 1; if (I != NULL) { MBEDTLS_MPI_CHK(mbedtls_mpi_grow(I, N->n)); - I->s = 1; } T = mbedtls_calloc(sizeof(mbedtls_mpi_uint) * N->n, T_factor); @@ -1787,16 +1787,19 @@ int mbedtls_mpi_gcd_modinv_odd(mbedtls_mpi *G, goto cleanup; } - /* We have to handle G and I carefully as they could be aliased - * to A or N. */ mbedtls_mpi_uint *Ip = I != NULL ? I->p : NULL; - /* If A is 0 (null), then A->p would be null, which would be an issue if - * A->p was passed to mbedtls_mpi_core_gcd_modinv_odd below. */ + /* If A is 0 (null), then A->p would be null, and A->n would be 0, + * which would be an issue if A->p and A->n were passed to + * mbedtls_mpi_core_gcd_modinv_odd below. */ const mbedtls_mpi_uint *Ap = A->p != NULL ? A->p : &zero; - size_t An = A->p == NULL ? 0 : A->n; - An = A->n <= N->n ? A->n : N->n; + size_t An = A->n >= N->n ? N->n : A->p != NULL ? A->n : 1; mbedtls_mpi_core_gcd_modinv_odd(G->p, Ip, Ap, An, N->p, N->n, T); + G->s = 1; + if (I != NULL) { + I->s = 1; + } + if (G->n > N->n) { memset(G->p + N->n, 0, ciL * (G->n - N->n)); } From 99270322ffac3546f813b1069fd6efb667cdce3f Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Wed, 6 Aug 2025 10:20:00 +0100 Subject: [PATCH 11/11] Improve mpi_gcd_invmod_odd() tests when I/G has more limbs than N Signed-off-by: Felix Conway --- tests/suites/test_suite_bignum.function | 60 ++++++++++++++----------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/tests/suites/test_suite_bignum.function b/tests/suites/test_suite_bignum.function index 9019053258..690d2bf509 100644 --- a/tests/suites/test_suite_bignum.function +++ b/tests/suites/test_suite_bignum.function @@ -1219,17 +1219,21 @@ void mpi_gcd_modinv_odd_both(char *input_A, char *input_N, res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); - /* Test G & I initialized to large number with non-zero limbs. */ - TEST_EQUAL(mbedtls_test_read_mpi(&G, "c7420eb50e52ce18795a1020896787ce18dcd6b0"), 0); - TEST_EQUAL(mbedtls_test_read_mpi(&I, "5702c227ee207cca33eb7a6151531b50541a47ef"), 0); - res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, &N); - TEST_EQUAL(res, return_code); - if (res == 0) { - TEST_ASSERT(sign_is_valid(&G)); - TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); - if (has_inverse) { - TEST_ASSERT(sign_is_valid(&I)); - TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + /* Test G & I initialized to a number with more limbs than N. */ + if (N.n > 0) { + TEST_EQUAL(mbedtls_mpi_grow(&G, N.n * 2), 0); + memset(G.p, 0x2d, G.n * sizeof(mbedtls_mpi_uint)); + TEST_EQUAL(mbedtls_mpi_grow(&I, N.n * 2), 0); + memset(I.p, 0x2f, I.n * sizeof(mbedtls_mpi_uint)); + res = mbedtls_mpi_gcd_modinv_odd(&G, &I, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + if (has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } } } @@ -1280,13 +1284,16 @@ void mpi_gcd_modinv_odd_only_gcd(char *input_A, char *input_N, res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); - /* Test G initialized to large number with non-zero limbs. */ - TEST_EQUAL(mbedtls_test_read_mpi(&G, "8d81032aaa52fb4d29831c7ed183fffee9baf169"), 0); - res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, &N); - TEST_EQUAL(res, return_code); - if (res == 0) { - TEST_ASSERT(sign_is_valid(&G)); - TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + /* Test G initialized to a number with more limbs than N. */ + if (N.n > 0) { + TEST_EQUAL(mbedtls_mpi_grow(&G, N.n * 2), 0); + memset(G.p, 0x2b, G.n * sizeof(mbedtls_mpi_uint)); + res = mbedtls_mpi_gcd_modinv_odd(&G, NULL, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&G)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&G, &exp_G), 0); + } } exit: @@ -1337,13 +1344,16 @@ void mpi_gcd_modinv_odd_only_modinv(char *input_A, char *input_N, res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, /* N */ &A); TEST_EQUAL(res, MBEDTLS_ERR_MPI_BAD_INPUT_DATA); - /* Test I initialized to large number with non-zero limbs. */ - TEST_EQUAL(mbedtls_test_read_mpi(&I, "bc0ccc030cb2d8b31e40e08fac727d2f4a8c9c1d"), 0); - res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, &N); - TEST_EQUAL(res, return_code); - if (res == 0 && has_inverse) { - TEST_ASSERT(sign_is_valid(&I)); - TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + /* Test I initialized to a number with more limbs than N. */ + if (N.n > 0) { + TEST_EQUAL(mbedtls_mpi_grow(&I, N.n * 2), 0); + memset(I.p, 0x29, I.n * sizeof(mbedtls_mpi_uint)); + res = mbedtls_mpi_gcd_modinv_odd(NULL, &I, &A, &N); + TEST_EQUAL(res, return_code); + if (res == 0 && has_inverse) { + TEST_ASSERT(sign_is_valid(&I)); + TEST_EQUAL(mbedtls_mpi_cmp_mpi(&I, &exp_I), 0); + } } exit: