Skip to content

Commit

Permalink
Merge bitcoin#732: Retry if r is zero during signing
Browse files Browse the repository at this point in the history
37ed51a Make ecdsa_sig_sign constant-time again after reverting 25e3cfb (Tim Ruffing)
93d343b Revert "ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign" (Tim Ruffing)

Pull request description:

ACKs for top commit:
  elichai:
    ACK 37ed51a makes sense.
  jonasnick:
    ACK 37ed51a

Tree-SHA512: 82b5b8e29f48e84fd7a0681b62923d3bd87d724b38ef18e8c7969b0dcc5a405ebb26c14b5c5f4c7ba0ccabd152d1531d217809d1daf40872fe0c1e079b55c64b
  • Loading branch information
jonasnick committed Apr 18, 2020
2 parents 59a8de8 + 37ed51a commit 39198a0
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions src/ecdsa_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,6 @@ 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);
/* 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.
Expand All @@ -310,7 +306,10 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec
if (recid) {
*recid ^= high;
}
return !secp256k1_scalar_is_zero(sigs);
/* P.x = order is on the curve, so technically sig->r could end up being zero, which would be an invalid signature.
* This is cryptographically unreachable as hitting it requires finding the discrete log of P.x = N.
*/
return !secp256k1_scalar_is_zero(sigr) & !secp256k1_scalar_is_zero(sigs);
}

#endif /* SECP256K1_ECDSA_IMPL_H */

0 comments on commit 39198a0

Please sign in to comment.