From e7f14a3090e6595eb3c8d821704ad9c90f6d3712 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 6 Apr 2022 06:11:26 +0100 Subject: [PATCH 01/26] Remove unused variable in mpi_mul_hlp() Signed-off-by: Hanno Becker --- library/bignum.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 288f85932984..22cbffacf919 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1398,7 +1398,7 @@ void mpi_mul_hlp( size_t i, mbedtls_mpi_uint *d, mbedtls_mpi_uint b ) { - mbedtls_mpi_uint c = 0, t = 0; + mbedtls_mpi_uint c = 0; /* carry */ #if defined(MULADDC_HUIT) for( ; i >= 8; i -= 8 ) @@ -1449,8 +1449,6 @@ void mpi_mul_hlp( size_t i, } #endif /* MULADDC_HUIT */ - t++; - while( c != 0 ) { *d += c; c = ( *d < c ); d++; From defe56928ea61d06adc0ae13e0599dcf324434fe Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 6 Apr 2022 06:12:09 +0100 Subject: [PATCH 02/26] Make length of output explicit in mpi_mul_hlp() The helper `mpi_mul_hlp()` performs a multiply-accumulate operation `d += s * b`, where `d,b` are MPIs and `b` is a scalar. Previously, only the length of `s` was specified, while `d` was assumed to be 0-terminated of unspecified length. This was leveraged at the end of the core multiplication steps computingg the first `limb(s)` limbs of `d + s*b`: Namely, the routine would keep on adding the current carry to `d` until none was left. This can, in theory, run for an arbitrarily long time if `d` has a tail of `0xFF`s, and hence the assumption of `0`-termination. This solution is both fragile and insecure -- the latter because the carry-loop depends on the result of the multiplication. This commit changes the signature of `mpi_mul_hlp()` to receive the length of the output buffer, which must be greater or equal to the length of the input buffer. It is _not_ assumed that the output buffer is strictly larger than the input buffer -- instead, the routine will simply return any carry that's left. This will be useful in some applications of this function. It is the responsibility of the caller to either size the output appropriately so that no carry will be left, or to handle the carry. NOTE: The commit leaves the library in a state where it cannot be compiled since the call-sites of mpi_mul_hlp() have not yet been adjusted. This will be done in the subsequent commits. Signed-off-by: Hanno Becker --- library/bignum.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 22cbffacf919..b7b90ec3e779 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1373,17 +1373,17 @@ int mbedtls_mpi_sub_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_sint * * Add \p b * \p s to \p d. * - * \param i The number of limbs of \p s. + * \param[in,out] d The bignum to add to. + * \param d_len The number of limbs of \p d. This must be + * at least \p s_len. + * \param s_len The number of limbs of \p s. * \param[in] s A bignum to multiply, of size \p i. * It may overlap with \p d, but only if * \p d <= \p s. * Its leading limb must not be \c 0. - * \param[in,out] d The bignum to add to. - * It must be sufficiently large to store the - * result of the multiplication. This means - * \p i + 1 limbs if \p d[\p i - 1] started as 0 and \p b - * is not known a priori. * \param b A scalar to multiply. + * + * \return c The carry at the end of the operation. */ static #if defined(__APPLE__) && defined(__arm__) @@ -1393,29 +1393,31 @@ static */ __attribute__ ((noinline)) #endif -void mpi_mul_hlp( size_t i, - const mbedtls_mpi_uint *s, - mbedtls_mpi_uint *d, - mbedtls_mpi_uint b ) +mbedtls_mpi_uint mpi_mul_hlp( mbedtls_mpi_uint *d, size_t d_len , + const mbedtls_mpi_uint *s, size_t s_len, + mbedtls_mpi_uint b ) { mbedtls_mpi_uint c = 0; /* carry */ + /* Remember the excess of d over s for later */ + d_len -= s_len; + #if defined(MULADDC_HUIT) - for( ; i >= 8; i -= 8 ) + for( ; s_len >= 8; s_len -= 8 ) { MULADDC_INIT MULADDC_HUIT MULADDC_STOP } - for( ; i > 0; i-- ) + for( ; s_len > 0; s_len-- ) { MULADDC_INIT MULADDC_CORE MULADDC_STOP } #else /* MULADDC_HUIT */ - for( ; i >= 16; i -= 16 ) + for( ; s_len >= 16; s_len -= 16 ) { MULADDC_INIT MULADDC_CORE MULADDC_CORE @@ -1430,7 +1432,7 @@ void mpi_mul_hlp( size_t i, MULADDC_STOP } - for( ; i >= 8; i -= 8 ) + for( ; s_len >= 8; s_len -= 8 ) { MULADDC_INIT MULADDC_CORE MULADDC_CORE @@ -1441,7 +1443,7 @@ void mpi_mul_hlp( size_t i, MULADDC_STOP } - for( ; i > 0; i-- ) + for( ; s_len > 0; s_len-- ) { MULADDC_INIT MULADDC_CORE @@ -1449,10 +1451,12 @@ void mpi_mul_hlp( size_t i, } #endif /* MULADDC_HUIT */ - while( c != 0 ) + while( d_len-- ) { *d += c; c = ( *d < c ); d++; } + + return( c ); } /* From fee261a50551fc45938103240ba53a88895f0405 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 6 Apr 2022 06:20:22 +0100 Subject: [PATCH 03/26] Adjust mbedtls_mpi_mul_mpi() to new signature of mpi_mul_hlp() The previous commit has changed the signature of mpi_mul_hlp(), making the length of the output explicit. This commit adjusts the call-site in mbedtls_mpi_mul_mpi() to this new signature. A notable change to the multiplication strategy had to be made: mbedtls_mpi_mul_mpi() performs a simple row-wise schoolbook multiplication, which however was so far computed iterating rows from top to bottom. This leads to the undesirable consequence that as lower rows are calculated and added to the temporary result, carry chains can grow. It is simpler and faster to iterate from bottom to top instead, as it is guaranteed that there will be no carry when adding the next row to the previous temporary result: The length of the output in each iteration can be fixed to len(B)+1. Signed-off-by: Hanno Becker --- library/bignum.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index b7b90ec3e779..da8e8ca3c775 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1465,7 +1465,7 @@ mbedtls_mpi_uint mpi_mul_hlp( mbedtls_mpi_uint *d, size_t d_len , int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi *B ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t i, j; + size_t i, j, k; mbedtls_mpi TA, TB; int result_is_zero = 0; MPI_VALIDATE_RET( X != NULL ); @@ -1492,8 +1492,14 @@ int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, i + j ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( X, 0 ) ); - for( ; j > 0; j-- ) - mpi_mul_hlp( i, A->p, X->p + j - 1, B->p[j - 1] ); + for( k = 0; k < j; k++ ) + { + /* We know that there cannot be any carry-out since we're + * iterating from bottom to top. */ + (void) mpi_mul_hlp( X->p + k, i + 1, + A->p, i, + B->p[k] ); + } /* If the result is 0, we don't shortcut the operation, which reduces * but does not eliminate side channels leaking the zero-ness. We do From 74a11a31cbaf3c1ad50b50e0068f3b9eb997211c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 6 Apr 2022 06:27:00 +0100 Subject: [PATCH 04/26] Adjust mbedtls_mpi_mul_int() to changed signature of mpi_mul_hlp() A previous commit has changed the signature of mpi_mul_hlp(), making the length of the output explicit. This commit adjusts mbedtls_mpi_mul_int() to this change. Along the way, we make the code simpler and more secure by not calculating the minimal limb-size of A. A previous comment indicated that this was functionally necessary because of the implementation of mpi_mul_hlp() -- if it ever was, it isn't anymore. Signed-off-by: Hanno Becker --- library/bignum.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index da8e8ca3c775..91ba824af6cf 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1525,17 +1525,9 @@ int mbedtls_mpi_mul_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_uint MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( A != NULL ); - /* mpi_mul_hlp can't deal with a leading 0. */ - size_t n = A->n; - while( n > 0 && A->p[n - 1] == 0 ) - --n; - - /* The general method below doesn't work if n==0 or b==0. By chance - * calculating the result is trivial in those cases. */ - if( b == 0 || n == 0 ) - { + /* The general method below doesn't work if b==0. */ + if( b == 0 ) return( mbedtls_mpi_lset( X, 0 ) ); - } /* Calculate A*b as A + A*(b-1) to take advantage of mpi_mul_hlp */ int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1547,9 +1539,9 @@ int mbedtls_mpi_mul_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_uint * calls to calloc() in ECP code, presumably because it reuses the * same mpi for a while and this way the mpi is more likely to directly * grow to its final size. */ - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, n + 1 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, A->n + 1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_copy( X, A ) ); - mpi_mul_hlp( n, A->p, X->p, b - 1 ); + mpi_mul_hlp( X->p, X->n, A->p, A->n, b - 1 ); cleanup: return( ret ); From e141702551bef58664a43e361b755c1e8c4cceb8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 6 Apr 2022 06:45:45 +0100 Subject: [PATCH 05/26] Adjust mpi_montmul() to new signature of mpi_mul_hlp() A previous commit has changed the signature of mpi_mul_hlp, making the length of the output explicit. This commit adjusts mpi_montmul() accordingly. It also fixes a comment on the required size of the temporary value passed to mpi_montmul() (but does not change the call-sites). Signed-off-by: Hanno Becker --- library/bignum.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 91ba824af6cf..a8f8f84be4f6 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1907,8 +1907,8 @@ static void mpi_montg_init( mbedtls_mpi_uint *mm, const mbedtls_mpi *N ) * \param mm The value calculated by `mpi_montg_init(&mm, N)`. * This is -N^-1 mod 2^ciL. * \param[in,out] T A bignum for temporary storage. - * It must be at least twice the limb size of N plus 2 - * (T->n >= 2 * (N->n + 1)). + * It must be at least twice the limb size of N plus 1 + * (T->n >= 2 * N->n + 1). * Its initial content is unused and * its final content is indeterminate. * Note that unlike the usual convention in the library @@ -1934,10 +1934,13 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi u0 = A->p[i]; u1 = ( d[0] + u0 * B->p[0] ) * mm; - mpi_mul_hlp( m, B->p, d, u0 ); - mpi_mul_hlp( n, N->p, d, u1 ); - - d++; d[n + 1] = 0; + (void) mpi_mul_hlp( d, n + 2, + B->p, m, + u0 ); + (void) mpi_mul_hlp( d, n + 2, + N->p, n, + u1 ); + d++; } /* At this point, d is either the desired result or the desired result From aef9cc4f96de57cccb50b318a18707d13c6f9095 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 06:36:29 +0100 Subject: [PATCH 06/26] Rename mpi_mul_hlp -> mbedtls_mpi_core_mla and expose internally This paves the way for the helper to be used from the ECP module Signed-off-by: Hanno Becker --- library/bignum.c | 37 ++++++++++++----------------- library/bignum_internal.h | 49 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 22 deletions(-) create mode 100644 library/bignum_internal.h diff --git a/library/bignum.c b/library/bignum.c index a8f8f84be4f6..3eb8dd9037f8 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -38,6 +38,7 @@ #if defined(MBEDTLS_BIGNUM_C) #include "mbedtls/bignum.h" +#include "bignum_internal.h" #include "bn_mul.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" @@ -1385,17 +1386,9 @@ int mbedtls_mpi_sub_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_sint * * \return c The carry at the end of the operation. */ -static -#if defined(__APPLE__) && defined(__arm__) -/* - * Apple LLVM version 4.2 (clang-425.0.24) (based on LLVM 3.2svn) - * appears to need this to prevent bad ARM code generation at -O3. - */ -__attribute__ ((noinline)) -#endif -mbedtls_mpi_uint mpi_mul_hlp( mbedtls_mpi_uint *d, size_t d_len , - const mbedtls_mpi_uint *s, size_t s_len, - mbedtls_mpi_uint b ) +mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len , + const mbedtls_mpi_uint *s, size_t s_len, + mbedtls_mpi_uint b ) { mbedtls_mpi_uint c = 0; /* carry */ @@ -1496,9 +1489,9 @@ int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi { /* We know that there cannot be any carry-out since we're * iterating from bottom to top. */ - (void) mpi_mul_hlp( X->p + k, i + 1, - A->p, i, - B->p[k] ); + (void) mbedtls_mpi_core_mla( X->p + k, i + 1, + A->p, i, + B->p[k] ); } /* If the result is 0, we don't shortcut the operation, which reduces @@ -1529,7 +1522,7 @@ int mbedtls_mpi_mul_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_uint if( b == 0 ) return( mbedtls_mpi_lset( X, 0 ) ); - /* Calculate A*b as A + A*(b-1) to take advantage of mpi_mul_hlp */ + /* Calculate A*b as A + A*(b-1) to take advantage of mbedtls_mpi_core_mla */ int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; /* In general, A * b requires 1 limb more than b. If * A->p[n - 1] * b / b == A->p[n - 1], then A * b fits in the same @@ -1541,7 +1534,7 @@ int mbedtls_mpi_mul_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_uint * grow to its final size. */ MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, A->n + 1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_copy( X, A ) ); - mpi_mul_hlp( X->p, X->n, A->p, A->n, b - 1 ); + mbedtls_mpi_core_mla( X->p, X->n, A->p, A->n, b - 1 ); cleanup: return( ret ); @@ -1934,12 +1927,12 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi u0 = A->p[i]; u1 = ( d[0] + u0 * B->p[0] ) * mm; - (void) mpi_mul_hlp( d, n + 2, - B->p, m, - u0 ); - (void) mpi_mul_hlp( d, n + 2, - N->p, n, - u1 ); + (void) mbedtls_mpi_core_mla( d, n + 2, + B->p, m, + u0 ); + (void) mbedtls_mpi_core_mla( d, n + 2, + N->p, n, + u1 ); d++; } diff --git a/library/bignum_internal.h b/library/bignum_internal.h new file mode 100644 index 000000000000..2af7510f9988 --- /dev/null +++ b/library/bignum_internal.h @@ -0,0 +1,49 @@ +/** + * Internal bignum functions + * + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef MBEDTLS_BIGNUM_INTERNAL_H +#define MBEDTLS_BIGNUM_INTERNAL_H + +#include "common.h" + +#if defined(MBEDTLS_BIGNUM_C) +#include "mbedtls/bignum.h" +#endif + +/** Helper for mbedtls_mpi multiplication. + * + * Add \p b * \p s to \p d. + * + * \param[in,out] d The bignum to add to. + * \param d_len The number of limbs of \p d. This must be + * at least \p s_len. + * \param s_len The number of limbs of \p s. + * \param[in] s A bignum to multiply, of size \p i. + * It may overlap with \p d, but only if + * \p d <= \p s. + * Its leading limb must not be \c 0. + * \param b A scalar to multiply. + * + * \return c The carry at the end of the operation. + */ +mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len , + const mbedtls_mpi_uint *s, size_t s_len, + mbedtls_mpi_uint b ); + +#endif /* MBEDTLS_BIGNUM_INTERNAL_H */ From 25bb732ea7a7f9ef7274dad252311eb969562093 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 07:03:48 +0100 Subject: [PATCH 07/26] Simplify x25519 reduction using internal bignum MLA helper Signed-off-by: Hanno Becker --- library/ecp_curves.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 421a067bbfe7..6bc859139de4 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -26,6 +26,7 @@ #include "mbedtls/error.h" #include "bn_mul.h" +#include "bignum_internal.h" #include "ecp_invasive.h" #include @@ -5213,40 +5214,29 @@ static int ecp_mod_p521( mbedtls_mpi *N ) /* * Fast quasi-reduction modulo p255 = 2^255 - 19 - * Write N as A0 + 2^255 A1, return A0 + 19 * A1 + * Write N as A0 + 2^256 A1, return A0 + 38 * A1 */ static int ecp_mod_p255( mbedtls_mpi *N ) { - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t i; - mbedtls_mpi M; - mbedtls_mpi_uint Mp[P255_WIDTH + 2]; + mbedtls_mpi_uint Mp[P255_WIDTH]; - if( N->n < P255_WIDTH ) + /* Helper references for top part of N */ + mbedtls_mpi_uint * const NT_p = N->p + P255_WIDTH; + unsigned const NT_n = N->n - P255_WIDTH; + if( NT_n > P255_WIDTH ) return( 0 ); - /* M = A1 */ - M.s = 1; - M.n = N->n - ( P255_WIDTH - 1 ); - if( M.n > P255_WIDTH + 1 ) - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - M.p = Mp; - memset( Mp, 0, sizeof Mp ); - memcpy( Mp, N->p + P255_WIDTH - 1, M.n * sizeof( mbedtls_mpi_uint ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( &M, 255 % ( 8 * sizeof( mbedtls_mpi_uint ) ) ) ); - M.n++; /* Make room for multiplication by 19 */ + /* Split N as N + 2^256 M */ + memset( Mp, 0, sizeof( Mp ) ); + memcpy( Mp, NT_p, sizeof( mbedtls_mpi_uint ) * NT_n ); + memset( NT_p, 0, sizeof( mbedtls_mpi_uint ) * NT_n ); - /* N = A0 */ - MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( N, 255, 0 ) ); - for( i = P255_WIDTH; i < N->n; i++ ) - N->p[i] = 0; - - /* N = A0 + 19 * A1 */ - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_int( &M, &M, 19 ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_add_abs( N, N, &M ) ); + /* N = A0 + 38 * A1 */ + mbedtls_mpi_core_mla( N->p, N->n, + Mp, P255_WIDTH, + 38 ); -cleanup: - return( ret ); + return( 0 ); } #endif /* MBEDTLS_ECP_DP_CURVE25519_ENABLED */ From 6454993e2e3c00ff2e23029e8a9357c3805382a9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 07:35:58 +0100 Subject: [PATCH 08/26] Safeguard against calling p255 reduction with single-width MPI (In this case, there's nothing to do anyway since we only do a quasi-reduction to N+1 limbs) Signed-off-by: Hanno Becker --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 6bc859139de4..5f541ad13738 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5223,7 +5223,7 @@ static int ecp_mod_p255( mbedtls_mpi *N ) /* Helper references for top part of N */ mbedtls_mpi_uint * const NT_p = N->p + P255_WIDTH; unsigned const NT_n = N->n - P255_WIDTH; - if( NT_n > P255_WIDTH ) + if( NT_n == 0 || NT_n > P255_WIDTH ) return( 0 ); /* Split N as N + 2^256 M */ From e9dd9a1f312ee1ba800dac75807f962e3666e4f0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 09:06:27 +0100 Subject: [PATCH 09/26] Use size_t for number of limbs Signed-off-by: Hanno Becker --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 5f541ad13738..d477fc1f8f95 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5222,7 +5222,7 @@ static int ecp_mod_p255( mbedtls_mpi *N ) /* Helper references for top part of N */ mbedtls_mpi_uint * const NT_p = N->p + P255_WIDTH; - unsigned const NT_n = N->n - P255_WIDTH; + size_t const NT_n = N->n - P255_WIDTH; if( NT_n == 0 || NT_n > P255_WIDTH ) return( 0 ); From 284d778d28405a6f2573ebd3f76618c097b7f486 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 09:19:24 +0100 Subject: [PATCH 10/26] Address review comments Signed-off-by: Hanno Becker --- library/bignum.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 3eb8dd9037f8..6eecfa38de5a 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1386,14 +1386,12 @@ int mbedtls_mpi_sub_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_sint * * \return c The carry at the end of the operation. */ -mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len , +mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len, const mbedtls_mpi_uint *s, size_t s_len, mbedtls_mpi_uint b ) { mbedtls_mpi_uint c = 0; /* carry */ - - /* Remember the excess of d over s for later */ - d_len -= s_len; + size_t const excess_len = d_len - s_len; #if defined(MULADDC_HUIT) for( ; s_len >= 8; s_len -= 8 ) @@ -1444,7 +1442,7 @@ mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len , } #endif /* MULADDC_HUIT */ - while( d_len-- ) + while( excess_len-- ) { *d += c; c = ( *d < c ); d++; } From 5d4ceeb25c4ce320ee46ef373ca1c78b72096e1e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 09:46:47 +0100 Subject: [PATCH 11/26] Remove const qualifier for mutable local variable in mpi_mul_hlp() Signed-off-by: Hanno Becker --- library/bignum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/bignum.c b/library/bignum.c index 6eecfa38de5a..581608d92592 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1391,7 +1391,7 @@ mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len, mbedtls_mpi_uint b ) { mbedtls_mpi_uint c = 0; /* carry */ - size_t const excess_len = d_len - s_len; + size_t excess_len = d_len - s_len; #if defined(MULADDC_HUIT) for( ; s_len >= 8; s_len -= 8 ) From efdc519864886da4fdf0ef3266d76091872a7014 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 10:44:02 +0100 Subject: [PATCH 12/26] Reintroduce though-to-be unused variable in correct place The variable is a local variable for the i386 bignum assembly only; introduce it as part of the start/finish macros. It can be noted that the variable is initialize to 0 within MULADDC_INIT, so there are no data dependencies across blocks of MULADDC_INIT/CORE/STOP. Signed-off-by: Hanno Becker --- library/bn_mul.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/bn_mul.h b/library/bn_mul.h index b71ddd881a04..aa1183fa5410 100644 --- a/library/bn_mul.h +++ b/library/bn_mul.h @@ -99,6 +99,7 @@ #if defined(__i386__) && defined(__OPTIMIZE__) #define MULADDC_INIT \ + { mbedtls_mpi_uint t; \ asm( \ "movl %%ebx, %0 \n\t" \ "movl %5, %%esi \n\t" \ @@ -190,7 +191,8 @@ : "=m" (t), "=m" (c), "=m" (d), "=m" (s) \ : "m" (t), "m" (s), "m" (d), "m" (c), "m" (b) \ : "eax", "ebx", "ecx", "edx", "esi", "edi" \ - ); + ); } \ + #else @@ -202,7 +204,7 @@ : "=m" (t), "=m" (c), "=m" (d), "=m" (s) \ : "m" (t), "m" (s), "m" (d), "m" (c), "m" (b) \ : "eax", "ebx", "ecx", "edx", "esi", "edi" \ - ); + ); } #endif /* SSE2 */ #endif /* i386 */ From 99ba4cc6d520aef5309e757f7b8cb650164b335d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 13:44:03 +0100 Subject: [PATCH 13/26] Remove Doxygen from mbedtls_mpi_core_mla() implementation Signed-off-by: Hanno Becker --- library/bignum.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 581608d92592..49e530fc7c04 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1370,22 +1370,6 @@ int mbedtls_mpi_sub_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_sint return( mbedtls_mpi_sub_mpi( X, A, &B ) ); } -/** Helper for mbedtls_mpi multiplication. - * - * Add \p b * \p s to \p d. - * - * \param[in,out] d The bignum to add to. - * \param d_len The number of limbs of \p d. This must be - * at least \p s_len. - * \param s_len The number of limbs of \p s. - * \param[in] s A bignum to multiply, of size \p i. - * It may overlap with \p d, but only if - * \p d <= \p s. - * Its leading limb must not be \c 0. - * \param b A scalar to multiply. - * - * \return c The carry at the end of the operation. - */ mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len, const mbedtls_mpi_uint *s, size_t s_len, mbedtls_mpi_uint b ) From dfcb2d084b5b95ad00cec5d9a045c1afb627b0e3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 13:44:15 +0100 Subject: [PATCH 14/26] Fix Doxygen for mbedtls_mpi_core_mla() Signed-off-by: Hanno Becker --- library/bignum_internal.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/library/bignum_internal.h b/library/bignum_internal.h index 2af7510f9988..8677dcf1f01f 100644 --- a/library/bignum_internal.h +++ b/library/bignum_internal.h @@ -26,19 +26,20 @@ #include "mbedtls/bignum.h" #endif -/** Helper for mbedtls_mpi multiplication. +/** Perform a known-size multiply accumulate operation * * Add \p b * \p s to \p d. * - * \param[in,out] d The bignum to add to. + * \param[in,out] d The pointer to the (little-endian) array + * representing the bignum to accumulate onto. * \param d_len The number of limbs of \p d. This must be * at least \p s_len. + * \param[in] s The pointer to the (little-endian) array + * representing the bignum to multiply with. + * This may be the same as \p d. Otherwise, + * it must be disjoint from \p d. * \param s_len The number of limbs of \p s. - * \param[in] s A bignum to multiply, of size \p i. - * It may overlap with \p d, but only if - * \p d <= \p s. - * Its leading limb must not be \c 0. - * \param b A scalar to multiply. + * \param b A scalar to multiply with. * * \return c The carry at the end of the operation. */ From 53b3c607a07218b859c0c0f029f27cfe9835d15f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 11 Apr 2022 13:46:30 +0100 Subject: [PATCH 15/26] Move `const` keyword prior to type name Signed-off-by: Hanno Becker --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index d477fc1f8f95..718d0d089401 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5222,7 +5222,7 @@ static int ecp_mod_p255( mbedtls_mpi *N ) /* Helper references for top part of N */ mbedtls_mpi_uint * const NT_p = N->p + P255_WIDTH; - size_t const NT_n = N->n - P255_WIDTH; + const size_t NT_n = N->n - P255_WIDTH; if( NT_n == 0 || NT_n > P255_WIDTH ) return( 0 ); From 808e666eeef4097ddf22618c0f54f37953033441 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 12 Apr 2022 10:49:53 +0100 Subject: [PATCH 16/26] Don't trim MPIs to minimal size in mbedtls_mpi_mul_mpi() Signed-off-by: Hanno Becker --- library/bignum.c | 43 +++++++++++++++++-------------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 49e530fc7c04..b17770702339 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1440,9 +1440,7 @@ mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len, int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi *B ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t i, j, k; mbedtls_mpi TA, TB; - int result_is_zero = 0; MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( A != NULL ); MPI_VALIDATE_RET( B != NULL ); @@ -1452,38 +1450,31 @@ int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi if( X == A ) { MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &TA, A ) ); A = &TA; } if( X == B ) { MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &TB, B ) ); B = &TB; } - for( i = A->n; i > 0; i-- ) - if( A->p[i - 1] != 0 ) - break; - if( i == 0 ) - result_is_zero = 1; - - for( j = B->n; j > 0; j-- ) - if( B->p[j - 1] != 0 ) - break; - if( j == 0 ) - result_is_zero = 1; - - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, i + j ) ); + const size_t A_len = A->n, B_len = B->n; + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, A_len + B_len ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( X, 0 ) ); - for( k = 0; k < j; k++ ) + for( size_t k = 0; k < B_len; k++ ) { /* We know that there cannot be any carry-out since we're * iterating from bottom to top. */ - (void) mbedtls_mpi_core_mla( X->p + k, i + 1, - A->p, i, + (void) mbedtls_mpi_core_mla( X->p + k, A_len + 1, + A->p, A_len, B->p[k] ); } - /* If the result is 0, we don't shortcut the operation, which reduces - * but does not eliminate side channels leaking the zero-ness. We do - * need to take care to set the sign bit properly since the library does - * not fully support an MPI object with a value of 0 and s == -1. */ - if( result_is_zero ) - X->s = 1; - else - X->s = A->s * B->s; + X->s = A->s * B->s; + + /* The library does not fully support a "negative zero". */ + if( X->s == -1 ) + { + size_t i; + for( i = X->n; i > 0; i-- ) + if( X->p[i - 1] != 0 ) + break; + if( i == 0 ) + X->s = 1; + } cleanup: From 9137b9c5879f521346289384aff2aeced35bf2a6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 12 Apr 2022 10:51:54 +0100 Subject: [PATCH 17/26] Note alternative implementation strategy in mbedtls_mpi_mul_int() Signed-off-by: Hanno Becker --- library/bignum.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/bignum.c b/library/bignum.c index b17770702339..acdb230361a9 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1504,7 +1504,10 @@ int mbedtls_mpi_mul_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_uint * making the call to grow() unconditional causes slightly fewer * calls to calloc() in ECP code, presumably because it reuses the * same mpi for a while and this way the mpi is more likely to directly - * grow to its final size. */ + * grow to its final size. + * + * Note that calculating A*b as 0 + A*b doesn't work as-is because + * A,X can be the same. */ MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, A->n + 1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_copy( X, A ) ); mbedtls_mpi_core_mla( X->p, X->n, A->p, A->n, b - 1 ); From 0235f7512f532535258e49b225dd86c28e995975 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 12 Apr 2022 10:54:46 +0100 Subject: [PATCH 18/26] Reduce scope of local variables in mpi_montmul() Signed-off-by: Hanno Becker --- library/bignum.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index acdb230361a9..78ba32f6dadf 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1886,8 +1886,8 @@ static void mpi_montg_init( mbedtls_mpi_uint *mm, const mbedtls_mpi *N ) static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi *N, mbedtls_mpi_uint mm, const mbedtls_mpi *T ) { - size_t i, n, m; - mbedtls_mpi_uint u0, u1, *d; + size_t n, m; + mbedtls_mpi_uint *d; memset( T->p, 0, T->n * ciL ); @@ -1895,8 +1895,10 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi n = N->n; m = ( B->n < n ) ? B->n : n; - for( i = 0; i < n; i++ ) + for( size_t i = 0; i < n; i++ ) { + mbedtls_mpi_uint u0, u1; + /* * T = (T + u0*B + u1*N) / 2^biL */ From 2ef0cff6c3d4b55462269e51921641e29f5fa12e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 12 Apr 2022 10:55:34 +0100 Subject: [PATCH 19/26] Fix size check in p25519 modular reduction The check was meant to precisely catch an underflow. Signed-off-by: Hanno Becker --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 718d0d089401..d3a14d679447 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5223,7 +5223,7 @@ static int ecp_mod_p255( mbedtls_mpi *N ) /* Helper references for top part of N */ mbedtls_mpi_uint * const NT_p = N->p + P255_WIDTH; const size_t NT_n = N->n - P255_WIDTH; - if( NT_n == 0 || NT_n > P255_WIDTH ) + if( NT_n == 0 || NT_n > N->n ) return( 0 ); /* Split N as N + 2^256 M */ From d830feb2561d99fa8b25c7ca68ed92db8b1d71bc Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 12 Apr 2022 11:10:19 +0100 Subject: [PATCH 20/26] Simplify check in p25519 quasi-reduction Signed-off-by: Hanno Becker --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index d3a14d679447..9e70e8ec922c 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5223,7 +5223,7 @@ static int ecp_mod_p255( mbedtls_mpi *N ) /* Helper references for top part of N */ mbedtls_mpi_uint * const NT_p = N->p + P255_WIDTH; const size_t NT_n = N->n - P255_WIDTH; - if( NT_n == 0 || NT_n > N->n ) + if( NT_n <= P255_WIDTH ) return( 0 ); /* Split N as N + 2^256 M */ From bb04cb992ff1731b6fded7847c140f68fb1a6d46 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 12 Apr 2022 11:18:11 +0100 Subject: [PATCH 21/26] Fix check in p25519 quasi-reduction Signed-off-by: Hanno Becker --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 9e70e8ec922c..571b0fe5795f 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5223,7 +5223,7 @@ static int ecp_mod_p255( mbedtls_mpi *N ) /* Helper references for top part of N */ mbedtls_mpi_uint * const NT_p = N->p + P255_WIDTH; const size_t NT_n = N->n - P255_WIDTH; - if( NT_n <= P255_WIDTH ) + if( N->n <= P255_WIDTH ) return( 0 ); /* Split N as N + 2^256 M */ From 127fcabb21d6f81664eb9c7f351032f018e52e19 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 12 Apr 2022 22:18:36 +0100 Subject: [PATCH 22/26] Fail gracefully upon unexpectedly large input to p25519 reduction Signed-off-by: Hanno Becker --- library/ecp_curves.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 571b0fe5795f..5788b0dba1d0 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5225,6 +5225,8 @@ static int ecp_mod_p255( mbedtls_mpi *N ) const size_t NT_n = N->n - P255_WIDTH; if( N->n <= P255_WIDTH ) return( 0 ); + if( NT_n > P255_WIDTH ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); /* Split N as N + 2^256 M */ memset( Mp, 0, sizeof( Mp ) ); From da763de7d0b6d5b5ad18fd82bc70565970eef59d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 13 Apr 2022 06:50:02 +0100 Subject: [PATCH 23/26] Revert "Don't trim MPIs to minimal size in mbedtls_mpi_mul_mpi()" This reverts commit 808e666eeef4097ddf22618c0f54f37953033441. Signed-off-by: Hanno Becker --- library/bignum.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 78ba32f6dadf..f121f405139d 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1440,7 +1440,9 @@ mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len, int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi *B ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t i, j, k; mbedtls_mpi TA, TB; + int result_is_zero = 0; MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( A != NULL ); MPI_VALIDATE_RET( B != NULL ); @@ -1450,31 +1452,38 @@ int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi if( X == A ) { MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &TA, A ) ); A = &TA; } if( X == B ) { MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &TB, B ) ); B = &TB; } - const size_t A_len = A->n, B_len = B->n; - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, A_len + B_len ) ); + for( i = A->n; i > 0; i-- ) + if( A->p[i - 1] != 0 ) + break; + if( i == 0 ) + result_is_zero = 1; + + for( j = B->n; j > 0; j-- ) + if( B->p[j - 1] != 0 ) + break; + if( j == 0 ) + result_is_zero = 1; + + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, i + j ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( X, 0 ) ); - for( size_t k = 0; k < B_len; k++ ) + for( k = 0; k < j; k++ ) { /* We know that there cannot be any carry-out since we're * iterating from bottom to top. */ - (void) mbedtls_mpi_core_mla( X->p + k, A_len + 1, - A->p, A_len, + (void) mbedtls_mpi_core_mla( X->p + k, i + 1, + A->p, i, B->p[k] ); } - X->s = A->s * B->s; - - /* The library does not fully support a "negative zero". */ - if( X->s == -1 ) - { - size_t i; - for( i = X->n; i > 0; i-- ) - if( X->p[i - 1] != 0 ) - break; - if( i == 0 ) - X->s = 1; - } + /* If the result is 0, we don't shortcut the operation, which reduces + * but does not eliminate side channels leaking the zero-ness. We do + * need to take care to set the sign bit properly since the library does + * not fully support an MPI object with a value of 0 and s == -1. */ + if( result_is_zero ) + X->s = 1; + else + X->s = A->s * B->s; cleanup: From 1772e05fca942dde3cd5a87346746fc77ec68e59 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 13 Apr 2022 06:51:40 +0100 Subject: [PATCH 24/26] Reduce the scope of local variable in mbedtls_mpi_mul_mpi() Signed-off-by: Hanno Becker --- library/bignum.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index f121f405139d..493ffa21e68f 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1440,7 +1440,7 @@ mbedtls_mpi_uint mbedtls_mpi_core_mla( mbedtls_mpi_uint *d, size_t d_len, int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi *B ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t i, j, k; + size_t i, j; mbedtls_mpi TA, TB; int result_is_zero = 0; MPI_VALIDATE_RET( X != NULL ); @@ -1467,7 +1467,7 @@ int mbedtls_mpi_mul_mpi( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, i + j ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( X, 0 ) ); - for( k = 0; k < j; k++ ) + for( size_t k = 0; k < j; k++ ) { /* We know that there cannot be any carry-out since we're * iterating from bottom to top. */ From 0dbf04a9a6f3f362187a99e472a9cb5958832ab1 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 13 Apr 2022 06:54:48 +0100 Subject: [PATCH 25/26] Remove unnecessary memory operations in p25519 quasireduction Signed-off-by: Hanno Becker --- library/ecp_curves.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 5788b0dba1d0..6b8ff5c7fba4 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5229,13 +5229,12 @@ static int ecp_mod_p255( mbedtls_mpi *N ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); /* Split N as N + 2^256 M */ - memset( Mp, 0, sizeof( Mp ) ); memcpy( Mp, NT_p, sizeof( mbedtls_mpi_uint ) * NT_n ); memset( NT_p, 0, sizeof( mbedtls_mpi_uint ) * NT_n ); /* N = A0 + 38 * A1 */ - mbedtls_mpi_core_mla( N->p, N->n, - Mp, P255_WIDTH, + mbedtls_mpi_core_mla( N->p, P255_WIDTH + 1, + Mp, NT_n, 38 ); return( 0 ); From 3577131bb4283b6d95c8dde85a4264fdb3c88278 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 14 Apr 2022 11:52:11 +0100 Subject: [PATCH 26/26] Reintroduce trimming of input in mbedtls_mpi_mul_int() Removing the trimming has significant memory impact. While it is clearly what we want to do eventually for constant-time'ness, it should be fixed alongside a strategy to contain the ramifications on memory usage. Signed-off-by: Hanno Becker --- library/bignum.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 493ffa21e68f..6f634b5d1717 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1500,8 +1500,12 @@ int mbedtls_mpi_mul_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_uint MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( A != NULL ); + size_t n = A->n; + while( n > 0 && A->p[n - 1] == 0 ) + --n; + /* The general method below doesn't work if b==0. */ - if( b == 0 ) + if( b == 0 || n == 0 ) return( mbedtls_mpi_lset( X, 0 ) ); /* Calculate A*b as A + A*(b-1) to take advantage of mbedtls_mpi_core_mla */ @@ -1517,9 +1521,9 @@ int mbedtls_mpi_mul_int( mbedtls_mpi *X, const mbedtls_mpi *A, mbedtls_mpi_uint * * Note that calculating A*b as 0 + A*b doesn't work as-is because * A,X can be the same. */ - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, A->n + 1 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, n + 1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_copy( X, A ) ); - mbedtls_mpi_core_mla( X->p, X->n, A->p, A->n, b - 1 ); + mbedtls_mpi_core_mla( X->p, X->n, A->p, n, b - 1 ); cleanup: return( ret );