From 909e03c52f7e2132656ed83e5f985466f51f9b24 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 Oct 2022 18:14:33 +0200 Subject: [PATCH 1/5] Bignum core: fill_random: prototype Signed-off-by: Gilles Peskine --- library/bignum_core.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/library/bignum_core.h b/library/bignum_core.h index ccccebbffa..624eaf49a3 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -470,4 +470,28 @@ void mbedtls_mpi_core_ct_uint_table_lookup( mbedtls_mpi_uint *dest, size_t count, size_t index ); +/** + * \brief Fill an integer with a number of random bytes. + * + * \param X The destination MPI. + * \param X_limbs The number of limbs of \p X. + * \param bytes The number of random bytes to generate. + * \param f_rng The RNG function to use. This must not be \c NULL. + * \param p_rng The RNG parameter to be passed to \p f_rng. This may be + * \c NULL if \p f_rng doesn't need a context argument. + * + * \return \c 0 if successful. + * \return #MBEDTLS_ERR_MPI_BAD_INPUT_DATA if \p X does not have + * enough room for \p bytes bytes. + * \return A negative error code on RNG failure. + * + * \note The bytes obtained from the RNG are interpreted + * as a big-endian representation of an MPI; this can + * be relevant in applications like deterministic ECDSA. + */ +int mbedtls_mpi_core_fill_random( mbedtls_mpi_uint *X, size_t X_limbs, + size_t bytes, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ); + #endif /* MBEDTLS_BIGNUM_CORE_H */ From 5980f2bd36b6e5508550085dd05d59069e4047ab Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Sep 2022 20:55:53 +0200 Subject: [PATCH 2/5] Implement mbedtls_mpi_core_fill_random Turn mpi_fill_random_internal() into mbedtls_mpi_core_fill_random(). It had basically the right code except for how X is passed to the function. Write unit tests. Signed-off-by: Gilles Peskine --- library/bignum.c | 23 ++++--- tests/suites/test_suite_bignum_core.function | 53 +++++++++++++++ tests/suites/test_suite_bignum_core.misc.data | 66 +++++++++++++++++++ 3 files changed, 132 insertions(+), 10 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index d33f07cc46..b5431a0fec 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1934,25 +1934,26 @@ cleanup: /* Fill X with n_bytes random bytes. * X must already have room for those bytes. * The ordering of the bytes returned from the RNG is suitable for - * deterministic ECDSA (see RFC 6979 §3.3 and mbedtls_mpi_random()). + * deterministic ECDSA (see RFC 6979 §3.3 and mbedtls_mpi_core_random()). * The size and sign of X are unchanged. * n_bytes must not be 0. */ -static int mpi_fill_random_internal( - mbedtls_mpi *X, size_t n_bytes, +int mbedtls_mpi_core_fill_random( + mbedtls_mpi_uint *X, size_t X_limbs, + size_t n_bytes, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const size_t limbs = CHARS_TO_LIMBS( n_bytes ); const size_t overhead = ( limbs * ciL ) - n_bytes; - if( X->n < limbs ) + if( X_limbs < limbs ) return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA ); - memset( X->p, 0, overhead ); - memset( (unsigned char *) X->p + limbs * ciL, 0, ( X->n - limbs ) * ciL ); - MBEDTLS_MPI_CHK( f_rng( p_rng, (unsigned char *) X->p + overhead, n_bytes ) ); - mbedtls_mpi_core_bigendian_to_host( X->p, limbs ); + memset( X, 0, overhead ); + memset( (unsigned char *) X + limbs * ciL, 0, ( X_limbs - limbs ) * ciL ); + MBEDTLS_MPI_CHK( f_rng( p_rng, (unsigned char *) X + overhead, n_bytes ) ); + mbedtls_mpi_core_bigendian_to_host( X, limbs ); cleanup: return( ret ); @@ -1980,7 +1981,7 @@ int mbedtls_mpi_fill_random( mbedtls_mpi *X, size_t size, if( size == 0 ) return( 0 ); - ret = mpi_fill_random_internal( X, size, f_rng, p_rng ); + ret = mbedtls_mpi_core_fill_random( X->p, X->n, size, f_rng, p_rng ); cleanup: return( ret ); @@ -2042,7 +2043,9 @@ int mbedtls_mpi_random( mbedtls_mpi *X, */ do { - MBEDTLS_MPI_CHK( mpi_fill_random_internal( X, n_bytes, f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_core_fill_random( X->p, X->n, + n_bytes, + f_rng, p_rng ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( X, 8 * n_bytes - n_bits ) ); if( --count == 0 ) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index d5d58d870b..745a8af46c 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -992,3 +992,56 @@ exit: mbedtls_free(dest); } /* END_CASE */ + +/* BEGIN_CASE */ +void mpi_core_fill_random( int wanted_bytes_arg, int extra_rng_bytes, + int extra_limbs, int before, int expected_ret ) +{ + size_t wanted_bytes = wanted_bytes_arg; + mbedtls_mpi_uint *X = NULL; + size_t X_limbs = CHARS_TO_LIMBS( wanted_bytes ) + extra_limbs; + size_t rng_bytes = wanted_bytes + extra_rng_bytes; + unsigned char *rnd_data = NULL; + mbedtls_test_rnd_buf_info rnd_info = {NULL, rng_bytes, NULL, NULL}; + int ret; + + /* Prepare an RNG with known output, limited to rng_bytes. */ + ASSERT_ALLOC( rnd_data, rng_bytes ); + TEST_EQUAL( 0, mbedtls_test_rnd_std_rand( NULL, rnd_data, rng_bytes ) ); + rnd_info.buf = rnd_data; + + /* Allocate an MPI with room for wanted_bytes plus extra_limbs. + * extra_limbs may be negative but the total limb count must be positive. + * Fill the MPI with the byte value in before. */ + TEST_LE_U( 1, X_limbs ); + ASSERT_ALLOC( X, X_limbs ); + memset( X, before, X_limbs * sizeof( *X ) ); + + ret = mbedtls_mpi_core_fill_random( X, X_limbs, wanted_bytes, + mbedtls_test_rnd_buffer_rand, + &rnd_info ); + TEST_EQUAL( expected_ret, ret ); + + if( expected_ret == 0 ) + { + /* mbedtls_mpi_core_fill_random is documented to use bytes from the + * RNG as a big-endian representation of the number. We used an RNG + * with known output, so check that the output contains the + * expected value. Bytes above wanted_bytes must be zero. */ + for( size_t i = 0; i < wanted_bytes; i++ ) + { + mbedtls_test_set_step( i ); + TEST_EQUAL( GET_BYTE( X, i ), rnd_data[wanted_bytes - 1 - i] ); + } + for( size_t i = wanted_bytes; i < X_limbs * ciL; i++ ) + { + mbedtls_test_set_step( i ); + TEST_EQUAL( GET_BYTE( X, i ), 0 ); + } + } + +exit: + mbedtls_free( rnd_data ); + mbedtls_free( X ); +} +/* END_CASE */ diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index a8fe9ab9d1..743bb3efe1 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -364,3 +364,69 @@ mpi_core_get_mont_r2_unsafe:"8335616aed761f1f7f44e6bd49e807b82e3bf2bf11bfa63":"5 mbedtls_mpi_core_get_mont_r2_unsafe #11 mpi_core_get_mont_r2_unsafe:"d1cece570f2f991013f26dd5b03c4c5b65f97be5905f36cb4664f2c78ff80aa8135a4aaf57ccb8a0aca2f394909a74cef1ef6758a64d11e2c149c393659d124bfc94196f0ce88f7d7d567efa5a649e2deefaa6e10fdc3deac60d606bf63fc540ac95294347031aefd73d6a9ee10188aaeb7a90d920894553cb196881691cadc51808715a07e8b24fcb1a63df047c7cdf084dd177ba368c806f3d51ddb5d3898c863e687ecaf7d649a57a46264a582f94d3c8f2edaf59f77a7f6bdaf83c991e8f06abe220ec8507386fce8c3da84c6c3903ab8f3ad4630a204196a7dbcbd9bcca4e40ec5cc5c09938d49f5e1e6181db8896f33bb12e6ef73f12ec5c5ea7a8a337":"12d7243d92ebc8338221f6dcec8ad8a2ec64c10a98339c8721beb1cb79e629253a7aa35e25d5421e6c2b43ddc4310cf4443875c070a7a5a5cc2c4c3eefa8a133af2e477fb7bb5b5058c6120946a7f9f08f2fab51e2f243b9ba206d2bfd62e4ef647dda49100d7004794f28172be2d715905fbd2e9ab8588c774523c0e096b49b6855a10e5ce0d8498370949a29d71d293788bf10a71e2447d4b2f11959a72f7290e2950772d14c83f15532468745fa58a83fca8883b0b6169a27ec0cf922c4f39d283bb20fca5ff1de01d9c66b8a710108b951af634d56c843d9505bf2edd5a7b8f0b72a5c95672151e60075a78084e83fbe284617a90c74c8335cce38bb012e":"12d7243d92ebc8338221f6dcec8ad8a2ec64c10a98339c8721beb1cb79e629253a7aa35e25d5421e6c2b43ddc4310cf4443875c070a7a5a5cc2c4c3eefa8a133af2e477fb7bb5b5058c6120946a7f9f08f2fab51e2f243b9ba206d2bfd62e4ef647dda49100d7004794f28172be2d715905fbd2e9ab8588c774523c0e096b49b6855a10e5ce0d8498370949a29d71d293788bf10a71e2447d4b2f11959a72f7290e2950772d14c83f15532468745fa58a83fca8883b0b6169a27ec0cf922c4f39d283bb20fca5ff1de01d9c66b8a710108b951af634d56c843d9505bf2edd5a7b8f0b72a5c95672151e60075a78084e83fbe284617a90c74c8335cce38bb012e" + +Fill random core: 0 bytes +mpi_core_fill_random:0:0:1:0:0 + +Fill random core: 1 byte, RNG stops at 0 +mpi_core_fill_random:1:-1:0:0:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random core: 1 byte, RNG just sufficient +mpi_core_fill_random:1:0:0:0:0 + +Fill random core: 1 byte, RNG not exhausted +mpi_core_fill_random:1:1:0:0:0 + +Fill random core: 1 byte, prior content nonzero +mpi_core_fill_random:1:0:0:0xba:0 + +Fill random core: 1 byte, 1 extra limb +mpi_core_fill_random:1:0:1:0:0 + +Fill random core: 1 byte, 1 extra limb, prior content nonzero +mpi_core_fill_random:1:0:1:0xba:0 + +Fill random core: 8 bytes, RNG stops before +mpi_core_fill_random:8:-1:0:0:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random core: 8 bytes, RNG just sufficient +mpi_core_fill_random:8:0:0:0:0 + +Fill random core: 8 bytes, RNG not exhausted +mpi_core_fill_random:8:1:0:0:0 + +Fill random core: 8 bytes, prior content nonzero +mpi_core_fill_random:8:0:0:0xba:0 + +Fill random core: 8 bytes, 1 extra limb +mpi_core_fill_random:8:0:1:0:0 + +Fill random core: 8 bytes, 1 extra limb, prior content nonzero +mpi_core_fill_random:8:0:1:0xba:0 + +Fill random core: 9 bytes, 1 missing limb +mpi_core_fill_random:9:0:-1:0:MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +Fill random core: 42 bytes, RNG stops before +mpi_core_fill_random:42:-1:0:0:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED + +Fill random core: 42 bytes, RNG just sufficient +mpi_core_fill_random:42:0:0:0:0 + +Fill random core: 42 bytes, RNG not exhausted +mpi_core_fill_random:42:1:0:0:0 + +Fill random core: 42 bytes, prior content nonzero +mpi_core_fill_random:42:0:0:0xba:0 + +Fill random core: 42 bytes, 1 extra limb +mpi_core_fill_random:42:0:1:0:0 + +Fill random core: 42 bytes, 1 extra limb, prior content nonzero +mpi_core_fill_random:42:0:1:0xba:0 + +Fill random core: 42 bytes, 1 missing limb +mpi_core_fill_random:42:0:-1:0:MBEDTLS_ERR_MPI_BAD_INPUT_DATA + +Fill random core: 42 bytes, 5 missing limbs +mpi_core_fill_random:42:0:-5:0:MBEDTLS_ERR_MPI_BAD_INPUT_DATA From 009d195a56522868cc7a887783f5db04706fd2ac Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Sep 2022 21:00:00 +0200 Subject: [PATCH 3/5] Move mbedtls_mpi_core_fill_random to the proper .c file Signed-off-by: Gilles Peskine --- library/bignum.c | 28 ---------------------------- library/bignum_core.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index b5431a0fec..76202fe514 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1931,34 +1931,6 @@ cleanup: return( ret ); } -/* Fill X with n_bytes random bytes. - * X must already have room for those bytes. - * The ordering of the bytes returned from the RNG is suitable for - * deterministic ECDSA (see RFC 6979 §3.3 and mbedtls_mpi_core_random()). - * The size and sign of X are unchanged. - * n_bytes must not be 0. - */ -int mbedtls_mpi_core_fill_random( - mbedtls_mpi_uint *X, size_t X_limbs, - size_t n_bytes, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - const size_t limbs = CHARS_TO_LIMBS( n_bytes ); - const size_t overhead = ( limbs * ciL ) - n_bytes; - - if( X_limbs < limbs ) - return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA ); - - memset( X, 0, overhead ); - memset( (unsigned char *) X + limbs * ciL, 0, ( X_limbs - limbs ) * ciL ); - MBEDTLS_MPI_CHK( f_rng( p_rng, (unsigned char *) X + overhead, n_bytes ) ); - mbedtls_mpi_core_bigendian_to_host( X, limbs ); - -cleanup: - return( ret ); -} - /* * Fill X with size bytes of random. * diff --git a/library/bignum_core.c b/library/bignum_core.c index b3bb3bcb88..e405995969 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -553,4 +553,33 @@ void mbedtls_mpi_core_ct_uint_table_lookup( mbedtls_mpi_uint *dest, } } + +/* Fill X with n_bytes random bytes. + * X must already have room for those bytes. + * The ordering of the bytes returned from the RNG is suitable for + * deterministic ECDSA (see RFC 6979 §3.3 and mbedtls_mpi_core_random()). + * The size and sign of X are unchanged. + * n_bytes must not be 0. + */ +int mbedtls_mpi_core_fill_random( + mbedtls_mpi_uint *X, size_t X_limbs, + size_t n_bytes, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + const size_t limbs = CHARS_TO_LIMBS( n_bytes ); + const size_t overhead = ( limbs * ciL ) - n_bytes; + + if( X_limbs < limbs ) + return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA ); + + memset( X, 0, overhead ); + memset( (unsigned char *) X + limbs * ciL, 0, ( X_limbs - limbs ) * ciL ); + MBEDTLS_MPI_CHK( f_rng( p_rng, (unsigned char *) X + overhead, n_bytes ) ); + mbedtls_mpi_core_bigendian_to_host( X, limbs ); + +cleanup: + return( ret ); +} + #endif /* MBEDTLS_BIGNUM_C */ From dd54324765bceba86a983a2ad4f8ba87d023e959 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 20 Sep 2022 23:07:23 +0200 Subject: [PATCH 4/5] Increase iterations for some statistical tests I ran into a sequence where the assertion `stats[8] > 0` failed for the range 1..272 with 100 iterations. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_bignum.misc.data | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_bignum.misc.data b/tests/suites/test_suite_bignum.misc.data index 78afcb64c5..29ba4ab46d 100644 --- a/tests/suites/test_suite_bignum.misc.data +++ b/tests/suites/test_suite_bignum.misc.data @@ -1766,16 +1766,16 @@ MPI random in range: 1..12 mpi_random_many:1:"0c":1000 MPI random in range: 1..255 -mpi_random_many:1:"ff":100 +mpi_random_many:1:"ff":200 MPI random in range: 1..256 -mpi_random_many:1:"0100":100 +mpi_random_many:1:"0100":200 MPI random in range: 1..257 -mpi_random_many:1:"0101":100 +mpi_random_many:1:"0101":200 MPI random in range: 1..272 -mpi_random_many:1:"0110":100 +mpi_random_many:1:"0110":200 MPI random in range: 1..2^64-1 mpi_random_many:1:"ffffffffffffffff":100 From 22cdd0ccd3af1f4721181ebcc949bf2f25db0ccd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Oct 2022 20:15:13 +0200 Subject: [PATCH 5/5] Update some internal comments The refactoring of fill_random had left some obsolete bits in comments. Signed-off-by: Gilles Peskine --- library/bignum.c | 7 +++---- library/bignum_core.c | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 76202fe514..521787d749 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1933,10 +1933,9 @@ cleanup: /* * Fill X with size bytes of random. - * - * Use a temporary bytes representation to make sure the result is the same - * regardless of the platform endianness (useful when f_rng is actually - * deterministic, eg for tests). + * The bytes returned from the RNG are used in a specific order which + * is suitable for deterministic ECDSA (see the specification of + * mbedtls_mpi_random() and the implementation in mbedtls_mpi_fill_random()). */ int mbedtls_mpi_fill_random( mbedtls_mpi *X, size_t size, int (*f_rng)(void *, unsigned char *, size_t), diff --git a/library/bignum_core.c b/library/bignum_core.c index e405995969..b990a8dcce 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -557,9 +557,8 @@ void mbedtls_mpi_core_ct_uint_table_lookup( mbedtls_mpi_uint *dest, /* Fill X with n_bytes random bytes. * X must already have room for those bytes. * The ordering of the bytes returned from the RNG is suitable for - * deterministic ECDSA (see RFC 6979 §3.3 and mbedtls_mpi_core_random()). - * The size and sign of X are unchanged. - * n_bytes must not be 0. + * deterministic ECDSA (see RFC 6979 §3.3 and the specification of + * mbedtls_mpi_core_random()). */ int mbedtls_mpi_core_fill_random( mbedtls_mpi_uint *X, size_t X_limbs,