From 678b0e5466d36cf430f550f24b02152dceb61e6b Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 28 Nov 2016 18:59:38 +0000 Subject: [PATCH] exhaustive tests: remove erroneous comment from ecdsa_sig_sign Mathematically, we always overflow when using the exhaustive tests (because our scalar order is 13 and our field order is on the order of 2^256), but the `overflow` variable returned when parsing a b32 as a scalar is always set to 0, to prevent infinite (or practically infinite) loops searching for non-overflowing scalars. --- src/ecdsa_impl.h | 4 +--- src/tests_exhaustive.c | 8 +++++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 52b2cb0..453bb11 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -225,14 +225,12 @@ static int secp256k1_ecdsa_sig_verify(const secp256k1_ecmult_context *ctx, const #if defined(EXHAUSTIVE_TEST_ORDER) { secp256k1_scalar computed_r; - int overflow = 0; secp256k1_ge pr_ge; secp256k1_ge_set_gej(&pr_ge, &pr); secp256k1_fe_normalize(&pr_ge.x); secp256k1_fe_get_b32(c, &pr_ge.x); - secp256k1_scalar_set_b32(&computed_r, c, &overflow); - /* we fully expect overflow */ + secp256k1_scalar_set_b32(&computed_r, c, NULL); return secp256k1_scalar_eq(sigr, &computed_r); } #else diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index bda6ee4..69d91c9 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -77,7 +77,7 @@ int secp256k1_nonce_function_smallint(unsigned char *nonce32, const unsigned cha * function with an increased `attempt`. So if attempt > 0 this means we * need to change the nonce to avoid an infinite loop. */ if (attempt > 0) { - (*idata)++; + *idata = (*idata + 1) % EXHAUSTIVE_TEST_ORDER; } secp256k1_scalar_set_int(&s, *idata); secp256k1_scalar_get_b32(nonce32, &s); @@ -244,6 +244,7 @@ void test_exhaustive_sign(const secp256k1_context *ctx, const secp256k1_ge *grou for (i = 1; i < order; i++) { /* message */ for (j = 1; j < order; j++) { /* key */ for (k = 1; k < order; k++) { /* nonce */ + const int starting_k = k; secp256k1_ecdsa_signature sig; secp256k1_scalar sk, msg, r, s, expected_r; unsigned char sk32[32], msg32[32]; @@ -262,6 +263,11 @@ void test_exhaustive_sign(const secp256k1_context *ctx, const secp256k1_ge *grou CHECK(r == expected_r); CHECK((k * s) % order == (i + r * j) % order || (k * (EXHAUSTIVE_TEST_ORDER - s)) % order == (i + r * j) % order); + + /* Overflow means we've tried every possible nonce */ + if (k < starting_k) { + break; + } } } }