aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormoneromooo-monero <moneromooo-monero@users.noreply.github.com>2017-10-26 10:21:06 +0100
committerJonathan Roelofs <jonathan@codesourcery.com>2017-12-16 15:40:33 -0700
commit7193b89fe567327bb78f4c61c887b2e2fad2ed51 (patch)
treec3064389fdc8d4a07b78882bc77cf475cb4e8d9d
parentMerge pull request #2881 (diff)
downloadmonero-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.h2
-rw-r--r--src/common/memwipe.h43
-rw-r--r--src/crypto/chacha8.h20
-rw-r--r--src/crypto/crypto.h7
-rw-r--r--src/wallet/wallet2.cpp10
-rw-r--r--tests/crypto/CMakeLists.txt1
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