From 7305002799416138979162cf425d9b9015bfacd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2024 12:52:45 +0200 Subject: [PATCH 01/35] Add optionally unsafe variant of exp_mod for perf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attempt to partially solve the performance regression in 3.6.0 without adding too much code size. Signed-off-by: Manuel Pégourié-Gonnard --- .../drivers/builtin/include/mbedtls/bignum.h | 49 ++++++++++++++++- tf-psa-crypto/drivers/builtin/src/bignum.c | 23 ++++++-- .../drivers/builtin/src/bignum_core.c | 55 +++++++++++++++---- .../drivers/builtin/src/bignum_core.h | 38 +++++++++++++ 4 files changed, 148 insertions(+), 17 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h index 71d7b97672..bb96f4fb89 100644 --- a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h +++ b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h @@ -44,6 +44,22 @@ goto cleanup; \ } while (0) +/* Constants to identify whether a value is public or secret. + * + * Parameters should be named X_public where X is the name of the + * corresponding input parameter. + * + * Implementation should always check using + * if (X_public == MBEDTLS_MPI_IS_PUBLIC) { + * // unsafe path + * } else { + * // safe path + * } + * not the other way round, in order to prevent misuse. (This is, if a value + * other than the two below is passed, default to the safe path.) */ +#define MBEDTLS_MPI_IS_PUBLIC 0x2a2a +#define MBEDTLS_MPI_IS_SECRET 0 + /* * Maximum size MPIs are allowed to grow to in number of limbs. */ @@ -880,7 +896,38 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_sint b); /** - * \brief Perform a sliding-window exponentiation: X = A^E mod N + * \brief Perform a modular exponentiation: X = A^E mod N + * + * \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. + * This must point to an initialized MPI. + * \param E The exponent MPI. This must point to an initialized MPI. + * \param N The base for the modular reduction. This must point to an + * initialized MPI. + * \param prec_RR A helper MPI depending solely on \p N which can be used to + * speed-up multiple modular exponentiations for the same value + * of \p N. This may be \c NULL. If it is not \c NULL, it must + * point to an initialized MPI. If it hasn't been used after + * the call to mbedtls_mpi_init(), this function will compute + * the helper value and store it in \p prec_RR for reuse on + * subsequent calls to this function. Otherwise, the function + * will assume that \p prec_RR holds the helper value set by a + * previous call to mbedtls_mpi_exp_mod(), and reuse it. + * + * \return \c 0 if successful. + * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if a memory allocation failed. + * \return #MBEDTLS_ERR_MPI_BAD_INPUT_DATA if \c N is negative or + * even, or if \c E is negative. + * \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); + +/** + * \brief Perform a modular exponentiation: X = A^E mod N * * \param X The destination MPI. This must point to an initialized MPI. * This must not alias E or N. diff --git a/tf-psa-crypto/drivers/builtin/src/bignum.c b/tf-psa-crypto/drivers/builtin/src/bignum.c index c45fd5bf24..4db2b10544 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum.c @@ -1610,9 +1610,9 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_s return 0; } -int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, - const mbedtls_mpi *E, const mbedtls_mpi *N, - mbedtls_mpi *prec_RR) +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,7 +1695,15 @@ int mbedtls_mpi_exp_mod(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(X->p, X->p, N->p, N->n, E->p, E->n, 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); mbedtls_mpi_core_from_mont_rep(X->p, X->p, N->p, N->n, mm, T); } @@ -1720,6 +1728,13 @@ cleanup: return ret; } +int mbedtls_mpi_exp_mod(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_SECRET); +} + /* * Greatest common divisor: G = gcd(A, B) (HAC 14.54) */ diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 39ab6e9091..d49b9d5a0f 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -759,14 +759,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(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) +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; @@ -804,6 +805,14 @@ void mbedtls_mpi_core_exp_mod(mbedtls_mpi_uint *X, * (limb_index=0, E_bit_index=0). */ size_t E_limb_index = E_limbs; size_t E_bit_index = 0; + if (E_public == MBEDTLS_MPI_IS_PUBLIC) { + size_t E_bits = mbedtls_mpi_core_bitlen(E, E_limbs); + if (E_bits != 0) { + E_limb_index = E_bits / biL; + E_bit_index = E_bits % biL; + } + } + /* At any given time, window contains window_bits bits from E. * window_bits can go up to wsize. */ size_t window_bits = 0; @@ -829,10 +838,14 @@ void mbedtls_mpi_core_exp_mod(mbedtls_mpi_uint *X, * when we've finished processing the exponent. */ if (window_bits == wsize || (E_bit_index == 0 && E_limb_index == 0)) { - /* Select Wtable[window] without leaking window through - * memory access patterns. */ - mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, - AN_limbs, welem, window); + if (E_public == MBEDTLS_MPI_IS_PUBLIC) { + memcpy(Wselect, Wtable + window * AN_limbs, AN_limbs * ciL); + } else { + /* Select Wtable[window] without leaking window through + * memory access patterns. */ + mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, + AN_limbs, welem, window); + } /* Multiply X by the selected element. */ mbedtls_mpi_core_montmul(X, X, Wselect, AN_limbs, N, AN_limbs, mm, temp); @@ -842,6 +855,24 @@ void mbedtls_mpi_core_exp_mod(mbedtls_mpi_uint *X, } while (!(E_bit_index == 0 && E_limb_index == 0)); } +void mbedtls_mpi_core_exp_mod(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_SECRET); +} + 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/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 51ecca5ff7..41d32f6e51 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -614,6 +614,44 @@ int mbedtls_mpi_core_random(mbedtls_mpi_uint *X, */ 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. + * + * \p X may be aliased to \p A, but not to \p RR or \p E, even if \p E_limbs == + * \p AN_limbs. + * + * \param[out] X The destination MPI, as a little endian array of length + * \p AN_limbs. + * \param[in] A The base MPI, as a little endian array of length \p AN_limbs. + * Must be in Montgomery form. + * \param[in] N The modulus, as a little endian array of length \p AN_limbs. + * \param AN_limbs The number of limbs in \p X, \p A, \p N, \p RR. + * \param[in] E The exponent, as a little endian array of length \p E_limbs. + * \param E_limbs The number of limbs in \p E. + * \param[in] RR The precomputed residue of 2^{2*biL} modulo N, as a little + * endian array of length \p AN_limbs. + * \param[in,out] T Temporary storage of at least the number of limbs returned + * by `mbedtls_mpi_core_exp_mod_working_limbs()`. + * Its initial content is unused and its final content is + * indeterminate. + * It must not alias or otherwise overlap any of the other + * parameters. + * 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); + /** * \brief Perform a modular exponentiation with secret exponent: * X = A^E mod N, where \p A is already in Montgomery form. From 91537eb09d9d01a60e4e6a97ce1c46a12d82958a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 12 Aug 2024 17:26:24 +0100 Subject: [PATCH 02/35] Improve documentation of MBEDTLS_MPI_IS_PUBLIC Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h index bb96f4fb89..cd7e2f6c66 100644 --- a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h +++ b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h @@ -44,7 +44,12 @@ goto cleanup; \ } while (0) -/* Constants to identify whether a value is public or secret. +/* Constants to identify whether a value is public or secret. If a parameter is marked as secret by + * this constant, the function must be constant time with respect to the parameter. + * + * This is only needed for functions with the _optionally_safe postfix. All other functions have + * fixed behavior that can't be changed at runtime and are constant time with respect to their + * parameters as prescribed by their documentation or by conventions in their module's documentation. * * Parameters should be named X_public where X is the name of the * corresponding input parameter. From 9a8b1f4a4c018356f51f76e507ead6eb99ec82f2 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 12 Aug 2024 18:20:59 +0100 Subject: [PATCH 03/35] 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 --- .../drivers/builtin/include/mbedtls/bignum.h | 8 ++-- tf-psa-crypto/drivers/builtin/src/bignum.c | 31 +++++++++------ .../drivers/builtin/src/bignum_core.c | 39 ++++++++++++++----- .../drivers/builtin/src/bignum_core.h | 18 ++++----- 4 files changed, 62 insertions(+), 34 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h index cd7e2f6c66..26c61f5db2 100644 --- a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h +++ b/tf-psa-crypto/drivers/builtin/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/tf-psa-crypto/drivers/builtin/src/bignum.c b/tf-psa-crypto/drivers/builtin/src/bignum.c index 4db2b10544..6ac041eef9 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum.c +++ b/tf-psa-crypto/drivers/builtin/src/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/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index d49b9d5a0f..5f4388d90d 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -748,6 +748,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. * @@ -759,15 +762,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; @@ -873,6 +876,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/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 41d32f6e51..01f0f3957e 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -618,6 +618,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. * @@ -640,17 +642,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: From 4726cb8f00dc94b6c87a1ac2c064a92b707cb204 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 12 Aug 2024 19:05:47 +0100 Subject: [PATCH 04/35] Move mixed security code to small local functions The complexity of having functions whose security properties depend on a runtime argument can be dangerous. Limit risk by isolating such code in small functions with limited scope. Signed-off-by: Janos Follath --- .../drivers/builtin/src/bignum_core.c | 70 +++++++++++++++---- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 5f4388d90d..3b9509f9b1 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -747,6 +747,56 @@ static void exp_mod_precompute_window(const mbedtls_mpi_uint *A, } } +/* + * This function calculates the indices of the exponent where the exponentiation algorithm should + * start processing. + * + * 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 inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint *E, + size_t E_limbs, + int E_public, + size_t *E_limb_index, + size_t *E_bit_index) +{ + if (E_public == MBEDTLS_MPI_IS_PUBLIC) { + size_t E_bits = mbedtls_mpi_core_bitlen(E, E_limbs); + if (E_bits != 0) { + *E_limb_index = E_bits / biL; + *E_bit_index = E_bits % biL; + } + } else { + /* + * Here we need to be constant time with respect to E and can't do anything better than + * start at the first allocated bit. + */ + *E_limb_index = E_limbs; + *E_bit_index = 0; + } +} + +/* + * Warning! If the parameter window_public has MBEDTLS_MPI_IS_PUBLIC as its value, this function is + * not constant time with respect to the window parameter and consequently the exponent of the + * exponentiation (parameter E of mbedtls_mpi_core_exp_mod_optionally_safe). + */ +static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselect, + mbedtls_mpi_uint *Wtable, + size_t AN_limbs, size_t welem, + mbedtls_mpi_uint window, + int window_public) +{ + if (window_public == MBEDTLS_MPI_IS_PUBLIC) { + memcpy(Wselect, Wtable + window * AN_limbs, AN_limbs * ciL); + } else { + /* Select Wtable[window] without leaking window through + * memory access patterns. */ + mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, + AN_limbs, welem, window); + } +} + /* Exponentiation: X := A^E mod N. * * Warning! If the parameter E_public has MBEDTLS_MPI_IS_PUBLIC as its value, @@ -808,13 +858,8 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, * (limb_index=0, E_bit_index=0). */ size_t E_limb_index = E_limbs; size_t E_bit_index = 0; - if (E_public == MBEDTLS_MPI_IS_PUBLIC) { - size_t E_bits = mbedtls_mpi_core_bitlen(E, E_limbs); - if (E_bits != 0) { - E_limb_index = E_bits / biL; - E_bit_index = E_bits % biL; - } - } + exp_mod_calc_first_bit_optionally_safe(E, E_limbs, E_public, + &E_limb_index, &E_bit_index); /* At any given time, window contains window_bits bits from E. * window_bits can go up to wsize. */ @@ -841,14 +886,9 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, * when we've finished processing the exponent. */ if (window_bits == wsize || (E_bit_index == 0 && E_limb_index == 0)) { - if (E_public == MBEDTLS_MPI_IS_PUBLIC) { - memcpy(Wselect, Wtable + window * AN_limbs, AN_limbs * ciL); - } else { - /* Select Wtable[window] without leaking window through - * memory access patterns. */ - mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, - AN_limbs, welem, window); - } + + exp_mod_table_lookup_optionally_safe(Wselect, Wtable, AN_limbs, welem, + window, E_public); /* Multiply X by the selected element. */ mbedtls_mpi_core_montmul(X, X, Wselect, AN_limbs, N, AN_limbs, mm, temp); From 5b69fade3171a8ab9d01977efadcc1a732c6e9c1 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 12 Aug 2024 19:32:45 +0100 Subject: [PATCH 05/35] Move MBEDTLS_MPI_IS_* macros to bignum_core.h These macros are not part of any public or internal API, ideally they would be defined in the source files. The reason to put them in bignum_core.h to avoid duplication as macros for this purpose are needed in both bignum.c and bignum_core.c. Signed-off-by: Janos Follath --- .../drivers/builtin/include/mbedtls/bignum.h | 21 ------------------- .../drivers/builtin/src/bignum_core.h | 21 +++++++++++++++++++ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h index 26c61f5db2..a945be3614 100644 --- a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h +++ b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h @@ -44,27 +44,6 @@ goto cleanup; \ } while (0) -/* Constants to identify whether a value is public or secret. If a parameter is marked as secret by - * this constant, the function must be constant time with respect to the parameter. - * - * This is only needed for functions with the _optionally_safe postfix. All other functions have - * fixed behavior that can't be changed at runtime and are constant time with respect to their - * parameters as prescribed by their documentation or by conventions in their module's documentation. - * - * Parameters should be named X_public where X is the name of the - * corresponding input parameter. - * - * Implementation should always check using - * if (X_public == MBEDTLS_MPI_IS_PUBLIC) { - * // unsafe path - * } else { - * // safe path - * } - * not the other way round, in order to prevent misuse. (This is, if a value - * other than the two below is passed, default to the safe path.) */ -#define MBEDTLS_MPI_IS_PUBLIC 0x2a2a -#define MBEDTLS_MPI_IS_SECRET 0 - /* * Maximum size MPIs are allowed to grow to in number of limbs. */ diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 01f0f3957e..1fc323ff90 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -90,6 +90,27 @@ #define GET_BYTE(X, i) \ (((X)[(i) / ciL] >> (((i) % ciL) * 8)) & 0xff) +/* Constants to identify whether a value is public or secret. If a parameter is marked as secret by + * this constant, the function must be constant time with respect to the parameter. + * + * This is only needed for functions with the _optionally_safe postfix. All other functions have + * fixed behavior that can't be changed at runtime and are constant time with respect to their + * parameters as prescribed by their documentation or by conventions in their module's documentation. + * + * Parameters should be named X_public where X is the name of the + * corresponding input parameter. + * + * Implementation should always check using + * if (X_public == MBEDTLS_MPI_IS_PUBLIC) { + * // unsafe path + * } else { + * // safe path + * } + * not the other way round, in order to prevent misuse. (This is, if a value + * other than the two below is passed, default to the safe path.) */ +#define MBEDTLS_MPI_IS_PUBLIC 0x2a2a +#define MBEDTLS_MPI_IS_SECRET 0 + /** Count leading zero bits in a given integer. * * \warning The result is undefined if \p a == 0 From 24fb8c9be5fbc333aa46953ac932f8ee4067e278 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 12 Aug 2024 19:55:02 +0100 Subject: [PATCH 06/35] Make MBEDTLS_MPI_IS_PUBLIC thumb friendly In Thumb instructions, constant can be: - any constant that can be produced by shifting an 8-bit value left by any number of bits within a 32-bit word - any constant of the form 0x00XY00XY - any constant of the form 0xXY00XY00 - any constant of the form 0xXYXYXYXY. Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 1fc323ff90..69fa3063d2 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -108,7 +108,7 @@ * } * not the other way round, in order to prevent misuse. (This is, if a value * other than the two below is passed, default to the safe path.) */ -#define MBEDTLS_MPI_IS_PUBLIC 0x2a2a +#define MBEDTLS_MPI_IS_PUBLIC 0x2a2a2a2a #define MBEDTLS_MPI_IS_SECRET 0 /** Count leading zero bits in a given integer. From 8fc736dc4ea73b99a7af15dcba705b7784038f50 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 12 Aug 2024 20:11:06 +0100 Subject: [PATCH 07/35] Move _public parameters next to their target It is easier to read if the parameter controlling constant timeness with respect to a parameter is next to that parameter. Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum.c | 8 ++++---- tf-psa-crypto/drivers/builtin/src/bignum_core.c | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum.c b/tf-psa-crypto/drivers/builtin/src/bignum.c index 6ac041eef9..b1f9c1bf0f 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum.c @@ -1615,8 +1615,8 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_s * 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) + const mbedtls_mpi *E, int E_public, + const mbedtls_mpi *N, mbedtls_mpi *prec_RR) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1732,14 +1732,14 @@ int mbedtls_mpi_exp_mod(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_SECRET); + return mbedtls_mpi_exp_mod_optionally_safe(X, A, E, MBEDTLS_MPI_IS_SECRET, N, prec_RR); } 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); + return mbedtls_mpi_exp_mod_optionally_safe(X, A, E, MBEDTLS_MPI_IS_PUBLIC, N, prec_RR); } /* diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 3b9509f9b1..b7454a57f4 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -818,9 +818,9 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, size_t AN_limbs, const mbedtls_mpi_uint *E, size_t E_limbs, + int E_public, const mbedtls_mpi_uint *RR, - mbedtls_mpi_uint *T, - int E_public) + mbedtls_mpi_uint *T) { const size_t wsize = exp_mod_get_window_size(E_limbs * biL); const size_t welem = ((size_t) 1) << wsize; @@ -911,9 +911,9 @@ void mbedtls_mpi_core_exp_mod(mbedtls_mpi_uint *X, AN_limbs, E, E_limbs, + MBEDTLS_MPI_IS_SECRET, RR, - T, - MBEDTLS_MPI_IS_SECRET); + T); } void mbedtls_mpi_core_exp_mod_unsafe(mbedtls_mpi_uint *X, @@ -929,9 +929,9 @@ void mbedtls_mpi_core_exp_mod_unsafe(mbedtls_mpi_uint *X, AN_limbs, E, E_limbs, + MBEDTLS_MPI_IS_PUBLIC, RR, - T, - MBEDTLS_MPI_IS_PUBLIC); + T); } mbedtls_mpi_uint mbedtls_mpi_core_sub_int(mbedtls_mpi_uint *X, From a099ac98127aec4f070b1a81bb713b25b47fd9ca Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 13 Aug 2024 07:53:20 +0100 Subject: [PATCH 08/35] Use actual exponent size for window calculation The allocated size can be significantly larger than the actual size. In the unsafe case we can use the actual size and gain some performance. Signed-off-by: Janos Follath --- .../drivers/builtin/src/bignum_core.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index b7454a57f4..f8e97fbf7b 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -822,7 +822,15 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, const mbedtls_mpi_uint *RR, mbedtls_mpi_uint *T) { - const size_t wsize = exp_mod_get_window_size(E_limbs * biL); + /* We'll process the bits of E from most significant + * (limb_index=E_limbs-1, E_bit_index=biL-1) to least significant + * (limb_index=0, E_bit_index=0). */ + size_t E_limb_index = E_limbs; + size_t E_bit_index = 0; + exp_mod_calc_first_bit_optionally_safe(E, E_limbs, E_public, + &E_limb_index, &E_bit_index); + + const size_t wsize = exp_mod_get_window_size(E_limb_index * biL); const size_t welem = ((size_t) 1) << wsize; /* This is how we will use the temporary storage T, which must have space @@ -853,14 +861,6 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, /* X = 1 (in Montgomery presentation) initially */ memcpy(X, Wtable, AN_limbs * ciL); - /* We'll process the bits of E from most significant - * (limb_index=E_limbs-1, E_bit_index=biL-1) to least significant - * (limb_index=0, E_bit_index=0). */ - size_t E_limb_index = E_limbs; - size_t E_bit_index = 0; - exp_mod_calc_first_bit_optionally_safe(E, E_limbs, E_public, - &E_limb_index, &E_bit_index); - /* At any given time, window contains window_bits bits from E. * window_bits can go up to wsize. */ size_t window_bits = 0; From df5e55bcb79ed50f22fc04b63d8cc6de611a2321 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 13 Aug 2024 08:40:31 +0100 Subject: [PATCH 09/35] Add tests for optionally safe codepaths The new test hooks allow to check whether there was an unsafe call of an optionally safe function in the codepath. For the sake of simplicity the MBEDTLS_MPI_IS_* macros are reused for signalling safe/unsafe codepaths here too. Signed-off-by: Janos Follath --- .../drivers/builtin/src/bignum_core.c | 18 ++++++++++++++++++ .../drivers/builtin/src/bignum_core.h | 10 ++++++++++ .../suites/test_suite_bignum_core.function | 12 ++++++++++++ 3 files changed, 40 insertions(+) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index f8e97fbf7b..9eae74cf03 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -766,6 +766,9 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint *E_limb_index = E_bits / biL; *E_bit_index = E_bits % biL; } +#if defined(MBEDTLS_TEST_HOOKS) + mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC; +#endif } else { /* * Here we need to be constant time with respect to E and can't do anything better than @@ -773,6 +776,12 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint */ *E_limb_index = E_limbs; *E_bit_index = 0; +#if defined(MBEDTLS_TEST_HOOKS) + // Only mark the codepath safe if there wasn't an unsafe codepath before + if (mbedtls_mpi_optionally_safe_codepath != MBEDTLS_MPI_IS_PUBLIC) { + mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_SECRET; + } +#endif } } @@ -789,11 +798,20 @@ static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselec { if (window_public == MBEDTLS_MPI_IS_PUBLIC) { memcpy(Wselect, Wtable + window * AN_limbs, AN_limbs * ciL); +#if defined(MBEDTLS_TEST_HOOKS) + mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC; +#endif } else { /* Select Wtable[window] without leaking window through * memory access patterns. */ mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, AN_limbs, welem, window); +#if defined(MBEDTLS_TEST_HOOKS) + // Only mark the codepath safe if there wasn't an unsafe codepath before + if (mbedtls_mpi_optionally_safe_codepath != MBEDTLS_MPI_IS_PUBLIC) { + mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_SECRET; + } +#endif } } diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 69fa3063d2..eb84161279 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -830,4 +830,14 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, mbedtls_mpi_uint mm, mbedtls_mpi_uint *T); +#if defined(MBEDTLS_TEST_HOOKS) +int mbedtls_mpi_optionally_safe_codepath; + +static inline void mbedtls_mpi_optionally_safe_codepath_reset() +{ + // Set to a default that is neither MBEDTLS_MPI_IS_PUBLIC nor MBEDTLS_MPI_IS_SECRET + mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC + MBEDTLS_MPI_IS_SECRET + 1; +} +#endif + #endif /* MBEDTLS_BIGNUM_CORE_H */ diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function index 6c0bd1e0c4..fe47cc69a6 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function @@ -1301,7 +1301,13 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); +#if defined(MBEDTLS_TEST_HOOKS) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif mbedtls_mpi_core_exp_mod(Y, A, N, N_limbs, E, E_limbs, R2, T); +#if defined(MBEDTLS_TEST_HOOKS) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif TEST_CF_PUBLIC(Y, N_limbs * sizeof(mbedtls_mpi_uint)); @@ -1312,7 +1318,13 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); /* Check when output aliased to input */ +#if defined(MBEDTLS_TEST_HOOKS) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif mbedtls_mpi_core_exp_mod(A, A, N, N_limbs, E, E_limbs, R2, T); +#if defined(MBEDTLS_TEST_HOOKS) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); From 1fa5f3a9297158fec9aa215dbd482b96be2f176c Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 13 Aug 2024 11:39:03 +0100 Subject: [PATCH 10/35] Add tests for optionally unsafe code paths Signed-off-by: Janos Follath --- .../suites/test_suite_bignum_core.function | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function index fe47cc69a6..accfffd816 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function @@ -1246,6 +1246,7 @@ void mpi_core_exp_mod(char *input_N, char *input_A, char *input_E, char *input_X) { mbedtls_mpi_uint *A = NULL; + mbedtls_mpi_uint *A_copy = NULL; mbedtls_mpi_uint *E = NULL; mbedtls_mpi_uint *N = NULL; mbedtls_mpi_uint *X = NULL; @@ -1297,10 +1298,10 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CALLOC(T, working_limbs); + /* Test the safe variant */ TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); - #if defined(MBEDTLS_TEST_HOOKS) mbedtls_mpi_optionally_safe_codepath_reset(); #endif @@ -1308,16 +1309,31 @@ void mpi_core_exp_mod(char *input_N, char *input_A, #if defined(MBEDTLS_TEST_HOOKS) TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); #endif + TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); TEST_CF_PUBLIC(Y, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); + /* Test the unsafe variant */ + +#if defined(MBEDTLS_TEST_HOOKS) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif + mbedtls_mpi_core_exp_mod_unsafe(Y, A, N, N_limbs, E, E_limbs, R2, T); +#if defined(MBEDTLS_TEST_HOOKS) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); +#endif + TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); + + /* Check both with output aliased to input */ + + TEST_CALLOC(A_copy, A_limbs); + memcpy(A_copy, A, sizeof(A_copy) * A_limbs); + TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); - - /* Check when output aliased to input */ #if defined(MBEDTLS_TEST_HOOKS) mbedtls_mpi_optionally_safe_codepath_reset(); #endif @@ -1325,13 +1341,23 @@ void mpi_core_exp_mod(char *input_N, char *input_A, #if defined(MBEDTLS_TEST_HOOKS) TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); #endif + TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); + memcpy(A, A_copy, sizeof(A) * A_limbs); +#if defined(MBEDTLS_TEST_HOOKS) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif + mbedtls_mpi_core_exp_mod_unsafe(A, A, N, N_limbs, E, E_limbs, R2, T); +#if defined(MBEDTLS_TEST_HOOKS) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); +#endif TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); exit: mbedtls_free(T); mbedtls_free(A); + mbedtls_free(A_copy); mbedtls_free(E); mbedtls_free(N); mbedtls_free(X); From b64f1b50acc4fb7444de01debf5a04c30498604e Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 15 Aug 2024 15:53:07 +0100 Subject: [PATCH 11/35] Fix mpi_core_exp_mod documentation Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 9eae74cf03..69707f8f3b 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -867,7 +867,7 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, const mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N); - /* Set Wtable[i] = A^(2^i) (in Montgomery representation) */ + /* Set Wtable[i] = A^i (in Montgomery representation) */ exp_mod_precompute_window(A, N, AN_limbs, mm, RR, welem, Wtable, temp); From 87253af893ca4de77a98ebebb4e0a65f0cfe7b79 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 15 Aug 2024 16:06:19 +0100 Subject: [PATCH 12/35] Optimise public RSA operations Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/rsa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tf-psa-crypto/drivers/builtin/src/rsa.c b/tf-psa-crypto/drivers/builtin/src/rsa.c index 8a4c3d0485..4f453ef882 100644 --- a/tf-psa-crypto/drivers/builtin/src/rsa.c +++ b/tf-psa-crypto/drivers/builtin/src/rsa.c @@ -1257,7 +1257,7 @@ int mbedtls_rsa_public(mbedtls_rsa_context *ctx, } olen = ctx->len; - MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&T, &T, &ctx->E, &ctx->N, &ctx->RN)); + MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod_unsafe(&T, &T, &ctx->E, &ctx->N, &ctx->RN)); MBEDTLS_MPI_CHK(mbedtls_mpi_write_binary(&T, output, olen)); cleanup: From 08091d79dd5b5799fb4959709cb1d5f21193eb63 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 20 Aug 2024 09:56:16 +0100 Subject: [PATCH 13/35] Fix optionally safe hooks declarations Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum_core.c | 5 +++++ tf-psa-crypto/drivers/builtin/src/bignum_core.h | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 69707f8f3b..3a24c85329 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -747,6 +747,11 @@ static void exp_mod_precompute_window(const mbedtls_mpi_uint *A, } } +#if defined(MBEDTLS_TEST_HOOKS) +// Set to a default that is neither MBEDTLS_MPI_IS_PUBLIC nor MBEDTLS_MPI_IS_SECRET +int mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC + MBEDTLS_MPI_IS_SECRET + 1; +#endif + /* * This function calculates the indices of the exponent where the exponentiation algorithm should * start processing. diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index eb84161279..90a2dee8c4 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -831,9 +831,9 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, mbedtls_mpi_uint *T); #if defined(MBEDTLS_TEST_HOOKS) -int mbedtls_mpi_optionally_safe_codepath; +extern int mbedtls_mpi_optionally_safe_codepath; -static inline void mbedtls_mpi_optionally_safe_codepath_reset() +static inline void mbedtls_mpi_optionally_safe_codepath_reset(void) { // Set to a default that is neither MBEDTLS_MPI_IS_PUBLIC nor MBEDTLS_MPI_IS_SECRET mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC + MBEDTLS_MPI_IS_SECRET + 1; From d6aaee10fdc21063b63e8d392b302eafe83f134a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 20 Aug 2024 10:21:54 +0100 Subject: [PATCH 14/35] Disable optionally safe test hook in threading builds Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum_core.c | 10 +++++----- tf-psa-crypto/drivers/builtin/src/bignum_core.h | 5 ++++- .../tests/suites/test_suite_bignum_core.function | 16 ++++++++-------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 3a24c85329..629fb9c500 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -747,7 +747,7 @@ static void exp_mod_precompute_window(const mbedtls_mpi_uint *A, } } -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) // Set to a default that is neither MBEDTLS_MPI_IS_PUBLIC nor MBEDTLS_MPI_IS_SECRET int mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC + MBEDTLS_MPI_IS_SECRET + 1; #endif @@ -771,7 +771,7 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint *E_limb_index = E_bits / biL; *E_bit_index = E_bits % biL; } -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC; #endif } else { @@ -781,7 +781,7 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint */ *E_limb_index = E_limbs; *E_bit_index = 0; -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) // Only mark the codepath safe if there wasn't an unsafe codepath before if (mbedtls_mpi_optionally_safe_codepath != MBEDTLS_MPI_IS_PUBLIC) { mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_SECRET; @@ -803,7 +803,7 @@ static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselec { if (window_public == MBEDTLS_MPI_IS_PUBLIC) { memcpy(Wselect, Wtable + window * AN_limbs, AN_limbs * ciL); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC; #endif } else { @@ -811,7 +811,7 @@ static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselec * memory access patterns. */ mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, AN_limbs, welem, window); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) // Only mark the codepath safe if there wasn't an unsafe codepath before if (mbedtls_mpi_optionally_safe_codepath != MBEDTLS_MPI_IS_PUBLIC) { mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_SECRET; diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 90a2dee8c4..3d32bf2c5d 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -830,7 +830,10 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, mbedtls_mpi_uint mm, mbedtls_mpi_uint *T); -#if defined(MBEDTLS_TEST_HOOKS) +/* + * Can't define thread local variables with our abstraction layer: do nothing if threading is on. + */ +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) extern int mbedtls_mpi_optionally_safe_codepath; static inline void mbedtls_mpi_optionally_safe_codepath_reset(void) diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function index accfffd816..39a50520aa 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function @@ -1302,11 +1302,11 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath_reset(); #endif mbedtls_mpi_core_exp_mod(Y, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); #endif TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); @@ -1317,11 +1317,11 @@ void mpi_core_exp_mod(char *input_N, char *input_A, /* Test the unsafe variant */ -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath_reset(); #endif mbedtls_mpi_core_exp_mod_unsafe(Y, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); #endif TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); @@ -1334,22 +1334,22 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath_reset(); #endif mbedtls_mpi_core_exp_mod(A, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); #endif TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); memcpy(A, A_copy, sizeof(A) * A_limbs); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath_reset(); #endif mbedtls_mpi_core_exp_mod_unsafe(A, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); #endif TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); From 76c0e6f3a255c58c1e52d74fcdc7e2b8a157bdd7 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 20 Aug 2024 10:41:55 +0100 Subject: [PATCH 15/35] Clean up initialization in _core_exp_mod() Signed-off-by: Janos Follath --- .../drivers/builtin/src/bignum_core.c | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 629fb9c500..2500644a6d 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -766,11 +766,21 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint size_t *E_bit_index) { if (E_public == MBEDTLS_MPI_IS_PUBLIC) { + /* + * Skip leading zero bits. + */ size_t E_bits = mbedtls_mpi_core_bitlen(E, E_limbs); - if (E_bits != 0) { - *E_limb_index = E_bits / biL; - *E_bit_index = E_bits % biL; + if (E_bits == 0) { + /* + * If E is 0 mbedtls_mpi_core_bitlen() returns 0. Even if that is the case, we will want + * to represent it as a single 0 bit and as such the bitlength will be 1. + */ + E_bits = 1; } + + *E_limb_index = E_bits / biL; + *E_bit_index = E_bits % biL; + #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC; #endif @@ -848,8 +858,8 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, /* We'll process the bits of E from most significant * (limb_index=E_limbs-1, E_bit_index=biL-1) to least significant * (limb_index=0, E_bit_index=0). */ - size_t E_limb_index = E_limbs; - size_t E_bit_index = 0; + size_t E_limb_index; + size_t E_bit_index; exp_mod_calc_first_bit_optionally_safe(E, E_limbs, E_public, &E_limb_index, &E_bit_index); From a7eb81290b178ad74adf27b5ac33060087e050b1 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 20 Aug 2024 12:33:42 +0100 Subject: [PATCH 16/35] Fix memory corruption in exp_mod tests Signed-off-by: Janos Follath --- tf-psa-crypto/tests/suites/test_suite_bignum_core.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function index 39a50520aa..3128307b5b 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function @@ -1329,7 +1329,7 @@ void mpi_core_exp_mod(char *input_N, char *input_A, /* Check both with output aliased to input */ TEST_CALLOC(A_copy, A_limbs); - memcpy(A_copy, A, sizeof(A_copy) * A_limbs); + memcpy(A_copy, A, sizeof(*A_copy) * A_limbs); TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); @@ -1344,7 +1344,7 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); - memcpy(A, A_copy, sizeof(A) * A_limbs); + memcpy(A, A_copy, sizeof(*A) * A_limbs); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) mbedtls_mpi_optionally_safe_codepath_reset(); #endif From e1d1854a32188303c2c37b0a0614ece9b651efc7 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 12:34:10 +0100 Subject: [PATCH 17/35] Add changelog Signed-off-by: Janos Follath --- ChangeLog.d/fix-rsa-performance-regression.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/fix-rsa-performance-regression.txt diff --git a/ChangeLog.d/fix-rsa-performance-regression.txt b/ChangeLog.d/fix-rsa-performance-regression.txt new file mode 100644 index 0000000000..abcc7a5adc --- /dev/null +++ b/ChangeLog.d/fix-rsa-performance-regression.txt @@ -0,0 +1,2 @@ +Bugfix + * Fix unintended performance regression when using short RSA public keys. See #9232. From 6872c5f67d1229b1baab1f47095d7eb8e7ca3391 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 13:00:12 +0100 Subject: [PATCH 18/35] Make mbedtls_mpi_exp_mod_unsafe internal Signed-off-by: Janos Follath --- .../drivers/builtin/include/mbedtls/bignum.h | 33 ------------------- tf-psa-crypto/drivers/builtin/src/rsa.c | 10 ++++++ 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h index a945be3614..8367cd34e6 100644 --- a/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h +++ b/tf-psa-crypto/drivers/builtin/include/mbedtls/bignum.h @@ -879,39 +879,6 @@ int mbedtls_mpi_mod_mpi(mbedtls_mpi *R, const mbedtls_mpi *A, int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_sint b); -/** - * \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. - * This must point to an initialized MPI. - * \param E The exponent MPI. This must point to an initialized MPI. - * \param N The base for the modular reduction. This must point to an - * initialized MPI. - * \param prec_RR A helper MPI depending solely on \p N which can be used to - * speed-up multiple modular exponentiations for the same value - * of \p N. This may be \c NULL. If it is not \c NULL, it must - * point to an initialized MPI. If it hasn't been used after - * the call to mbedtls_mpi_init(), this function will compute - * the helper value and store it in \p prec_RR for reuse on - * subsequent calls to this function. Otherwise, the function - * will assume that \p prec_RR holds the helper value set by a - * previous call to mbedtls_mpi_exp_mod(), and reuse it. - * - * \return \c 0 if successful. - * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if a memory allocation failed. - * \return #MBEDTLS_ERR_MPI_BAD_INPUT_DATA if \c N is negative or - * even, or if \c E is negative. - * \return Another negative error code on different kinds of failures. - * - */ -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/tf-psa-crypto/drivers/builtin/src/rsa.c b/tf-psa-crypto/drivers/builtin/src/rsa.c index 4f453ef882..3df00c06a0 100644 --- a/tf-psa-crypto/drivers/builtin/src/rsa.c +++ b/tf-psa-crypto/drivers/builtin/src/rsa.c @@ -1226,6 +1226,16 @@ int mbedtls_rsa_check_pub_priv(const mbedtls_rsa_context *pub, return 0; } +/* + * This function is identical to mbedtls_mpi_exp_mod() the only difference is that this function is + * not constant time. + * + * WARNING! This function is not constant time. + */ +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); + /* * Do an RSA public key operation */ From 6154765c1b5ac11bf5850852260d7247e4350cf5 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 14:49:58 +0100 Subject: [PATCH 19/35] Improve ChangeLog Co-authored-by: Gilles Peskine Signed-off-by: Janos Follath --- ChangeLog.d/fix-rsa-performance-regression.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/fix-rsa-performance-regression.txt b/ChangeLog.d/fix-rsa-performance-regression.txt index abcc7a5adc..1624ac2618 100644 --- a/ChangeLog.d/fix-rsa-performance-regression.txt +++ b/ChangeLog.d/fix-rsa-performance-regression.txt @@ -1,2 +1,2 @@ Bugfix - * Fix unintended performance regression when using short RSA public keys. See #9232. + * Fix unintended performance regression when using short RSA public keys. Fixes #9232. From c870e05a09e7115dfb44537f4a33258efe5eee89 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 14:53:13 +0100 Subject: [PATCH 20/35] Add header for mbedtls_mpi_exp_mod_unsafe() To silence no previous prototype warnings. And this is the proper way to do it anyway. Signed-off-by: Janos Follath --- library/bignum_internal.h | 50 +++++++++++++++++++ tf-psa-crypto/drivers/builtin/src/bignum.c | 1 + tf-psa-crypto/drivers/builtin/src/rsa.c | 11 +--- .../tests/suites/test_suite_bignum.function | 1 + 4 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 library/bignum_internal.h diff --git a/library/bignum_internal.h b/library/bignum_internal.h new file mode 100644 index 0000000000..aceaf55ea2 --- /dev/null +++ b/library/bignum_internal.h @@ -0,0 +1,50 @@ +/** + * \file bignum_internal.h + * + * \brief Internal-only bignum public-key cryptosystem API. + * + * This file declares bignum-related functions that are to be used + * only from within the Mbed TLS library itself. + * + */ +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ +#ifndef MBEDTLS_BIGNUM_INTERNAL_H +#define MBEDTLS_BIGNUM_INTERNAL_H + +/** + * \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. + * This must point to an initialized MPI. + * \param E The exponent MPI. This must point to an initialized MPI. + * \param N The base for the modular reduction. This must point to an + * initialized MPI. + * \param prec_RR A helper MPI depending solely on \p N which can be used to + * speed-up multiple modular exponentiations for the same value + * of \p N. This may be \c NULL. If it is not \c NULL, it must + * point to an initialized MPI. If it hasn't been used after + * the call to mbedtls_mpi_init(), this function will compute + * the helper value and store it in \p prec_RR for reuse on + * subsequent calls to this function. Otherwise, the function + * will assume that \p prec_RR holds the helper value set by a + * previous call to mbedtls_mpi_exp_mod(), and reuse it. + * + * \return \c 0 if successful. + * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if a memory allocation failed. + * \return #MBEDTLS_ERR_MPI_BAD_INPUT_DATA if \c N is negative or + * even, or if \c E is negative. + * \return Another negative error code on different kinds of failures. + * + */ +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); + +#endif /* bignum_internal.h */ diff --git a/tf-psa-crypto/drivers/builtin/src/bignum.c b/tf-psa-crypto/drivers/builtin/src/bignum.c index b1f9c1bf0f..424490951d 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum.c @@ -27,6 +27,7 @@ #include "mbedtls/bignum.h" #include "bignum_core.h" +#include "bignum_internal.h" #include "bn_mul.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" diff --git a/tf-psa-crypto/drivers/builtin/src/rsa.c b/tf-psa-crypto/drivers/builtin/src/rsa.c index 3df00c06a0..16912fd9a7 100644 --- a/tf-psa-crypto/drivers/builtin/src/rsa.c +++ b/tf-psa-crypto/drivers/builtin/src/rsa.c @@ -29,6 +29,7 @@ #include "mbedtls/rsa.h" #include "bignum_core.h" +#include "bignum_internal.h" #include "rsa_alt_helpers.h" #include "rsa_internal.h" #include "mbedtls/oid.h" @@ -1226,16 +1227,6 @@ int mbedtls_rsa_check_pub_priv(const mbedtls_rsa_context *pub, return 0; } -/* - * This function is identical to mbedtls_mpi_exp_mod() the only difference is that this function is - * not constant time. - * - * WARNING! This function is not constant time. - */ -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); - /* * Do an RSA public key operation */ diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum.function b/tf-psa-crypto/tests/suites/test_suite_bignum.function index 1830e5aa1c..3ac4e10ea6 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum.function @@ -3,6 +3,7 @@ #include "mbedtls/entropy.h" #include "constant_time_internal.h" #include "bignum_core.h" +#include "bignum_internal.h" #include "test/constant_flow.h" #if MBEDTLS_MPI_MAX_BITS > 792 From b6769598c6d7f72b19374409d892a59e9d13d3ec Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 15:45:18 +0100 Subject: [PATCH 21/35] Fix Changelog formatting Signed-off-by: Janos Follath --- ChangeLog.d/fix-rsa-performance-regression.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog.d/fix-rsa-performance-regression.txt b/ChangeLog.d/fix-rsa-performance-regression.txt index 1624ac2618..603612a314 100644 --- a/ChangeLog.d/fix-rsa-performance-regression.txt +++ b/ChangeLog.d/fix-rsa-performance-regression.txt @@ -1,2 +1,3 @@ Bugfix - * Fix unintended performance regression when using short RSA public keys. Fixes #9232. + * Fix unintended performance regression when using short RSA public keys. + Fixes #9232. From 64467ff6d221396e478fb8f2d14f9041601a7cf0 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 21 Aug 2024 13:15:13 +0100 Subject: [PATCH 22/35] Add tests for optionally safe code paths in bignum Not adding _unsafe version to the tests targeting behaviour related to RR as it is independent from the secret involved in the safe/unsafe distinction. Signed-off-by: Janos Follath --- .../tests/suites/test_suite_bignum.function | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum.function b/tf-psa-crypto/tests/suites/test_suite_bignum.function index 3ac4e10ea6..5e18441d7e 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum.function @@ -989,7 +989,13 @@ void mpi_exp_mod_min_RR(char *input_A, char *input_E, * against a smaller RR. */ TEST_LE_U(RR.n, N.n - 1); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif /* We know that exp_mod internally needs RR to be as large as N. * Validate that it is the case now, otherwise there was probably * a buffer overread. */ @@ -1022,7 +1028,26 @@ void mpi_exp_mod(char *input_A, char *input_E, TEST_ASSERT(mbedtls_test_read_mpi(&N, input_N) == 0); TEST_ASSERT(mbedtls_test_read_mpi(&X, input_X) == 0); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, NULL); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif + TEST_ASSERT(res == exp_result); + if (res == 0) { + TEST_ASSERT(sign_is_valid(&Z)); + TEST_ASSERT(mbedtls_mpi_cmp_mpi(&Z, &X) == 0); + } + +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif + res = mbedtls_mpi_exp_mod_unsafe(&Z, &A, &E, &N, NULL); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); +#endif TEST_ASSERT(res == exp_result); if (res == 0) { TEST_ASSERT(sign_is_valid(&Z)); @@ -1030,7 +1055,13 @@ void mpi_exp_mod(char *input_A, char *input_E, } /* Now test again with the speed-up parameter supplied as an output. */ +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif TEST_ASSERT(res == exp_result); if (res == 0) { TEST_ASSERT(sign_is_valid(&Z)); @@ -1038,7 +1069,13 @@ void mpi_exp_mod(char *input_A, char *input_E, } /* Now test again with the speed-up parameter supplied in calculated form. */ +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif TEST_ASSERT(res == exp_result); if (res == 0) { TEST_ASSERT(sign_is_valid(&Z)); @@ -1078,7 +1115,21 @@ void mpi_exp_mod_size(int A_bytes, int E_bytes, int N_bytes, TEST_ASSERT(mbedtls_test_read_mpi(&RR, input_RR) == 0); } +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif TEST_ASSERT(mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR) == exp_result); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif + +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif + TEST_ASSERT(mbedtls_mpi_exp_mod_unsafe(&Z, &A, &E, &N, &RR) == exp_result); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif exit: mbedtls_mpi_free(&A); mbedtls_mpi_free(&E); mbedtls_mpi_free(&N); From e0825bba493e286284e7aeb974ac982d7d35583a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 21 Aug 2024 13:24:01 +0100 Subject: [PATCH 23/35] Add tests for optionally safe code paths in RSA Only add the test hooks where it is meaningful. That is, not adding where the operation is essentially the same or the target is not the function that is being tested. Signed-off-by: Janos Follath --- tf-psa-crypto/tests/suites/test_suite_rsa.function | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tf-psa-crypto/tests/suites/test_suite_rsa.function b/tf-psa-crypto/tests/suites/test_suite_rsa.function index e82452927e..e0206eca29 100644 --- a/tf-psa-crypto/tests/suites/test_suite_rsa.function +++ b/tf-psa-crypto/tests/suites/test_suite_rsa.function @@ -1,5 +1,6 @@ /* BEGIN_HEADER */ #include "mbedtls/rsa.h" +#include "bignum_core.h" #include "rsa_alt_helpers.h" #include "rsa_internal.h" /* END_HEADER */ @@ -489,7 +490,13 @@ void mbedtls_rsa_public(data_t *message_str, int mod, TEST_EQUAL(mbedtls_rsa_get_bitlen(&ctx), (size_t) mod); TEST_ASSERT(mbedtls_rsa_check_pubkey(&ctx) == 0); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif TEST_ASSERT(mbedtls_rsa_public(&ctx, message_str->x, output) == result); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); +#endif if (result == 0) { TEST_ASSERT(mbedtls_test_hexcmp(output, result_str->x, @@ -554,9 +561,15 @@ void mbedtls_rsa_private(data_t *message_str, int mod, /* repeat three times to test updating of blinding values */ for (i = 0; i < 3; i++) { memset(output, 0x00, sizeof(output)); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_mpi_optionally_safe_codepath_reset(); +#endif TEST_ASSERT(mbedtls_rsa_private(&ctx, mbedtls_test_rnd_pseudo_rand, &rnd_info, message_str->x, output) == result); +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); +#endif if (result == 0) { TEST_ASSERT(mbedtls_test_hexcmp(output, result_str->x, From 816a71f85e6e8f7a79d77a56709a67a5bf8a5853 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 08:25:33 +0100 Subject: [PATCH 24/35] Introduce MBEDTLS_MPI_IS_TEST A + B + 1 is not a good way to get a number that's neither A nor B. This can be a problem for example if values later are changed to A = 0 and B = -1. Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum_core.c | 3 +-- tf-psa-crypto/drivers/builtin/src/bignum_core.h | 7 +++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 2500644a6d..58e01723a5 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -748,8 +748,7 @@ static void exp_mod_precompute_window(const mbedtls_mpi_uint *A, } #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) -// Set to a default that is neither MBEDTLS_MPI_IS_PUBLIC nor MBEDTLS_MPI_IS_SECRET -int mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC + MBEDTLS_MPI_IS_SECRET + 1; +int mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_TEST; #endif /* diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 3d32bf2c5d..16788ef71f 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -110,6 +110,10 @@ * other than the two below is passed, default to the safe path.) */ #define MBEDTLS_MPI_IS_PUBLIC 0x2a2a2a2a #define MBEDTLS_MPI_IS_SECRET 0 +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) +// Default value for testing that is neither MBEDTLS_MPI_IS_PUBLIC nor MBEDTLS_MPI_IS_SECRET +#define MBEDTLS_MPI_IS_TEST 1 +#endif /** Count leading zero bits in a given integer. * @@ -838,8 +842,7 @@ extern int mbedtls_mpi_optionally_safe_codepath; static inline void mbedtls_mpi_optionally_safe_codepath_reset(void) { - // Set to a default that is neither MBEDTLS_MPI_IS_PUBLIC nor MBEDTLS_MPI_IS_SECRET - mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC + MBEDTLS_MPI_IS_SECRET + 1; + mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_TEST; } #endif From 47847ca78bd8be9b946c066b734dd4784890a01d Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 17:07:58 +0100 Subject: [PATCH 25/35] Initial local variables to secure default Unfortunately compilers aren't good at analyzing whether variables are analyzed on all code paths, and it is better to initialize to the safe-path values. Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 58e01723a5..97e212db89 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -857,8 +857,8 @@ static void mbedtls_mpi_core_exp_mod_optionally_safe(mbedtls_mpi_uint *X, /* We'll process the bits of E from most significant * (limb_index=E_limbs-1, E_bit_index=biL-1) to least significant * (limb_index=0, E_bit_index=0). */ - size_t E_limb_index; - size_t E_bit_index; + size_t E_limb_index = E_limbs; + size_t E_bit_index = 0; exp_mod_calc_first_bit_optionally_safe(E, E_limbs, E_public, &E_limb_index, &E_bit_index); From 7e909c80eaee6aa3048a856b750b866366142654 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 17:13:25 +0100 Subject: [PATCH 26/35] Explain the choice of the value of MBEDTLS_MPI_IS_PUBLIC Signed-off-by: Janos Follath --- tf-psa-crypto/drivers/builtin/src/bignum_core.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 16788ef71f..c7db590c7a 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -107,7 +107,10 @@ * // safe path * } * not the other way round, in order to prevent misuse. (This is, if a value - * other than the two below is passed, default to the safe path.) */ + * other than the two below is passed, default to the safe path.) + * + * The value of MBEDTLS_MPI_IS_PUBLIC is chosen in a way that is unlikely to happen by accident, but + * which can be used as an immediate value in a Thumb2 comparison (for code size). */ #define MBEDTLS_MPI_IS_PUBLIC 0x2a2a2a2a #define MBEDTLS_MPI_IS_SECRET 0 #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) From 514e62c8332fb1121d98a7215b99f9d87d9173f2 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 18:30:06 +0100 Subject: [PATCH 27/35] Move bignum code path testing out of the library Without this, it's not at all obvious that turning on MBEDTLS_TEST_HOOKS doesn't change the functional behavior of the code. Signed-off-by: Janos Follath --- library/bignum_core_invasive.h | 23 +++++++++ tests/include/test/bignum_codepath_check.h | 48 +++++++++++++++++++ tests/src/bignum_codepath_check.c | 39 +++++++++++++++ tests/src/helpers.c | 12 +++++ .../drivers/builtin/src/bignum_core.c | 21 ++++---- .../drivers/builtin/src/bignum_core.h | 12 ----- .../tests/suites/test_suite_bignum.function | 29 +++++------ .../suites/test_suite_bignum_core.function | 17 +++---- .../tests/suites/test_suite_rsa.function | 9 ++-- 9 files changed, 161 insertions(+), 49 deletions(-) create mode 100644 library/bignum_core_invasive.h create mode 100644 tests/include/test/bignum_codepath_check.h create mode 100644 tests/src/bignum_codepath_check.c diff --git a/library/bignum_core_invasive.h b/library/bignum_core_invasive.h new file mode 100644 index 0000000000..167099dc91 --- /dev/null +++ b/library/bignum_core_invasive.h @@ -0,0 +1,23 @@ +/** + * \file bignum_core_invasive.h + * + * \brief Function declarations for invasive functions of bignum core. + */ +/** + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#ifndef MBEDTLS_BIGNUM_CORE_INVASIVE_H +#define MBEDTLS_BIGNUM_CORE_INVASIVE_H + +#include "bignum_core.h" + +#if defined(MBEDTLS_TEST_HOOKS) && !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_BIGNUM_CORE_INVASIVE_H */ diff --git a/tests/include/test/bignum_codepath_check.h b/tests/include/test/bignum_codepath_check.h new file mode 100644 index 0000000000..6ab68bb5a5 --- /dev/null +++ b/tests/include/test/bignum_codepath_check.h @@ -0,0 +1,48 @@ +/** Support for path tracking in optionally safe bignum functions + * + * The functions are called when an optionally safe path is taken and logs it with a single + * variable. This variable is at any time in one of three states: + * - MBEDTLS_MPI_IS_TEST: No optionally safe path has been taken since the last reset + * - MBEDTLS_MPI_IS_SECRET: Only safe paths were teken since the last reset + * - MBEDTLS_MPI_IS_PUBLIC: At least one unsafe path has been taken since the last reset + * + * Using a simple global variable to track execution path. Making it work with multithreading + * doesn't worth the effort as multithreaded tests add little to no value here. + */ +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#ifndef BIGNUM_CODEPATH_CHECK_H +#define BIGNUM_CODEPATH_CHECK_H + +#include "bignum_core.h" + +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + +extern int mbedtls_codepath_check; + +/** + * \brief Setup the codepath test hooks used by optionally safe bignum functions to signal + * the path taken. + */ +void mbedtls_codepath_test_hooks_setup(void); + +/** + * \brief Teardown the codepath test hooks used by optionally safe bignum functions to + * signal the path taken. + */ +void mbedtls_codepath_test_hooks_teardown(void); + +/** + * \brief Reset the state of the codepath to the initial state. + */ +static inline void mbedtls_codepath_reset(void) +{ + mbedtls_codepath_check = MBEDTLS_MPI_IS_TEST; +} + +#endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ + +#endif /* BIGNUM_CODEPATH_CHECK_H */ diff --git a/tests/src/bignum_codepath_check.c b/tests/src/bignum_codepath_check.c new file mode 100644 index 0000000000..b6b85d9aba --- /dev/null +++ b/tests/src/bignum_codepath_check.c @@ -0,0 +1,39 @@ +/** Support for path tracking in optionally safe bignum functions + */ +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#include "test/bignum_codepath_check.h" +#include "bignum_core_invasive.h" + +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) +int mbedtls_codepath_check = MBEDTLS_MPI_IS_TEST; + +void mbedtls_codepath_take_safe(void) +{ + if(mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST) { + mbedtls_codepath_check = MBEDTLS_MPI_IS_SECRET; + } +} + +void mbedtls_codepath_take_unsafe(void) +{ + mbedtls_codepath_check = MBEDTLS_MPI_IS_PUBLIC; +} + +void mbedtls_codepath_test_hooks_setup(void) +{ + mbedtls_safe_codepath_hook = mbedtls_codepath_take_safe; + mbedtls_unsafe_codepath_hook = mbedtls_codepath_take_unsafe; +} + +void mbedtls_codepath_test_hooks_teardown(void) +{ + mbedtls_safe_codepath_hook = NULL; + mbedtls_unsafe_codepath_hook = NULL; +} + +#endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ + diff --git a/tests/src/helpers.c b/tests/src/helpers.c index 065d17d3e0..db50296e01 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -16,6 +16,9 @@ #if defined(MBEDTLS_TEST_HOOKS) && defined(MBEDTLS_PSA_CRYPTO_C) #include #endif +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) +#include +#endif #if defined(MBEDTLS_THREADING_C) #include "mbedtls/threading.h" #endif @@ -342,6 +345,11 @@ int mbedtls_test_platform_setup(void) mbedtls_mutex_init(&mbedtls_test_info_mutex); #endif /* MBEDTLS_THREADING_C */ + +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_codepath_test_hooks_setup(); +#endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ + return ret; } @@ -359,6 +367,10 @@ void mbedtls_test_platform_teardown(void) #if defined(MBEDTLS_PLATFORM_C) mbedtls_platform_teardown(&platform_ctx); #endif /* MBEDTLS_PLATFORM_C */ + +#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) + mbedtls_codepath_test_hooks_teardown(); +#endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ } int mbedtls_test_ascii2uc(const char c, unsigned char *uc) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index 97e212db89..da3b6f4dee 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -748,7 +748,8 @@ static void exp_mod_precompute_window(const mbedtls_mpi_uint *A, } #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) -int mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_TEST; +void (*mbedtls_safe_codepath_hook)(void) = NULL; +void (*mbedtls_unsafe_codepath_hook)(void) = NULL; #endif /* @@ -781,7 +782,8 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint *E_bit_index = E_bits % biL; #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC; + if(mbedtls_unsafe_codepath_hook != NULL) + mbedtls_unsafe_codepath_hook(); #endif } else { /* @@ -791,10 +793,8 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint *E_limb_index = E_limbs; *E_bit_index = 0; #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - // Only mark the codepath safe if there wasn't an unsafe codepath before - if (mbedtls_mpi_optionally_safe_codepath != MBEDTLS_MPI_IS_PUBLIC) { - mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_SECRET; - } + if(mbedtls_safe_codepath_hook != NULL) + mbedtls_safe_codepath_hook(); #endif } } @@ -813,7 +813,8 @@ static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselec if (window_public == MBEDTLS_MPI_IS_PUBLIC) { memcpy(Wselect, Wtable + window * AN_limbs, AN_limbs * ciL); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_PUBLIC; + if(mbedtls_unsafe_codepath_hook != NULL) + mbedtls_unsafe_codepath_hook(); #endif } else { /* Select Wtable[window] without leaking window through @@ -821,10 +822,8 @@ static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselec mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, AN_limbs, welem, window); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - // Only mark the codepath safe if there wasn't an unsafe codepath before - if (mbedtls_mpi_optionally_safe_codepath != MBEDTLS_MPI_IS_PUBLIC) { - mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_SECRET; - } + if(mbedtls_safe_codepath_hook != NULL) + mbedtls_safe_codepath_hook(); #endif } } diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index c7db590c7a..3c6b105e43 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -837,16 +837,4 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, mbedtls_mpi_uint mm, mbedtls_mpi_uint *T); -/* - * Can't define thread local variables with our abstraction layer: do nothing if threading is on. - */ -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) -extern int mbedtls_mpi_optionally_safe_codepath; - -static inline void mbedtls_mpi_optionally_safe_codepath_reset(void) -{ - mbedtls_mpi_optionally_safe_codepath = MBEDTLS_MPI_IS_TEST; -} -#endif - #endif /* MBEDTLS_BIGNUM_CORE_H */ diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum.function b/tf-psa-crypto/tests/suites/test_suite_bignum.function index 5e18441d7e..c71f3c8bb1 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum.function @@ -5,6 +5,7 @@ #include "bignum_core.h" #include "bignum_internal.h" #include "test/constant_flow.h" +#include "test/bignum_codepath_check.h" #if MBEDTLS_MPI_MAX_BITS > 792 #define MPI_MAX_BITS_LARGER_THAN_792 @@ -990,11 +991,11 @@ void mpi_exp_mod_min_RR(char *input_A, char *input_E, TEST_LE_U(RR.n, N.n - 1); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif /* We know that exp_mod internally needs RR to be as large as N. * Validate that it is the case now, otherwise there was probably @@ -1029,11 +1030,11 @@ void mpi_exp_mod(char *input_A, char *input_E, TEST_ASSERT(mbedtls_test_read_mpi(&X, input_X) == 0); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, NULL); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1042,11 +1043,11 @@ void mpi_exp_mod(char *input_A, char *input_E, } #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif res = mbedtls_mpi_exp_mod_unsafe(&Z, &A, &E, &N, NULL); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1056,11 +1057,11 @@ void mpi_exp_mod(char *input_A, char *input_E, /* Now test again with the speed-up parameter supplied as an output. */ #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1070,11 +1071,11 @@ void mpi_exp_mod(char *input_A, char *input_E, /* Now test again with the speed-up parameter supplied in calculated form. */ #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1116,19 +1117,19 @@ void mpi_exp_mod_size(int A_bytes, int E_bytes, int N_bytes, } #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif TEST_ASSERT(mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR) == exp_result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif TEST_ASSERT(mbedtls_mpi_exp_mod_unsafe(&Z, &A, &E, &N, &RR) == exp_result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif exit: diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function index 3128307b5b..d5cc08e56d 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function @@ -4,6 +4,7 @@ #include "bignum_core.h" #include "constant_time_internal.h" #include "test/constant_flow.h" +#include "test/bignum_codepath_check.h" /** Verifies mbedtls_mpi_core_add(). * @@ -1303,11 +1304,11 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif mbedtls_mpi_core_exp_mod(Y, A, N, N_limbs, E, E_limbs, R2, T); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); @@ -1318,11 +1319,11 @@ void mpi_core_exp_mod(char *input_N, char *input_A, /* Test the unsafe variant */ #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif mbedtls_mpi_core_exp_mod_unsafe(Y, A, N, N_limbs, E, E_limbs, R2, T); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); #endif TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); @@ -1335,22 +1336,22 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif mbedtls_mpi_core_exp_mod(A, A, N, N_limbs, E, E_limbs, R2, T); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); memcpy(A, A_copy, sizeof(*A) * A_limbs); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif mbedtls_mpi_core_exp_mod_unsafe(A, A, N, N_limbs, E, E_limbs, R2, T); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); #endif TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); diff --git a/tf-psa-crypto/tests/suites/test_suite_rsa.function b/tf-psa-crypto/tests/suites/test_suite_rsa.function index e0206eca29..75f3f428c0 100644 --- a/tf-psa-crypto/tests/suites/test_suite_rsa.function +++ b/tf-psa-crypto/tests/suites/test_suite_rsa.function @@ -3,6 +3,7 @@ #include "bignum_core.h" #include "rsa_alt_helpers.h" #include "rsa_internal.h" +#include "test/bignum_codepath_check.h" /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -491,11 +492,11 @@ void mbedtls_rsa_public(data_t *message_str, int mod, TEST_ASSERT(mbedtls_rsa_check_pubkey(&ctx) == 0); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif TEST_ASSERT(mbedtls_rsa_public(&ctx, message_str->x, output) == result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_PUBLIC); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); #endif if (result == 0) { @@ -562,13 +563,13 @@ void mbedtls_rsa_private(data_t *message_str, int mod, for (i = 0; i < 3; i++) { memset(output, 0x00, sizeof(output)); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_mpi_optionally_safe_codepath_reset(); + mbedtls_codepath_reset(); #endif TEST_ASSERT(mbedtls_rsa_private(&ctx, mbedtls_test_rnd_pseudo_rand, &rnd_info, message_str->x, output) == result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_mpi_optionally_safe_codepath, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); #endif if (result == 0) { From 44eca95ace9b7aae3e6e37c16a25089f0a790b9f Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 18:55:40 +0100 Subject: [PATCH 28/35] Fix incorrect test result Signed-off-by: Janos Follath --- tf-psa-crypto/tests/suites/test_suite_bignum.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum.function b/tf-psa-crypto/tests/suites/test_suite_bignum.function index c71f3c8bb1..1102e18403 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum.function @@ -1129,7 +1129,7 @@ void mpi_exp_mod_size(int A_bytes, int E_bytes, int N_bytes, #endif TEST_ASSERT(mbedtls_mpi_exp_mod_unsafe(&Z, &A, &E, &N, &RR) == exp_result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); + TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); #endif exit: From 21445c580f81cad5a91c5fbf96440abd8beabfb7 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 22 Aug 2024 20:00:23 +0100 Subject: [PATCH 29/35] Prepare codepath tests for early termination Signed-off-by: Janos Follath --- tests/include/test/bignum_codepath_check.h | 43 +++++++++++++++++++ .../tests/suites/test_suite_bignum.function | 14 +++--- .../tests/suites/test_suite_rsa.function | 4 +- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/tests/include/test/bignum_codepath_check.h b/tests/include/test/bignum_codepath_check.h index 6ab68bb5a5..34dfc5651c 100644 --- a/tests/include/test/bignum_codepath_check.h +++ b/tests/include/test/bignum_codepath_check.h @@ -43,6 +43,49 @@ static inline void mbedtls_codepath_reset(void) mbedtls_codepath_check = MBEDTLS_MPI_IS_TEST; } +/** Check the codepath taken and fail if it doesn't match. + * + * When a function returns with an error, it can do so before reaching any interesting codepath. The + * same can happen if a parameter to the function is zero. In these cases we need to allow + * uninitialised value for the codepath tracking variable. + * + * This macro expands to an instruction, not an expression. + * It may jump to the \c exit label. + * + * \param path The expected codepath. + * This expression may be evaluated multiple times. + * \param ret The expected return value. + * \param E The MPI parameter that can cause shortcuts. + */ +#define ASSERT_BIGNUM_CODEPATH(path, ret, E) \ + do { \ + if((ret)!=0 || (E).n == 0) \ + TEST_ASSERT(mbedtls_codepath_check == (path) || \ + mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST); \ + else \ + TEST_EQUAL(mbedtls_codepath_check, (path)); \ + } while (0) + +/** Check the codepath taken and fail if it doesn't match. + * + * When a function returns with an error, it can do so before reaching any interesting codepath. In + * this case we need to allow uninitialised value for the codepath tracking variable. + * + * This macro expands to an instruction, not an expression. + * It may jump to the \c exit label. + * + * \param path The expected codepath. + * This expression may be evaluated multiple times. + * \param ret The expected return value. + */ +#define ASSERT_RSA_CODEPATH(path, ret) \ + do { \ + if((ret)!=0) \ + TEST_ASSERT(mbedtls_codepath_check == (path) || \ + mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST); \ + else \ + TEST_EQUAL(mbedtls_codepath_check, (path)); \ + } while (0) #endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ #endif /* BIGNUM_CODEPATH_CHECK_H */ diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum.function b/tf-psa-crypto/tests/suites/test_suite_bignum.function index 1102e18403..3d2b8a1240 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum.function @@ -995,7 +995,7 @@ void mpi_exp_mod_min_RR(char *input_A, char *input_E, #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); + ASSERT_BIGNUM_CODEPATH(MBEDTLS_MPI_IS_SECRET, res, E); #endif /* We know that exp_mod internally needs RR to be as large as N. * Validate that it is the case now, otherwise there was probably @@ -1034,7 +1034,7 @@ void mpi_exp_mod(char *input_A, char *input_E, #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, NULL); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); + ASSERT_BIGNUM_CODEPATH(MBEDTLS_MPI_IS_SECRET, res, E); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1047,7 +1047,7 @@ void mpi_exp_mod(char *input_A, char *input_E, #endif res = mbedtls_mpi_exp_mod_unsafe(&Z, &A, &E, &N, NULL); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); + ASSERT_BIGNUM_CODEPATH(MBEDTLS_MPI_IS_PUBLIC, res, E); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1061,7 +1061,7 @@ void mpi_exp_mod(char *input_A, char *input_E, #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); + ASSERT_BIGNUM_CODEPATH(MBEDTLS_MPI_IS_SECRET, res, E); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1075,7 +1075,7 @@ void mpi_exp_mod(char *input_A, char *input_E, #endif res = mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); + ASSERT_BIGNUM_CODEPATH(MBEDTLS_MPI_IS_SECRET, res, E); #endif TEST_ASSERT(res == exp_result); if (res == 0) { @@ -1121,7 +1121,7 @@ void mpi_exp_mod_size(int A_bytes, int E_bytes, int N_bytes, #endif TEST_ASSERT(mbedtls_mpi_exp_mod(&Z, &A, &E, &N, &RR) == exp_result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); + ASSERT_BIGNUM_CODEPATH(MBEDTLS_MPI_IS_SECRET, exp_result, E); #endif #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) @@ -1129,7 +1129,7 @@ void mpi_exp_mod_size(int A_bytes, int E_bytes, int N_bytes, #endif TEST_ASSERT(mbedtls_mpi_exp_mod_unsafe(&Z, &A, &E, &N, &RR) == exp_result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); + ASSERT_BIGNUM_CODEPATH(MBEDTLS_MPI_IS_PUBLIC, exp_result, E); #endif exit: diff --git a/tf-psa-crypto/tests/suites/test_suite_rsa.function b/tf-psa-crypto/tests/suites/test_suite_rsa.function index 75f3f428c0..98ea9efb1c 100644 --- a/tf-psa-crypto/tests/suites/test_suite_rsa.function +++ b/tf-psa-crypto/tests/suites/test_suite_rsa.function @@ -496,7 +496,7 @@ void mbedtls_rsa_public(data_t *message_str, int mod, #endif TEST_ASSERT(mbedtls_rsa_public(&ctx, message_str->x, output) == result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); + ASSERT_RSA_CODEPATH(MBEDTLS_MPI_IS_PUBLIC, result); #endif if (result == 0) { @@ -569,7 +569,7 @@ void mbedtls_rsa_private(data_t *message_str, int mod, &rnd_info, message_str->x, output) == result); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); + ASSERT_RSA_CODEPATH(MBEDTLS_MPI_IS_SECRET, result); #endif if (result == 0) { From e91d924821c31d2313cbb0d937ee7985d4c0d726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 2 Sep 2024 10:42:46 +0200 Subject: [PATCH 30/35] Fix code style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tf-psa-crypto/drivers/builtin/src/bignum_core.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.c b/tf-psa-crypto/drivers/builtin/src/bignum_core.c index da3b6f4dee..60f48f92de 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.c +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.c @@ -782,8 +782,9 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint *E_bit_index = E_bits % biL; #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - if(mbedtls_unsafe_codepath_hook != NULL) + if (mbedtls_unsafe_codepath_hook != NULL) { mbedtls_unsafe_codepath_hook(); + } #endif } else { /* @@ -793,8 +794,9 @@ static inline void exp_mod_calc_first_bit_optionally_safe(const mbedtls_mpi_uint *E_limb_index = E_limbs; *E_bit_index = 0; #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - if(mbedtls_safe_codepath_hook != NULL) + if (mbedtls_safe_codepath_hook != NULL) { mbedtls_safe_codepath_hook(); + } #endif } } @@ -813,8 +815,9 @@ static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselec if (window_public == MBEDTLS_MPI_IS_PUBLIC) { memcpy(Wselect, Wtable + window * AN_limbs, AN_limbs * ciL); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - if(mbedtls_unsafe_codepath_hook != NULL) + if (mbedtls_unsafe_codepath_hook != NULL) { mbedtls_unsafe_codepath_hook(); + } #endif } else { /* Select Wtable[window] without leaking window through @@ -822,8 +825,9 @@ static inline void exp_mod_table_lookup_optionally_safe(mbedtls_mpi_uint *Wselec mbedtls_mpi_core_ct_uint_table_lookup(Wselect, Wtable, AN_limbs, welem, window); #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - if(mbedtls_safe_codepath_hook != NULL) + if (mbedtls_safe_codepath_hook != NULL) { mbedtls_safe_codepath_hook(); + } #endif } } From 464bf78396df4236e30e07879018f6d5b1f1be7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 2 Sep 2024 11:12:09 +0200 Subject: [PATCH 31/35] Fix guards on #include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rest of the file uses mbedtls_mpi_uint_t unconditionally, so its definition should also be #include'd unconditionally. Signed-off-by: Manuel Pégourié-Gonnard --- tf-psa-crypto/drivers/builtin/src/bignum_core.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index 3c6b105e43..da75546d51 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -70,9 +70,7 @@ #include "common.h" -#if defined(MBEDTLS_BIGNUM_C) #include "mbedtls/bignum.h" -#endif #include "constant_time_internal.h" From 3106013e172d454d1bc5e640bd008678bc65ea4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 2 Sep 2024 12:41:05 +0200 Subject: [PATCH 32/35] Fix code style (for real this time, hopefully) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For some reason I didn't think about other files in the previous commit. Signed-off-by: Manuel Pégourié-Gonnard --- tests/include/test/bignum_codepath_check.h | 10 ++++++---- tests/src/bignum_codepath_check.c | 3 +-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/include/test/bignum_codepath_check.h b/tests/include/test/bignum_codepath_check.h index 34dfc5651c..2f954e94fa 100644 --- a/tests/include/test/bignum_codepath_check.h +++ b/tests/include/test/bignum_codepath_check.h @@ -59,11 +59,12 @@ static inline void mbedtls_codepath_reset(void) */ #define ASSERT_BIGNUM_CODEPATH(path, ret, E) \ do { \ - if((ret)!=0 || (E).n == 0) \ + if ((ret) != 0 || (E).n == 0) { \ TEST_ASSERT(mbedtls_codepath_check == (path) || \ mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST); \ - else \ + } else { \ TEST_EQUAL(mbedtls_codepath_check, (path)); \ + } \ } while (0) /** Check the codepath taken and fail if it doesn't match. @@ -80,11 +81,12 @@ static inline void mbedtls_codepath_reset(void) */ #define ASSERT_RSA_CODEPATH(path, ret) \ do { \ - if((ret)!=0) \ + if ((ret) != 0) { \ TEST_ASSERT(mbedtls_codepath_check == (path) || \ mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST); \ - else \ + } else { \ TEST_EQUAL(mbedtls_codepath_check, (path)); \ + } \ } while (0) #endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ diff --git a/tests/src/bignum_codepath_check.c b/tests/src/bignum_codepath_check.c index b6b85d9aba..b752d13f82 100644 --- a/tests/src/bignum_codepath_check.c +++ b/tests/src/bignum_codepath_check.c @@ -13,7 +13,7 @@ int mbedtls_codepath_check = MBEDTLS_MPI_IS_TEST; void mbedtls_codepath_take_safe(void) { - if(mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST) { + if (mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST) { mbedtls_codepath_check = MBEDTLS_MPI_IS_SECRET; } } @@ -36,4 +36,3 @@ void mbedtls_codepath_test_hooks_teardown(void) } #endif /* MBEDTLS_TEST_HOOKS && !MBEDTLS_THREADING_C */ - From 49645f64904abcd4d08e38cd528bad3af60c2b57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 3 Sep 2024 10:10:18 +0200 Subject: [PATCH 33/35] Misc improvements to comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/include/test/bignum_codepath_check.h | 9 +++++---- tf-psa-crypto/drivers/builtin/src/bignum_core.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/include/test/bignum_codepath_check.h b/tests/include/test/bignum_codepath_check.h index 2f954e94fa..3d72be1b2e 100644 --- a/tests/include/test/bignum_codepath_check.h +++ b/tests/include/test/bignum_codepath_check.h @@ -6,8 +6,8 @@ * - MBEDTLS_MPI_IS_SECRET: Only safe paths were teken since the last reset * - MBEDTLS_MPI_IS_PUBLIC: At least one unsafe path has been taken since the last reset * - * Using a simple global variable to track execution path. Making it work with multithreading - * doesn't worth the effort as multithreaded tests add little to no value here. + * Use a simple global variable to track execution path. Making it work with multithreading + * isn't worth the effort as multithreaded tests add little to no value here. */ /* * Copyright The Mbed TLS Contributors @@ -47,7 +47,7 @@ static inline void mbedtls_codepath_reset(void) * * When a function returns with an error, it can do so before reaching any interesting codepath. The * same can happen if a parameter to the function is zero. In these cases we need to allow - * uninitialised value for the codepath tracking variable. + * the codepath tracking variable to still have its initial "not set" value. * * This macro expands to an instruction, not an expression. * It may jump to the \c exit label. @@ -70,7 +70,8 @@ static inline void mbedtls_codepath_reset(void) /** Check the codepath taken and fail if it doesn't match. * * When a function returns with an error, it can do so before reaching any interesting codepath. In - * this case we need to allow uninitialised value for the codepath tracking variable. + * this case we need to allow the codepath tracking variable to still have its + * initial "not set" value. * * This macro expands to an instruction, not an expression. * It may jump to the \c exit label. diff --git a/tf-psa-crypto/drivers/builtin/src/bignum_core.h b/tf-psa-crypto/drivers/builtin/src/bignum_core.h index da75546d51..484fa8c321 100644 --- a/tf-psa-crypto/drivers/builtin/src/bignum_core.h +++ b/tf-psa-crypto/drivers/builtin/src/bignum_core.h @@ -104,7 +104,7 @@ * } else { * // safe path * } - * not the other way round, in order to prevent misuse. (This is, if a value + * not the other way round, in order to prevent misuse. (That is, if a value * other than the two below is passed, default to the safe path.) * * The value of MBEDTLS_MPI_IS_PUBLIC is chosen in a way that is unlikely to happen by accident, but From 0c4a115442fbd4fac8d10f4d10e4221775773b5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 5 Sep 2024 11:01:44 +0200 Subject: [PATCH 34/35] Remove codepath testing where it's redundant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constant-time testing with Memsan or Valgrind is much more robust, as it doesn't require cooperation from the code under test. Signed-off-by: Manuel Pégourié-Gonnard --- .../suites/test_suite_bignum_core.function | 50 ++++++++----------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function index d5cc08e56d..c755287221 100644 --- a/tf-psa-crypto/tests/suites/test_suite_bignum_core.function +++ b/tf-psa-crypto/tests/suites/test_suite_bignum_core.function @@ -1303,56 +1303,46 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_codepath_reset(); -#endif + mbedtls_mpi_core_exp_mod(Y, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); -#endif - TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); TEST_CF_PUBLIC(Y, N_limbs * sizeof(mbedtls_mpi_uint)); - TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); /* Test the unsafe variant */ + TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(N, N_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_codepath_reset(); -#endif mbedtls_mpi_core_exp_mod_unsafe(Y, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); -#endif + TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); - /* Check both with output aliased to input */ + /* + * Check both with output aliased to input + */ TEST_CALLOC(A_copy, A_limbs); - memcpy(A_copy, A, sizeof(*A_copy) * A_limbs); + memcpy(A_copy, A, sizeof(*A_copy) * A_limbs); // save A + /* Safe */ TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_codepath_reset(); -#endif + mbedtls_mpi_core_exp_mod(A, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_SECRET); -#endif - TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); - memcpy(A, A_copy, sizeof(*A) * A_limbs); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - mbedtls_codepath_reset(); -#endif + TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); + + /* Unsafe */ + memcpy(A, A_copy, sizeof(*A) * A_limbs); // restore A + TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(N, N_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(E, E_limbs * sizeof(mbedtls_mpi_uint)); + mbedtls_mpi_core_exp_mod_unsafe(A, A, N, N_limbs, E, E_limbs, R2, T); -#if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) - TEST_EQUAL(mbedtls_codepath_check, MBEDTLS_MPI_IS_PUBLIC); -#endif + TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); exit: From b70ef8690ae862145b6e9ed5386195516ec6fabc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 5 Sep 2024 12:33:57 +0200 Subject: [PATCH 35/35] Move new files to their correct location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- .../drivers/builtin/src}/bignum_core_invasive.h | 0 {library => tf-psa-crypto/drivers/builtin/src}/bignum_internal.h | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {library => tf-psa-crypto/drivers/builtin/src}/bignum_core_invasive.h (100%) rename {library => tf-psa-crypto/drivers/builtin/src}/bignum_internal.h (100%) diff --git a/library/bignum_core_invasive.h b/tf-psa-crypto/drivers/builtin/src/bignum_core_invasive.h similarity index 100% rename from library/bignum_core_invasive.h rename to tf-psa-crypto/drivers/builtin/src/bignum_core_invasive.h diff --git a/library/bignum_internal.h b/tf-psa-crypto/drivers/builtin/src/bignum_internal.h similarity index 100% rename from library/bignum_internal.h rename to tf-psa-crypto/drivers/builtin/src/bignum_internal.h