maxmustermann/dogecoin
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge #17369: Refactor: Move encryption code between KeyMan and Wallet

Browse source 
7cecf10ac3 Replace LegacyScriptPubKeyMan::IsCrypted with LegacyScriptPubKeyMan::HasEncryptionKeys (Andrew Chow)
bf6417142f Remove SetCrypted() and fUseCrypto; Change IsCrypted()'s implementation (Andrew Chow)
77a777118e Rename EncryptKeys to Encrypt and pass in the encrypted batch to use (Andrew Chow)
35f962fcf0 Clear mapKeys before encrypting (Andrew Chow)
14b5efd66f Move fDecryptionThoroughlyChecked from CWallet to LegacyScriptPubKeyMan (Andrew Chow)
97c0374a46 Move Unlock implementation to LegacyScriptPubKeyMan (Andrew Chow)
e576b135d6 Replace LegacyScriptPubKeyMan::vMasterKey with GetDecryptionKey() (Andrew Chow)
fd9d6eebc1 Add GetEncryptionKey() and HasEncryptionKeys() to WalletStorage (Andrew Chow)

Pull request description:

  Let wallet class handle locked/unlocked status and master key, and let keyman
  handle encrypting its data and determining whether there is encrypted data.

  There should be no change in behavior, but state is tracked differently. The
  fUseCrypto atomic bool is eliminated and replaced with equivalent
  HasEncryptionKeys checks.

  Split from #17261

ACKs for top commit:
  laanwj:
    ACK 7cecf10ac3

Tree-SHA512: 95a997c366ca539abba0c0a7a0015f39d27b55220683d8d86344ff2d926db4724da67700d2c8ec2d82ed75d07404318c6cb81544af8aadeefab312167257e673
This commit is contained in:
Wladimir J. van der Laan 2019-12-12 12:16:47 +01:00
parent 3914e877c4 7cecf10ac3
commit 0192bd0652
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
4 changed files with 75 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

59
src/wallet/scriptpubkeyman.cpp
View file

@ -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<unsigned char> &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<unsigned char> 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<unsigned char> 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<unsigned char> &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<unsigned char> &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<CScript>& script_
std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const
{
LOCK(cs_KeyStore);
if (!IsCrypted()) {
if (!m_storage.HasEncryptionKeys()) {
return FillableSigningProvider::GetKeys();
}
std::set<CKeyID> set_address;
@ -1397,13 +1401,8 @@ std::set<CKeyID> 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<typename... Params> void LegacyScriptPubKeyMan::WalletLogPrintf(const std::string& fmt, const Params&... parameters) const { return m_wallet.WalletLogPrintf(fmt, parameters...); }

18
src/wallet/scriptpubkeyman.h
View file

@ -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<CScript>;
using WatchKeyMap = std::map<CKeyID, CPubKey>;
@ -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<typename... Params> void WalletLogPrintf(const std::string& fmt, const Params&... parameters) const;
CWallet& m_wallet;
CCriticalSection& cs_wallet;
CKeyingMaterial& vMasterKey GUARDED_BY(cs_KeyStore);
std::atomic<bool>& fUseCrypto;
bool& fDecryptionThoroughlyChecked;
};
#endif // BITCOIN_WALLET_SCRIPTPUBKEYMAN_H

42
src/wallet/wallet.cpp
View file

@ -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<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& 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();
}

20
src/wallet/wallet.h
View file

@ -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<bool> 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<bool> 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<WalletDatabase> 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<LegacyScriptPubKeyMan> m_spk_man = MakeUnique<LegacyScriptPubKeyMan>(*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)