Skip to content

Commit bd0830b

Browse files
hodlinatorhodlinator
committed
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.
1 parent d99c816 commit bd0830b

File tree

4 files changed

+88
-75
lines changed

4 files changed

+88
-75
lines changed

‎src/wallet/crypter.cpp‎

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include <vector>
1313

1414
namespace wallet {
15-
int CCrypter::BytesToKeySHA512AES(const std::vector<unsigned char>& chSalt, const SecureString& strKeyData, int count, unsigned char *key,unsigned char *iv) const
15+
int CCrypter::BytesToKeySHA512AES(const std::vector<unsigned char>& salt, const SecureString& key_data, int count, unsigned char* key, unsigned char* iv) const
1616
{
1717
// This mimics the behavior of openssl's EVP_BytesToKey with an aes256cbc
1818
// cipher and sha512 message digest. Because sha512's output size (64b) is
@@ -25,8 +25,8 @@ int CCrypter::BytesToKeySHA512AES(const std::vector<unsigned char>& chSalt, cons
2525
unsigned char buf[CSHA512::OUTPUT_SIZE];
2626
CSHA512 di;
2727

28-
di.Write(UCharCast(strKeyData.data()), strKeyData.size());
29-
di.Write(chSalt.data(), chSalt.size());
28+
di.Write(UCharCast(key_data.data()), key_data.size());
29+
di.Write(salt.data(), salt.size());
3030
di.Finalize(buf);
3131

3232
for(int i = 0; i != count - 1; i++)
@@ -38,14 +38,16 @@ int CCrypter::BytesToKeySHA512AES(const std::vector<unsigned char>& chSalt, cons
3838
return WALLET_CRYPTO_KEY_SIZE;
3939
}
4040

41-
bool CCrypter::SetKeyFromPassphrase(const SecureString& strKeyData, const std::vector<unsigned char>& chSalt, const unsigned int nRounds, const unsigned int nDerivationMethod)
41+
bool CCrypter::SetKeyFromPassphrase(const SecureString& key_data, const std::vector<unsigned char>& salt, const unsigned int rounds, const unsigned int derivation_method)
4242
{
43-
if (nRounds < 1 || chSalt.size() != WALLET_CRYPTO_SALT_SIZE)
43+
if (rounds < 1 || salt.size() != WALLET_CRYPTO_SALT_SIZE) {
4444
return false;
45+
}
4546

4647
int i = 0;
47-
if (nDerivationMethod == 0)
48-
i = BytesToKeySHA512AES(chSalt, strKeyData, nRounds, vchKey.data(), vchIV.data());
48+
if (derivation_method == 0) {
49+
i = BytesToKeySHA512AES(salt, key_data, rounds, vchKey.data(), vchIV.data());
50+
}
4951

5052
if (i != (int)WALLET_CRYPTO_KEY_SIZE)
5153
{
@@ -58,13 +60,14 @@ bool CCrypter::SetKeyFromPassphrase(const SecureString& strKeyData, const std::v
5860
return true;
5961
}
6062

61-
bool CCrypter::SetKey(const CKeyingMaterial& chNewKey, const std::vector<unsigned char>& chNewIV)
63+
bool CCrypter::SetKey(const CKeyingMaterial& new_key, const std::vector<unsigned char>& new_iv)
6264
{
63-
if (chNewKey.size() != WALLET_CRYPTO_KEY_SIZE || chNewIV.size() != WALLET_CRYPTO_IV_SIZE)
65+
if (new_key.size() != WALLET_CRYPTO_KEY_SIZE || new_iv.size() != WALLET_CRYPTO_IV_SIZE) {
6466
return false;
67+
}
6568

66-
memcpy(vchKey.data(), chNewKey.data(), chNewKey.size());
67-
memcpy(vchIV.data(), chNewIV.data(), chNewIV.size());
69+
memcpy(vchKey.data(), new_key.data(), new_key.size());
70+
memcpy(vchIV.data(), new_iv.data(), new_iv.size());
6871

6972
fKeySet = true;
7073
return true;
@@ -88,19 +91,20 @@ bool CCrypter::Encrypt(const CKeyingMaterial& vchPlaintext, std::vector<unsigned
8891
return true;
8992
}
9093

91-
bool CCrypter::Decrypt(const std::vector<unsigned char>& vchCiphertext, CKeyingMaterial& vchPlaintext) const
94+
bool CCrypter::Decrypt(const std::vector<unsigned char>& ciphertext, CKeyingMaterial& plaintext) const
9295
{
9396
if (!fKeySet)
9497
return false;
9598

9699
// plaintext will always be equal to or lesser than length of ciphertext
97-
vchPlaintext.resize(vchCiphertext.size());
100+
plaintext.resize(ciphertext.size());
98101

99102
AES256CBCDecrypt dec(vchKey.data(), vchIV.data(), true);
100-
int nLen = dec.Decrypt(vchCiphertext.data(), vchCiphertext.size(), vchPlaintext.data());
101-
if(nLen == 0)
103+
int len = dec.Decrypt(ciphertext.data(), ciphertext.size(), plaintext.data());
104+
if (len == 0) {
102105
return false;
103-
vchPlaintext.resize(nLen);
106+
}
107+
plaintext.resize(len);
104108
return true;
105109
}
106110

@@ -114,26 +118,29 @@ bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vch
114118
return cKeyCrypter.Encrypt(vchPlaintext, vchCiphertext);
115119
}
116120

117-
bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char>& vchCiphertext, const uint256& nIV, CKeyingMaterial& vchPlaintext)
121+
bool DecryptSecret(const CKeyingMaterial& master_key, const std::vector<unsigned char>& ciphertext, const uint256& iv, CKeyingMaterial& plaintext)
118122
{
119-
CCrypter cKeyCrypter;
120-
static_assert(WALLET_CRYPTO_IV_SIZE <= std::remove_reference_t<decltype(nIV)>::size());
121-
std::vector<unsigned char> chIV{nIV.begin(), nIV.begin() + WALLET_CRYPTO_IV_SIZE};
122-
if(!cKeyCrypter.SetKey(vMasterKey, chIV))
123+
CCrypter key_crypter;
124+
static_assert(WALLET_CRYPTO_IV_SIZE <= std::remove_reference_t<decltype(iv)>::size());
125+
std::vector<unsigned char> iv_prefix{iv.begin(), iv.begin() + WALLET_CRYPTO_IV_SIZE};
126+
if (!key_crypter.SetKey(master_key, iv_prefix)) {
123127
return false;
124-
return cKeyCrypter.Decrypt(vchCiphertext, vchPlaintext);
128+
}
129+
return key_crypter.Decrypt(ciphertext, plaintext);
125130
}
126131

127-
bool DecryptKey(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char>& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key)
132+
bool DecryptKey(const CKeyingMaterial& master_key, const std::vector<unsigned char>& crypted_secret, const CPubKey& pub_key, CKey& key)
128133
{
129-
CKeyingMaterial vchSecret;
130-
if(!DecryptSecret(vMasterKey, vchCryptedSecret, vchPubKey.GetHash(), vchSecret))
134+
CKeyingMaterial secret;
135+
if (!DecryptSecret(master_key, crypted_secret, pub_key.GetHash(), secret)) {
131136
return false;
137+
}
132138

133-
if (vchSecret.size() != 32)
139+
if (secret.size() != 32) {
134140
return false;
141+
}
135142

136-
key.Set(vchSecret.begin(), vchSecret.end(), vchPubKey.IsCompressed());
137-
return key.VerifyPubKey(vchPubKey);
143+
key.Set(secret.begin(), secret.end(), pub_key.IsCompressed());
144+
return key.VerifyPubKey(pub_key);
138145
}
139146
} // namespace wallet

‎src/wallet/crypter.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,13 @@ friend class wallet_crypto_tests::TestCrypter; // for test access to chKey/chIV
7575
std::vector<unsigned char, secure_allocator<unsigned char>> vchIV;
7676
bool fKeySet;
7777

78-
int BytesToKeySHA512AES(const std::vector<unsigned char>& chSalt, const SecureString& strKeyData, int count, unsigned char *key,unsigned char *iv) const;
78+
int BytesToKeySHA512AES(const std::vector<unsigned char>& salt, const SecureString& key_data, int count, unsigned char* key, unsigned char* iv) const;
7979

8080
public:
81-
bool SetKeyFromPassphrase(const SecureString &strKeyData, const std::vector<unsigned char>& chSalt, const unsigned int nRounds, const unsigned int nDerivationMethod);
81+
bool SetKeyFromPassphrase(const SecureString& key_data, const std::vector<unsigned char>& salt, const unsigned int rounds, const unsigned int derivation_method);
8282
bool Encrypt(const CKeyingMaterial& vchPlaintext, std::vector<unsigned char> &vchCiphertext) const;
83-
bool Decrypt(const std::vector<unsigned char>& vchCiphertext, CKeyingMaterial& vchPlaintext) const;
84-
bool SetKey(const CKeyingMaterial& chNewKey, const std::vector<unsigned char>& chNewIV);
83+
bool Decrypt(const std::vector<unsigned char>& ciphertext, CKeyingMaterial& plaintext) const;
84+
bool SetKey(const CKeyingMaterial& new_key, const std::vector<unsigned char>& new_iv);
8585

8686
void CleanKey()
8787
{
@@ -104,8 +104,8 @@ friend class wallet_crypto_tests::TestCrypter; // for test access to chKey/chIV
104104
};
105105

106106
bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vchPlaintext, const uint256& nIV, std::vector<unsigned char> &vchCiphertext);
107-
bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char>& vchCiphertext, const uint256& nIV, CKeyingMaterial& vchPlaintext);
108-
bool DecryptKey(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char>& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key);
107+
bool DecryptSecret(const CKeyingMaterial& master_key, const std::vector<unsigned char>& ciphertext, const uint256& iv, CKeyingMaterial& plaintext);
108+
bool DecryptKey(const CKeyingMaterial& master_key, const std::vector<unsigned char>& crypted_secret, const CPubKey& pub_key, CKey& key);
109109
} // namespace wallet
110110

111111
#endif // BITCOIN_WALLET_CRYPTER_H

‎src/wallet/test/fuzz/crypter.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ FUZZ_TARGET(crypter, .init = initialize_crypter)
3535

3636
const unsigned int derivation_method = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<unsigned int>();
3737

38-
// Limiting the value of nRounds since it is otherwise uselessly expensive and causes a timeout when fuzzing.
39-
crypt.SetKeyFromPassphrase(/*strKeyData=*/secure_string,
40-
/*chSalt=*/ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_SALT_SIZE),
41-
/*nRounds=*/fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 25000),
42-
/*nDerivationMethod=*/derivation_method);
38+
// Limiting the value of rounds since it is otherwise uselessly expensive and causes a timeout when fuzzing.
39+
crypt.SetKeyFromPassphrase(/*key_data=*/secure_string,
40+
/*salt=*/ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_SALT_SIZE),
41+
/*rounds=*/fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 25000),
42+
/*derivation_method=*/derivation_method);
4343
}
4444

4545
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 100)

‎src/wallet/test/wallet_crypto_tests.cpp‎

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,58 +17,64 @@ BOOST_FIXTURE_TEST_SUITE(wallet_crypto_tests, BasicTestingSetup)
1717
class TestCrypter
1818
{
1919
public:
20-
static void TestPassphraseSingle(const std::vector<unsigned char>& vchSalt, const SecureString& passphrase, uint32_t rounds,
21-
const std::vector<unsigned char>& correctKey = {},
22-
const std::vector<unsigned char>& correctIV = {})
20+
static void TestPassphraseSingle(const std::vector<unsigned char>& salt, const SecureString& passphrase, uint32_t rounds,
21+
const std::vector<unsigned char>& correct_key = {},
22+
const std::vector<unsigned char>& correct_iv = {})
2323
{
2424
CCrypter crypt;
25-
crypt.SetKeyFromPassphrase(passphrase, vchSalt, rounds, 0);
26-
27-
if(!correctKey.empty())
28-
BOOST_CHECK_MESSAGE(memcmp(crypt.vchKey.data(), correctKey.data(), crypt.vchKey.size()) == 0,
29-
HexStr(crypt.vchKey) + std::string(" != ") + HexStr(correctKey));
30-
if(!correctIV.empty())
31-
BOOST_CHECK_MESSAGE(memcmp(crypt.vchIV.data(), correctIV.data(), crypt.vchIV.size()) == 0,
32-
HexStr(crypt.vchIV) + std::string(" != ") + HexStr(correctIV));
25+
crypt.SetKeyFromPassphrase(passphrase, salt, rounds, 0);
26+
27+
if (!correct_key.empty()) {
28+
BOOST_CHECK_MESSAGE(memcmp(crypt.vchKey.data(), correct_key.data(), crypt.vchKey.size()) == 0,
29+
HexStr(crypt.vchKey) + std::string(" != ") + HexStr(correct_key));
30+
}
31+
if (!correct_iv.empty()) {
32+
BOOST_CHECK_MESSAGE(memcmp(crypt.vchIV.data(), correct_iv.data(), crypt.vchIV.size()) == 0,
33+
HexStr(crypt.vchIV) + std::string(" != ") + HexStr(correct_iv));
34+
}
3335
}
3436

35-
static void TestPassphrase(const std::vector<unsigned char>& vchSalt, const SecureString& passphrase, uint32_t rounds,
36-
const std::vector<unsigned char>& correctKey = {},
37-
const std::vector<unsigned char>& correctIV = {})
37+
static void TestPassphrase(const std::vector<unsigned char>& salt, const SecureString& passphrase, uint32_t rounds,
38+
const std::vector<unsigned char>& correct_key = {},
39+
const std::vector<unsigned char>& correct_iv = {})
3840
{
39-
TestPassphraseSingle(vchSalt, passphrase, rounds, correctKey, correctIV);
40-
for(SecureString::const_iterator i(passphrase.begin()); i != passphrase.end(); ++i)
41-
TestPassphraseSingle(vchSalt, SecureString(i, passphrase.end()), rounds);
41+
TestPassphraseSingle(salt, passphrase, rounds, correct_key, correct_iv);
42+
for (SecureString::const_iterator it{passphrase.begin()}; it != passphrase.end(); ++it) {
43+
TestPassphraseSingle(salt, SecureString{it, passphrase.end()}, rounds);
44+
}
4245
}
4346

44-
static void TestDecrypt(const CCrypter& crypt, const std::vector<unsigned char>& vchCiphertext,
45-
const std::vector<unsigned char>& vchCorrectPlaintext = {})
47+
static void TestDecrypt(const CCrypter& crypt, const std::vector<unsigned char>& ciphertext,
48+
const std::vector<unsigned char>& correct_plaintext = {})
4649
{
47-
CKeyingMaterial vchDecrypted;
48-
crypt.Decrypt(vchCiphertext, vchDecrypted);
49-
if (vchPlaintext.size())
50-
BOOST_CHECK_EQUAL_COLLECTIONS(vchDecrypted.begin(), vchDecrypted.end(), vchCorrectPlaintext.begin(), vchCorrectPlaintext.end());
50+
CKeyingMaterial decrypted;
51+
crypt.Decrypt(ciphertext, decrypted);
52+
if (!correct_plaintext.empty()) {
53+
BOOST_CHECK_EQUAL_COLLECTIONS(decrypted.begin(), decrypted.end(), correct_plaintext.begin(), correct_plaintext.end());
54+
}
5155
}
5256

53-
static void TestEncryptSingle(const CCrypter& crypt, const CKeyingMaterial& vchPlaintext,
54-
const std::vector<unsigned char>& vchCiphertextCorrect = {})
57+
static void TestEncryptSingle(const CCrypter& crypt, const CKeyingMaterial& plaintext,
58+
const std::vector<unsigned char>& correct_ciphertext = {})
5559
{
56-
std::vector<unsigned char> vchCiphertext;
57-
crypt.Encrypt(vchPlaintext, vchCiphertext);
60+
std::vector<unsigned char> ciphertext;
61+
crypt.Encrypt(plaintext, ciphertext);
5862

59-
if (!vchCiphertextCorrect.empty())
60-
BOOST_CHECK_EQUAL_COLLECTIONS(vchCiphertext.begin(), vchCiphertext.end(), vchCiphertextCorrect.begin(), vchCiphertextCorrect.end());
63+
if (!correct_ciphertext.empty()) {
64+
BOOST_CHECK_EQUAL_COLLECTIONS(ciphertext.begin(), ciphertext.end(), correct_ciphertext.begin(), correct_ciphertext.end());
65+
}
6166

62-
const std::vector<unsigned char> vchPlaintext2(vchPlaintext.begin(), vchPlaintext.end());
63-
TestDecrypt(crypt, vchCiphertext, vchPlaintext2);
67+
const std::vector<unsigned char> plaintext2(plaintext.begin(), plaintext.end());
68+
TestDecrypt(crypt, ciphertext, /*correct_plaintext=*/plaintext2);
6469
}
6570

66-
static void TestEncrypt(const CCrypter& crypt, const std::vector<unsigned char>& vchPlaintextIn,
67-
const std::vector<unsigned char>& vchCiphertextCorrect = {})
71+
static void TestEncrypt(const CCrypter& crypt, const std::vector<unsigned char>& plaintext,
72+
const std::vector<unsigned char>& correct_ciphertext = {})
6873
{
69-
TestEncryptSingle(crypt, CKeyingMaterial{vchPlaintextIn.begin(), vchPlaintextIn.end()}, vchCiphertextCorrect);
70-
for(std::vector<unsigned char>::const_iterator i(vchPlaintextIn.begin()); i != vchPlaintextIn.end(); ++i)
71-
TestEncryptSingle(crypt, CKeyingMaterial(i, vchPlaintextIn.end()));
74+
TestEncryptSingle(crypt, CKeyingMaterial{plaintext.begin(), plaintext.end()}, correct_ciphertext);
75+
for (auto it{plaintext.begin()}; it != plaintext.end(); ++it) {
76+
TestEncryptSingle(crypt, CKeyingMaterial{it, plaintext.end()});
77+
}
7278
}
7379

7480
};

0 commit comments

Comments
 (0)