From 07ec748c8245b42de0ec9964b2f0062c0034f2f8 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 27 Aug 2018 08:43:56 +0000 Subject: [PATCH 1/2] wipeable_string: add hex_to_pod function --- contrib/epee/include/hex.h | 1 + contrib/epee/include/wipeable_string.h | 19 ++++++++++++++++++- contrib/epee/src/wipeable_string.cpp | 3 ++- tests/unit_tests/wipeable_string.cpp | 7 +++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/contrib/epee/include/hex.h b/contrib/epee/include/hex.h index 02600c320..901c666a9 100644 --- a/contrib/epee/include/hex.h +++ b/contrib/epee/include/hex.h @@ -44,6 +44,7 @@ namespace epee static std::string string(const span src); //! \return A epee::wipeable_string containing hex of `src`. static epee::wipeable_string wipeable_string(const span src); + template static epee::wipeable_string wipeable_string(const T &pod) { return wipeable_string(span((const uint8_t*)&pod, sizeof(pod))); } //! \return An array containing hex of `src`. template diff --git a/contrib/epee/include/wipeable_string.h b/contrib/epee/include/wipeable_string.h index 4cebe5fdf..31854fe2e 100644 --- a/contrib/epee/include/wipeable_string.h +++ b/contrib/epee/include/wipeable_string.h @@ -28,10 +28,11 @@ #pragma once -#include +#include #include #include #include +#include "memwipe.h" #include "fnv1.h" namespace epee @@ -65,6 +66,8 @@ namespace epee void trim(); void split(std::vector &fields) const; boost::optional parse_hexstr() const; + template inline bool hex_to_pod(T &pod) const; + template inline bool hex_to_pod(tools::scrubbed &pod) const { return hex_to_pod(unwrap(pod)); } void resize(size_t sz); void reserve(size_t sz); void clear(); @@ -79,6 +82,20 @@ namespace epee private: std::vector buffer; }; + + template inline bool wipeable_string::hex_to_pod(T &pod) const + { + static_assert(std::is_pod::value, "expected pod type"); + if (size() != sizeof(T) * 2) + return false; + boost::optional blob = parse_hexstr(); + if (!blob) + return false; + if (blob->size() != sizeof(T)) + return false; + pod = *(const T*)blob->data(); + return true; + } } namespace std diff --git a/contrib/epee/src/wipeable_string.cpp b/contrib/epee/src/wipeable_string.cpp index 7c9722765..69f92e106 100644 --- a/contrib/epee/src/wipeable_string.cpp +++ b/contrib/epee/src/wipeable_string.cpp @@ -32,6 +32,8 @@ #include "misc_log_ex.h" #include "wipeable_string.h" +static constexpr const char hex[] = u8"0123456789abcdef"; + namespace { int atolower(int c) @@ -197,7 +199,6 @@ boost::optional wipeable_string::parse_hexstr() const const size_t len = size(); const char *d = data(); res->grow(0, len / 2); - static constexpr const char hex[] = u8"0123456789abcdef"; for (size_t i = 0; i < len; i += 2) { char c = atolower(d[i]); diff --git a/tests/unit_tests/wipeable_string.cpp b/tests/unit_tests/wipeable_string.cpp index 5ea1c1729..65718fd45 100644 --- a/tests/unit_tests/wipeable_string.cpp +++ b/tests/unit_tests/wipeable_string.cpp @@ -32,6 +32,7 @@ #include "misc_log_ex.h" #include "wipeable_string.h" +#include "hex.h" TEST(wipeable_string, ctor) { @@ -202,3 +203,9 @@ TEST(wipeable_string, parse_hexstr) ASSERT_TRUE((s = epee::wipeable_string("414243").parse_hexstr())); ASSERT_EQ(*s, epee::wipeable_string("ABC")); } + +TEST(wipeable_string, to_hex) +{ + ASSERT_TRUE(epee::to_hex::wipeable_string(epee::span((const uint8_t*)"", 0)) == epee::wipeable_string("")); + ASSERT_TRUE(epee::to_hex::wipeable_string(epee::span((const uint8_t*)"abc", 3)) == epee::wipeable_string("616263")); +} From 56b50faab20166fc77e67d12bcd9f424c621f875 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 27 Aug 2018 08:44:15 +0000 Subject: [PATCH 2/2] wallet: use wipeable_string in more places where a secret is used --- src/simplewallet/simplewallet.cpp | 39 +++++++++++++---------------- src/wallet/wallet_rpc_server.cpp | 41 +++++++++++++++++++------------ 2 files changed, 42 insertions(+), 38 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index bdf4212ce..f5d972ab4 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -2943,20 +2943,19 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) } // parse view secret key - std::string viewkey_string = input_line("View key: "); + epee::wipeable_string viewkey_string = input_secure_line("Secret view key: "); if (std::cin.eof()) return false; if (viewkey_string.empty()) { fail_msg_writer() << tr("No data supplied, cancelled"); return false; } - cryptonote::blobdata viewkey_data; - if(!epee::string_tools::parse_hexstr_to_binbuff(viewkey_string, viewkey_data) || viewkey_data.size() != sizeof(crypto::secret_key)) + crypto::secret_key viewkey; + if (viewkey_string.hex_to_pod(unwrap(unwrap(viewkey)))) { fail_msg_writer() << tr("failed to parse view key secret key"); return false; } - crypto::secret_key viewkey = *reinterpret_cast(viewkey_data.data()); m_wallet_file=m_generate_from_view_key; @@ -2979,14 +2978,14 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) { m_wallet_file = m_generate_from_spend_key; // parse spend secret key - std::string spendkey_string = input_line("Secret spend key: "); + epee::wipeable_string spendkey_string = input_secure_line("Secret spend key: "); if (std::cin.eof()) return false; if (spendkey_string.empty()) { fail_msg_writer() << tr("No data supplied, cancelled"); return false; } - if (!epee::string_tools::hex_to_pod(spendkey_string, m_recovery_key)) + if (!spendkey_string.hex_to_pod(unwrap(unwrap(m_recovery_key)))) { fail_msg_writer() << tr("failed to parse spend key secret key"); return false; @@ -3019,36 +3018,34 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) } // parse spend secret key - std::string spendkey_string = input_line("Secret spend key: "); + epee::wipeable_string spendkey_string = input_secure_line("Secret spend key: "); if (std::cin.eof()) return false; if (spendkey_string.empty()) { fail_msg_writer() << tr("No data supplied, cancelled"); return false; } - cryptonote::blobdata spendkey_data; - if(!epee::string_tools::parse_hexstr_to_binbuff(spendkey_string, spendkey_data) || spendkey_data.size() != sizeof(crypto::secret_key)) + crypto::secret_key spendkey; + if (!spendkey_string.hex_to_pod(unwrap(unwrap(spendkey)))) { fail_msg_writer() << tr("failed to parse spend key secret key"); return false; } - crypto::secret_key spendkey = *reinterpret_cast(spendkey_data.data()); // parse view secret key - std::string viewkey_string = input_line("Secret view key: "); + epee::wipeable_string viewkey_string = input_secure_line("Secret view key: "); if (std::cin.eof()) return false; if (viewkey_string.empty()) { fail_msg_writer() << tr("No data supplied, cancelled"); return false; } - cryptonote::blobdata viewkey_data; - if(!epee::string_tools::parse_hexstr_to_binbuff(viewkey_string, viewkey_data) || viewkey_data.size() != sizeof(crypto::secret_key)) + crypto::secret_key viewkey; + if(!viewkey_string.hex_to_pod(unwrap(unwrap(viewkey)))) { fail_msg_writer() << tr("failed to parse view key secret key"); return false; } - crypto::secret_key viewkey = *reinterpret_cast(viewkey_data.data()); m_wallet_file=m_generate_from_keys; @@ -3124,7 +3121,7 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) } // parse secret view key - std::string viewkey_string = input_line("Secret view key: "); + epee::wipeable_string viewkey_string = input_secure_line("Secret view key: "); if (std::cin.eof()) return false; if (viewkey_string.empty()) @@ -3132,13 +3129,12 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) fail_msg_writer() << tr("No data supplied, cancelled"); return false; } - cryptonote::blobdata viewkey_data; - if(!epee::string_tools::parse_hexstr_to_binbuff(viewkey_string, viewkey_data) || viewkey_data.size() != sizeof(crypto::secret_key)) + crypto::secret_key viewkey; + if(!viewkey_string.hex_to_pod(unwrap(unwrap(viewkey)))) { fail_msg_writer() << tr("failed to parse secret view key"); return false; } - crypto::secret_key viewkey = *reinterpret_cast(viewkey_data.data()); // check that the view key matches the given address crypto::public_key pkey; @@ -3159,12 +3155,12 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) if(multisig_m == multisig_n) { std::vector multisig_secret_spendkeys(multisig_n); - std::string spendkey_string; + epee::wipeable_string spendkey_string; cryptonote::blobdata spendkey_data; // get N secret spend keys from user for(unsigned int i=0; i(spendkey_data.data()); } // sum the spend keys together to get the master spend key diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 2cddea25d..bb064f5a8 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -773,10 +773,10 @@ namespace tools { if (get_tx_key) { - std::string s = epee::string_tools::pod_to_hex(ptx.tx_key); + epee::wipeable_string s = epee::to_hex::wipeable_string(ptx.tx_key); for (const crypto::secret_key& additional_tx_key : ptx.additional_tx_keys) - s += epee::string_tools::pod_to_hex(additional_tx_key); - fill(tx_key, s); + s += epee::to_hex::wipeable_string(additional_tx_key); + fill(tx_key, std::string(s.data(), s.size())); } // Compute amount leaving wallet in tx. By convention dests does not include change outputs fill(amount, total_amount(ptx)); @@ -1584,11 +1584,13 @@ namespace tools } else if(req.key_type.compare("view_key") == 0) { - res.key = string_tools::pod_to_hex(m_wallet->get_account().get_keys().m_view_secret_key); + epee::wipeable_string key = epee::to_hex::wipeable_string(m_wallet->get_account().get_keys().m_view_secret_key); + res.key = std::string(key.data(), key.size()); } else if(req.key_type.compare("spend_key") == 0) { - res.key = string_tools::pod_to_hex(m_wallet->get_account().get_keys().m_spend_secret_key); + epee::wipeable_string key = epee::to_hex::wipeable_string(m_wallet->get_account().get_keys().m_spend_secret_key); + res.key = std::string(key.data(), key.size()); } else { @@ -1814,11 +1816,11 @@ namespace tools return false; } - std::ostringstream oss; - oss << epee::string_tools::pod_to_hex(tx_key); + epee::wipeable_string s; + s += epee::to_hex::wipeable_string(tx_key); for (size_t i = 0; i < additional_tx_keys.size(); ++i) - oss << epee::string_tools::pod_to_hex(additional_tx_keys[i]); - res.tx_key = oss.str(); + s += epee::to_hex::wipeable_string(additional_tx_keys[i]); + res.tx_key = std::string(s.data(), s.size()); return true; } //------------------------------------------------------------------------------------------------------------------------------ @@ -1834,26 +1836,33 @@ namespace tools return false; } - std::string tx_key_str = req.tx_key; - crypto::secret_key tx_key; - if (!epee::string_tools::hex_to_pod(tx_key_str.substr(0, 64), tx_key)) + epee::wipeable_string tx_key_str = req.tx_key; + if (tx_key_str.size() < 64 || tx_key_str.size() % 64) { er.code = WALLET_RPC_ERROR_CODE_WRONG_KEY; er.message = "Tx key has invalid format"; return false; } - tx_key_str = tx_key_str.substr(64); + const char *data = tx_key_str.data(); + crypto::secret_key tx_key; + if (!epee::wipeable_string(data, 64).hex_to_pod(unwrap(unwrap(tx_key)))) + { + er.code = WALLET_RPC_ERROR_CODE_WRONG_KEY; + er.message = "Tx key has invalid format"; + return false; + } + size_t offset = 64; std::vector additional_tx_keys; - while (!tx_key_str.empty()) + while (offset < tx_key_str.size()) { additional_tx_keys.resize(additional_tx_keys.size() + 1); - if (!epee::string_tools::hex_to_pod(tx_key_str.substr(0, 64), additional_tx_keys.back())) + if (!epee::wipeable_string(data + offset, 64).hex_to_pod(unwrap(unwrap(additional_tx_keys.back())))) { er.code = WALLET_RPC_ERROR_CODE_WRONG_KEY; er.message = "Tx key has invalid format"; return false; } - tx_key_str = tx_key_str.substr(64); + offset += 64; } cryptonote::address_parse_info info;