diff options
author | moneromooo-monero <moneromooo-monero@users.noreply.github.com> | 2017-10-26 10:21:06 +0100 |
---|---|---|
committer | Jonathan Roelofs <jonathan@codesourcery.com> | 2017-12-16 15:40:33 -0700 |
commit | 7193b89fe567327bb78f4c61c887b2e2fad2ed51 (patch) | |
tree | c3064389fdc8d4a07b78882bc77cf475cb4e8d9d | |
parent | Merge pull request #2881 (diff) | |
download | monero-7193b89fe567327bb78f4c61c887b2e2fad2ed51.tar.xz |
Scrub keys from memory just before scope end.
Partially implements #74.
Securely erases keys from memory after they are no longer needed. Might have a
performance impact, which I haven't measured (perf measurements aren't
generally reliable on laptops).
Thanks to @stoffu for the suggestion to specialize the pod_to_hex/hex_to_pod
functions. Using overloads + SFINAE instead generalizes it so other types can
be marked as scrubbed without adding more boilerplate.
-rw-r--r-- | contrib/epee/include/span.h | 2 | ||||
-rw-r--r-- | src/common/memwipe.h | 43 | ||||
-rw-r--r-- | src/crypto/chacha8.h | 20 | ||||
-rw-r--r-- | src/crypto/crypto.h | 7 | ||||
-rw-r--r-- | src/wallet/wallet2.cpp | 10 | ||||
-rw-r--r-- | tests/crypto/CMakeLists.txt | 1 |
6 files changed, 60 insertions, 23 deletions
diff --git a/contrib/epee/include/span.h b/contrib/epee/include/span.h index ea4ba63dd..5660c09d1 100644 --- a/contrib/epee/include/span.h +++ b/contrib/epee/include/span.h @@ -108,7 +108,7 @@ namespace epee template<typename T> constexpr bool has_padding() noexcept { - return !std::is_pod<T>() || alignof(T) != 1; + return !std::is_pod<T>::value || alignof(T) != 1; } //! \return Cast data from `src` as `span<const std::uint8_t>`. diff --git a/src/common/memwipe.h b/src/common/memwipe.h index e9a3fba7b..c3b4ce8ab 100644 --- a/src/common/memwipe.h +++ b/src/common/memwipe.h @@ -31,6 +31,8 @@ #pragma once #ifdef __cplusplus +#include <array> + extern "C" { #endif @@ -39,3 +41,44 @@ void *memwipe(void *src, size_t n); #ifdef __cplusplus } #endif + +#ifdef __cplusplus +namespace tools { + + /// Scrubs data in the contained type upon destruction. + /// + /// Primarily useful for making sure that private keys don't stick around in + /// memory after the objects that held them have gone out of scope. + template <class T> + struct scrubbed : public T { + using type = T; + + ~scrubbed() { + scrub(); + } + + /// Destroy the contents of the contained type. + void scrub() { + static_assert(std::is_pod<T>::value, + "T cannot be auto-scrubbed. T must be POD."); + static_assert(std::is_trivially_destructible<T>::value, + "T cannot be auto-scrubbed. T must be trivially destructable."); + memwipe(this, sizeof(T)); + } + }; + + template <class T, size_t N> + using scrubbed_arr = scrubbed<std::array<T, N>>; +} // namespace tools + +// Partial specialization for std::is_pod<tools::scrubbed<T>> so that it can +// pretend to be the containted type in those contexts. +namespace std +{ + template<class t_scrubbee> + struct is_pod<tools::scrubbed<t_scrubbee>> { + static const bool value = is_pod<t_scrubbee>::value; + }; +} + +#endif // __cplusplus diff --git a/src/crypto/chacha8.h b/src/crypto/chacha8.h index 1bf695731..dcbe6a933 100644 --- a/src/crypto/chacha8.h +++ b/src/crypto/chacha8.h @@ -49,16 +49,9 @@ namespace crypto { #if defined(__cplusplus) } -#pragma pack(push, 1) - struct chacha8_key { - uint8_t data[CHACHA8_KEY_SIZE]; - - ~chacha8_key() - { - memwipe(data, sizeof(data)); - } - }; + using chacha8_key = tools::scrubbed_arr<uint8_t, CHACHA8_KEY_SIZE>; +#pragma pack(push, 1) // MS VC 2012 doesn't interpret `class chacha8_iv` as POD in spite of [9.0.10], so it is a struct struct chacha8_iv { uint8_t data[CHACHA8_IV_SIZE]; @@ -68,15 +61,14 @@ namespace crypto { static_assert(sizeof(chacha8_key) == CHACHA8_KEY_SIZE && sizeof(chacha8_iv) == CHACHA8_IV_SIZE, "Invalid structure size"); inline void chacha8(const void* data, std::size_t length, const chacha8_key& key, const chacha8_iv& iv, char* cipher) { - chacha8(data, length, reinterpret_cast<const uint8_t*>(&key), reinterpret_cast<const uint8_t*>(&iv), cipher); + chacha8(data, length, key.data(), reinterpret_cast<const uint8_t*>(&iv), cipher); } inline void generate_chacha8_key(const void *data, size_t size, chacha8_key& key) { static_assert(sizeof(chacha8_key) <= sizeof(hash), "Size of hash must be at least that of chacha8_key"); - char pwd_hash[HASH_SIZE]; - crypto::cn_slow_hash(data, size, pwd_hash); - memcpy(&key, pwd_hash, sizeof(key)); - memwipe(pwd_hash, sizeof(pwd_hash)); + tools::scrubbed_arr<char, HASH_SIZE> pwd_hash; + crypto::cn_slow_hash(data, size, pwd_hash.data()); + memcpy(&key, pwd_hash.data(), sizeof(key)); } inline void generate_chacha8_key(std::string password, chacha8_key& key) { diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h index abdea0165..0ce5e6d7a 100644 --- a/src/crypto/crypto.h +++ b/src/crypto/crypto.h @@ -36,9 +36,12 @@ #include <boost/thread/lock_guard.hpp> #include <boost/utility/value_init.hpp> #include <boost/optional.hpp> +#include <type_traits> #include <vector> #include "common/pod-class.h" +#include "common/util.h" +#include "common/memwipe.h" #include "generic-ops.h" #include "hex.h" #include "span.h" @@ -65,9 +68,7 @@ namespace crypto { friend class crypto_ops; }; - POD_CLASS secret_key: ec_scalar { - friend class crypto_ops; - }; + using secret_key = tools::scrubbed<ec_scalar>; POD_CLASS public_keyV { std::vector<public_key> keys; diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 59e759bfc..ef8a75375 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -57,6 +57,7 @@ using namespace epee; #include "rapidjson/writer.h" #include "rapidjson/stringbuffer.h" #include "common/json_util.h" +#include "common/memwipe.h" #include "common/base58.h" #include "ringct/rctSigs.h" @@ -2761,12 +2762,11 @@ bool wallet2::generate_chacha8_key_from_secret_keys(crypto::chacha8_key &key) co const account_keys &keys = m_account.get_keys(); const crypto::secret_key &view_key = keys.m_view_secret_key; const crypto::secret_key &spend_key = keys.m_spend_secret_key; - char data[sizeof(view_key) + sizeof(spend_key) + 1]; - memcpy(data, &view_key, sizeof(view_key)); - memcpy(data + sizeof(view_key), &spend_key, sizeof(spend_key)); + tools::scrubbed_arr<char, sizeof(view_key) + sizeof(spend_key) + 1> data; + memcpy(data.data(), &view_key, sizeof(view_key)); + memcpy(data.data() + sizeof(view_key), &spend_key, sizeof(spend_key)); data[sizeof(data) - 1] = CHACHA8_KEY_TAIL; - crypto::generate_chacha8_key(data, sizeof(data), key); - memset(data, 0, sizeof(data)); + crypto::generate_chacha8_key(data.data(), sizeof(data), key); return true; } //---------------------------------------------------------------------------------------------------- diff --git a/tests/crypto/CMakeLists.txt b/tests/crypto/CMakeLists.txt index 573c62ad4..f4abb3a9a 100644 --- a/tests/crypto/CMakeLists.txt +++ b/tests/crypto/CMakeLists.txt @@ -42,6 +42,7 @@ add_executable(cncrypto-tests ${crypto_headers}) target_link_libraries(cncrypto-tests PRIVATE + common ${Boost_SYSTEM_LIBRARY} ${EXTRA_LIBRARIES}) set_property(TARGET cncrypto-tests |