diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 7059f2fc8..6b4d08a59 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -35,10 +35,10 @@ TEST(keystore_tests, StoreAndRetrieveHDSeed) { auto seed2 = HDSeed::Random(); EXPECT_NE(seed, seed2); - // We should be able to set and retrieve a different seed - ASSERT_TRUE(keyStore.SetHDSeed(seed2)); + // We should not be able to set and retrieve a different seed + EXPECT_FALSE(keyStore.SetHDSeed(seed2)); ASSERT_TRUE(keyStore.GetHDSeed(seedOut)); - EXPECT_EQ(seed2, seedOut); + EXPECT_EQ(seed, seedOut); } TEST(keystore_tests, sapling_keys) { @@ -276,12 +276,12 @@ TEST(keystore_tests, StoreAndRetrieveHDSeedInEncryptedStore) { ASSERT_TRUE(keyStore.GetHDSeed(seedOut)); EXPECT_EQ(seed, seedOut); - // 2) Test replacing the seed in an already-encrypted key store + // 2) Test replacing the seed in an already-encrypted key store fails auto seed2 = HDSeed::Random(); - ASSERT_TRUE(keyStore.SetHDSeed(seed2)); + EXPECT_FALSE(keyStore.SetHDSeed(seed2)); EXPECT_TRUE(keyStore.HaveHDSeed()); ASSERT_TRUE(keyStore.GetHDSeed(seedOut)); - EXPECT_EQ(seed2, seedOut); + EXPECT_EQ(seed, seedOut); // 3) Test adding a new seed to an already-encrypted key store TestCCryptoKeyStore keyStore2; diff --git a/src/keystore.cpp b/src/keystore.cpp index 4fa6c8081..8262c8aab 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -26,6 +26,11 @@ bool CKeyStore::AddKey(const CKey &key) { bool CBasicKeyStore::SetHDSeed(const HDSeed& seed) { LOCK(cs_SpendingKeyStore); + if (!hdSeed.IsNull()) { + // Don't allow an existing seed to be changed. We can maybe relax this + // restriction later once we have worked out the UX implications. + return false; + } hdSeed = seed; return true; } diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index fea563159..88807598a 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -219,7 +219,7 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) bool keyPass = false; bool keyFail = false; - { + if (!cryptedHDSeed.first.IsNull()) { HDSeed seed; if (!DecryptHDSeed(vMasterKeyIn, cryptedHDSeed.second, cryptedHDSeed.first, seed)) { @@ -322,6 +322,12 @@ bool CCryptoKeyStore::SetCryptedHDSeed( return false; } + if (!cryptedHDSeed.first.IsNull()) { + // Don't allow an existing seed to be changed. We can maybe relax this + // restriction later once we have worked out the UX implications. + return false; + } + cryptedHDSeed = std::make_pair(seedFp, vchCryptedSecret); } return true; @@ -550,20 +556,22 @@ 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; + if (!hdSeed.IsNull()) { + { + 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(); } - hdSeed = HDSeed(); BOOST_FOREACH(KeyMap::value_type& mKey, mapKeys) { const CKey &key = mKey.second;