Skip to content

Commit 1745e52

Browse files
real-or-randomdeadalnix
authored andcommitted
Retry if r is zero during signing
Summary: * Revert "ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign" This reverts commit 25e3cfbf9b52d2f5afa543f967a73aa8850d2038. The reverted commit was probably based on the assumption that this is about the touched checks cover the secret nonce k instead of r, which is the x-coord of the public nonce. A signature with a zero r is invalid by the spec, so we should return 0 to make the caller retry with a different nonce. Overflow is not an issue. Fixes #720. * Make ecdsa_sig_sign constant-time again after reverting 25e3cfb This is a backport of libsecp256k1 [[bitcoin-core/secp256k1#732 | PR732]] Test Plan: ninja check-secp256k1 Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D7586
1 parent 08fc810 commit 1745e52

File tree

1 file changed

+4
-5
lines changed

1 file changed

+4
-5
lines changed

src/secp256k1/src/ecdsa_impl.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,6 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec
288288
secp256k1_fe_normalize(&r.y);
289289
secp256k1_fe_get_b32(b, &r.x);
290290
secp256k1_scalar_set_b32(sigr, b, &overflow);
291-
/* These two conditions should be checked before calling */
292-
VERIFY_CHECK(!secp256k1_scalar_is_zero(sigr));
293-
VERIFY_CHECK(overflow == 0);
294-
295291
if (recid) {
296292
/* The overflow condition is cryptographically unreachable as hitting it requires finding the discrete log
297293
* of some P where P.x >= order, and only 1 in about 2^127 points meet this criteria.
@@ -310,7 +306,10 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec
310306
if (recid) {
311307
*recid ^= high;
312308
}
313-
return !secp256k1_scalar_is_zero(sigs);
309+
/* P.x = order is on the curve, so technically sig->r could end up being zero, which would be an invalid signature.
310+
* This is cryptographically unreachable as hitting it requires finding the discrete log of P.x = N.
311+
*/
312+
return !secp256k1_scalar_is_zero(sigr) & !secp256k1_scalar_is_zero(sigs);
314313
}
315314

316315
#endif /* SECP256K1_ECDSA_IMPL_H */

0 commit comments

Comments
 (0)