From 79fd28cacbbcb86ea03d3d468845001f84a76de3 Mon Sep 17 00:00:00 2001 From: amadeuszpawlik Date: Thu, 9 Sep 2021 20:18:50 +0200 Subject: 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. --- src/key.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'src/key.cpp') diff --git a/src/key.cpp b/src/key.cpp index 40df248e02..eb94aa537c 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -229,6 +229,12 @@ bool CKey::Sign(const uint256 &hash, std::vector& vchSig, bool gr assert(ret); secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig); vchSig.resize(nSigLen); + // Additional verification step to prevent using a potentially corrupted signature + secp256k1_pubkey pk; + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pk, begin()); + assert(ret); + ret = secp256k1_ecdsa_verify(GetVerifyContext(), &sig, hash.begin(), &pk); + assert(ret); return true; } @@ -251,13 +257,21 @@ bool CKey::SignCompact(const uint256 &hash, std::vector& vchSig) return false; vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE); int rec = -1; - secp256k1_ecdsa_recoverable_signature sig; - int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); + secp256k1_ecdsa_recoverable_signature rsig; + int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &rsig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); assert(ret); - ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &sig); + ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &rsig); assert(ret); assert(rec != -1); vchSig[0] = 27 + rec + (fCompressed ? 4 : 0); + // Additional verification step to prevent using a potentially corrupted signature + secp256k1_pubkey epk, rpk; + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &epk, begin()); + assert(ret); + ret = secp256k1_ecdsa_recover(GetVerifyContext(), &rpk, &rsig, hash.begin()); + assert(ret); + ret = secp256k1_ec_pubkey_cmp(GetVerifyContext(), &epk, &rpk); + assert(ret == 0); return true; } @@ -275,6 +289,13 @@ bool CKey::SignSchnorr(const uint256& hash, Span sig, const uint2 if (!secp256k1_keypair_xonly_tweak_add(GetVerifyContext(), &keypair, tweak.data())) return false; } bool ret = secp256k1_schnorrsig_sign(secp256k1_context_sign, sig.data(), hash.data(), &keypair, aux ? (unsigned char*)aux->data() : nullptr); + if (ret) { + // Additional verification step to prevent using a potentially corrupted signature + secp256k1_xonly_pubkey pubkey_verify; + ret = secp256k1_keypair_xonly_pub(GetVerifyContext(), &pubkey_verify, nullptr, &keypair); + ret &= secp256k1_schnorrsig_verify(GetVerifyContext(), sig.data(), hash.begin(), 32, &pubkey_verify); + } + if (!ret) memory_cleanse(sig.data(), sig.size()); memory_cleanse(&keypair, sizeof(keypair)); return ret; } -- cgit v1.2.3