diff --git a/src/cryptonote_basic/account.cpp b/src/cryptonote_basic/account.cpp index b366985ab..36ff41684 100644 --- a/src/cryptonote_basic/account.cpp +++ b/src/cryptonote_basic/account.cpp @@ -61,8 +61,7 @@ DISABLE_VS_WARNINGS(4244 4345) m_device = &hwdev; MCDEBUG("device", "account_keys::set_device device type: "<(); - - // Now do the chacha - chacha_wrapper(key, all_keys); - } + // derive a new key + crypto::chacha_key key; + derive_key(base_key, key); - // Start chacha for decryption - void account_keys::decrypt_wrapper(const crypto::chacha_key &key, const bool all_keys) - { - 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; } - - // Decrypt keys using the legacy method - void account_keys::decrypt_legacy(const crypto::chacha_key &key) + //----------------------------------------------------------------- + void account_keys::xor_with_key_stream(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) - { - // 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 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()); - } - - // 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)); + m_encryption_iv = crypto::rand(); + xor_with_key_stream(key); } - + //----------------------------------------------------------------- + void account_keys::decrypt(const crypto::chacha_key &key) + { + xor_with_key_stream(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(); diff --git a/src/cryptonote_basic/account.h b/src/cryptonote_basic/account.h index c71c06edd..5288b9b04 100644 --- a/src/cryptonote_basic/account.h +++ b/src/cryptonote_basic/account.h @@ -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 &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 inline void serialize(t_archive &a, const unsigned int /*ver*/) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 353f20b51..18072f350 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -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(); diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 44ae342c3..4a10e3d23 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -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 diff --git a/tests/unit_tests/account.cpp b/tests/unit_tests/account.cpp index 68bf4dce7..2ab2f893a 100644 --- a/tests/unit_tests/account.cpp +++ b/tests/unit_tests/account.cpp @@ -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 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); } diff --git a/tests/unit_tests/serialization.cpp b/tests/unit_tests/serialization.cpp index b460559ff..ee205e666 100644 --- a/tests/unit_tests/serialization.cpp +++ b/tests/unit_tests/serialization.cpp @@ -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;