From d0c41a73501a0bf94fca91be5fb38ab039490843 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 6 Nov 2014 01:17:48 -0800 Subject: [PATCH 1/2] Add sanity check after key generation Add a sanity check to prevent cosmic rays from flipping a bit in the generated public key, or bugs in the elliptic curve code. This is simply done by signing a (randomized) message, and verifying the result. --- src/key.cpp | 19 +++++++++++++++---- src/key.h | 6 ++++++ src/rpcdump.cpp | 2 ++ src/test/key_tests.cpp | 20 ++++++++++++++++++++ src/wallet.cpp | 1 + 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/key.cpp b/src/key.cpp index 76256b864..826af7f44 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -86,6 +86,20 @@ bool CKey::Sign(const uint256 &hash, std::vector& vchSig) const { return true; } +bool CKey::VerifyPubKey(const CPubKey& pubkey) const { + if (pubkey.IsCompressed() != fCompressed) { + return false; + } + unsigned char rnd[8]; + std::string str = "Bitcoin key verification\n"; + GetRandBytes(rnd, sizeof(rnd)); + uint256 hash; + CHash256().Write((unsigned char*)str.data(), str.size()).Write(rnd, sizeof(rnd)).Finalize((unsigned char*)&hash); + std::vector vchSig; + Sign(hash, vchSig); + return pubkey.Verify(hash, vchSig); +} + bool CKey::SignCompact(const uint256 &hash, std::vector& vchSig) const { if (!fValid) return false; @@ -111,10 +125,7 @@ bool CKey::Load(CPrivKey &privkey, CPubKey &vchPubKey, bool fSkipCheck=false) { if (fSkipCheck) return true; - if (GetPubKey() != vchPubKey) - return false; - - return true; + return VerifyPubKey(vchPubKey); } bool CKey::Derive(CKey& keyChild, unsigned char ccChild[32], unsigned int nChild, const unsigned char cc[32]) const { diff --git a/src/key.h b/src/key.h index 0bb05482c..d8c82b6f0 100644 --- a/src/key.h +++ b/src/key.h @@ -136,6 +136,12 @@ public: //! Derive BIP32 child key. bool Derive(CKey& keyChild, unsigned char ccChild[32], unsigned int nChild, const unsigned char cc[32]) const; + /** + * Verify thoroughly whether a private key and a public key match. + * This is done using a different mechanism than just regenerating it. + */ + bool VerifyPubKey(const CPubKey& vchPubKey) const; + //! Load private key and check that public key matches. bool Load(CPrivKey& privkey, CPubKey& vchPubKey, bool fSkipCheck); diff --git a/src/rpcdump.cpp b/src/rpcdump.cpp index c3ffe38cc..8b95373cf 100644 --- a/src/rpcdump.cpp +++ b/src/rpcdump.cpp @@ -112,6 +112,7 @@ Value importprivkey(const Array& params, bool fHelp) if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); CPubKey pubkey = key.GetPubKey(); + assert(key.VerifyPubKey(pubkey)); CKeyID vchAddress = pubkey.GetID(); { pwalletMain->MarkDirty(); @@ -253,6 +254,7 @@ Value importwallet(const Array& params, bool fHelp) continue; CKey key = vchSecret.GetKey(); CPubKey pubkey = key.GetPubKey(); + assert(key.VerifyPubKey(pubkey)); CKeyID keyid = pubkey.GetID(); if (pwalletMain->HaveKey(keyid)) { LogPrintf("Skipping import of %s (key already present)\n", CBitcoinAddress(keyid).ToString()); diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index b32f3774f..f9e35e016 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -82,6 +82,26 @@ BOOST_AUTO_TEST_CASE(key_test1) CPubKey pubkey1C = key1C.GetPubKey(); CPubKey pubkey2C = key2C.GetPubKey(); + BOOST_CHECK(key1.VerifyPubKey(pubkey1)); + BOOST_CHECK(!key1.VerifyPubKey(pubkey1C)); + BOOST_CHECK(!key1.VerifyPubKey(pubkey2)); + BOOST_CHECK(!key1.VerifyPubKey(pubkey2C)); + + BOOST_CHECK(!key1C.VerifyPubKey(pubkey1)); + BOOST_CHECK(key1C.VerifyPubKey(pubkey1C)); + BOOST_CHECK(!key1C.VerifyPubKey(pubkey2)); + BOOST_CHECK(!key1C.VerifyPubKey(pubkey2C)); + + BOOST_CHECK(!key2.VerifyPubKey(pubkey1)); + BOOST_CHECK(!key2.VerifyPubKey(pubkey1C)); + BOOST_CHECK(key2.VerifyPubKey(pubkey2)); + BOOST_CHECK(!key2.VerifyPubKey(pubkey2C)); + + BOOST_CHECK(!key2C.VerifyPubKey(pubkey1)); + BOOST_CHECK(!key2C.VerifyPubKey(pubkey1C)); + BOOST_CHECK(!key2C.VerifyPubKey(pubkey2)); + BOOST_CHECK(key2C.VerifyPubKey(pubkey2C)); + BOOST_CHECK(addr1.Get() == CTxDestination(pubkey1.GetID())); BOOST_CHECK(addr2.Get() == CTxDestination(pubkey2.GetID())); BOOST_CHECK(addr1C.Get() == CTxDestination(pubkey1C.GetID())); diff --git a/src/wallet.cpp b/src/wallet.cpp index 5aea9881c..353010ae0 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -79,6 +79,7 @@ CPubKey CWallet::GenerateNewKey() SetMinVersion(FEATURE_COMPRPUBKEY); CPubKey pubkey = secret.GetPubKey(); + assert(secret.VerifyPubKey(pubkey)); // Create new metadata int64_t nCreationTime = GetTime(); From f321d6bfff4dbbb4c52d0f175a27d54b287e81ff Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 8 Nov 2014 14:29:45 -0800 Subject: [PATCH 2/2] Add key generation/verification to ECC sanity check --- src/key.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/key.cpp b/src/key.cpp index 826af7f44..a91ed1cc1 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -201,5 +201,13 @@ void CExtKey::Decode(const unsigned char code[74]) { } bool ECC_InitSanityCheck() { - return CECKey::SanityCheck(); +#if !defined(USE_SECP256K1) + if (!CECKey::SanityCheck()) { + return false; + } +#endif + CKey key; + key.MakeNewKey(true); + CPubKey pubkey = key.GetPubKey(); + return key.VerifyPubKey(pubkey); }