From 38ff70e169e5b00bbe63038456638f357f01db3e Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 12 Aug 2024 18:20:59 +0100 Subject: [PATCH] Make _optionally_safe functions internal The complexity of having functions whose security properties depend on a runtime argument can be dangerous. Limit misuse by making any such functions local. Signed-off-by: Janos Follath --- include/mbedtls/bignum.h | 8 +++++--- library/bignum.c | 31 +++++++++++++++++++------------ library/bignum_core.c | 39 ++++++++++++++++++++++++++++++--------- library/bignum_core.h | 18 ++++++++---------- 4 files changed, 62 insertions(+), 34 deletions(-) diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index cd7e2f6c66..26c61f5db2 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -903,6 +903,8 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, /** * \brief Perform a modular exponentiation: X = A^E mod N * + * \warning This function is not constant time with respect to \p E (the exponent). + * * \param X The destination MPI. This must point to an initialized MPI. * This must not alias E or N. * \param A The base of the exponentiation. @@ -927,9 +929,9 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, * \return Another negative error code on different kinds of failures. * */ -int mbedtls_mpi_exp_mod_optionally_safe(mbedtls_mpi *X, const mbedtls_mpi *A, - const mbedtls_mpi *E, const mbedtls_mpi *N, - mbedtls_mpi *prec_RR, int E_public); +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 Perform a modular exponentiation: X = A^E mod N diff --git a/library/bignum.c b/library/bignum.c index 4db2b10544..6ac041eef9 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1610,9 +1610,13 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_s return 0; } -int mbedtls_mpi_exp_mod_optionally_safe(mbedtls_mpi *X, const mbedtls_mpi *A, - const mbedtls_mpi *E, const mbedtls_mpi *N, - mbedtls_mpi *prec_RR, int E_public) +/* + * Warning! If the parameter E_public has MBEDTLS_MPI_IS_PUBLIC as its value, + * this function is not constant time with respect to the exponent (parameter E). + */ +static int mbedtls_mpi_exp_mod_optionally_safe(mbedtls_mpi *X, const mbedtls_mpi *A, + const mbedtls_mpi *E, const mbedtls_mpi *N, + mbedtls_mpi *prec_RR, int E_public) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1695,15 +1699,11 @@ int mbedtls_mpi_exp_mod_optionally_safe(mbedtls_mpi *X, const mbedtls_mpi *A, { mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N->p); mbedtls_mpi_core_to_mont_rep(X->p, X->p, N->p, N->n, mm, RR.p, T); - mbedtls_mpi_core_exp_mod_optionally_safe(X->p, - X->p, - N->p, - N->n, - E->p, - E->n, - RR.p, - T, - E_public); + if (E_public == MBEDTLS_MPI_IS_PUBLIC) { + mbedtls_mpi_core_exp_mod_unsafe(X->p, X->p, N->p, N->n, E->p, E->n, RR.p, T); + } else { + mbedtls_mpi_core_exp_mod(X->p, X->p, N->p, N->n, E->p, E->n, RR.p, T); + } mbedtls_mpi_core_from_mont_rep(X->p, X->p, N->p, N->n, mm, T); } @@ -1735,6 +1735,13 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, return mbedtls_mpi_exp_mod_optionally_safe(X, A, E, N, prec_RR, MBEDTLS_MPI_IS_SECRET); } +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) +{ + return mbedtls_mpi_exp_mod_optionally_safe(X, A, E, N, prec_RR, MBEDTLS_MPI_IS_PUBLIC); +} + /* * Greatest common divisor: G = gcd(A, B) (HAC 14.54) */ diff --git a/library/bignum_core.c b/library/bignum_core.c index 518b1bd3f3..ab6cf8fcdc 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -747,6 +747,9 @@ static void exp_mod_precompute_window(const mbedtls_mpi_uint *A, } /* Exponentiation: X := A^E mod N. + * + * Warning! If the parameter E_public has MBEDTLS_MPI_IS_PUBLIC as its value, + * this function is not constant time with respect to the exponent (parameter E). * * A must already be in Montgomery form. * @@ -758,15 +761,15 @@ static void exp_mod_precompute_window(const mbedtls_mpi_uint *A, * (The difference is that the body in our loop processes a single bit instead * of a full window.) */ -void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, - const mbedtls_mpi_uint *A, - const mbedtls_mpi_uint *N, - size_t AN_limbs, - const mbedtls_mpi_uint *E, - size_t E_limbs, - const mbedtls_mpi_uint *RR, - mbedtls_mpi_uint *T, - int E_public) +static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, + const mbedtls_mpi_uint *A, + const mbedtls_mpi_uint *N, + size_t AN_limbs, + const mbedtls_mpi_uint *E, + size_t E_limbs, + const mbedtls_mpi_uint *RR, + mbedtls_mpi_uint *T, + int E_public) { const size_t wsize = exp_mod_get_window_size(E_limbs * biL); const size_t welem = ((size_t) 1) << wsize; @@ -872,6 +875,24 @@ void mbedtls_mpi_core_exp_mod(mbedtls_mpi_uint *X, MBEDTLS_MPI_IS_SECRET); } +void mbedtls_mpi_core_exp_mod_unsafe(mbedtls_mpi_uint *X, + const mbedtls_mpi_uint *A, + const mbedtls_mpi_uint *N, size_t AN_limbs, + const mbedtls_mpi_uint *E, size_t E_limbs, + const mbedtls_mpi_uint *RR, + mbedtls_mpi_uint *T) +{ + mbedtls_mpi_core_exp_mod_optionally_safe(X, + A, + N, + AN_limbs, + E, + E_limbs, + RR, + T, + MBEDTLS_MPI_IS_PUBLIC); +} + mbedtls_mpi_uint mbedtls_mpi_core_sub_int(mbedtls_mpi_uint *X, const mbedtls_mpi_uint *A, mbedtls_mpi_uint c, /* doubles as carry */ diff --git a/library/bignum_core.h b/library/bignum_core.h index c63cdee465..d208daf7ba 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -608,6 +608,8 @@ size_t mbedtls_mpi_core_exp_mod_working_limbs(size_t AN_limbs, size_t E_limbs); * \brief Perform a modular exponentiation with public or secret exponent: * X = A^E mod N, where \p A is already in Montgomery form. * + * \warning This function is not constant time with respect to \p E (the exponent). + * * \p X may be aliased to \p A, but not to \p RR or \p E, even if \p E_limbs == * \p AN_limbs. * @@ -630,17 +632,13 @@ size_t mbedtls_mpi_core_exp_mod_working_limbs(size_t AN_limbs, size_t E_limbs); * It is up to the caller to zeroize \p T when it is no * longer needed, and before freeing it if it was dynamically * allocated. - * \param[in] E_public Set to MBEDTLS_MPI_IS_PUBLIC to gain some performance - * when the value of E is public. - * Set to MBEDTLS_MPI_IS_SECRET when the value of E is secret. */ -void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, - const mbedtls_mpi_uint *A, - const mbedtls_mpi_uint *N, size_t AN_limbs, - const mbedtls_mpi_uint *E, size_t E_limbs, - const mbedtls_mpi_uint *RR, - mbedtls_mpi_uint *T, - int E_public); +void mbedtls_mpi_core_exp_mod_unsafe(mbedtls_mpi_uint *X, + const mbedtls_mpi_uint *A, + const mbedtls_mpi_uint *N, size_t AN_limbs, + const mbedtls_mpi_uint *E, size_t E_limbs, + const mbedtls_mpi_uint *RR, + mbedtls_mpi_uint *T); /** * \brief Perform a modular exponentiation with secret exponent: