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 <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine 2020-06-08 21:58:22 +02:00
parent 46bf7da684
commit 36acd547c5

View File

@ -1265,12 +1265,23 @@ cleanup:
} }
/* /*
* Helper for mbedtls_mpi subtraction: * Helper for mbedtls_mpi subtraction.
* d -= s where d and s have the same size and d >= s. *
* 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, static mbedtls_mpi_uint mpi_sub_hlp( size_t n,
mbedtls_mpi_uint *d, mbedtls_mpi_uint *d,
const mbedtls_mpi_uint *s ) const mbedtls_mpi_uint *s )
{ {
size_t i; size_t i;
mbedtls_mpi_uint c, z; mbedtls_mpi_uint c, z;
@ -1281,11 +1292,7 @@ static void mpi_sub_hlp( size_t n,
c = ( *d < *s ) + z; *d -= *s; c = ( *d < *s ) + z; *d -= *s;
} }
while( c != 0 ) return( c );
{
z = ( *d < c ); *d -= c;
c = z; d++;
}
} }
/* /*
@ -1296,6 +1303,7 @@ int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi
mbedtls_mpi TB; mbedtls_mpi TB;
int ret; int ret;
size_t n; size_t n;
mbedtls_mpi_uint c, z;
MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( X != NULL );
MPI_VALIDATE_RET( A != NULL ); MPI_VALIDATE_RET( A != NULL );
MPI_VALIDATE_RET( B != NULL ); MPI_VALIDATE_RET( B != NULL );
@ -1325,7 +1333,12 @@ int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi
if( B->p[n - 1] != 0 ) if( B->p[n - 1] != 0 )
break; 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: cleanup:
@ -1959,7 +1972,7 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi
* timing attacks. */ * timing attacks. */
/* Set d to A + (2^biL)^n - N. */ /* Set d to A + (2^biL)^n - N. */
d[n] += 1; 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. /* 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. * 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. */ * Note that d->p[n] is either 0 or 1 since A - N <= N <= (2^biL)^n. */