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 files changed, 24 insertions, 3 deletions

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<unsigned char>& 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<unsigned char>& 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<unsigned char> 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;
 }