From 25e3cfbf9b52d2f5afa543f967a73aa8850d2038 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 26 Nov 2016 20:14:19 +0000 Subject: [PATCH] ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign Whenever ecdsa_sig_sign is called, in the case that r == 0 or r overflows, we want to retry with a different nonce rather than fail signing entirely. Because of this, we always check the nonce conditions before calling sig_sign, so these checks should always pass (and in particular, they are inaccessible through the API and appear as uncovered code in test coverage). --- src/ecdsa_impl.h | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 9a42e51..52b2cb0 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -285,14 +285,10 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec secp256k1_fe_normalize(&r.y); secp256k1_fe_get_b32(b, &r.x); secp256k1_scalar_set_b32(sigr, b, &overflow); - if (secp256k1_scalar_is_zero(sigr)) { - /* P.x = order is on the curve, so technically sig->r could end up zero, which would be an invalid signature. - * This branch is cryptographically unreachable as hitting it requires finding the discrete log of P.x = N. - */ - secp256k1_gej_clear(&rp); - secp256k1_ge_clear(&r); - return 0; - } + /* These two conditions should be checked before calling */ + VERIFY_CHECK(!secp256k1_scalar_is_zero(sigr)); + VERIFY_CHECK(overflow == 0); + if (recid) { /* The overflow condition is cryptographically unreachable as hitting it requires finding the discrete log * of some P where P.x >= order, and only 1 in about 2^127 points meet this criteria.