Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove a secret-dependent branch in Montgomery multiplication #3398

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ChangeLog.d/montmul-cmp-branch.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Security
* Fix a side channel vulnerability in modular exponentiation that could
reveal an RSA private key used in a secure enclave. Noticed by Sangho Lee,
Ming-Wei Shih, Prasun Gera, Taesoo Kim and Hyesoon Kim (Georgia Institute
of Technology); and Marcus Peinado (Microsoft Research). Reported by Raoul
Strackx (Fortanix) in #3394.
128 changes: 90 additions & 38 deletions library/bignum.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,22 @@ void mbedtls_mpi_swap( mbedtls_mpi *X, mbedtls_mpi *Y )
memcpy( Y, &T, sizeof( mbedtls_mpi ) );
}

/*
* Conditionally assign dest = src, without leaking information
* about whether the assignment was made or not.
* dest and src must be arrays of limbs of size n.
* assign must be 0 or 1.
*/
static void mpi_safe_cond_assign( size_t n,
mbedtls_mpi_uint *dest,
const mbedtls_mpi_uint *src,
unsigned char assign )
{
size_t i;
for( i = 0; i < n; i++ )
dest[i] = dest[i] * ( 1 - assign ) + src[i] * assign;
}

/*
* Conditionally assign X = Y, without leaking information
* about whether the assignment was made or not.
Expand All @@ -262,10 +278,9 @@ int mbedtls_mpi_safe_cond_assign( mbedtls_mpi *X, const mbedtls_mpi *Y, unsigned

X->s = X->s * ( 1 - assign ) + Y->s * assign;

for( i = 0; i < Y->n; i++ )
X->p[i] = X->p[i] * ( 1 - assign ) + Y->p[i] * assign;
mpi_safe_cond_assign( Y->n, X->p, Y->p, assign );

for( ; i < X->n; i++ )
for( i = Y->n; i < X->n; i++ )
X->p[i] *= ( 1 - assign );

cleanup:
Expand Down Expand Up @@ -1328,9 +1343,23 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi
}

/*
* Helper for mbedtls_mpi subtraction
* 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax error: I think you mean param[in] s.

(Btw, less important, but you're using doxygen syntax in something that's not a doxygen comment. Maybe it would be cleaner to start the comment with /**, so that various tools such as clang's -Wdocumentation can check it.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the second part, I was a bit surprised check-doxy-blocks didn't complain. I checked the CI results, and it did complain.

*
* \return 1 if `d < s`.
* 0 if `d >= s`.
*/
static void mpi_sub_hlp( size_t n, mbedtls_mpi_uint *s, mbedtls_mpi_uint *d )
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;
Expand All @@ -1341,11 +1370,7 @@ static void mpi_sub_hlp( size_t n, mbedtls_mpi_uint *s, mbedtls_mpi_uint *d )
c = ( *d < *s ) + z; *d -= *s;
}

while( c != 0 )
{
z = ( *d < c ); *d -= c;
c = z; d++;
}
return( c );
}

/*
Expand All @@ -1356,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 );
Expand Down Expand Up @@ -1385,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, B->p, X->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:

Expand Down Expand Up @@ -1975,18 +2006,34 @@ static void mpi_montg_init( mbedtls_mpi_uint *mm, const mbedtls_mpi *N )
*mm = ~x + 1;
}

/*
* Montgomery multiplication: A = A * B * R^-1 mod N (HAC 14.36)
*/
static int mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi *N, mbedtls_mpi_uint mm,
/** Montgomery multiplication: A = A * B * R^-1 mod N (HAC 14.36)
yanesca marked this conversation as resolved.
Show resolved Hide resolved
*
* \param[in,out] A One of the numbers to multiply.
* It must have at least one more limb than N
* (A->n >= N->n + 1).
* On successful completion, A contains the result of
* the multiplication A * B * R^-1 mod N where
* R = (2^ciL)^n.
* \param[in] B One of the numbers to multiply.
* It must be nonzero and must not have more limbs than N
* (B->n <= N->n).
* \param[in] N The modulo. N must be odd.
* \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)).
* Its initial content is unused and
* its final content is indeterminate.
* Note that unlike the usual convention in the library
* for `const mbedtls_mpi*`, the content of T can change.
yanesca marked this conversation as resolved.
Show resolved Hide resolved
*/
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;

if( T->n < N->n + 1 || T->p == NULL )
return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA );

memset( T->p, 0, T->n * ciL );

d = T->p;
Expand All @@ -2009,28 +2056,33 @@ static int mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi

memcpy( A->p, d, ( n + 1 ) * ciL );

if( mbedtls_mpi_cmp_abs( A, N ) >= 0 )
mpi_sub_hlp( n, N->p, A->p );
else
/* prevent timing attacks */
mpi_sub_hlp( n, A->p, T->p );

return( 0 );
/* If A >= N then A -= N. Do the subtraction unconditionally to prevent
* timing attacks. */
/* Set d to A + (2^biL)^n - N. */
d[n] += 1;
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. */
mpi_safe_cond_assign( n + 1, A->p, d, (unsigned char) d[n] );
A->p[n] = 0;
yanesca marked this conversation as resolved.
Show resolved Hide resolved
}

/*
* Montgomery reduction: A = A * R^-1 mod N
*
* See mpi_montmul() regarding constraints and guarantees on the parameters.
*/
static int mpi_montred( mbedtls_mpi *A, const mbedtls_mpi *N,
mbedtls_mpi_uint mm, const mbedtls_mpi *T )
static void mpi_montred( mbedtls_mpi *A, const mbedtls_mpi *N,
mbedtls_mpi_uint mm, const mbedtls_mpi *T )
{
mbedtls_mpi_uint z = 1;
mbedtls_mpi U;

U.n = U.s = (int) z;
U.p = &z;

return( mpi_montmul( A, &U, N, mm, T ) );
mpi_montmul( A, &U, N, mm, T );
}

/*
Expand Down Expand Up @@ -2116,13 +2168,13 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A,
else
MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &W[1], A ) );

MBEDTLS_MPI_CHK( mpi_montmul( &W[1], &RR, N, mm, &T ) );
mpi_montmul( &W[1], &RR, N, mm, &T );

/*
* X = R^2 * R^-1 mod N = R mod N
*/
MBEDTLS_MPI_CHK( mbedtls_mpi_copy( X, &RR ) );
MBEDTLS_MPI_CHK( mpi_montred( X, N, mm, &T ) );
mpi_montred( X, N, mm, &T );

if( wsize > 1 )
{
Expand All @@ -2135,7 +2187,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A,
MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &W[j], &W[1] ) );

for( i = 0; i < wsize - 1; i++ )
MBEDTLS_MPI_CHK( mpi_montmul( &W[j], &W[j], N, mm, &T ) );
mpi_montmul( &W[j], &W[j], N, mm, &T );

/*
* W[i] = W[i - 1] * W[1]
Expand All @@ -2145,7 +2197,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A,
MBEDTLS_MPI_CHK( mbedtls_mpi_grow( &W[i], N->n + 1 ) );
MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &W[i], &W[i - 1] ) );

MBEDTLS_MPI_CHK( mpi_montmul( &W[i], &W[1], N, mm, &T ) );
mpi_montmul( &W[i], &W[1], N, mm, &T );
}
}

Expand Down Expand Up @@ -2182,7 +2234,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A,
/*
* out of window, square X
*/
MBEDTLS_MPI_CHK( mpi_montmul( X, X, N, mm, &T ) );
mpi_montmul( X, X, N, mm, &T );
continue;
}

Expand All @@ -2200,12 +2252,12 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A,
* X = X^wsize R^-1 mod N
*/
for( i = 0; i < wsize; i++ )
MBEDTLS_MPI_CHK( mpi_montmul( X, X, N, mm, &T ) );
mpi_montmul( X, X, N, mm, &T );

/*
* X = X * W[wbits] R^-1 mod N
*/
MBEDTLS_MPI_CHK( mpi_montmul( X, &W[wbits], N, mm, &T ) );
mpi_montmul( X, &W[wbits], N, mm, &T );

state--;
nbits = 0;
Expand All @@ -2218,18 +2270,18 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A,
*/
for( i = 0; i < nbits; i++ )
{
MBEDTLS_MPI_CHK( mpi_montmul( X, X, N, mm, &T ) );
mpi_montmul( X, X, N, mm, &T );

wbits <<= 1;

if( ( wbits & ( one << wsize ) ) != 0 )
MBEDTLS_MPI_CHK( mpi_montmul( X, &W[1], N, mm, &T ) );
mpi_montmul( X, &W[1], N, mm, &T );
}

/*
* X = A^E * R * R^-1 mod N = A^E mod N
*/
MBEDTLS_MPI_CHK( mpi_montred( X, N, mm, &T ) );
mpi_montred( X, N, mm, &T );

if( neg && E->n != 0 && ( E->p[0] & 1 ) != 0 )
{
Expand Down