From 381d4ba03be79df1a3d2777aa74cdf11f5d0b5b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 4 Aug 2025 10:57:13 +0200 Subject: [PATCH] Make mbedtls_mpi_gcd() more consistent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/gcd-sign.txt | 5 +++++ include/mbedtls/bignum.h | 3 +-- library/bignum.c | 19 ++++++++++--------- tests/suites/test_suite_bignum.misc.data | 4 ++-- 4 files changed, 18 insertions(+), 13 deletions(-) create mode 100644 ChangeLog.d/gcd-sign.txt diff --git a/ChangeLog.d/gcd-sign.txt b/ChangeLog.d/gcd-sign.txt new file mode 100644 index 0000000000..52d1e1f24f --- /dev/null +++ b/ChangeLog.d/gcd-sign.txt @@ -0,0 +1,5 @@ +Changes + * The function mbedtls_mpi_gcd() now always gives a non-negative output. + Previously the output was negative when B = 0 and A < 0, which was not + documented, and inconsistent as all other inputs resulted in a non-negative + output. diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index 297c0a6ba4..6187856713 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -974,8 +974,7 @@ int mbedtls_mpi_random(mbedtls_mpi *X, * \brief Compute the greatest common divisor: G = gcd(A, B) * * \param G The destination MPI. This must point to an initialized MPI. - * This will be positive unless \p B is 0, in which case \p A - * will be returned, where \p A could be negative. + * This will always be positive or 0. * \param A The first operand. This must point to an initialized MPI. * \param B The second operand. This must point to an initialized MPI. * diff --git a/library/bignum.c b/library/bignum.c index 7d5103ebb8..96cade4bfe 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1834,18 +1834,19 @@ int mbedtls_mpi_gcd(mbedtls_mpi *G, const mbedtls_mpi *A, const mbedtls_mpi *B) MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&TB, B)); TA.s = TB.s = 1; - /* Handle special cases (that don't happen in crypto usage) */ - if (mbedtls_mpi_core_check_zero_ct(A.p, A.n) == MBEDTLS_CT_FALSE) { - return mbedtls_mpi_copy(G, TB); // GCD(0, B) = abs(B) - } - if (mbedtls_mpi_core_check_zero_ct(B.p, B.n) == MBEDTLS_CT_FALSE) { - return mbedtls_mpi_copy(G, A); // GCD(A, 0) = A (for now) - } - - /* Make the two values the same (non-zero) number of limbs */ + /* Make the two values the same (non-zero) number of limbs. + * This is needed to use mbedtls_mpi_core functions below. */ MBEDTLS_MPI_CHK(mbedtls_mpi_grow(&TA, TB.n != 0 ? TB.n : 1)); MBEDTLS_MPI_CHK(mbedtls_mpi_grow(&TB, TA.n)); // non-zero from above + /* Handle special cases (that don't happen in crypto usage) */ + if (mbedtls_mpi_core_check_zero_ct(TA.p, TA.n) == MBEDTLS_CT_FALSE) { + return mbedtls_mpi_copy(G, &TB); // GCD(0, B) = abs(B) + } + if (mbedtls_mpi_core_check_zero_ct(TB.p, TB.n) == MBEDTLS_CT_FALSE) { + return mbedtls_mpi_copy(G, &TA); // GCD(A, 0) = abs(A) + } + const size_t za = mbedtls_mpi_lsb(&TA); const size_t zb = mbedtls_mpi_lsb(&TB); diff --git a/tests/suites/test_suite_bignum.misc.data b/tests/suites/test_suite_bignum.misc.data index 7b0c1e7323..4798512e76 100644 --- a/tests/suites/test_suite_bignum.misc.data +++ b/tests/suites/test_suite_bignum.misc.data @@ -1466,10 +1466,10 @@ Test GCD: 6, 0 (1 limb) mpi_gcd:"06":"00":"6" Test GCD: negative, 0 (null) -mpi_gcd:"-50000":"":"-50000" +mpi_gcd:"-50000":"":"50000" Test GCD: negative, 0 (1 limb) -mpi_gcd:"-a782374b2ee927df28802745833a":"00":"-a782374b2ee927df28802745833a" +mpi_gcd:"-a782374b2ee927df28802745833a":"00":"a782374b2ee927df28802745833a" Test GCD: 0 (null), negative mpi_gcd:"":"-50000":"50000"