From 723e8ca8f7ee75126bac4240feeac825c23a0d44 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 8 May 2023 12:07:26 -0400 Subject: [PATCH 1/3] Remove randomness tests Our RNG has been replaced with Xoshiro256++, a well-analyzed RNG. Our unit tests should not be resposible for verifying its statistical qualities. --- src/tests.c | 76 ----------------------------------------------------- 1 file changed, 76 deletions(-) diff --git a/src/tests.c b/src/tests.c index 6d46b39d74..462407be2e 100644 --- a/src/tests.c +++ b/src/tests.c @@ -804,78 +804,6 @@ static void run_tagged_sha256_tests(void) { CHECK(secp256k1_memcmp_var(hash32, hash_expected, sizeof(hash32)) == 0); } -/***** RANDOM TESTS *****/ - -static void test_rand_bits(int rand32, int bits) { - /* (1-1/2^B)^rounds[B] < 1/10^9, so rounds is the number of iterations to - * get a false negative chance below once in a billion */ - static const unsigned int rounds[7] = {1, 30, 73, 156, 322, 653, 1316}; - /* We try multiplying the results with various odd numbers, which shouldn't - * influence the uniform distribution modulo a power of 2. */ - static const uint32_t mults[6] = {1, 3, 21, 289, 0x9999, 0x80402011}; - /* We only select up to 6 bits from the output to analyse */ - unsigned int usebits = bits > 6 ? 6 : bits; - unsigned int maxshift = bits - usebits; - /* For each of the maxshift+1 usebits-bit sequences inside a bits-bit - number, track all observed outcomes, one per bit in a uint64_t. */ - uint64_t x[6][27] = {{0}}; - unsigned int i, shift, m; - /* Multiply the output of all rand calls with the odd number m, which - should not change the uniformity of its distribution. */ - for (i = 0; i < rounds[usebits]; i++) { - uint32_t r = (rand32 ? secp256k1_testrand32() : secp256k1_testrand_bits(bits)); - CHECK((((uint64_t)r) >> bits) == 0); - for (m = 0; m < sizeof(mults) / sizeof(mults[0]); m++) { - uint32_t rm = r * mults[m]; - for (shift = 0; shift <= maxshift; shift++) { - x[m][shift] |= (((uint64_t)1) << ((rm >> shift) & ((1 << usebits) - 1))); - } - } - } - for (m = 0; m < sizeof(mults) / sizeof(mults[0]); m++) { - for (shift = 0; shift <= maxshift; shift++) { - /* Test that the lower usebits bits of x[shift] are 1 */ - CHECK(((~x[m][shift]) << (64 - (1 << usebits))) == 0); - } - } -} - -/* Subrange must be a whole divisor of range, and at most 64 */ -static void test_rand_int(uint32_t range, uint32_t subrange) { - /* (1-1/subrange)^rounds < 1/10^9 */ - int rounds = (subrange * 2073) / 100; - int i; - uint64_t x = 0; - CHECK((range % subrange) == 0); - for (i = 0; i < rounds; i++) { - uint32_t r = secp256k1_testrand_int(range); - CHECK(r < range); - r = r % subrange; - x |= (((uint64_t)1) << r); - } - /* Test that the lower subrange bits of x are 1. */ - CHECK(((~x) << (64 - subrange)) == 0); -} - -static void run_rand_bits(void) { - size_t b; - test_rand_bits(1, 32); - for (b = 1; b <= 32; b++) { - test_rand_bits(0, b); - } -} - -static void run_rand_int(void) { - static const uint32_t ms[] = {1, 3, 17, 1000, 13771, 999999, 33554432}; - static const uint32_t ss[] = {1, 3, 6, 9, 13, 31, 64}; - unsigned int m, s; - for (m = 0; m < sizeof(ms) / sizeof(ms[0]); m++) { - for (s = 0; s < sizeof(ss) / sizeof(ss[0]); s++) { - test_rand_int(ms[m] * ss[s], ss[s]); - } - } -} - /***** MODINV TESTS *****/ /* Compute the modular inverse of (odd) x mod 2^64. */ @@ -7730,10 +7658,6 @@ int main(int argc, char **argv) { /* scratch tests */ run_scratch_tests(); - /* randomness tests */ - run_rand_bits(); - run_rand_int(); - /* integer arithmetic tests */ #ifdef SECP256K1_WIDEMUL_INT128 run_int128_tests(); From fb5bfa4eed834dcd58109525408a2d88dabc48c5 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 9 May 2023 18:07:11 +0200 Subject: [PATCH 2/3] Add static test vector for Xoshiro256++ --- src/tests.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/tests.c b/src/tests.c index 462407be2e..664c541d9c 100644 --- a/src/tests.c +++ b/src/tests.c @@ -172,6 +172,35 @@ static void random_scalar_order_b32(unsigned char *b32) { secp256k1_scalar_get_b32(b32, &num); } +static void run_xoshiro256pp_tests(void) { + { + size_t i; + /* Sanity check that we run before the actual seeding. */ + for (i = 0; i < sizeof(secp256k1_test_state)/sizeof(secp256k1_test_state[0]); i++) { + CHECK(secp256k1_test_state[i] == 0); + } + } + { + int i; + unsigned char buf32[32]; + unsigned char seed16[16] = { + 'C', 'H', 'I', 'C', 'K', 'E', 'N', '!', + 'C', 'H', 'I', 'C', 'K', 'E', 'N', '!', + }; + unsigned char buf32_expected[32] = { + 0xAF, 0xCC, 0xA9, 0x16, 0xB5, 0x6C, 0xE3, 0xF0, + 0x44, 0x3F, 0x45, 0xE0, 0x47, 0xA5, 0x08, 0x36, + 0x4C, 0xCC, 0xC1, 0x18, 0xB2, 0xD8, 0x8F, 0xEF, + 0x43, 0x26, 0x15, 0x57, 0x37, 0x00, 0xEF, 0x30, + }; + secp256k1_testrand_seed(seed16); + for (i = 0; i < 17; i++) { + secp256k1_testrand256(buf32); + } + CHECK(secp256k1_memcmp_var(buf32, buf32_expected, sizeof(buf32)) == 0); + } +} + static void run_selftest_tests(void) { /* Test public API */ secp256k1_selftest(); @@ -7621,6 +7650,9 @@ int main(int argc, char **argv) { } printf("test count = %i\n", COUNT); + /* run test RNG tests (must run before we really initialize the test RNG) */ + run_xoshiro256pp_tests(); + /* find random seed */ secp256k1_testrand_init(argc > 2 ? argv[2] : NULL); From 6ec3731e8c53658fcf68634c81bb1e47cad791ad Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 10 May 2023 10:40:08 -0400 Subject: [PATCH 3/3] Simplify test PRNG implementation --- src/testrand_impl.h | 65 ++++++++++++--------------------------------- 1 file changed, 17 insertions(+), 48 deletions(-) diff --git a/src/testrand_impl.h b/src/testrand_impl.h index 1b7481a53b..fe97620435 100644 --- a/src/testrand_impl.h +++ b/src/testrand_impl.h @@ -16,8 +16,6 @@ #include "util.h" static uint64_t secp256k1_test_state[4]; -static uint64_t secp256k1_test_rng_integer; -static int secp256k1_test_rng_integer_bits_left = 0; SECP256K1_INLINE static void secp256k1_testrand_seed(const unsigned char *seed16) { static const unsigned char PREFIX[19] = "secp256k1 test init"; @@ -36,7 +34,6 @@ SECP256K1_INLINE static void secp256k1_testrand_seed(const unsigned char *seed16 for (j = 0; j < 8; ++j) s = (s << 8) | out32[8*i + j]; secp256k1_test_state[i] = s; } - secp256k1_test_rng_integer_bits_left = 0; } SECP256K1_INLINE static uint64_t rotl(const uint64_t x, int k) { @@ -57,58 +54,30 @@ SECP256K1_INLINE static uint64_t secp256k1_testrand64(void) { } SECP256K1_INLINE static uint64_t secp256k1_testrand_bits(int bits) { - uint64_t ret; - if (secp256k1_test_rng_integer_bits_left < bits) { - secp256k1_test_rng_integer = secp256k1_testrand64(); - secp256k1_test_rng_integer_bits_left = 64; - } - ret = secp256k1_test_rng_integer; - secp256k1_test_rng_integer >>= bits; - secp256k1_test_rng_integer_bits_left -= bits; - ret &= ((~((uint64_t)0)) >> (64 - bits)); - return ret; + if (bits == 0) return 0; + return secp256k1_testrand64() >> (64 - bits); } SECP256K1_INLINE static uint32_t secp256k1_testrand32(void) { - return secp256k1_testrand_bits(32); + return secp256k1_testrand64() >> 32; } static uint32_t secp256k1_testrand_int(uint32_t range) { - /* We want a uniform integer between 0 and range-1, inclusive. - * B is the smallest number such that range <= 2**B. - * two mechanisms implemented here: - * - generate B bits numbers until one below range is found, and return it - * - find the largest multiple M of range that is <= 2**(B+A), generate B+A - * bits numbers until one below M is found, and return it modulo range - * The second mechanism consumes A more bits of entropy in every iteration, - * but may need fewer iterations due to M being closer to 2**(B+A) then - * range is to 2**B. The array below (indexed by B) contains a 0 when the - * first mechanism is to be used, and the number A otherwise. - */ - static const int addbits[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 2, 1, 0}; - uint32_t trange, mult; - int bits = 0; - if (range <= 1) { - return 0; - } - trange = range - 1; - while (trange > 0) { - trange >>= 1; - bits++; + uint32_t mask = 0; + uint32_t range_copy; + /* Reduce range by 1, changing its meaning to "maximum value". */ + VERIFY_CHECK(range != 0); + range -= 1; + /* Count the number of bits in range. */ + range_copy = range; + while (range_copy) { + mask = (mask << 1) | 1U; + range_copy >>= 1; } - if (addbits[bits]) { - bits = bits + addbits[bits]; - mult = ((~((uint32_t)0)) >> (32 - bits)) / range; - trange = range * mult; - } else { - trange = range; - mult = 1; - } - while(1) { - uint32_t x = secp256k1_testrand_bits(bits); - if (x < trange) { - return (mult == 1) ? x : (x % range); - } + /* Generation loop. */ + while (1) { + uint32_t val = secp256k1_testrand64() & mask; + if (val <= range) return val; } }