From ec653523ad99ab4bcf7a3eeebf00350c885609e3 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 4 Aug 2018 00:49:11 +0100 Subject: [PATCH] Store HDSeed in CCryptoKeyStore --- src/gtest/test_keystore.cpp | 60 +++++++++++++++++++++ src/wallet/crypter.cpp | 101 ++++++++++++++++++++++++++++++++++++ src/wallet/crypter.h | 7 +++ 3 files changed, 168 insertions(+) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 4f2d0ea44..33d5f0411 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -240,6 +240,66 @@ public: bool Unlock(const CKeyingMaterial& vMasterKeyIn) { return CCryptoKeyStore::Unlock(vMasterKeyIn); } }; +TEST(keystore_tests, StoreAndRetrieveHDSeedInEncryptedStore) { + TestCCryptoKeyStore keyStore; + CKeyingMaterial vMasterKey(32, 0); + GetRandBytes(vMasterKey.data(), 32); + HDSeed seedOut; + + // 1) Test adding a seed to an unencrypted key store, then encrypting it + auto seed = HDSeed::Random(); + EXPECT_FALSE(keyStore.HaveHDSeed()); + EXPECT_FALSE(keyStore.GetHDSeed(seedOut)); + + ASSERT_TRUE(keyStore.SetHDSeed(seed)); + EXPECT_TRUE(keyStore.HaveHDSeed()); + ASSERT_TRUE(keyStore.GetHDSeed(seedOut)); + EXPECT_EQ(seed, seedOut); + + ASSERT_TRUE(keyStore.EncryptKeys(vMasterKey)); + EXPECT_FALSE(keyStore.GetHDSeed(seedOut)); + + // Unlocking with a random key should fail + CKeyingMaterial vRandomKey(32, 0); + GetRandBytes(vRandomKey.data(), 32); + EXPECT_FALSE(keyStore.Unlock(vRandomKey)); + + // Unlocking with a slightly-modified vMasterKey should fail + CKeyingMaterial vModifiedKey(vMasterKey); + vModifiedKey[0] += 1; + EXPECT_FALSE(keyStore.Unlock(vModifiedKey)); + + // Unlocking with vMasterKey should succeed + ASSERT_TRUE(keyStore.Unlock(vMasterKey)); + ASSERT_TRUE(keyStore.GetHDSeed(seedOut)); + EXPECT_EQ(seed, seedOut); + + // 2) Test replacing the seed in an already-encrypted key store + auto seed2 = HDSeed::Random(); + ASSERT_TRUE(keyStore.SetHDSeed(seed2)); + EXPECT_TRUE(keyStore.HaveHDSeed()); + ASSERT_TRUE(keyStore.GetHDSeed(seedOut)); + EXPECT_EQ(seed2, seedOut); + + // 3) Test adding a new seed to an already-encrypted key store + TestCCryptoKeyStore keyStore2; + + // Add a Sprout address so the wallet has something to test when decrypting + ASSERT_TRUE(keyStore2.AddSproutSpendingKey(libzcash::SproutSpendingKey::random())); + + ASSERT_TRUE(keyStore2.EncryptKeys(vMasterKey)); + ASSERT_TRUE(keyStore2.Unlock(vMasterKey)); + + EXPECT_FALSE(keyStore2.HaveHDSeed()); + EXPECT_FALSE(keyStore2.GetHDSeed(seedOut)); + + auto seed3 = HDSeed::Random(); + ASSERT_TRUE(keyStore2.SetHDSeed(seed3)); + EXPECT_TRUE(keyStore2.HaveHDSeed()); + ASSERT_TRUE(keyStore2.GetHDSeed(seedOut)); + EXPECT_EQ(seed3, seedOut); +} + TEST(keystore_tests, store_and_retrieve_spending_key_in_encrypted_store) { TestCCryptoKeyStore keyStore; uint256 r {GetRandHash()}; diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index da8529ba6..798fc14db 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -121,6 +121,23 @@ static bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector& vchCryptedSecret, + const uint256& seedFp, + HDSeed& seed) +{ + CKeyingMaterial vchSecret; + + // Use seed's fingerprint as IV + // TODO: Handle IV properly when we make encryption a supported feature + if(!DecryptSecret(vMasterKey, vchCryptedSecret, seedFp, vchSecret)) + return false; + + seed = HDSeed(vchSecret); + return seed.Fingerprint() == seedFp; +} + static bool DecryptKey(const CKeyingMaterial& vMasterKey, const std::vector& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key) { CKeyingMaterial vchSecret; @@ -202,6 +219,15 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) bool keyPass = false; bool keyFail = false; + { + HDSeed seed; + if (!DecryptHDSeed(vMasterKeyIn, cryptedHDSeed.second, cryptedHDSeed.first, seed)) + { + keyFail = true; + } else { + keyPass = true; + } + } CryptedKeyMap::const_iterator mi = mapCryptedKeys.begin(); for (; mi != mapCryptedKeys.end(); ++mi) { @@ -261,6 +287,67 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) return true; } +bool CCryptoKeyStore::SetHDSeed(const HDSeed& seed) +{ + { + LOCK(cs_SpendingKeyStore); + if (!IsCrypted()) { + return CBasicKeyStore::SetHDSeed(seed); + } + + if (IsLocked()) + return false; + + std::vector vchCryptedSecret; + // Use seed's fingerprint as IV + // TODO: Handle this properly when we make encryption a supported feature + auto seedFp = seed.Fingerprint(); + if (!EncryptSecret(vMasterKey, seed.RawSeed(), seedFp, vchCryptedSecret)) + return false; + + // This will call into CWallet to store the crypted seed to disk + if (!SetCryptedHDSeed(seedFp, vchCryptedSecret)) + return false; + } + return true; +} + +bool CCryptoKeyStore::SetCryptedHDSeed( + const uint256& seedFp, + const std::vector& vchCryptedSecret) +{ + { + LOCK(cs_SpendingKeyStore); + if (!IsCrypted()) { + return false; + } + + cryptedHDSeed = std::make_pair(seedFp, vchCryptedSecret); + } + return true; +} + +bool CCryptoKeyStore::HaveHDSeed() const +{ + LOCK(cs_SpendingKeyStore); + if (!IsCrypted()) + return CBasicKeyStore::HaveHDSeed(); + + return !cryptedHDSeed.second.empty(); +} + +bool CCryptoKeyStore::GetHDSeed(HDSeed& seedOut) const +{ + LOCK(cs_SpendingKeyStore); + if (!IsCrypted()) + return CBasicKeyStore::GetHDSeed(seedOut); + + if (cryptedHDSeed.second.empty()) + return false; + + return DecryptHDSeed(vMasterKey, cryptedHDSeed.second, cryptedHDSeed.first, seedOut); +} + bool CCryptoKeyStore::AddKeyPubKey(const CKey& key, const CPubKey &pubkey) { { @@ -463,6 +550,20 @@ bool CCryptoKeyStore::EncryptKeys(CKeyingMaterial& vMasterKeyIn) return false; fUseCrypto = true; + { + std::vector vchCryptedSecret; + // Use seed's fingerprint as IV + // TODO: Handle this properly when we make encryption a supported feature + auto seedFp = hdSeed.Fingerprint(); + if (!EncryptSecret(vMasterKeyIn, hdSeed.RawSeed(), seedFp, vchCryptedSecret)) { + return false; + } + // This will call into CWallet to store the crypted seed to disk + if (!SetCryptedHDSeed(seedFp, vchCryptedSecret)) { + return false; + } + } + hdSeed = HDSeed(); BOOST_FOREACH(KeyMap::value_type& mKey, mapKeys) { const CKey &key = mKey.second; diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 169237cc9..11231ead2 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -10,6 +10,7 @@ #include "streams.h" #include "support/allocators/secure.h" #include "zcash/Address.hpp" +#include "zcash/zip32.h" class uint256; @@ -127,6 +128,7 @@ public: class CCryptoKeyStore : public CBasicKeyStore { private: + std::pair> cryptedHDSeed; CryptedKeyMap mapCryptedKeys; CryptedSproutSpendingKeyMap mapCryptedSproutSpendingKeys; CryptedSaplingSpendingKeyMap mapCryptedSaplingSpendingKeys; @@ -172,6 +174,11 @@ public: bool Lock(); + virtual bool SetCryptedHDSeed(const uint256& seedFp, const std::vector &vchCryptedSecret); + bool SetHDSeed(const HDSeed& seed); + bool HaveHDSeed() const; + bool GetHDSeed(HDSeed& seedOut) const; + virtual bool AddCryptedKey(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret); bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey); bool HaveKey(const CKeyID &address) const