diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 2b9e8fbf2..b4315014c 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -202,12 +202,11 @@ isminetype LegacyScriptPubKeyMan::IsMine(const CScript& script) const assert(false); } -bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys) +bool LegacyScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys) { { LOCK(cs_KeyStore); - if (!SetCrypted()) - return false; + assert(mapKeys.empty()); bool keyPass = mapCryptedKeys.empty(); // Always pass when there are no encrypted keys bool keyFail = false; @@ -217,7 +216,7 @@ bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys) const CPubKey &vchPubKey = (*mi).second.first; const std::vector &vchCryptedSecret = (*mi).second.second; CKey key; - if (!DecryptKey(vMasterKeyIn, vchCryptedSecret, vchPubKey, key)) + if (!DecryptKey(master_key, vchCryptedSecret, vchPubKey, key)) { keyFail = true; break; @@ -233,32 +232,39 @@ bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys) } if (keyFail || (!keyPass && !accept_no_keys)) return false; - vMasterKey = vMasterKeyIn; fDecryptionThoroughlyChecked = true; } - NotifyStatusChanged(this); return true; } -bool LegacyScriptPubKeyMan::EncryptKeys(CKeyingMaterial& vMasterKeyIn) +bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { + AssertLockHeld(cs_wallet); LOCK(cs_KeyStore); - if (!mapCryptedKeys.empty() || IsCrypted()) + encrypted_batch = batch; + if (!mapCryptedKeys.empty()) { + encrypted_batch = nullptr; return false; + } - fUseCrypto = true; - for (const KeyMap::value_type& mKey : mapKeys) + KeyMap keys_to_encrypt; + keys_to_encrypt.swap(mapKeys); // Clear mapKeys so AddCryptedKeyInner will succeed. + for (const KeyMap::value_type& mKey : keys_to_encrypt) { const CKey &key = mKey.second; CPubKey vchPubKey = key.GetPubKey(); CKeyingMaterial vchSecret(key.begin(), key.end()); std::vector vchCryptedSecret; - if (!EncryptSecret(vMasterKeyIn, vchSecret, vchPubKey.GetHash(), vchCryptedSecret)) + if (!EncryptSecret(master_key, vchSecret, vchPubKey.GetHash(), vchCryptedSecret)) { + encrypted_batch = nullptr; return false; - if (!AddCryptedKey(vchPubKey, vchCryptedSecret)) + } + if (!AddCryptedKey(vchPubKey, vchCryptedSecret)) { + encrypted_batch = nullptr; return false; + } } - mapKeys.clear(); + encrypted_batch = nullptr; return true; } @@ -543,7 +549,7 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyWithDB(WalletBatch& batch, const CKey& s RemoveWatchOnly(script); } - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return batch.WriteKey(pubkey, secret.GetPrivKey(), mapKeyMetadata[pubkey.GetID()]); @@ -584,7 +590,7 @@ void LegacyScriptPubKeyMan::LoadScriptMetadata(const CScriptID& script_id, const bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pubkey) { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return FillableSigningProvider::AddKeyPubKey(key, pubkey); } @@ -594,7 +600,7 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu std::vector vchCryptedSecret; CKeyingMaterial vchSecret(key.begin(), key.end()); - if (!EncryptSecret(vMasterKey, vchSecret, pubkey.GetHash(), vchCryptedSecret)) { + if (!EncryptSecret(m_storage.GetEncryptionKey(), vchSecret, pubkey.GetHash(), vchCryptedSecret)) { return false; } @@ -612,9 +618,7 @@ bool LegacyScriptPubKeyMan::LoadCryptedKey(const CPubKey &vchPubKey, const std:: bool LegacyScriptPubKeyMan::AddCryptedKeyInner(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret) { LOCK(cs_KeyStore); - if (!SetCrypted()) { - return false; - } + assert(mapKeys.empty()); mapCryptedKeys[vchPubKey.GetID()] = make_pair(vchPubKey, vchCryptedSecret); ImplicitlyLearnRelatedKeyScripts(vchPubKey); @@ -741,7 +745,7 @@ void LegacyScriptPubKeyMan::SetHDChain(const CHDChain& chain, bool memonly) bool LegacyScriptPubKeyMan::HaveKey(const CKeyID &address) const { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return FillableSigningProvider::HaveKey(address); } return mapCryptedKeys.count(address) > 0; @@ -750,7 +754,7 @@ bool LegacyScriptPubKeyMan::HaveKey(const CKeyID &address) const bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey& keyOut) const { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return FillableSigningProvider::GetKey(address, keyOut); } @@ -759,7 +763,7 @@ bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey& keyOut) const { const CPubKey &vchPubKey = (*mi).second.first; const std::vector &vchCryptedSecret = (*mi).second.second; - return DecryptKey(vMasterKey, vchCryptedSecret, vchPubKey, keyOut); + return DecryptKey(m_storage.GetEncryptionKey(), vchCryptedSecret, vchPubKey, keyOut); } return false; } @@ -797,7 +801,7 @@ bool LegacyScriptPubKeyMan::GetWatchPubKey(const CKeyID &address, CPubKey &pubke bool LegacyScriptPubKeyMan::GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { if (!FillableSigningProvider::GetPubKey(address, vchPubKeyOut)) { return GetWatchPubKey(address, vchPubKeyOut); } @@ -1383,7 +1387,7 @@ bool LegacyScriptPubKeyMan::ImportScriptPubKeys(const std::set& script_ std::set LegacyScriptPubKeyMan::GetKeys() const { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return FillableSigningProvider::GetKeys(); } std::set set_address; @@ -1397,13 +1401,8 @@ std::set LegacyScriptPubKeyMan::GetKeys() const LegacyScriptPubKeyMan::LegacyScriptPubKeyMan(CWallet& wallet) : ScriptPubKeyMan(wallet), m_wallet(wallet), - cs_wallet(wallet.cs_wallet), - vMasterKey(wallet.vMasterKey), - fUseCrypto(wallet.fUseCrypto), - fDecryptionThoroughlyChecked(wallet.fDecryptionThoroughlyChecked) {} + cs_wallet(wallet.cs_wallet) {} -bool LegacyScriptPubKeyMan::SetCrypted() { return m_wallet.SetCrypted(); } -bool LegacyScriptPubKeyMan::IsCrypted() const { return m_wallet.IsCrypted(); } void LegacyScriptPubKeyMan::NotifyWatchonlyChanged(bool fHaveWatchOnly) const { return m_wallet.NotifyWatchonlyChanged(fHaveWatchOnly); } void LegacyScriptPubKeyMan::NotifyCanGetAddressesChanged() const { return m_wallet.NotifyCanGetAddressesChanged(); } template void LegacyScriptPubKeyMan::WalletLogPrintf(const std::string& fmt, const Params&... parameters) const { return m_wallet.WalletLogPrintf(fmt, parameters...); } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 6ed9a4787..aa5eac3a8 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -31,6 +31,8 @@ public: virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; virtual bool CanSupportFeature(enum WalletFeature) const = 0; virtual void SetMinVersion(enum WalletFeature, WalletBatch* = nullptr, bool = false) = 0; + virtual const CKeyingMaterial& GetEncryptionKey() const = 0; + virtual bool HasEncryptionKeys() const = 0; virtual bool IsLocked() const = 0; }; @@ -150,6 +152,10 @@ public: virtual bool GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) { return false; } virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; } + //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it. + virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; } + virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; } + virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) { return false; } virtual void KeepDestination(int64_t index, const OutputType& type) {} virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {} @@ -193,6 +199,9 @@ public: class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProvider { private: + //! keeps track of whether Unlock has run a thorough check before + bool fDecryptionThoroughlyChecked = false; + using WatchOnlySet = std::set; using WatchKeyMap = std::map; @@ -272,8 +281,8 @@ public: bool GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) override; isminetype IsMine(const CScript& script) const override; - //! will encrypt previously unencrypted keys - bool EncryptKeys(CKeyingMaterial& vMasterKeyIn); + bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; + bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override; void KeepDestination(int64_t index, const OutputType& type) override; @@ -403,16 +412,11 @@ public: friend class CWallet; friend class ReserveDestination; LegacyScriptPubKeyMan(CWallet& wallet); - bool SetCrypted(); - bool IsCrypted() const; void NotifyWatchonlyChanged(bool fHaveWatchOnly) const; void NotifyCanGetAddressesChanged() const; template void WalletLogPrintf(const std::string& fmt, const Params&... parameters) const; CWallet& m_wallet; CCriticalSection& cs_wallet; - CKeyingMaterial& vMasterKey GUARDED_BY(cs_KeyStore); - std::atomic& fUseCrypto; - bool& fDecryptionThoroughlyChecked; }; #endif // BITCOIN_WALLET_SCRIPTPUBKEYMAN_H diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index abee497c1..41a816312 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -532,8 +532,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) { LOCK(cs_wallet); mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; - assert(!encrypted_batch); - encrypted_batch = new WalletBatch(*database); + WalletBatch* encrypted_batch = new WalletBatch(*database); if (!encrypted_batch->TxnBegin()) { delete encrypted_batch; encrypted_batch = nullptr; @@ -542,7 +541,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) encrypted_batch->WriteMasterKey(nMasterKeyMaxID, kMasterKey); if (auto spk_man = m_spk_man.get()) { - if (!spk_man->EncryptKeys(_vMasterKey)) { + if (!spk_man->Encrypt(_vMasterKey, encrypted_batch)) { encrypted_batch->TxnAbort(); delete encrypted_batch; encrypted_batch = nullptr; @@ -4003,15 +4002,9 @@ std::vector CWallet::GroupOutputs(const std::vector& outpu return groups; } -bool CWallet::SetCrypted() +bool CWallet::IsCrypted() const { - LOCK(cs_KeyStore); - if (fUseCrypto) - return true; - if (!mapKeys.empty()) - return false; - fUseCrypto = true; - return true; + return HasEncryptionKeys(); } bool CWallet::IsLocked() const @@ -4025,7 +4018,7 @@ bool CWallet::IsLocked() const bool CWallet::Lock() { - if (!SetCrypted()) + if (!IsCrypted()) return false; { @@ -4037,6 +4030,21 @@ bool CWallet::Lock() return true; } +bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys) +{ + { + LOCK(cs_KeyStore); + if (m_spk_man) { + if (!m_spk_man->CheckDecryptionKey(vMasterKeyIn, accept_no_keys)) { + return false; + } + } + vMasterKey = vMasterKeyIn; + } + NotifyStatusChanged(this); + return true; +} + ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const CScript& script) const { return m_spk_man.get(); @@ -4056,3 +4064,13 @@ LegacyScriptPubKeyMan* CWallet::GetLegacyScriptPubKeyMan() const { return m_spk_man.get(); } + +const CKeyingMaterial& CWallet::GetEncryptionKey() const +{ + return vMasterKey; +} + +bool CWallet::HasEncryptionKeys() const +{ + return !mapMasterKeys.empty(); +} diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b02e092f0..c4511601d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -597,14 +597,7 @@ class CWallet final : public WalletStorage, private interfaces::Chain::Notificat private: CKeyingMaterial vMasterKey GUARDED_BY(cs_KeyStore); - //! if fUseCrypto is true, mapKeys must be empty - //! if fUseCrypto is false, vMasterKey must be empty - std::atomic fUseCrypto; - //! keeps track of whether Unlock has run a thorough check before - bool fDecryptionThoroughlyChecked; - - bool SetCrypted(); bool Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys = false); std::atomic fAbortRescan{false}; @@ -734,9 +727,7 @@ public: /** Construct wallet with specified name and database implementation. */ CWallet(interfaces::Chain* chain, const WalletLocation& location, std::unique_ptr database) - : fUseCrypto(false), - fDecryptionThoroughlyChecked(false), - m_chain(chain), + : m_chain(chain), m_location(location), database(std::move(database)) { @@ -746,11 +737,9 @@ public: { // Should not have slots connected at this point. assert(NotifyUnload.empty()); - delete encrypted_batch; - encrypted_batch = nullptr; } - bool IsCrypted() const { return fUseCrypto; } + bool IsCrypted() const; bool IsLocked() const override; bool Lock(); @@ -1136,6 +1125,9 @@ public: LegacyScriptPubKeyMan* GetLegacyScriptPubKeyMan() const; + const CKeyingMaterial& GetEncryptionKey() const override; + bool HasEncryptionKeys() const override; + // Temporary LegacyScriptPubKeyMan accessors and aliases. friend class LegacyScriptPubKeyMan; std::unique_ptr m_spk_man = MakeUnique(*this); @@ -1145,8 +1137,6 @@ public: LegacyScriptPubKeyMan::CryptedKeyMap& mapCryptedKeys GUARDED_BY(cs_KeyStore) = m_spk_man->mapCryptedKeys; LegacyScriptPubKeyMan::WatchOnlySet& setWatchOnly GUARDED_BY(cs_KeyStore) = m_spk_man->setWatchOnly; LegacyScriptPubKeyMan::WatchKeyMap& mapWatchKeys GUARDED_BY(cs_KeyStore) = m_spk_man->mapWatchKeys; - WalletBatch*& encrypted_batch GUARDED_BY(cs_wallet) = m_spk_man->encrypted_batch; - using CryptedKeyMap = LegacyScriptPubKeyMan::CryptedKeyMap; /** Get last block processed height */ int GetLastBlockHeight() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet)