nihilist/monero
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "Use domain-separated ChaCha20 for in-memory key encryption"

Browse Source 
This reverts commit 921dd8dde5.
This commit is contained in:
luigi1111 2020-08-17 14:08:59 -05:00
parent 43a4fd9e16
commit 765db1ae7a
No known key found for this signature in database
GPG Key ID: F4ACA0183641E010
6 changed files with 50 additions and 172 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

98
src/cryptonote_basic/account.cpp
View File

@ -61,8 +61,7 @@ DISABLE_VS_WARNINGS(4244 4345)
m_device = &hwdev;
MCDEBUG("device", "account_keys::set_device device type: "<<typeid(hwdev).name());
}
// Generate a derived chacha key
//-----------------------------------------------------------------
static void derive_key(const crypto::chacha_key &base_key, crypto::chacha_key &key)
{
static_assert(sizeof(base_key) == sizeof(crypto::hash), "chacha key and hash should be the same size");
@ -71,38 +70,25 @@ DISABLE_VS_WARNINGS(4244 4345)
data[sizeof(base_key)] = config::HASH_KEY_MEMORY;
crypto::generate_chacha_key(data.data(), sizeof(data), key, 1);
}
// Prepare IVs and start chacha for encryption
void account_keys::encrypt_wrapper(const crypto::chacha_key &key, const bool all_keys)
//-----------------------------------------------------------------
static epee::wipeable_string get_key_stream(const crypto::chacha_key &base_key, const crypto::chacha_iv &iv, size_t bytes)
{
// Set a fresh IV only for all-key encryption
if (all_keys)
m_encryption_iv = crypto::rand<crypto::chacha_iv>();
// derive a new key
crypto::chacha_key key;
derive_key(base_key, key);
// Now do the chacha
chacha_wrapper(key, all_keys);
// chacha
epee::wipeable_string buffer0(std::string(bytes, '\0'));
epee::wipeable_string buffer1 = buffer0;
crypto::chacha20(buffer0.data(), buffer0.size(), key, iv, buffer1.data());
return buffer1;
}
// Start chacha for decryption
void account_keys::decrypt_wrapper(const crypto::chacha_key &key, const bool all_keys)
//-----------------------------------------------------------------
void account_keys::xor_with_key_stream(const crypto::chacha_key &key)
{
chacha_wrapper(key, all_keys);
}
// Decrypt keys using the legacy method
void account_keys::decrypt_legacy(const crypto::chacha_key &key)
{
// Derive domain-separated chacha key
crypto::chacha_key derived_key;
derive_key(key, derived_key);
// Build key stream
epee::wipeable_string temp(std::string(sizeof(crypto::secret_key)*(2 + m_multisig_keys.size()), '\0'));
epee::wipeable_string stream = temp;
crypto::chacha20(temp.data(), temp.size(), derived_key, m_encryption_iv, stream.data());
// Decrypt all keys
const char *ptr = stream.data();
// encrypt a large enough byte stream with chacha20
epee::wipeable_string key_stream = get_key_stream(key, m_encryption_iv, sizeof(crypto::secret_key) * (2 + m_multisig_keys.size()));
const char *ptr = key_stream.data();
for (size_t i = 0; i < sizeof(crypto::secret_key); ++i)
m_spend_secret_key.data[i] ^= *ptr++;
for (size_t i = 0; i < sizeof(crypto::secret_key); ++i)
@ -113,39 +99,33 @@ DISABLE_VS_WARNINGS(4244 4345)
k.data[i] ^= *ptr++;
}
}
// Perform chacha on either the view key or all keys
void account_keys::chacha_wrapper(const crypto::chacha_key &key, const bool all_keys)
//-----------------------------------------------------------------
void account_keys::encrypt(const crypto::chacha_key &key)
{
// Derive domain-seprated chacha key
crypto::chacha_key derived_key;
derive_key(key, derived_key);
// Chacha the specified keys using the appropriate IVs
if (all_keys)
m_encryption_iv = crypto::rand<crypto::chacha_iv>();
xor_with_key_stream(key);
}
//-----------------------------------------------------------------
void account_keys::decrypt(const crypto::chacha_key &key)
{
// Spend key
crypto::secret_key temp_key;
chacha20((char *) &m_spend_secret_key, sizeof(crypto::secret_key), derived_key, m_encryption_iv, (char *) &temp_key);
memcpy(&m_spend_secret_key, &temp_key, sizeof(crypto::secret_key));
memwipe(&temp_key, sizeof(crypto::secret_key));
// Multisig keys
std::vector<crypto::secret_key> temp_keys;
temp_keys.reserve(m_multisig_keys.size());
temp_keys.resize(m_multisig_keys.size());
chacha20((char *) &m_multisig_keys[0], sizeof(crypto::secret_key)*m_multisig_keys.size(), derived_key, m_encryption_iv, (char *) &temp_keys[0]);
memcpy(&m_multisig_keys[0], &temp_keys[0], sizeof(crypto::secret_key)*temp_keys.size());
memwipe(&temp_keys[0], sizeof(crypto::secret_key)*temp_keys.size());
xor_with_key_stream(key);
}
// View key
crypto::secret_key temp_key;
chacha20((char *) &m_view_secret_key, sizeof(crypto::secret_key), derived_key, m_encryption_iv, (char *) &temp_key);
memcpy(&m_view_secret_key, &temp_key, sizeof(crypto::secret_key));
memwipe(&temp_key, sizeof(crypto::secret_key));
//-----------------------------------------------------------------
void account_keys::encrypt_viewkey(const crypto::chacha_key &key)
{
// encrypt a large enough byte stream with chacha20
epee::wipeable_string key_stream = get_key_stream(key, m_encryption_iv, sizeof(crypto::secret_key) * 2);
const char *ptr = key_stream.data();
ptr += sizeof(crypto::secret_key);
for (size_t i = 0; i < sizeof(crypto::secret_key); ++i)
m_view_secret_key.data[i] ^= *ptr++;
}
//-----------------------------------------------------------------
void account_keys::decrypt_viewkey(const crypto::chacha_key &key)
{
encrypt_viewkey(key);
}
//-----------------------------------------------------------------
account_base::account_base()
{
set_null();

19
src/cryptonote_basic/account.h
View File

@ -57,15 +57,16 @@ namespace cryptonote
account_keys& operator=(account_keys const&) = default;
void encrypt_wrapper(const crypto::chacha_key &key, const bool all_keys);
void decrypt_wrapper(const crypto::chacha_key &key, const bool all_keys);
void decrypt_legacy(const crypto::chacha_key &key);
void encrypt(const crypto::chacha_key &key);
void decrypt(const crypto::chacha_key &key);
void encrypt_viewkey(const crypto::chacha_key &key);
void decrypt_viewkey(const crypto::chacha_key &key);
hw::device& get_device() const ;
void set_device( hw::device &hwdev) ;
private:
void chacha_wrapper(const crypto::chacha_key &key, const bool all_keys);
void xor_with_key_stream(const crypto::chacha_key &key);
};
/************************************************************************/
@ -99,12 +100,10 @@ namespace cryptonote
void forget_spend_key();
const std::vector<crypto::secret_key> &get_multisig_keys() const { return m_keys.m_multisig_keys; }
void encrypt_keys(const crypto::chacha_key &key) { m_keys.encrypt_wrapper(key, true); }
void encrypt_keys_same_iv(const crypto::chacha_key &key) { m_keys.decrypt_wrapper(key, true); } // encryption with the same IV is the same as decryption due to symmetry
void decrypt_keys(const crypto::chacha_key &key) { m_keys.decrypt_wrapper(key, true); }
void encrypt_viewkey(const crypto::chacha_key &key) { m_keys.encrypt_wrapper(key, false); }
void decrypt_viewkey(const crypto::chacha_key &key) { m_keys.decrypt_wrapper(key, false); }
void decrypt_legacy(const crypto::chacha_key &key) { m_keys.decrypt_legacy(key); }
void encrypt_keys(const crypto::chacha_key &key) { m_keys.encrypt(key); }
void decrypt_keys(const crypto::chacha_key &key) { m_keys.decrypt(key); }
void encrypt_viewkey(const crypto::chacha_key &key) { m_keys.encrypt_viewkey(key); }
void decrypt_viewkey(const crypto::chacha_key &key) { m_keys.decrypt_viewkey(key); }
template <class t_archive>
inline void serialize(t_archive &a, const unsigned int /*ver*/)

16
src/wallet/wallet2.cpp
View File

@ -4336,24 +4336,9 @@ bool wallet2::load_keys_buf(const std::string& keys_buf, const epee::wipeable_st
if (r)
{
// Decrypt keys, using one of two possible methods
if (encrypted_secret_keys)
{
// First try the updated method
m_account.decrypt_keys(key);
load_info.is_legacy_key_encryption = false;
// Test address construction to see if decryption succeeded
const cryptonote::account_keys &keys = m_account.get_keys();
hw::device &hwdev = m_account.get_device();
if (!hwdev.verify_keys(keys.m_view_secret_key, keys.m_account_address.m_view_public_key) || !hwdev.verify_keys(keys.m_spend_secret_key, keys.m_account_address.m_spend_public_key))
{
// Updated method failed; try the legacy method
// Note that we must first encrypt the keys again with the same IV
m_account.encrypt_keys_same_iv(key);
m_account.decrypt_legacy(key);
load_info.is_legacy_key_encryption = true;
}
}
else
{
@ -5557,7 +5542,6 @@ void wallet2::load(const std::string& wallet_, const epee::wipeable_string& pass
{
clear();
prepare_file_names(wallet_);
MINFO("Keys file: " << m_keys_file);
// determine if loading from file system or string buffer
bool use_fs = !wallet_.empty();

11
src/wallet/wallet2.h
View File

@ -219,15 +219,6 @@ private:
friend class wallet_keys_unlocker;
friend class wallet_device_callback;
public:
// Contains data on how keys were loaded, primarily for unit test purposes
struct load_info_t {
bool is_legacy_key_encryption;
};
const load_info_t &get_load_info() const {
return load_info;
}
static constexpr const std::chrono::seconds rpc_timeout = std::chrono::minutes(3) + std::chrono::seconds(30);
enum RefreshType {
@ -1417,8 +1408,6 @@ private:
static std::string get_default_daemon_address() { CRITICAL_REGION_LOCAL(default_daemon_address_lock); return default_daemon_address; }
private:
load_info_t load_info;
/*!
* \brief Stores wallet information to wallet file.
* \param keys_file_name Name of wallet file

34
tests/unit_tests/account.cpp
View File

@ -29,30 +29,14 @@
#include "gtest/gtest.h"
#include "cryptonote_basic/account.h"
#include "ringct/rctOps.h"
// Tests in-memory encryption of account secret keys
TEST(account, encrypt_keys)
{
// Generate account keys and random multisig keys
cryptonote::keypair recovery_key = cryptonote::keypair::generate(hw::get_device("default"));
cryptonote::account_base account;
crypto::secret_key key = account.generate(recovery_key.sec);
const size_t n_multisig = 4;
std::vector<crypto::secret_key> multisig_keys;
multisig_keys.reserve(n_multisig);
multisig_keys.resize(0);
for (size_t i = 0; i < n_multisig; ++i)
{
multisig_keys.push_back(rct::rct2sk(rct::skGen()));
}
ASSERT_TRUE(account.make_multisig(account.get_keys().m_view_secret_key, account.get_keys().m_spend_secret_key, account.get_keys().m_account_address.m_spend_public_key, multisig_keys));
const cryptonote::account_keys keys = account.get_keys();
ASSERT_EQ(keys.m_multisig_keys.size(),n_multisig);
// Encrypt and decrypt keys
ASSERT_EQ(account.get_keys().m_account_address, keys.m_account_address);
ASSERT_EQ(account.get_keys().m_spend_secret_key, keys.m_spend_secret_key);
ASSERT_EQ(account.get_keys().m_view_secret_key, keys.m_view_secret_key);
@ -66,40 +50,22 @@ TEST(account, encrypt_keys)
ASSERT_EQ(account.get_keys().m_account_address, keys.m_account_address);
ASSERT_NE(account.get_keys().m_spend_secret_key, keys.m_spend_secret_key);
ASSERT_NE(account.get_keys().m_view_secret_key, keys.m_view_secret_key);
ASSERT_NE(account.get_keys().m_multisig_keys, keys.m_multisig_keys);
account.decrypt_viewkey(chacha_key);
ASSERT_EQ(account.get_keys().m_account_address, keys.m_account_address);
ASSERT_NE(account.get_keys().m_spend_secret_key, keys.m_spend_secret_key);
ASSERT_EQ(account.get_keys().m_view_secret_key, keys.m_view_secret_key);
ASSERT_NE(account.get_keys().m_multisig_keys, keys.m_multisig_keys);
account.encrypt_viewkey(chacha_key);
ASSERT_EQ(account.get_keys().m_account_address, keys.m_account_address);
ASSERT_NE(account.get_keys().m_spend_secret_key, keys.m_spend_secret_key);
ASSERT_NE(account.get_keys().m_view_secret_key, keys.m_view_secret_key);
ASSERT_NE(account.get_keys().m_multisig_keys, keys.m_multisig_keys);
account.decrypt_viewkey(chacha_key);
ASSERT_EQ(account.get_keys().m_account_address, keys.m_account_address);
ASSERT_NE(account.get_keys().m_spend_secret_key, keys.m_spend_secret_key);
ASSERT_EQ(account.get_keys().m_view_secret_key, keys.m_view_secret_key);
ASSERT_NE(account.get_keys().m_multisig_keys, keys.m_multisig_keys);
account.encrypt_viewkey(chacha_key);
ASSERT_EQ(account.get_keys().m_account_address, keys.m_account_address);
ASSERT_NE(account.get_keys().m_spend_secret_key, keys.m_spend_secret_key);
ASSERT_NE(account.get_keys().m_view_secret_key, keys.m_view_secret_key);
ASSERT_NE(account.get_keys().m_multisig_keys, keys.m_multisig_keys);
account.decrypt_keys(chacha_key);
ASSERT_EQ(account.get_keys().m_account_address, keys.m_account_address);
ASSERT_EQ(account.get_keys().m_spend_secret_key, keys.m_spend_secret_key);
ASSERT_EQ(account.get_keys().m_view_secret_key, keys.m_view_secret_key);
ASSERT_EQ(account.get_keys().m_multisig_keys, keys.m_multisig_keys);
}

40
tests/unit_tests/serialization.cpp
View File

@ -616,46 +616,6 @@ TEST(Serialization, serializes_ringct_types)
ASSERT_EQ(bp0, bp1);
}
TEST(Serialization, key_encryption_transition)
{
const cryptonote::network_type nettype = cryptonote::TESTNET;
tools::wallet2 w(nettype);
const boost::filesystem::path wallet_file = unit_test::data_dir / "wallet_9svHk1";
const boost::filesystem::path key_file = unit_test::data_dir / "wallet_9svHk1.keys";
const boost::filesystem::path temp_wallet_file = unit_test::data_dir / "wallet_9svHk1_temp";
const boost::filesystem::path temp_key_file = unit_test::data_dir / "wallet_9svHk1_temp.keys";
string password = "test";
bool r = false;
// Copy the original files for this test
boost::filesystem::copy(wallet_file,temp_wallet_file);
boost::filesystem::copy(key_file,temp_key_file);
try
{
// Key transition
w.load(temp_wallet_file.string(), password); // legacy decryption method
ASSERT_TRUE(w.get_load_info().is_legacy_key_encryption);
const crypto::secret_key view_secret_key = w.get_account().get_keys().m_view_secret_key;
w.rewrite(temp_wallet_file.string(), password); // transition to new key format
w.load(temp_wallet_file.string(), password); // new decryption method
ASSERT_FALSE(w.get_load_info().is_legacy_key_encryption);
ASSERT_EQ(w.get_account().get_keys().m_view_secret_key,view_secret_key);
r = true;
}
catch (const exception& e)
{}
// Remove the temporary files
boost::filesystem::remove(temp_wallet_file);
boost::filesystem::remove(temp_key_file);
ASSERT_TRUE(r);
}
TEST(Serialization, portability_wallet)
{
const cryptonote::network_type nettype = cryptonote::TESTNET;