7b18a0c Wallet: Check crypter return values:
Now that we return early on failure, we shouldn't modify the output parameter: a failed calibration can leave nDeriveIterations changed while vchCryptedKey still contains the old ciphertext.
Could we copy master_key and only move it to the output after encryption succeeds?
<details><summary>preserve master key on failure</summary>
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index bbb930864d..b76c5da070 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -566,11 +566,12 @@ static bool EncryptMasterKey(const SecureString& wallet_passphrase, const CKeyin
{
constexpr MillisecondsDouble target_time{100};
CCrypter crypter;
+ CMasterKey updated_master_key{master_key};
// Get the weighted average of iterations we can do in 100ms over 2 runs.
for (int i = 0; i < 2; i++){
auto start_time{NodeClock::now()};
- const bool key_set{crypter.SetKeyFromPassphrase(wallet_passphrase, master_key.vchSalt, master_key.nDeriveIterations, master_key.nDerivationMethod)};
+ const bool key_set{crypter.SetKeyFromPassphrase(wallet_passphrase, updated_master_key.vchSalt, updated_master_key.nDeriveIterations, updated_master_key.nDerivationMethod)};
auto elapsed_time{NodeClock::now() - start_time};
if (!key_set) {
return false;
@@ -578,27 +579,28 @@ static bool EncryptMasterKey(const SecureString& wallet_passphrase, const CKeyin
if (elapsed_time <= 0s) {
// We are probably in a test with a mocked clock.
- master_key.nDeriveIterations = CMasterKey::DEFAULT_DERIVE_ITERATIONS;
+ updated_master_key.nDeriveIterations = CMasterKey::DEFAULT_DERIVE_ITERATIONS;
break;
}
// target_iterations : elapsed_iterations :: target_time : elapsed_time
- unsigned int target_iterations = master_key.nDeriveIterations * target_time / elapsed_time;
+ unsigned int target_iterations = updated_master_key.nDeriveIterations * target_time / elapsed_time;
// Get the weighted average with previous runs.
- master_key.nDeriveIterations = (i * master_key.nDeriveIterations + target_iterations) / (i + 1);
+ updated_master_key.nDeriveIterations = (i * updated_master_key.nDeriveIterations + target_iterations) / (i + 1);
}
- if (master_key.nDeriveIterations < CMasterKey::DEFAULT_DERIVE_ITERATIONS) {
- master_key.nDeriveIterations = CMasterKey::DEFAULT_DERIVE_ITERATIONS;
+ if (updated_master_key.nDeriveIterations < CMasterKey::DEFAULT_DERIVE_ITERATIONS) {
+ updated_master_key.nDeriveIterations = CMasterKey::DEFAULT_DERIVE_ITERATIONS;
}
- if (!crypter.SetKeyFromPassphrase(wallet_passphrase, master_key.vchSalt, master_key.nDeriveIterations, master_key.nDerivationMethod)) {
+ if (!crypter.SetKeyFromPassphrase(wallet_passphrase, updated_master_key.vchSalt, updated_master_key.nDeriveIterations, updated_master_key.nDerivationMethod)) {
return false;
}
- if (!crypter.Encrypt(plain_master_key, master_key.vchCryptedKey)) {
+ if (!crypter.Encrypt(plain_master_key, updated_master_key.vchCryptedKey)) {
return false;
}
+ master_key = std::move(updated_master_key);
return true;
}
</details>
