LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #15235: Do not import private keys to wallets with private keys disabled

Browse source 
e6c58d3b01 Do not import private keys to wallets with private keys disabled (Andrew Chow)
b5c5021b64 Refactor importwallet to extract data from the file and then import (Andrew Chow)
1f77f6754c tests: unify RPC argument to cli argument conversion and handle dicts and lists (Andrew Chow)

Pull request description:

  Fixes a bug where private keys could be imported to wallets with private keys disabled. Now every RPC which can import private keys checks for whether the wallet has private keys are disabled and errors if it is. Also added an belt-and-suspenders check to `AddKeyPubkeyWithDB` to have it assert that the wallet has private keys enabled.

Tree-SHA512: 5cd04febce9aa2bd9bfd02f312c6ff8705e37278cae59efd3895f6d6e2f1b477aefd297e2dd0860791bdd3d4f3cad8eb1a404f8f3d4e2035b91314ad2c1028ae
This commit is contained in:
Wladimir J. van der Laan 2019-02-01 13:39:43 +01:00
commit 3e38d40873
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
4 changed files with 86 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

102
src/wallet/rpcdump.cpp
View file

@ -134,6 +134,9 @@ UniValue importprivkey(const JSONRPCRequest& request)
},
}.ToString());
if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled");
}
WalletRescanReserver reserver(pwallet);
bool fRescan = true;
@ -587,8 +590,10 @@ UniValue importwallet(const JSONRPCRequest& request)
// Use uiInterface.ShowProgress instead of pwallet.ShowProgress because pwallet.ShowProgress has a cancel button tied to AbortRescan which
// we don't want for this progress bar showing the import progress. uiInterface.ShowProgress does not have a cancel button.
uiInterface.ShowProgress(strprintf("%s " + _("Importing..."), pwallet->GetDisplayName()), 0, false); // show progress dialog in GUI
std::vector<std::tuple<CKey, int64_t, bool, std::string>> keys;
std::vector<std::pair<CScript, int64_t>> scripts;
while (file.good()) {
uiInterface.ShowProgress("", std::max(1, std::min(99, (int)(((double)file.tellg() / (double)nFilesize) * 100))), false);
uiInterface.ShowProgress("", std::max(1, std::min(50, (int)(((double)file.tellg() / (double)nFilesize) * 100))), false);
std::string line;
std::getline(file, line);
if (line.empty() || line[0] == '#')
@ -600,13 +605,6 @@ UniValue importwallet(const JSONRPCRequest& request)
continue;
CKey key = DecodeSecret(vstr[0]);
if (key.IsValid()) {
CPubKey pubkey = key.GetPubKey();
assert(key.VerifyPubKey(pubkey));
CKeyID keyid = pubkey.GetID();
if (pwallet->HaveKey(keyid)) {
pwallet->WalletLogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid));
continue;
}
int64_t nTime = DecodeDumpTime(vstr[1]);
std::string strLabel;
bool fLabel = true;
@ -622,36 +620,67 @@ UniValue importwallet(const JSONRPCRequest& request)
fLabel = true;
}
}
pwallet->WalletLogPrintf("Importing %s...\n", EncodeDestination(keyid));
if (!pwallet->AddKeyPubKey(key, pubkey)) {
fGood = false;
continue;
}
pwallet->mapKeyMetadata[keyid].nCreateTime = nTime;
if (fLabel)
pwallet->SetAddressBook(keyid, strLabel, "receive");
nTimeBegin = std::min(nTimeBegin, nTime);
keys.push_back(std::make_tuple(key, nTime, fLabel, strLabel));
} else if(IsHex(vstr[0])) {
std::vector<unsigned char> vData(ParseHex(vstr[0]));
CScript script = CScript(vData.begin(), vData.end());
CScriptID id(script);
if (pwallet->HaveCScript(id)) {
pwallet->WalletLogPrintf("Skipping import of %s (script already present)\n", vstr[0]);
continue;
}
if(!pwallet->AddCScript(script)) {
pwallet->WalletLogPrintf("Error importing script %s\n", vstr[0]);
fGood = false;
continue;
}
int64_t birth_time = DecodeDumpTime(vstr[1]);
if (birth_time > 0) {
pwallet->m_script_metadata[id].nCreateTime = birth_time;
nTimeBegin = std::min(nTimeBegin, birth_time);
}
std::vector<unsigned char> vData(ParseHex(vstr[0]));
CScript script = CScript(vData.begin(), vData.end());
int64_t birth_time = DecodeDumpTime(vstr[1]);
scripts.push_back(std::pair<CScript, int64_t>(script, birth_time));
}
}
file.close();
// We now know whether we are importing private keys, so we can error if private keys are disabled
if (keys.size() > 0 && pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI
throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled when private keys are disabled");
}
double total = (double)(keys.size() + scripts.size());
double progress = 0;
for (const auto& key_tuple : keys) {
uiInterface.ShowProgress("", std::max(50, std::min(75, (int)((progress / total) * 100) + 50)), false);
const CKey& key = std::get<0>(key_tuple);
int64_t time = std::get<1>(key_tuple);
bool has_label = std::get<2>(key_tuple);
std::string label = std::get<3>(key_tuple);
CPubKey pubkey = key.GetPubKey();
assert(key.VerifyPubKey(pubkey));
CKeyID keyid = pubkey.GetID();
if (pwallet->HaveKey(keyid)) {
pwallet->WalletLogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid));
continue;
}
pwallet->WalletLogPrintf("Importing %s...\n", EncodeDestination(keyid));
if (!pwallet->AddKeyPubKey(key, pubkey)) {
fGood = false;
continue;
}
pwallet->mapKeyMetadata[keyid].nCreateTime = time;
if (has_label)
pwallet->SetAddressBook(keyid, label, "receive");
nTimeBegin = std::min(nTimeBegin, time);
progress++;
}
for (const auto& script_pair : scripts) {
uiInterface.ShowProgress("", std::max(50, std::min(75, (int)((progress / total) * 100) + 50)), false);
const CScript& script = script_pair.first;
int64_t time = script_pair.second;
CScriptID id(script);
if (pwallet->HaveCScript(id)) {
pwallet->WalletLogPrintf("Skipping import of %s (script already present)\n", HexStr(script));
continue;
}
if(!pwallet->AddCScript(script)) {
pwallet->WalletLogPrintf("Error importing script %s\n", HexStr(script));
fGood = false;
continue;
}
if (time > 0) {
pwallet->m_script_metadata[id].nCreateTime = time;
nTimeBegin = std::min(nTimeBegin, time);
}
progress++;
}
uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI
pwallet->UpdateTimeFirstKey(nTimeBegin);
}
@ -958,6 +987,11 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
const bool watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false;
const std::string& label = data.exists("label") ? data["label"].get_str() : "";
// If private keys are disabled, abort if private keys are being imported
if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !keys.isNull()) {
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled");
}
// Generate the script and destination for the scriptPubKey provided
CScript script;
CTxDestination dest;

4
src/wallet/rpcwallet.cpp
View file

@ -3872,6 +3872,10 @@ UniValue sethdseed(const JSONRPCRequest& request)
throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set a new HD seed while still in Initial Block Download");
}
if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed to a wallet with private keys disabled");
}
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet);

3
src/wallet/wallet.cpp
View file

@ -251,6 +251,9 @@ bool CWallet::AddKeyPubKeyWithDB(WalletBatch &batch, const CKey& secret, const C
{
AssertLockHeld(cs_wallet); // mapKeyMetadata
// Make sure we aren't adding private keys to private key disabled wallets
assert(!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
// CCryptoKeyStore has no concept of wallet databases, but calls AddCryptedKey
// which is overridden below. To avoid flushes, the database handle is
// tunneled through to it.

11
test/functional/wallet_disableprivatekeys.py
View file

@ -7,6 +7,7 @@
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
)
@ -31,5 +32,15 @@ class DisablePrivateKeysTest(BitcoinTestFramework):
assert_raises_rpc_error(-4,"Error: Private keys are disabled for this wallet", w1.getrawchangeaddress)
w1.importpubkey(w2.getaddressinfo(w2.getnewaddress())['pubkey'])
self.log.info('Test that private keys cannot be imported')
addr = w2.getnewaddress('', 'legacy')
privkey = w2.dumpprivkey(addr)
assert_raises_rpc_error(-4, 'Cannot import private keys to a wallet with private keys disabled', w1.importprivkey, privkey)
result = w1.importmulti([{'scriptPubKey': {'address': addr}, 'timestamp': 'now', 'keys': [privkey]}])
assert(not result[0]['success'])
assert('warning' not in result[0])
assert_equal(result[0]['error']['code'], -4)
assert_equal(result[0]['error']['message'], 'Cannot import private keys to a wallet with private keys disabled')
if __name__ == '__main__':
DisablePrivateKeysTest().main()