diff --git a/ChangeLog b/ChangeLog index df9e1a7da..96fca0226 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,13 @@ Security being leaked to memory after release. * Fix dhm_check_range() failing to detect trivial subgroups and potentially leaking 1 bit of the private key. Reported by prashantkspatil. + * Make mbedtls_mpi_read_binary constant-time with respect to + the input data. Previously, trailing zero bytes were detected + and omitted for the sake of saving memory, but potentially + leading to slight timing differences. + Reported by Marco Macchetti, Kudelski Group. + * Wipe stack buffer temporarily holding EC private exponent + after keypair generation. Features * Allow comments in test data files. diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index 456a80420..214e83c2d 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -683,6 +683,10 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi * * \return 0 if successful, * MBEDTLS_ERR_MPI_ALLOC_FAILED if memory allocation failed + * + * \note The bytes obtained from the PRNG are interpreted + * as a big-endian representation of an MPI; this can + * be relevant in applications like deterministic ECDSA. */ int mbedtls_mpi_fill_random( mbedtls_mpi *X, size_t size, int (*f_rng)(void *, unsigned char *, size_t), diff --git a/library/bignum.c b/library/bignum.c index bd8280b6f..d27c130bc 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -63,6 +63,7 @@ static void mbedtls_mpi_zeroize( mbedtls_mpi_uint *v, size_t n ) { volatile mbedtls_mpi_uint *p = v; while( n-- ) *p++ = 0; } +/* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } @@ -676,16 +677,20 @@ cleanup: int mbedtls_mpi_read_binary( mbedtls_mpi *X, const unsigned char *buf, size_t buflen ) { int ret; - size_t i, j, n; + size_t i, j; + size_t const limbs = CHARS_TO_LIMBS( buflen ); - for( n = 0; n < buflen; n++ ) - if( buf[n] != 0 ) - break; + /* Ensure that target MPI has exactly the necessary number of limbs */ + if( X->n != limbs ) + { + mbedtls_mpi_free( X ); + mbedtls_mpi_init( X ); + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, limbs ) ); + } - MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, CHARS_TO_LIMBS( buflen - n ) ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( X, 0 ) ); - for( i = buflen, j = 0; i > n; i--, j++ ) + for( i = buflen, j = 0; i > 0; i--, j++ ) X->p[j / ciL] |= ((mbedtls_mpi_uint) buf[i - 1]) << ((j % ciL) << 3); cleanup: @@ -1887,7 +1892,6 @@ int mbedtls_mpi_fill_random( mbedtls_mpi *X, size_t size, cleanup: mbedtls_zeroize( buf, sizeof( buf ) ); - return( ret ); } diff --git a/library/ecp.c b/library/ecp.c index 5ad686398..b41baef27 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -1953,7 +1953,6 @@ int mbedtls_ecp_gen_keypair_base( mbedtls_ecp_group *grp, { /* SEC1 3.2.1: Generate d such that 1 <= n < N */ int count = 0; - unsigned char rnd[MBEDTLS_ECP_MAX_BYTES]; /* * Match the procedure given in RFC 6979 (deterministic ECDSA): @@ -1964,8 +1963,7 @@ int mbedtls_ecp_gen_keypair_base( mbedtls_ecp_group *grp, */ do { - MBEDTLS_MPI_CHK( f_rng( p_rng, rnd, n_size ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( d, rnd, n_size ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( d, n_size, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( d, 8 * n_size - grp->nbits ) ); /*