Skip to content

Commit 79fd28c

Browse files
Adds verification step to Schnorr and ECDSA signing
As defined in BIP340, a verification step should be executed after `secp256k1_schnorrsig_sign` to ensure that a potentially corrupted signature isn't used; using corrupted signatures could reveal information about the private key used. This applies to ECSDA as well. Additionally clears schnorr signature if signing failed.
1 parent 42fedb4 commit 79fd28c

File tree

1 file changed

+24
-3
lines changed

1 file changed

+24
-3
lines changed

src/key.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,12 @@ bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, bool gr
229229
assert(ret);
230230
secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig);
231231
vchSig.resize(nSigLen);
232+
// Additional verification step to prevent using a potentially corrupted signature
233+
secp256k1_pubkey pk;
234+
ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pk, begin());
235+
assert(ret);
236+
ret = secp256k1_ecdsa_verify(GetVerifyContext(), &sig, hash.begin(), &pk);
237+
assert(ret);
232238
return true;
233239
}
234240

@@ -251,13 +257,21 @@ bool CKey::SignCompact(const uint256 &hash, std::vector<unsigned char>& vchSig)
251257
return false;
252258
vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE);
253259
int rec = -1;
254-
secp256k1_ecdsa_recoverable_signature sig;
255-
int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr);
260+
secp256k1_ecdsa_recoverable_signature rsig;
261+
int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &rsig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr);
256262
assert(ret);
257-
ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &sig);
263+
ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &rsig);
258264
assert(ret);
259265
assert(rec != -1);
260266
vchSig[0] = 27 + rec + (fCompressed ? 4 : 0);
267+
// Additional verification step to prevent using a potentially corrupted signature
268+
secp256k1_pubkey epk, rpk;
269+
ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &epk, begin());
270+
assert(ret);
271+
ret = secp256k1_ecdsa_recover(GetVerifyContext(), &rpk, &rsig, hash.begin());
272+
assert(ret);
273+
ret = secp256k1_ec_pubkey_cmp(GetVerifyContext(), &epk, &rpk);
274+
assert(ret == 0);
261275
return true;
262276
}
263277

@@ -275,6 +289,13 @@ bool CKey::SignSchnorr(const uint256& hash, Span<unsigned char> sig, const uint2
275289
if (!secp256k1_keypair_xonly_tweak_add(GetVerifyContext(), &keypair, tweak.data())) return false;
276290
}
277291
bool ret = secp256k1_schnorrsig_sign(secp256k1_context_sign, sig.data(), hash.data(), &keypair, aux ? (unsigned char*)aux->data() : nullptr);
292+
if (ret) {
293+
// Additional verification step to prevent using a potentially corrupted signature
294+
secp256k1_xonly_pubkey pubkey_verify;
295+
ret = secp256k1_keypair_xonly_pub(GetVerifyContext(), &pubkey_verify, nullptr, &keypair);
296+
ret &= secp256k1_schnorrsig_verify(GetVerifyContext(), sig.data(), hash.begin(), 32, &pubkey_verify);
297+
}
298+
if (!ret) memory_cleanse(sig.data(), sig.size());
278299
memory_cleanse(&keypair, sizeof(keypair));
279300
return ret;
280301
}

0 commit comments

Comments
 (0)