Browse Source

wallet: Change CCrypter to use vectors with secure allocator

Change CCrypter to use vectors with secure allocator instead of buffers
on in the object itself which will end up on the stack. This avoids
having to call LockedPageManager to lock stack memory pages to prevent the
memory from being swapped to disk. This is wasteful.
0.14
Wladimir J. van der Laan 8 years ago
parent
commit
999e4c91c2
  1. 14
      src/wallet/crypter.cpp
  2. 19
      src/wallet/crypter.h
  3. 12
      src/wallet/test/crypto_tests.cpp

14
src/wallet/crypter.cpp

@ -48,12 +48,12 @@ bool CCrypter::SetKeyFromPassphrase(const SecureString& strKeyData, const std::v
int i = 0; int i = 0;
if (nDerivationMethod == 0) if (nDerivationMethod == 0)
i = BytesToKeySHA512AES(chSalt, strKeyData, nRounds, chKey, chIV); i = BytesToKeySHA512AES(chSalt, strKeyData, nRounds, vchKey.data(), vchIV.data());
if (i != (int)WALLET_CRYPTO_KEY_SIZE) if (i != (int)WALLET_CRYPTO_KEY_SIZE)
{ {
memory_cleanse(chKey, sizeof(chKey)); memory_cleanse(vchKey.data(), vchKey.size());
memory_cleanse(chIV, sizeof(chIV)); memory_cleanse(vchIV.data(), vchIV.size());
return false; return false;
} }
@ -66,8 +66,8 @@ bool CCrypter::SetKey(const CKeyingMaterial& chNewKey, const std::vector<unsigne
if (chNewKey.size() != WALLET_CRYPTO_KEY_SIZE || chNewIV.size() != WALLET_CRYPTO_IV_SIZE) if (chNewKey.size() != WALLET_CRYPTO_KEY_SIZE || chNewIV.size() != WALLET_CRYPTO_IV_SIZE)
return false; return false;
memcpy(&chKey[0], &chNewKey[0], sizeof chKey); memcpy(vchKey.data(), chNewKey.data(), chNewKey.size());
memcpy(&chIV[0], &chNewIV[0], sizeof chIV); memcpy(vchIV.data(), chNewIV.data(), chNewIV.size());
fKeySet = true; fKeySet = true;
return true; return true;
@ -82,7 +82,7 @@ bool CCrypter::Encrypt(const CKeyingMaterial& vchPlaintext, std::vector<unsigned
// n + AES_BLOCKSIZE bytes // n + AES_BLOCKSIZE bytes
vchCiphertext.resize(vchPlaintext.size() + AES_BLOCKSIZE); vchCiphertext.resize(vchPlaintext.size() + AES_BLOCKSIZE);
AES256CBCEncrypt enc(chKey, chIV, true); AES256CBCEncrypt enc(vchKey.data(), vchIV.data(), true);
size_t nLen = enc.Encrypt(&vchPlaintext[0], vchPlaintext.size(), &vchCiphertext[0]); size_t nLen = enc.Encrypt(&vchPlaintext[0], vchPlaintext.size(), &vchCiphertext[0]);
if(nLen < vchPlaintext.size()) if(nLen < vchPlaintext.size())
return false; return false;
@ -101,7 +101,7 @@ bool CCrypter::Decrypt(const std::vector<unsigned char>& vchCiphertext, CKeyingM
vchPlaintext.resize(nLen); vchPlaintext.resize(nLen);
AES256CBCDecrypt dec(chKey, chIV, true); AES256CBCDecrypt dec(vchKey.data(), vchIV.data(), true);
nLen = dec.Decrypt(&vchCiphertext[0], vchCiphertext.size(), &vchPlaintext[0]); nLen = dec.Decrypt(&vchCiphertext[0], vchCiphertext.size(), &vchPlaintext[0]);
if(nLen == 0) if(nLen == 0)
return false; return false;

19
src/wallet/crypter.h

@ -77,8 +77,8 @@ class CCrypter
{ {
friend class wallet_crypto::TestCrypter; // for test access to chKey/chIV friend class wallet_crypto::TestCrypter; // for test access to chKey/chIV
private: private:
unsigned char chKey[WALLET_CRYPTO_KEY_SIZE]; std::vector<unsigned char, secure_allocator<unsigned char>> vchKey;
unsigned char chIV[WALLET_CRYPTO_IV_SIZE]; std::vector<unsigned char, secure_allocator<unsigned char>> vchIV;
bool fKeySet; bool fKeySet;
int BytesToKeySHA512AES(const std::vector<unsigned char>& chSalt, const SecureString& strKeyData, int count, unsigned char *key,unsigned char *iv) const; int BytesToKeySHA512AES(const std::vector<unsigned char>& chSalt, const SecureString& strKeyData, int count, unsigned char *key,unsigned char *iv) const;
@ -91,28 +91,21 @@ public:
void CleanKey() void CleanKey()
{ {
memory_cleanse(chKey, sizeof(chKey)); memory_cleanse(vchKey.data(), vchKey.size());
memory_cleanse(chIV, sizeof(chIV)); memory_cleanse(vchIV.data(), vchIV.size());
fKeySet = false; fKeySet = false;
} }
CCrypter() CCrypter()
{ {
fKeySet = false; fKeySet = false;
vchKey.resize(WALLET_CRYPTO_KEY_SIZE);
// Try to keep the key data out of swap (and be a bit over-careful to keep the IV that we don't even use out of swap) vchIV.resize(WALLET_CRYPTO_IV_SIZE);
// Note that this does nothing about suspend-to-disk (which will put all our key data on disk)
// Note as well that at no point in this program is any attempt made to prevent stealing of keys by reading the memory of the running process.
LockedPageManager::Instance().LockRange(&chKey[0], sizeof chKey);
LockedPageManager::Instance().LockRange(&chIV[0], sizeof chIV);
} }
~CCrypter() ~CCrypter()
{ {
CleanKey(); CleanKey();
LockedPageManager::Instance().UnlockRange(&chKey[0], sizeof chKey);
LockedPageManager::Instance().UnlockRange(&chIV[0], sizeof chIV);
} }
}; };

12
src/wallet/test/crypto_tests.cpp

@ -97,10 +97,10 @@ static void TestPassphraseSingle(const std::vector<unsigned char>& vchSalt, cons
OldSetKeyFromPassphrase(passphrase, vchSalt, rounds, 0, chKey, chIV); OldSetKeyFromPassphrase(passphrase, vchSalt, rounds, 0, chKey, chIV);
BOOST_CHECK_MESSAGE(memcmp(chKey, crypt.chKey, sizeof(chKey)) == 0, \ BOOST_CHECK_MESSAGE(memcmp(chKey, crypt.vchKey.data(), crypt.vchKey.size()) == 0, \
HexStr(chKey, chKey+sizeof(chKey)) + std::string(" != ") + HexStr(crypt.chKey, crypt.chKey + (sizeof crypt.chKey))); HexStr(chKey, chKey+sizeof(chKey)) + std::string(" != ") + HexStr(crypt.vchKey));
BOOST_CHECK_MESSAGE(memcmp(chIV, crypt.chIV, sizeof(chIV)) == 0, \ BOOST_CHECK_MESSAGE(memcmp(chIV, crypt.vchIV.data(), crypt.vchIV.size()) == 0, \
HexStr(chIV, chIV+sizeof(chIV)) + std::string(" != ") + HexStr(crypt.chIV, crypt.chIV + (sizeof crypt.chIV))); HexStr(chIV, chIV+sizeof(chIV)) + std::string(" != ") + HexStr(crypt.vchIV));
if(!correctKey.empty()) if(!correctKey.empty())
BOOST_CHECK_MESSAGE(memcmp(chKey, &correctKey[0], sizeof(chKey)) == 0, \ BOOST_CHECK_MESSAGE(memcmp(chKey, &correctKey[0], sizeof(chKey)) == 0, \
@ -127,7 +127,7 @@ static void TestDecrypt(const CCrypter& crypt, const std::vector<unsigned char>&
CKeyingMaterial vchDecrypted2; CKeyingMaterial vchDecrypted2;
int result1, result2; int result1, result2;
result1 = crypt.Decrypt(vchCiphertext, vchDecrypted1); result1 = crypt.Decrypt(vchCiphertext, vchDecrypted1);
result2 = OldDecrypt(vchCiphertext, vchDecrypted2, crypt.chKey, crypt.chIV); result2 = OldDecrypt(vchCiphertext, vchDecrypted2, crypt.vchKey.data(), crypt.vchIV.data());
BOOST_CHECK(result1 == result2); BOOST_CHECK(result1 == result2);
// These two should be equal. However, OpenSSL 1.0.1j introduced a change // These two should be equal. However, OpenSSL 1.0.1j introduced a change
@ -152,7 +152,7 @@ static void TestEncryptSingle(const CCrypter& crypt, const CKeyingMaterial& vchP
std::vector<unsigned char> vchCiphertext2; std::vector<unsigned char> vchCiphertext2;
int result1 = crypt.Encrypt(vchPlaintext, vchCiphertext1); int result1 = crypt.Encrypt(vchPlaintext, vchCiphertext1);
int result2 = OldEncrypt(vchPlaintext, vchCiphertext2, crypt.chKey, crypt.chIV); int result2 = OldEncrypt(vchPlaintext, vchCiphertext2, crypt.vchKey.data(), crypt.vchIV.data());
BOOST_CHECK(result1 == result2); BOOST_CHECK(result1 == result2);
BOOST_CHECK(vchCiphertext1 == vchCiphertext2); BOOST_CHECK(vchCiphertext1 == vchCiphertext2);

Loading…
Cancel
Save