diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -34,6 +34,8 @@ 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; }; @@ -164,6 +166,17 @@ } 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) { @@ -219,6 +232,9 @@ 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; @@ -313,8 +329,10 @@ 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, @@ -476,8 +494,6 @@ friend class CWallet; friend class ReserveDestination; LegacyScriptPubKeyMan(CWallet &wallet); - bool SetCrypted(); - bool IsCrypted() const; void NotifyWatchonlyChanged(bool fHaveWatchOnly) const; void NotifyCanGetAddressesChanged() const; template @@ -485,9 +501,6 @@ const Params &... parameters) const; CWallet &m_wallet; RecursiveMutex &cs_wallet; - CKeyingMaterial &vMasterKey GUARDED_BY(cs_KeyStore); - std::atomic &fUseCrypto; - bool &fDecryptionThoroughlyChecked; }; #endif // BITCOIN_WALLET_SCRIPTPUBKEYMAN_H diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -148,12 +148,11 @@ 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()); // Always pass when there are no encrypted keys bool keyPass = mapCryptedKeys.empty(); @@ -163,7 +162,7 @@ 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; } @@ -182,34 +181,40 @@ 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; + // Clear mapKeys so AddCryptedKeyInner will succeed. + keys_to_encrypt.swap(mapKeys); + 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(), + if (!EncryptSecret(master_key, vchSecret, vchPubKey.GetHash(), vchCryptedSecret)) { + encrypted_batch = nullptr; return false; } if (!AddCryptedKey(vchPubKey, vchCryptedSecret)) { + encrypted_batch = nullptr; return false; } } - mapKeys.clear(); + encrypted_batch = nullptr; return true; } @@ -506,7 +511,7 @@ RemoveWatchOnly(script); } - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return batch.WriteKey(pubkey, secret.GetPrivKey(), mapKeyMetadata[pubkey.GetID()]); } @@ -551,7 +556,7 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey &key, const CPubKey &pubkey) { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return FillableSigningProvider::AddKeyPubKey(key, pubkey); } @@ -561,8 +566,8 @@ 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; } @@ -580,9 +585,7 @@ 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); return true; @@ -699,7 +702,7 @@ 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; @@ -707,7 +710,7 @@ bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey &keyOut) const { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return FillableSigningProvider::GetKey(address, keyOut); } @@ -715,7 +718,8 @@ if (mi != mapCryptedKeys.end()) { 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; } @@ -755,7 +759,7 @@ 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); } @@ -1404,7 +1408,7 @@ std::set LegacyScriptPubKeyMan::GetKeys() const { LOCK(cs_KeyStore); - if (!IsCrypted()) { + if (!m_storage.HasEncryptionKeys()) { return FillableSigningProvider::GetKeys(); } std::set set_address; @@ -1416,16 +1420,8 @@ // Temporary CWallet accessors and aliases. LegacyScriptPubKeyMan::LegacyScriptPubKeyMan(CWallet &wallet) - : ScriptPubKeyMan(wallet), m_wallet(wallet), cs_wallet(wallet.cs_wallet), - vMasterKey(wallet.vMasterKey), fUseCrypto(wallet.fUseCrypto), - fDecryptionThoroughlyChecked(wallet.fDecryptionThoroughlyChecked) {} + : ScriptPubKeyMan(wallet), m_wallet(wallet), 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); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -643,14 +643,6 @@ 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); @@ -813,19 +805,16 @@ /** Construct wallet with specified name and database implementation. */ CWallet(const CChainParams &chainParamsIn, interfaces::Chain *chain, const WalletLocation &location, - std::unique_ptr databaseIn) - : fUseCrypto(false), fDecryptionThoroughlyChecked(false), - m_chain(chain), m_location(location), database(std::move(databaseIn)), + std::unique_ptr _database) + : m_chain(chain), m_location(location), database(std::move(_database)), chainParams(chainParamsIn) {} ~CWallet() { // 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(); @@ -1369,6 +1358,9 @@ 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 = @@ -1384,9 +1376,6 @@ 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) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -639,8 +639,7 @@ { 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; @@ -649,7 +648,7 @@ 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; @@ -4468,16 +4467,8 @@ return groups; } -bool CWallet::SetCrypted() { - LOCK(cs_KeyStore); - if (fUseCrypto) { - return true; - } - if (!mapKeys.empty()) { - return false; - } - fUseCrypto = true; - return true; +bool CWallet::IsCrypted() const { + return HasEncryptionKeys(); } bool CWallet::IsLocked() const { @@ -4489,7 +4480,7 @@ } bool CWallet::Lock() { - if (!SetCrypted()) { + if (!IsCrypted()) { return false; } @@ -4502,6 +4493,20 @@ 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(); } @@ -4520,3 +4525,11 @@ LegacyScriptPubKeyMan *CWallet::GetLegacyScriptPubKeyMan() const { return m_spk_man.get(); } + +const CKeyingMaterial &CWallet::GetEncryptionKey() const { + return vMasterKey; +} + +bool CWallet::HasEncryptionKeys() const { + return !mapMasterKeys.empty(); +}