From 0727ca41b7418378d6d063b8ade62b7437c98d0a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 16:07:09 +0100 Subject: [PATCH 1/4] Make mpi_read_binary time constant This commit modifies mpi_read_binary to always allocate the minimum number of limbs required to hold the entire buffer provided to the function, regardless of its content. Previously, leading zero bytes in the input data were detected and used to reduce memory footprint and time, but this non-constant behavior turned out to be non-tolerable for the cryptographic applications this function is used for. --- library/bignum.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 4829d916b..f5130b406 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -678,16 +678,20 @@ cleanup: int mpi_read_binary( 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 ) + { + mpi_free( X ); + mpi_init( X ); + MPI_CHK( mpi_grow( X, limbs ) ); + } - MPI_CHK( mpi_grow( X, CHARS_TO_LIMBS( buflen - n ) ) ); MPI_CHK( 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] |= ((t_uint) buf[i - 1]) << ((j % ciL) << 3); cleanup: From 754663f8c497c43f169638c3903e37b323b7c98f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 16:08:19 +0100 Subject: [PATCH 2/4] Fix information leak in ecp_gen_keypair_base The function ecp_gen_keypair_base did not wipe the stack buffer used to hold the private exponent before returning. This commit fixes this by not using a stack buffer in the first place but instead calling mpi_fill_random directly to acquire the necessary random MPI. --- library/ecp.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 79066dc91..f39e7ebe8 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -1854,7 +1854,6 @@ int ecp_gen_keypair( ecp_group *grp, mpi *d, ecp_point *Q, { /* SEC1 3.2.1: Generate d such that 1 <= n < N */ int count = 0; - unsigned char rnd[POLARSSL_ECP_MAX_BYTES]; /* * Match the procedure given in RFC 6979 (deterministic ECDSA): @@ -1865,8 +1864,7 @@ int ecp_gen_keypair( ecp_group *grp, mpi *d, ecp_point *Q, */ do { - MPI_CHK( f_rng( p_rng, rnd, n_size ) ); - MPI_CHK( mpi_read_binary( d, rnd, n_size ) ); + MPI_CHK( mpi_fill_random( d, n_size, f_rng, p_rng ) ); MPI_CHK( mpi_shift_r( d, 8 * n_size - grp->nbits ) ); /* From c2102893af3257a9215176d8cb9c5ee969f39f91 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 16:09:08 +0100 Subject: [PATCH 3/4] Zeroize stack before returning from mpi_fill_random --- library/bignum.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/bignum.c b/library/bignum.c index f5130b406..0a9560734 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1884,6 +1884,7 @@ int mpi_fill_random( mpi *X, size_t size, MPI_CHK( mpi_read_binary( X, buf, size ) ); cleanup: + polarssl_zeroize( buf, sizeof( buf ) ); return( ret ); } From 825c3db149abc89de130c73f7266f59b844fd926 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 16:10:07 +0100 Subject: [PATCH 4/4] Adapt ChangeLog --- ChangeLog | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ChangeLog b/ChangeLog index a3171d7eb..360a72db8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 1.3.22 branch released xxxx-xx-xx + +Security + * Make 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. + = mbed TLS 1.3.21 branch released 2017-08-10 Security