From 2f78062e7542100cc5f7a832f1b509af121ac167 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 25 Nov 2020 15:37:20 +0100 Subject: [PATCH 1/2] Test mbedtls_mpi_fill_random Positive tests: test that the RNG has the expected size, given that we know how many leading zeros it has because we know how the function consumes bytes and when the test RNG produces null bytes. Negative tests: test that if the RNG is willing to emit less than the number of wanted bytes, the function fails. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_mpi.data | 42 ++++++++++++++++++++++++ tests/suites/test_suite_mpi.function | 48 ++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data index 00926ff97..a3e4eb974 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -929,6 +929,48 @@ mbedtls_mpi_set_bit:16:"00":32:1:16:"0100000000":0 Test bit set (Invalid bit value) mbedtls_mpi_set_bit:16:"00":5:2:16:"00":MBEDTLS_ERR_MPI_BAD_INPUT_DATA +Fill random: 0 bytes +mpi_fill_random:0:0:0 + +Fill random: 1 byte, good +mpi_fill_random:1:1:0 + +Fill random: 2 bytes, good, no leading zero +mpi_fill_random:2:2:0 + +Fill random: 2 bytes, good, 1 leading zero +mpi_fill_random:2:256:0 + +Fill random: MAX_SIZE - 7, good +mpi_fill_random:MBEDTLS_MPI_MAX_SIZE - 7:MBEDTLS_MPI_MAX_SIZE - 7:0 + +Fill random: MAX_SIZE, good +mpi_fill_random:MBEDTLS_MPI_MAX_SIZE:MBEDTLS_MPI_MAX_SIZE:0 + +Fill random: 1 byte, RNG failure +mpi_fill_random:1:0:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random: 2 bytes, RNG failure after 1 byte +mpi_fill_random:2:1:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random: 4 bytes, RNG failure after 3 bytes +mpi_fill_random:4:3:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random: 8 bytes, RNG failure after 7 bytes +mpi_fill_random:8:7:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random: 16 bytes, RNG failure after 1 bytes +mpi_fill_random:16:1:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random: 16 bytes, RNG failure after 8 bytes +mpi_fill_random:16:8:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random: 16 bytes, RNG failure after 15 bytes +mpi_fill_random:16:15:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random: MAX_SIZE bytes, RNG failure after MAX_SIZE-1 bytes +mpi_fill_random:MBEDTLS_MPI_MAX_SIZE:MBEDTLS_MPI_MAX_SIZE-1:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + MPI Selftest depends_on:MBEDTLS_SELF_TEST mpi_selftest: diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index f2702f12b..a6d9effc2 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -1,5 +1,6 @@ /* BEGIN_HEADER */ #include "mbedtls/bignum.h" +#include "mbedtls/entropy.h" typedef struct mbedtls_test_mpi_random { @@ -43,6 +44,22 @@ int mbedtls_test_mpi_miller_rabin_determinizer( void* state, return( 0 ); } + +/* Random generator that is told how many bytes to return. */ +static int f_rng_bytes_left( void *state, unsigned char *buf, size_t len ) +{ + size_t *bytes_left = state; + size_t i; + for( i = 0; i < len; i++ ) + { + if( *bytes_left == 0 ) + return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); + buf[i] = *bytes_left & 0xff; + --( *bytes_left ); + } + return( 0 ); +} + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -1251,6 +1268,37 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mpi_fill_random( int wanted_bytes, int rng_bytes, int expected_ret ) +{ + mbedtls_mpi X; + int ret; + size_t bytes_left = rng_bytes; + mbedtls_mpi_init( &X ); + + ret = mbedtls_mpi_fill_random( &X, wanted_bytes, + f_rng_bytes_left, &bytes_left ); + TEST_ASSERT( ret == expected_ret ); + + if( expected_ret == 0 ) + { + /* mbedtls_mpi_fill_random is documented to use bytes from the RNG + * as a big-endian representation of the number. We know when + * our RNG function returns null bytes, so we know how many + * leading zero bytes the number has. */ + size_t leading_zeros = 0; + if( wanted_bytes > 0 && rng_bytes % 256 == 0 ) + leading_zeros = 1; + TEST_ASSERT( mbedtls_mpi_size( &X ) + leading_zeros == + (size_t) wanted_bytes ); + TEST_ASSERT( (int) bytes_left == rng_bytes - wanted_bytes ); + } + +exit: + mbedtls_mpi_free( &X ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SELF_TEST */ void mpi_selftest( ) { From 05251147521930cbd3ac2b415d7d25404230fd98 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 25 Nov 2020 16:15:14 +0100 Subject: [PATCH 2/2] Handle random generator failure in mbedtls_mpi_fill_random() Discuss the impact in a changelog entry. Signed-off-by: Gilles Peskine --- ChangeLog.d/mpi_fill_random-rng_failure.txt | 8 ++++++++ library/bignum.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 ChangeLog.d/mpi_fill_random-rng_failure.txt diff --git a/ChangeLog.d/mpi_fill_random-rng_failure.txt b/ChangeLog.d/mpi_fill_random-rng_failure.txt new file mode 100644 index 000000000..8addf180c --- /dev/null +++ b/ChangeLog.d/mpi_fill_random-rng_failure.txt @@ -0,0 +1,8 @@ +Security + * A failure of the random generator was ignored in mbedtls_mpi_fill_random(), + which is how most uses of randomization in asymmetric cryptography + (including key generation, intermediate value randomization and blinding) + are implemented. This could cause failures or the silent use of non-random + values. A random generator can fail if it needs reseeding and cannot not + obtain entropy, or due to an internal failure (which, for Mbed TLS's own + CTR_DRBG or HMAC_DRBG, can only happen due to a misconfiguration). diff --git a/library/bignum.c b/library/bignum.c index dfe976d64..0d363e6e0 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -2334,7 +2334,7 @@ int mbedtls_mpi_fill_random( mbedtls_mpi *X, size_t size, MBEDTLS_MPI_CHK( mbedtls_mpi_lset( X, 0 ) ); Xp = (unsigned char*) X->p; - f_rng( p_rng, Xp + overhead, size ); + MBEDTLS_MPI_CHK( f_rng( p_rng, Xp + overhead, size ) ); mpi_bigendian_to_host( X->p, limbs );