From 3cb1e296a48fce2546c13264f619d0a199660377 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 202df1d29..3aebd6c7b 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -941,6 +941,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 e54aaffe6..b3c5cbab3 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 @@ -1308,6 +1325,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 436400eec3a772caf17b59850ac6adae2bded8fc 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 9325632b4..1f99ca7be 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -2391,7 +2391,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 );