rpc: make sure upgradetohd always has the passphrase for UpgradeToHD

earlier it was possible to make it all the way to `EncryptSecret`
without actually having the passphrase in hand until being told off
by `CCrypter::SetKey`, we should avoid that.

also, let's get rid of checks that `UpgradeToHD` is now taking
responsibility for. no point in checking if the wallet is unlocked
as it has no bearing on your ability to upgrade the wallet.
This commit is contained in:
Kittywhiskers Van Gogh 2024-07-17 13:20:22 +00:00
parent 619b640a77
commit 69c37f4ec2
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
3 changed files with 19 additions and 13 deletions

View File

@ -2779,11 +2779,13 @@ static RPCHelpMan upgradetohd()
{
return RPCHelpMan{"upgradetohd",
"\nUpgrades non-HD wallets to HD.\n"
"\nIf your wallet is encrypted, the wallet passphrase must be supplied. Supplying an incorrect"
"\npassphrase may result in your wallet getting locked.\n"
"\nWarning: You will need to make a new backup of your wallet after setting the HD wallet mnemonic.\n",
{
{"mnemonic", RPCArg::Type::STR, /* default */ "", "Mnemonic as defined in BIP39 to use for the new HD wallet. Use an empty string \"\" to generate a new random mnemonic."},
{"mnemonicpassphrase", RPCArg::Type::STR, /* default */ "", "Optional mnemonic passphrase as defined in BIP39"},
{"walletpassphrase", RPCArg::Type::STR, /* default */ "", "If your wallet is encrypted you must have your wallet passphrase here. If your wallet is not encrypted specifying wallet passphrase will trigger wallet encryption."},
{"walletpassphrase", RPCArg::Type::STR, /* default */ "", "If your wallet is encrypted you must have your wallet passphrase here. If your wallet is not encrypted, specifying wallet passphrase will trigger wallet encryption."},
{"rescan", RPCArg::Type::BOOL, /* default */ "false if mnemonic is empty", "Whether to rescan the blockchain for missing transactions or not"},
},
RPCResult{
@ -2793,6 +2795,7 @@ static RPCHelpMan upgradetohd()
HelpExampleCli("upgradetohd", "")
+ HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\"")
+ HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\" \"mnemonicpassphrase\"")
+ HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\" \"\" \"walletpassphrase\"")
+ HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\" \"mnemonicpassphrase\" \"walletpassphrase\"")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
@ -2803,16 +2806,16 @@ static RPCHelpMan upgradetohd()
bool generate_mnemonic = request.params[0].isNull() || request.params[0].get_str().empty();
SecureString secureWalletPassphrase;
secureWalletPassphrase.reserve(100);
if (request.params[2].isNull()) {
if (pwallet->IsCrypted()) {
throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Wallet encrypted but passphrase not supplied to RPC.");
}
} else {
// TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make request.params[0] mlock()'d to begin with.
if (!request.params[2].isNull()) {
secureWalletPassphrase = request.params[2].get_str().c_str();
if (!pwallet->Unlock(secureWalletPassphrase)) {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "The wallet passphrase entered was incorrect");
}
}
EnsureWalletIsUnlocked(pwallet.get());
SecureString secureMnemonic;
secureMnemonic.reserve(256);
@ -2825,6 +2828,7 @@ static RPCHelpMan upgradetohd()
if (!request.params[1].isNull()) {
secureMnemonicPassphrase = request.params[1].get_str().c_str();
}
// TODO: breaking changes kept for v21!
// instead upgradetohd let's use more straightforward 'sethdseed'
constexpr bool is_v21 = false;

View File

@ -5683,7 +5683,9 @@ bool CWallet::GenerateNewHDChain(const SecureString& secureMnemonic, const Secur
// We got a gibberish key...
if (vMasterKey.empty()) {
throw std::runtime_error(strprintf("%s: supplied incorrect passphrase", __func__));
// Mimicking the error message of RPC_WALLET_PASSPHRASE_INCORRECT as it's possible
// that the user may see this error when interacting with the upgradetohd RPC
throw std::runtime_error("Error: The wallet passphrase entered was incorrect");
}
spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase, vMasterKey);

View File

@ -190,8 +190,8 @@ class WalletUpgradeToHDTest(BitcoinTestFramework):
node.stop()
node.wait_until_stopped()
self.start_node(0, extra_args=['-rescan'])
assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.upgradetohd, mnemonic)
assert_raises_rpc_error(-14, "The wallet passphrase entered was incorrect", node.upgradetohd, mnemonic, "", "wrongpass")
assert_raises_rpc_error(-13, "Error: Wallet encrypted but passphrase not supplied to RPC.", node.upgradetohd, mnemonic)
assert_raises_rpc_error(-1, "Error: The wallet passphrase entered was incorrect", node.upgradetohd, mnemonic, "", "wrongpass")
assert node.upgradetohd(mnemonic, "", walletpass)
assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.dumphdinfo)
node.walletpassphrase(walletpass, 100)