Merge #12101: Clamp walletpassphrase timeout to 2^30 seconds and check its bounds

134cdc7 Test walletpassphrase timeout bounds and clamping (Andrew Chow)
0b63e3c Clamp walletpassphrase timeout to 2^(30) seconds and check its bounds (Andrew Chow)

Pull request description:

  Fixes #12100

  Makes the timeout be clamped to 2^30 seconds to avoid the issue with sign flipping with large timeout values and thus relocking the wallet instantly. Unlocking for at most ~34 years should be sufficient.

  Also checks that the timeout is not negative to avoid instant relocks.

Tree-SHA512: 426922f08c54e323d259e25dcdbebc2cd560708a65111ce6051493a7e7c61e79d9da1ea4026cc0d68807d728f5d7c0d7c58168c6ef4167b94cf6c2877af88794
This commit is contained in:
Wladimir J. van der Laan 2018-01-17 12:15:45 +01:00
commit c7978be899
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
2 changed files with 32 additions and 2 deletions

View file

@ -2283,7 +2283,8 @@ UniValue walletpassphrase(const JSONRPCRequest& request)
"This is needed prior to performing transactions related to private keys such as sending bitcoins\n"
"\nArguments:\n"
"1. \"passphrase\" (string, required) The wallet passphrase\n"
"2. timeout (numeric, required) The time to keep the decryption key in seconds.\n"
"2. timeout (numeric, required) The time to keep the decryption key in seconds. Limited to at most 1073741824 (2^30) seconds.\n"
" Any value greater than 1073741824 seconds will be set to 1073741824 seconds.\n"
"\nNote:\n"
"Issuing the walletpassphrase command while the wallet is already unlocked will set a new unlock\n"
"time that overrides the old one.\n"
@ -2312,6 +2313,17 @@ UniValue walletpassphrase(const JSONRPCRequest& request)
// Alternately, find a way to make request.params[0] mlock()'d to begin with.
strWalletPass = request.params[0].get_str().c_str();
// Get the timeout
int64_t nSleepTime = request.params[1].get_int64();
// Timeout cannot be negative, otherwise it will relock immediately
if (nSleepTime < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative.");
}
// Clamp timeout to 2^30 seconds
if (nSleepTime > (int64_t)1 << 30) {
nSleepTime = (int64_t)1 << 30;
}
if (strWalletPass.length() > 0)
{
if (!pwallet->Unlock(strWalletPass)) {
@ -2325,7 +2337,6 @@ UniValue walletpassphrase(const JSONRPCRequest& request)
pwallet->TopUpKeyPool();
int64_t nSleepTime = request.params[1].get_int64();
pwallet->nRelockTime = GetTime() + nSleepTime;
RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), boost::bind(LockWallet, pwallet), nSleepTime);

View file

@ -10,6 +10,8 @@ from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
assert_greater_than,
assert_greater_than_or_equal,
)
class WalletEncryptionTest(BitcoinTestFramework):
@ -56,6 +58,23 @@ class WalletEncryptionTest(BitcoinTestFramework):
assert_raises_rpc_error(-14, "wallet passphrase entered was incorrect", self.nodes[0].walletpassphrase, passphrase, 10)
self.nodes[0].walletpassphrase(passphrase2, 10)
assert_equal(privkey, self.nodes[0].dumpprivkey(address))
self.nodes[0].walletlock()
# Test timeout bounds
assert_raises_rpc_error(-8, "Timeout cannot be negative.", self.nodes[0].walletpassphrase, passphrase2, -10)
# Check the timeout
# Check a time less than the limit
expected_time = int(time.time()) + (1 << 30) - 600
self.nodes[0].walletpassphrase(passphrase2, (1 << 30) - 600)
actual_time = self.nodes[0].getwalletinfo()['unlocked_until']
assert_greater_than_or_equal(actual_time, expected_time)
assert_greater_than(expected_time + 5, actual_time) # 5 second buffer
# Check a time greater than the limit
expected_time = int(time.time()) + (1 << 30) - 1
self.nodes[0].walletpassphrase(passphrase2, (1 << 33))
actual_time = self.nodes[0].getwalletinfo()['unlocked_until']
assert_greater_than_or_equal(actual_time, expected_time)
assert_greater_than(expected_time + 5, actual_time) # 5 second buffer
if __name__ == '__main__':
WalletEncryptionTest().main()