Merge pull request #771 from gilles-peskine-arm/mpi_fill_random-rng_failure-development

Handle RNG failure in mbedtls_mpi_fill_random

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).

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 );
 

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:

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(  )
 {