From bd0830bbd4105af1953b6b897ba6bc35098cbe13 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Tue, 20 Aug 2024 23:44:57 +0200 Subject: [PATCH] refactor: de-Hungarianize CCrypter Beyond renaming it also adjusts whitespace and adds braces to conform to current doc/developer-notes.md. TestEncrypt: Change iterator type to auto in ahead of vector -> span conversion. Only touches functions that will be modified in next commit. --- src/wallet/crypter.cpp | 63 +++++++++++--------- src/wallet/crypter.h | 12 ++-- src/wallet/test/fuzz/crypter.cpp | 10 ++-- src/wallet/test/wallet_crypto_tests.cpp | 76 +++++++++++++------------ 4 files changed, 87 insertions(+), 74 deletions(-) diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 5ee755452d2..1d6af9dda44 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -12,7 +12,7 @@ #include namespace wallet { -int CCrypter::BytesToKeySHA512AES(const std::vector& chSalt, const SecureString& strKeyData, int count, unsigned char *key,unsigned char *iv) const +int CCrypter::BytesToKeySHA512AES(const std::vector& salt, const SecureString& key_data, int count, unsigned char* key, unsigned char* iv) const { // This mimics the behavior of openssl's EVP_BytesToKey with an aes256cbc // cipher and sha512 message digest. Because sha512's output size (64b) is @@ -25,8 +25,8 @@ int CCrypter::BytesToKeySHA512AES(const std::vector& chSalt, cons unsigned char buf[CSHA512::OUTPUT_SIZE]; CSHA512 di; - di.Write(UCharCast(strKeyData.data()), strKeyData.size()); - di.Write(chSalt.data(), chSalt.size()); + di.Write(UCharCast(key_data.data()), key_data.size()); + di.Write(salt.data(), salt.size()); di.Finalize(buf); for(int i = 0; i != count - 1; i++) @@ -38,14 +38,16 @@ int CCrypter::BytesToKeySHA512AES(const std::vector& chSalt, cons return WALLET_CRYPTO_KEY_SIZE; } -bool CCrypter::SetKeyFromPassphrase(const SecureString& strKeyData, const std::vector& chSalt, const unsigned int nRounds, const unsigned int nDerivationMethod) +bool CCrypter::SetKeyFromPassphrase(const SecureString& key_data, const std::vector& salt, const unsigned int rounds, const unsigned int derivation_method) { - if (nRounds < 1 || chSalt.size() != WALLET_CRYPTO_SALT_SIZE) + if (rounds < 1 || salt.size() != WALLET_CRYPTO_SALT_SIZE) { return false; + } int i = 0; - if (nDerivationMethod == 0) - i = BytesToKeySHA512AES(chSalt, strKeyData, nRounds, vchKey.data(), vchIV.data()); + if (derivation_method == 0) { + i = BytesToKeySHA512AES(salt, key_data, rounds, vchKey.data(), vchIV.data()); + } if (i != (int)WALLET_CRYPTO_KEY_SIZE) { @@ -58,13 +60,14 @@ bool CCrypter::SetKeyFromPassphrase(const SecureString& strKeyData, const std::v return true; } -bool CCrypter::SetKey(const CKeyingMaterial& chNewKey, const std::vector& chNewIV) +bool CCrypter::SetKey(const CKeyingMaterial& new_key, const std::vector& new_iv) { - if (chNewKey.size() != WALLET_CRYPTO_KEY_SIZE || chNewIV.size() != WALLET_CRYPTO_IV_SIZE) + if (new_key.size() != WALLET_CRYPTO_KEY_SIZE || new_iv.size() != WALLET_CRYPTO_IV_SIZE) { return false; + } - memcpy(vchKey.data(), chNewKey.data(), chNewKey.size()); - memcpy(vchIV.data(), chNewIV.data(), chNewIV.size()); + memcpy(vchKey.data(), new_key.data(), new_key.size()); + memcpy(vchIV.data(), new_iv.data(), new_iv.size()); fKeySet = true; return true; @@ -88,19 +91,20 @@ bool CCrypter::Encrypt(const CKeyingMaterial& vchPlaintext, std::vector& vchCiphertext, CKeyingMaterial& vchPlaintext) const +bool CCrypter::Decrypt(const std::vector& ciphertext, CKeyingMaterial& plaintext) const { if (!fKeySet) return false; // plaintext will always be equal to or lesser than length of ciphertext - vchPlaintext.resize(vchCiphertext.size()); + plaintext.resize(ciphertext.size()); AES256CBCDecrypt dec(vchKey.data(), vchIV.data(), true); - int nLen = dec.Decrypt(vchCiphertext.data(), vchCiphertext.size(), vchPlaintext.data()); - if(nLen == 0) + int len = dec.Decrypt(ciphertext.data(), ciphertext.size(), plaintext.data()); + if (len == 0) { return false; - vchPlaintext.resize(nLen); + } + plaintext.resize(len); return true; } @@ -114,26 +118,29 @@ bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vch return cKeyCrypter.Encrypt(vchPlaintext, vchCiphertext); } -bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector& vchCiphertext, const uint256& nIV, CKeyingMaterial& vchPlaintext) +bool DecryptSecret(const CKeyingMaterial& master_key, const std::vector& ciphertext, const uint256& iv, CKeyingMaterial& plaintext) { - CCrypter cKeyCrypter; - static_assert(WALLET_CRYPTO_IV_SIZE <= std::remove_reference_t::size()); - std::vector chIV{nIV.begin(), nIV.begin() + WALLET_CRYPTO_IV_SIZE}; - if(!cKeyCrypter.SetKey(vMasterKey, chIV)) + CCrypter key_crypter; + static_assert(WALLET_CRYPTO_IV_SIZE <= std::remove_reference_t::size()); + std::vector iv_prefix{iv.begin(), iv.begin() + WALLET_CRYPTO_IV_SIZE}; + if (!key_crypter.SetKey(master_key, iv_prefix)) { return false; - return cKeyCrypter.Decrypt(vchCiphertext, vchPlaintext); + } + return key_crypter.Decrypt(ciphertext, plaintext); } -bool DecryptKey(const CKeyingMaterial& vMasterKey, const std::vector& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key) +bool DecryptKey(const CKeyingMaterial& master_key, const std::vector& crypted_secret, const CPubKey& pub_key, CKey& key) { - CKeyingMaterial vchSecret; - if(!DecryptSecret(vMasterKey, vchCryptedSecret, vchPubKey.GetHash(), vchSecret)) + CKeyingMaterial secret; + if (!DecryptSecret(master_key, crypted_secret, pub_key.GetHash(), secret)) { return false; + } - if (vchSecret.size() != 32) + if (secret.size() != 32) { return false; + } - key.Set(vchSecret.begin(), vchSecret.end(), vchPubKey.IsCompressed()); - return key.VerifyPubKey(vchPubKey); + key.Set(secret.begin(), secret.end(), pub_key.IsCompressed()); + return key.VerifyPubKey(pub_key); } } // namespace wallet diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index b776a9c4979..4c3d49175c4 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -75,13 +75,13 @@ private: std::vector> vchIV; bool fKeySet; - int BytesToKeySHA512AES(const std::vector& chSalt, const SecureString& strKeyData, int count, unsigned char *key,unsigned char *iv) const; + int BytesToKeySHA512AES(const std::vector& salt, const SecureString& key_data, int count, unsigned char* key, unsigned char* iv) const; public: - bool SetKeyFromPassphrase(const SecureString &strKeyData, const std::vector& chSalt, const unsigned int nRounds, const unsigned int nDerivationMethod); + bool SetKeyFromPassphrase(const SecureString& key_data, const std::vector& salt, const unsigned int rounds, const unsigned int derivation_method); bool Encrypt(const CKeyingMaterial& vchPlaintext, std::vector &vchCiphertext) const; - bool Decrypt(const std::vector& vchCiphertext, CKeyingMaterial& vchPlaintext) const; - bool SetKey(const CKeyingMaterial& chNewKey, const std::vector& chNewIV); + bool Decrypt(const std::vector& ciphertext, CKeyingMaterial& plaintext) const; + bool SetKey(const CKeyingMaterial& new_key, const std::vector& new_iv); void CleanKey() { @@ -104,8 +104,8 @@ public: }; bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vchPlaintext, const uint256& nIV, std::vector &vchCiphertext); -bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector& vchCiphertext, const uint256& nIV, CKeyingMaterial& vchPlaintext); -bool DecryptKey(const CKeyingMaterial& vMasterKey, const std::vector& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key); +bool DecryptSecret(const CKeyingMaterial& master_key, const std::vector& ciphertext, const uint256& iv, CKeyingMaterial& plaintext); +bool DecryptKey(const CKeyingMaterial& master_key, const std::vector& crypted_secret, const CPubKey& pub_key, CKey& key); } // namespace wallet #endif // BITCOIN_WALLET_CRYPTER_H diff --git a/src/wallet/test/fuzz/crypter.cpp b/src/wallet/test/fuzz/crypter.cpp index 4d6dd43c5f7..7869f5f39c8 100644 --- a/src/wallet/test/fuzz/crypter.cpp +++ b/src/wallet/test/fuzz/crypter.cpp @@ -35,11 +35,11 @@ FUZZ_TARGET(crypter, .init = initialize_crypter) const unsigned int derivation_method = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral(); - // Limiting the value of nRounds since it is otherwise uselessly expensive and causes a timeout when fuzzing. - crypt.SetKeyFromPassphrase(/*strKeyData=*/secure_string, - /*chSalt=*/ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_SALT_SIZE), - /*nRounds=*/fuzzed_data_provider.ConsumeIntegralInRange(0, 25000), - /*nDerivationMethod=*/derivation_method); + // 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(0, 25000), + /*derivation_method=*/derivation_method); } LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 100) diff --git a/src/wallet/test/wallet_crypto_tests.cpp b/src/wallet/test/wallet_crypto_tests.cpp index 09b08fa4337..34b909e85dd 100644 --- a/src/wallet/test/wallet_crypto_tests.cpp +++ b/src/wallet/test/wallet_crypto_tests.cpp @@ -17,58 +17,64 @@ BOOST_FIXTURE_TEST_SUITE(wallet_crypto_tests, BasicTestingSetup) class TestCrypter { public: -static void TestPassphraseSingle(const std::vector& vchSalt, const SecureString& passphrase, uint32_t rounds, - const std::vector& correctKey = {}, - const std::vector& correctIV = {}) +static void TestPassphraseSingle(const std::vector& salt, const SecureString& passphrase, uint32_t rounds, + const std::vector& correct_key = {}, + const std::vector& correct_iv = {}) { CCrypter crypt; - crypt.SetKeyFromPassphrase(passphrase, vchSalt, rounds, 0); + crypt.SetKeyFromPassphrase(passphrase, salt, rounds, 0); - if(!correctKey.empty()) - BOOST_CHECK_MESSAGE(memcmp(crypt.vchKey.data(), correctKey.data(), crypt.vchKey.size()) == 0, - HexStr(crypt.vchKey) + std::string(" != ") + HexStr(correctKey)); - if(!correctIV.empty()) - BOOST_CHECK_MESSAGE(memcmp(crypt.vchIV.data(), correctIV.data(), crypt.vchIV.size()) == 0, - HexStr(crypt.vchIV) + std::string(" != ") + HexStr(correctIV)); + if (!correct_key.empty()) { + BOOST_CHECK_MESSAGE(memcmp(crypt.vchKey.data(), correct_key.data(), crypt.vchKey.size()) == 0, + HexStr(crypt.vchKey) + std::string(" != ") + HexStr(correct_key)); + } + if (!correct_iv.empty()) { + BOOST_CHECK_MESSAGE(memcmp(crypt.vchIV.data(), correct_iv.data(), crypt.vchIV.size()) == 0, + HexStr(crypt.vchIV) + std::string(" != ") + HexStr(correct_iv)); + } } -static void TestPassphrase(const std::vector& vchSalt, const SecureString& passphrase, uint32_t rounds, - const std::vector& correctKey = {}, - const std::vector& correctIV = {}) +static void TestPassphrase(const std::vector& salt, const SecureString& passphrase, uint32_t rounds, + const std::vector& correct_key = {}, + const std::vector& correct_iv = {}) { - TestPassphraseSingle(vchSalt, passphrase, rounds, correctKey, correctIV); - for(SecureString::const_iterator i(passphrase.begin()); i != passphrase.end(); ++i) - TestPassphraseSingle(vchSalt, SecureString(i, passphrase.end()), rounds); + TestPassphraseSingle(salt, passphrase, rounds, correct_key, correct_iv); + for (SecureString::const_iterator it{passphrase.begin()}; it != passphrase.end(); ++it) { + TestPassphraseSingle(salt, SecureString{it, passphrase.end()}, rounds); + } } -static void TestDecrypt(const CCrypter& crypt, const std::vector& vchCiphertext, - const std::vector& vchCorrectPlaintext = {}) +static void TestDecrypt(const CCrypter& crypt, const std::vector& ciphertext, + const std::vector& correct_plaintext = {}) { - CKeyingMaterial vchDecrypted; - crypt.Decrypt(vchCiphertext, vchDecrypted); - if (vchPlaintext.size()) - BOOST_CHECK_EQUAL_COLLECTIONS(vchDecrypted.begin(), vchDecrypted.end(), vchCorrectPlaintext.begin(), vchCorrectPlaintext.end()); + CKeyingMaterial decrypted; + crypt.Decrypt(ciphertext, decrypted); + if (!correct_plaintext.empty()) { + BOOST_CHECK_EQUAL_COLLECTIONS(decrypted.begin(), decrypted.end(), correct_plaintext.begin(), correct_plaintext.end()); + } } -static void TestEncryptSingle(const CCrypter& crypt, const CKeyingMaterial& vchPlaintext, - const std::vector& vchCiphertextCorrect = {}) +static void TestEncryptSingle(const CCrypter& crypt, const CKeyingMaterial& plaintext, + const std::vector& correct_ciphertext = {}) { - std::vector vchCiphertext; - crypt.Encrypt(vchPlaintext, vchCiphertext); + std::vector ciphertext; + crypt.Encrypt(plaintext, ciphertext); - if (!vchCiphertextCorrect.empty()) - BOOST_CHECK_EQUAL_COLLECTIONS(vchCiphertext.begin(), vchCiphertext.end(), vchCiphertextCorrect.begin(), vchCiphertextCorrect.end()); + if (!correct_ciphertext.empty()) { + BOOST_CHECK_EQUAL_COLLECTIONS(ciphertext.begin(), ciphertext.end(), correct_ciphertext.begin(), correct_ciphertext.end()); + } - const std::vector vchPlaintext2(vchPlaintext.begin(), vchPlaintext.end()); - TestDecrypt(crypt, vchCiphertext, vchPlaintext2); + const std::vector plaintext2(plaintext.begin(), plaintext.end()); + TestDecrypt(crypt, ciphertext, /*correct_plaintext=*/plaintext2); } -static void TestEncrypt(const CCrypter& crypt, const std::vector& vchPlaintextIn, - const std::vector& vchCiphertextCorrect = {}) +static void TestEncrypt(const CCrypter& crypt, const std::vector& plaintext, + const std::vector& correct_ciphertext = {}) { - TestEncryptSingle(crypt, CKeyingMaterial{vchPlaintextIn.begin(), vchPlaintextIn.end()}, vchCiphertextCorrect); - for(std::vector::const_iterator i(vchPlaintextIn.begin()); i != vchPlaintextIn.end(); ++i) - TestEncryptSingle(crypt, CKeyingMaterial(i, vchPlaintextIn.end())); + TestEncryptSingle(crypt, CKeyingMaterial{plaintext.begin(), plaintext.end()}, correct_ciphertext); + for (auto it{plaintext.begin()}; it != plaintext.end(); ++it) { + TestEncryptSingle(crypt, CKeyingMaterial{it, plaintext.end()}); + } } };