From c097e9ea45f12bd8cb2f2ffa40a9181dba898b91 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 8 Jun 2020 21:58:22 +0200 Subject: [PATCH] Move carry propagation out of mpi_sub_hlp The function mpi_sub_hlp had confusing semantics: although it took a size parameter, it accessed the limb array d beyond this size, to propagate the carry. This made the function difficult to understand and analyze, with a potential buffer overflow if misused (not enough room to propagate the carry). Change the function so that it only performs the subtraction within the specified number of limbs, and returns the carry. Move the carry propagation out of mpi_sub_hlp and into its caller mbedtls_mpi_sub_abs. This makes the code of subtraction very slightly less neat, but not significantly different. In the one other place where mpi_sub_hlp is used, namely mpi_montmul, this is a net win because the carry is potentially sensitive data and the function carefully arranges to not have to propagate it. Signed-off-by: Gilles Peskine --- library/bignum.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 64ea872e0..0fddef2ac 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1343,12 +1343,23 @@ cleanup: } /* - * Helper for mbedtls_mpi subtraction: - * d -= s where d and s have the same size and d >= s. + * Helper for mbedtls_mpi subtraction. + * + * Calculate d - s where d and s have the same size. + * This function operates modulo (2^ciL)^n and returns the carry + * (1 if there was a wraparound, i.e. if `d < s`, and 0 otherwise). + * + * \param n Number of limbs of \p d and \p s. + * \param[in,out] d On input, the left operand. + * On output, the result of the subtraction: + * \param[s] The right operand. + * + * \return 1 if `d < s`. + * 0 if `d >= s`. */ -static void mpi_sub_hlp( size_t n, - mbedtls_mpi_uint *d, - const mbedtls_mpi_uint *s ) +static mbedtls_mpi_uint mpi_sub_hlp( size_t n, + mbedtls_mpi_uint *d, + const mbedtls_mpi_uint *s ) { size_t i; mbedtls_mpi_uint c, z; @@ -1359,11 +1370,7 @@ static void mpi_sub_hlp( size_t n, c = ( *d < *s ) + z; *d -= *s; } - while( c != 0 ) - { - z = ( *d < c ); *d -= c; - c = z; d++; - } + return( c ); } /* @@ -1374,6 +1381,7 @@ int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi mbedtls_mpi TB; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t n; + mbedtls_mpi_uint c, z; MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( A != NULL ); MPI_VALIDATE_RET( B != NULL ); @@ -1403,7 +1411,12 @@ int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi if( B->p[n - 1] != 0 ) break; - mpi_sub_hlp( n, X->p, B->p ); + c = mpi_sub_hlp( n, X->p, B->p ); + while( c != 0 ) + { + z = ( X->p[n] < c ); X->p[n] -= c; + c = z; n++; + } cleanup: @@ -2047,7 +2060,7 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi * timing attacks. */ /* Set d to A + (2^biL)^n - N. */ d[n] += 1; - mpi_sub_hlp( n, d, N->p ); + d[n] -= mpi_sub_hlp( n, d, N->p ); /* Now d - (2^biL)^n = A - N so d >= (2^biL)^n iff A >= N. * So we want to copy the result of the subtraction iff d->p[n] != 0. * Note that d->p[n] is either 0 or 1 since A - N <= N <= (2^biL)^n. */