highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-11 21:20:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

Wallet: Check crypter return values

Browse Source 
Mark CCrypter's fallible methods and crypto helpers as nodiscard, and
handle key-derivation calibration failures.

Keep the output master key unchanged until encryption succeeds. Validate
calibrated iteration counts before integer conversion. This prevents a
failed calibration from leaving mismatched parameters and ciphertext or
triggering undefined conversion behavior.
This commit is contained in:
benthecarman
2026-08-01 23:17:41 -05:00
parent b76afff274
commit cf36df070b
4 changed files with 39 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/wallet/crypter.h
View File

@@ -76,13 +76,13 @@ private:
std::vector<unsigned char, secure_allocator<unsigned char>> vchIV;
bool fKeySet;
int BytesToKeySHA512AES(std::span<const unsigned char> salt, const SecureString& key_data, unsigned int count, unsigned char* key, unsigned char* iv) const;
[[nodiscard]] int BytesToKeySHA512AES(std::span<const unsigned char> salt, const SecureString& key_data, unsigned int count, unsigned char* key, unsigned char* iv) const;
public:
bool SetKeyFromPassphrase(const SecureString& key_data, std::span<const unsigned char> salt, unsigned int rounds, unsigned int derivation_method);
bool Encrypt(const CKeyingMaterial& vchPlaintext, std::vector<unsigned char> &vchCiphertext) const;
bool Decrypt(std::span<const unsigned char> ciphertext, CKeyingMaterial& plaintext) const;
bool SetKey(const CKeyingMaterial& new_key, std::span<const unsigned char> new_iv);
[[nodiscard]] bool SetKeyFromPassphrase(const SecureString& key_data, std::span<const unsigned char> salt, unsigned int rounds, unsigned int derivation_method);
[[nodiscard]] bool Encrypt(const CKeyingMaterial& vchPlaintext, std::vector<unsigned char> &vchCiphertext) const;
[[nodiscard]] bool Decrypt(std::span<const unsigned char> ciphertext, CKeyingMaterial& plaintext) const;
[[nodiscard]] bool SetKey(const CKeyingMaterial& new_key, std::span<const unsigned char> new_iv);
void CleanKey()
{
@@ -104,9 +104,9 @@ public:
}
};
bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vchPlaintext, const uint256& nIV, std::vector<unsigned char> &vchCiphertext);
bool DecryptSecret(const CKeyingMaterial& master_key, std::span<const unsigned char> ciphertext, const uint256& iv, CKeyingMaterial& plaintext);
bool DecryptKey(const CKeyingMaterial& master_key, std::span<const unsigned char> crypted_secret, const CPubKey& pub_key, CKey& key);
[[nodiscard]] bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vchPlaintext, const uint256& nIV, std::vector<unsigned char> &vchCiphertext);
[[nodiscard]] bool DecryptSecret(const CKeyingMaterial& master_key, std::span<const unsigned char> ciphertext, const uint256& iv, CKeyingMaterial& plaintext);
[[nodiscard]] bool DecryptKey(const CKeyingMaterial& master_key, std::span<const unsigned char> crypted_secret, const CPubKey& pub_key, CKey& key);
} // namespace wallet
#endif // BITCOIN_WALLET_CRYPTER_H

8
src/wallet/test/fuzz/crypter.cpp
View File

@@ -35,10 +35,10 @@ FUZZ_TARGET(crypter, .init = initialize_crypter)
const unsigned int derivation_method = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<unsigned int>();
// Limiting the value of rounds since it is otherwise uselessly expensive and causes a timeout when fuzzing.
crypt.SetKeyFromPassphrase(/*key_data=*/secure_string,
/*salt=*/ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_SALT_SIZE),
/*rounds=*/fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, CMasterKey::DEFAULT_DERIVE_ITERATIONS),
/*derivation_method=*/derivation_method);
(void)crypt.SetKeyFromPassphrase(/*key_data=*/secure_string,
/*salt=*/ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_SALT_SIZE),
/*rounds=*/fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, CMasterKey::DEFAULT_DERIVE_ITERATIONS),
/*derivation_method=*/derivation_method);
}
CKey random_ckey;

13
src/wallet/test/wallet_crypto_tests.cpp
View File

@@ -24,7 +24,7 @@ static void TestPassphraseSingle(const std::span<const unsigned char> salt, cons
const std::span<const unsigned char> correct_iv = {})
{
CCrypter crypt;
crypt.SetKeyFromPassphrase(passphrase, salt, rounds, 0);
BOOST_REQUIRE(crypt.SetKeyFromPassphrase(passphrase, salt, rounds, /*derivation_method=*/0));
if (!correct_key.empty()) {
BOOST_CHECK_MESSAGE(memcmp(crypt.vchKey.data(), correct_key.data(), crypt.vchKey.size()) == 0,
@@ -50,8 +50,9 @@ static void TestDecrypt(const CCrypter& crypt, const std::span<const unsigned ch
const std::span<const unsigned char> correct_plaintext = {})
{
CKeyingMaterial decrypted;
crypt.Decrypt(ciphertext, decrypted);
const bool decrypt_ok{crypt.Decrypt(ciphertext, decrypted)};
if (!correct_plaintext.empty()) {
BOOST_REQUIRE(decrypt_ok);
BOOST_CHECK_EQUAL_COLLECTIONS(decrypted.begin(), decrypted.end(), correct_plaintext.begin(), correct_plaintext.end());
}
}
@@ -60,7 +61,7 @@ static void TestEncryptSingle(const CCrypter& crypt, const CKeyingMaterial& plai
const std::span<const unsigned char> correct_ciphertext = {})
{
std::vector<unsigned char> ciphertext;
crypt.Encrypt(plaintext, ciphertext);
BOOST_REQUIRE(crypt.Encrypt(plaintext, ciphertext));
if (!correct_ciphertext.empty()) {
BOOST_CHECK_EQUAL_COLLECTIONS(ciphertext.begin(), ciphertext.end(), correct_ciphertext.begin(), correct_ciphertext.end());
@@ -106,7 +107,7 @@ BOOST_AUTO_TEST_CASE(passphrase_zero_rounds) {
BOOST_AUTO_TEST_CASE(encrypt) {
constexpr std::array<uint8_t, WALLET_CRYPTO_SALT_SIZE> salt{"0000deadbeef0000"_hex_u8};
CCrypter crypt;
crypt.SetKeyFromPassphrase("passphrase", salt, CMasterKey::DEFAULT_DERIVE_ITERATIONS, 0);
BOOST_REQUIRE(crypt.SetKeyFromPassphrase("passphrase", salt, CMasterKey::DEFAULT_DERIVE_ITERATIONS, /*derivation_method=*/0));
TestCrypter::TestEncrypt(crypt, "22bcade09ac03ff6386914359cfe885cfeb5f77ff0d670f102f619687453b29d"_hex_u8);
for (int i = 0; i != 100; i++)
@@ -120,9 +121,9 @@ BOOST_AUTO_TEST_CASE(encrypt) {
BOOST_AUTO_TEST_CASE(decrypt) {
constexpr std::array<uint8_t, WALLET_CRYPTO_SALT_SIZE> salt{"0000deadbeef0000"_hex_u8};
CCrypter crypt;
crypt.SetKeyFromPassphrase("passphrase", salt, CMasterKey::DEFAULT_DERIVE_ITERATIONS, 0);
BOOST_REQUIRE(crypt.SetKeyFromPassphrase("passphrase", salt, CMasterKey::DEFAULT_DERIVE_ITERATIONS, /*derivation_method=*/0));
// Some corner cases the came up while testing
// Some corner cases that came up while testing
TestCrypter::TestDecrypt(crypt,"795643ce39d736088367822cdc50535ec6f103715e3e48f4f3b1a60a08ef59ca"_hex_u8);
TestCrypter::TestDecrypt(crypt,"de096f4a8f9bd97db012aa9d90d74de8cdea779c3ee8bc7633d8b5d6da703486"_hex_u8);
TestCrypter::TestDecrypt(crypt,"32d0a8974e3afd9c6c3ebf4d66aa4e6419f8c173de25947f98cf8b7ace49449c"_hex_u8);

29
src/wallet/wallet.cpp
View File

@@ -76,10 +76,12 @@
#include <cassert>
#include <condition_variable>
#include <exception>
#include <limits>
#include <optional>
#include <stdexcept>
#include <thread>
#include <tuple>
#include <utility>
#include <variant>
struct KeyOriginInfo;
@@ -566,36 +568,45 @@ 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()};
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;
}
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;
// Get the weighted average with previous runs.
master_key.nDeriveIterations = (i * master_key.nDeriveIterations + target_iterations) / (i + 1);
const double target_iterations{updated_master_key.nDeriveIterations * target_time / elapsed_time};
if (target_iterations < 1 || target_iterations > std::numeric_limits<unsigned int>::max()) {
return false;
}
// Get the weighted average with previous runs. Use 64-bit math so the
// sum cannot wrap; the average of two unsigned int values fits in one.
updated_master_key.nDeriveIterations = (uint64_t{updated_master_key.nDeriveIterations} * i + static_cast<unsigned int>(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;
}