diff options
author | Riccardo Spagni <ric@spagni.net> | 2017-12-16 23:19:27 +0200 |
---|---|---|
committer | Riccardo Spagni <ric@spagni.net> | 2017-12-16 23:19:27 +0200 |
commit | 6c0953b15a27a49403c6ec2a48c1b0dcefa1eda5 (patch) | |
tree | 90c341b746beeb252953081f75bef7f315704068 | |
parent | Merge pull request #2856 (diff) | |
parent | new wipeable_string class to replace std::string passphrases (diff) | |
download | monero-6c0953b15a27a49403c6ec2a48c1b0dcefa1eda5.tar.xz |
Merge pull request #2860
3dffe71b new wipeable_string class to replace std::string passphrases (moneromooo-monero)
7a2a5741 utils: initialize easylogging++ in on_startup (moneromooo-monero)
54950829 use memwipe in a few relevant places (moneromooo-monero)
000666ff add a memwipe function (moneromooo-monero)
-rw-r--r-- | CMakeLists.txt | 8 | ||||
-rw-r--r-- | contrib/epee/include/net/http_auth.h | 6 | ||||
-rw-r--r-- | contrib/epee/include/wipeable_string.h | 70 | ||||
-rw-r--r-- | contrib/epee/src/CMakeLists.txt | 2 | ||||
-rw-r--r-- | contrib/epee/src/http_auth.cpp | 8 | ||||
-rw-r--r-- | contrib/epee/src/wipeable_string.cpp | 146 | ||||
-rw-r--r-- | src/common/CMakeLists.txt | 6 | ||||
-rw-r--r-- | src/common/memwipe.c | 106 | ||||
-rw-r--r-- | src/common/memwipe.h | 41 | ||||
-rw-r--r-- | src/common/password.cpp | 37 | ||||
-rw-r--r-- | src/common/password.h | 6 | ||||
-rw-r--r-- | src/common/util.cpp | 6 | ||||
-rw-r--r-- | src/crypto/chacha8.h | 5 | ||||
-rw-r--r-- | src/cryptonote_basic/cryptonote_format_utils.cpp | 5 | ||||
-rw-r--r-- | src/cryptonote_basic/cryptonote_format_utils.h | 9 | ||||
-rw-r--r-- | src/simplewallet/simplewallet.cpp | 8 | ||||
-rw-r--r-- | src/wallet/wallet2.cpp | 32 | ||||
-rw-r--r-- | src/wallet/wallet2.h | 24 | ||||
-rw-r--r-- | src/wallet/wallet_rpc_server.cpp | 3 | ||||
-rw-r--r-- | tests/unit_tests/CMakeLists.txt | 3 | ||||
-rw-r--r-- | tests/unit_tests/http.cpp | 2 | ||||
-rw-r--r-- | tests/unit_tests/memwipe.cpp | 64 |
22 files changed, 528 insertions, 69 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt index d63b50510..b11d6ba6f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -768,3 +768,11 @@ option(BUILD_GUI_DEPS "Build GUI dependencies." OFF) option(INSTALL_VENDORED_LIBUNBOUND "Install libunbound binary built from source vendored with this repo." OFF) +include(CheckCCompilerFlag) + +CHECK_C_COMPILER_FLAG(-std=c11 HAVE_C11) + +include(CheckLibraryExists) + +check_library_exists(c memset_s "string.h" HAVE_MEMSET_S) +check_library_exists(c explicit_bzero "strings.h" HAVE_EXPLICIT_BZERO) diff --git a/contrib/epee/include/net/http_auth.h b/contrib/epee/include/net/http_auth.h index bf368e6f4..841cebc17 100644 --- a/contrib/epee/include/net/http_auth.h +++ b/contrib/epee/include/net/http_auth.h @@ -33,7 +33,7 @@ #include <functional> #include <string> #include <utility> - +#include "wipeable_string.h" #include "http_base.h" #undef MONERO_DEFAULT_LOG_CATEGORY @@ -48,12 +48,12 @@ namespace net_utils struct login { login() : username(), password() {} - login(std::string username_, std::string password_) + login(std::string username_, wipeable_string password_) : username(std::move(username_)), password(std::move(password_)) {} std::string username; - std::string password; + wipeable_string password; }; //! Implements RFC 2617 digest auth. Digests from RFC 7616 can be added. diff --git a/contrib/epee/include/wipeable_string.h b/contrib/epee/include/wipeable_string.h new file mode 100644 index 000000000..66d3e8e2b --- /dev/null +++ b/contrib/epee/include/wipeable_string.h @@ -0,0 +1,70 @@ +// Copyright (c) 2017, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#pragma once + +#include <stddef.h> +#include <vector> +#include <string> + +namespace epee +{ + class wipeable_string + { + public: + wipeable_string() {} + wipeable_string(const wipeable_string &other); + wipeable_string(wipeable_string &&other); + wipeable_string(const std::string &other); + wipeable_string(std::string &&other); + wipeable_string(const char *s); + ~wipeable_string(); + void wipe(); + void push_back(char c); + void pop_back(); + const char *data() const noexcept { return buffer.data(); } + size_t size() const noexcept { return buffer.size(); } + bool empty() const noexcept { return buffer.empty(); } + void resize(size_t sz); + void reserve(size_t sz); + void clear(); + bool operator==(const wipeable_string &other) const noexcept { return buffer == other.buffer; } + bool operator!=(const wipeable_string &other) const noexcept { return buffer != other.buffer; } + wipeable_string &operator=(wipeable_string &&other); + wipeable_string &operator=(const wipeable_string &other); + + static void set_wipe(void *(*f)(void*, size_t)) { wipefunc = f; } + + private: + void grow(size_t sz, size_t reserved = 0); + + private: + std::vector<char> buffer; + static void *(*wipefunc)(void*, size_t); + }; +} diff --git a/contrib/epee/src/CMakeLists.txt b/contrib/epee/src/CMakeLists.txt index bd6714791..5cd6d7813 100644 --- a/contrib/epee/src/CMakeLists.txt +++ b/contrib/epee/src/CMakeLists.txt @@ -26,7 +26,7 @@ # STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF # THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -add_library(epee STATIC hex.cpp http_auth.cpp mlog.cpp net_utils_base.cpp string_tools.cpp) +add_library(epee STATIC hex.cpp http_auth.cpp mlog.cpp net_utils_base.cpp string_tools.cpp wipeable_string.cpp) if (USE_READLINE AND GNU_READLINE_FOUND) add_library(epee_readline STATIC readline_buffer.cpp) endif() diff --git a/contrib/epee/src/http_auth.cpp b/contrib/epee/src/http_auth.cpp index 30e562700..f06f05528 100644 --- a/contrib/epee/src/http_auth.cpp +++ b/contrib/epee/src/http_auth.cpp @@ -125,6 +125,14 @@ namespace { (*this)(boost::string_ref(arg)); } + void operator()(const epee::wipeable_string& arg) const + { + md5::MD5Update( + std::addressof(ctx), + reinterpret_cast<const std::uint8_t*>(arg.data()), + arg.size() + ); + } md5::MD5_CTX& ctx; }; diff --git a/contrib/epee/src/wipeable_string.cpp b/contrib/epee/src/wipeable_string.cpp new file mode 100644 index 000000000..75191df71 --- /dev/null +++ b/contrib/epee/src/wipeable_string.cpp @@ -0,0 +1,146 @@ +// Copyright (c) 2017, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include <string.h> +#include "misc_log_ex.h" +#include "wipeable_string.h" + +namespace epee +{ + +void *(*wipeable_string::wipefunc)(void*, size_t) = NULL; + +wipeable_string::wipeable_string(const wipeable_string &other): + buffer(other.buffer) +{ +} + +wipeable_string::wipeable_string(wipeable_string &&other) +{ + if (&other == this) + return; + buffer = std::move(other.buffer); +} + +wipeable_string::wipeable_string(const std::string &other) +{ + grow(other.size()); + memcpy(buffer.data(), other.c_str(), size()); +} + +wipeable_string::wipeable_string(std::string &&other) +{ + CHECK_AND_ASSERT_THROW_MES(wipefunc, "wipefunc is not set"); + grow(other.size()); + memcpy(buffer.data(), other.c_str(), size()); + if (!other.empty()) + { + wipefunc(&other[0], other.size()); // we're kinda left with this again aren't we + other = std::string(); + } +} + +wipeable_string::wipeable_string(const char *s) +{ + grow(strlen(s)); + memcpy(buffer.data(), s, size()); +} + +wipeable_string::~wipeable_string() +{ + wipe(); +} + +void wipeable_string::wipe() +{ + CHECK_AND_ASSERT_THROW_MES(wipefunc, "wipefunc is not set"); + wipefunc(buffer.data(), buffer.size() * sizeof(char)); +} + +void wipeable_string::grow(size_t sz, size_t reserved) +{ + CHECK_AND_ASSERT_THROW_MES(wipefunc, "wipefunc is not set"); + if (reserved == 0) + reserved = sz; + CHECK_AND_ASSERT_THROW_MES(reserved >= sz, "reserved < sz"); + if (reserved <= buffer.capacity()) + return; + size_t old_sz = buffer.size(); + std::unique_ptr<char[]> tmp{new char[old_sz]}; + memcpy(tmp.get(), buffer.data(), old_sz * sizeof(char)); + wipefunc(buffer.data(), old_sz * sizeof(char)); + buffer.reserve(reserved); + buffer.resize(sz); + memcpy(buffer.data(), tmp.get(), sz * sizeof(char)); + wipefunc(tmp.get(), old_sz * sizeof(char)); +} + +void wipeable_string::push_back(char c) +{ + grow(size() + 1); + buffer.push_back(c); +} + +void wipeable_string::pop_back() +{ + resize(size() - 1); +} + +void wipeable_string::resize(size_t sz) +{ + CHECK_AND_ASSERT_THROW_MES(wipefunc, "wipefunc is not set"); + if (sz < buffer.size()) + wipefunc(buffer.data() + sz, buffer.size() - sz); + grow(sz); +} + +void wipeable_string::reserve(size_t sz) +{ + grow(size(), sz); +} + +void wipeable_string::clear() +{ + resize(0); +} + +wipeable_string &wipeable_string::operator=(wipeable_string &&other) +{ + if (&other != this) + buffer = std::move(other.buffer); + return *this; +} + +wipeable_string &wipeable_string::operator=(const wipeable_string &other) +{ + if (&other != this) + buffer = other.buffer; + return *this; +} + +} diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 50887e35c..7ad08ea83 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -35,6 +35,7 @@ set(common_sources download.cpp util.cpp i18n.cpp + memwipe.c password.cpp perf_timer.cpp threadpool.cpp @@ -63,6 +64,7 @@ set(common_private_headers util.h varint.h i18n.h + memwipe.h password.h perf_timer.h stack_trace.h @@ -90,5 +92,9 @@ target_link_libraries(common ${OPENSSL_LIBRARIES} ${EXTRA_LIBRARIES}) +if(HAVE_C11) +SET_PROPERTY(SOURCE memwipe.c PROPERTY COMPILE_FLAGS -std=c11) +endif() + #monero_install_headers(common # ${common_headers}) diff --git a/src/common/memwipe.c b/src/common/memwipe.c new file mode 100644 index 000000000..da7e9f346 --- /dev/null +++ b/src/common/memwipe.c @@ -0,0 +1,106 @@ +// Copyright (c) 2017, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// Parts of this file Copyright (c) 2009-2015 The Bitcoin Core developers + +#define __STDC_WANT_LIB_EXT1__ 1 +#include <string.h> +#include <stdlib.h> +#ifdef HAVE_EXPLICIT_BZERO +#include <strings.h> +#endif +#include "memwipe.h" + +#if defined(_MSC_VER) +#define SCARECROW \ + __asm; +#else +#define SCARECROW \ + __asm__ __volatile__("" : : "r"(ptr) : "memory"); +#endif + +#ifdef HAVE_MEMSET_S + +void *memwipe(void *ptr, size_t n) +{ + if (memset_s(ptr, n, 0, n)) + { + abort(); + } + SCARECROW // might as well... + return ptr; +} + +#elif defined HAVE_EXPLICIT_BZERO + +void *memwipe(void *ptr, size_t n) +{ + explicit_bzero(ptr, n); + SCARECROW + return ptr; +} + +#else + +/* The memory_cleanse implementation is taken from Bitcoin */ + +/* Compilers have a bad habit of removing "superfluous" memset calls that + * are trying to zero memory. For example, when memset()ing a buffer and + * then free()ing it, the compiler might decide that the memset is + * unobservable and thus can be removed. + * + * Previously we used OpenSSL which tried to stop this by a) implementing + * memset in assembly on x86 and b) putting the function in its own file + * for other platforms. + * + * This change removes those tricks in favour of using asm directives to + * scare the compiler away. As best as our compiler folks can tell, this is + * sufficient and will continue to be so. + * + * Adam Langley <agl@google.com> + * Commit: ad1907fe73334d6c696c8539646c21b11178f20f + * BoringSSL (LICENSE: ISC) + */ +static void memory_cleanse(void *ptr, size_t len) +{ + memset(ptr, 0, len); + + /* As best as we can tell, this is sufficient to break any optimisations that + might try to eliminate "superfluous" memsets. If there's an easy way to + detect memset_s, it would be better to use that. */ + SCARECROW +} + +void *memwipe(void *ptr, size_t n) +{ + memory_cleanse(ptr, n); + SCARECROW + return ptr; +} + +#endif diff --git a/src/common/memwipe.h b/src/common/memwipe.h new file mode 100644 index 000000000..e9a3fba7b --- /dev/null +++ b/src/common/memwipe.h @@ -0,0 +1,41 @@ +// Copyright (c) 2017, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers + +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif + +void *memwipe(void *src, size_t n); + +#ifdef __cplusplus +} +#endif diff --git a/src/common/password.cpp b/src/common/password.cpp index 5d56464a5..98a49eb80 100644 --- a/src/common/password.cpp +++ b/src/common/password.cpp @@ -46,6 +46,8 @@ #include "readline_buffer.h" #endif +#include "common/memwipe.h" + namespace { #if defined(_WIN32) @@ -54,7 +56,7 @@ namespace return 0 != _isatty(_fileno(stdin)); } - bool read_from_tty(std::string& pass) + bool read_from_tty(epee::wipeable_string& pass) { static constexpr const char BACKSPACE = 8; @@ -86,8 +88,7 @@ namespace { if (!pass.empty()) { - pass.back() = '\0'; - pass.resize(pass.size() - 1); + pass.pop_back(); } } else @@ -125,7 +126,7 @@ namespace return ch; } - bool read_from_tty(std::string& aPass) + bool read_from_tty(epee::wipeable_string& aPass) { static constexpr const char BACKSPACE = 127; @@ -146,8 +147,7 @@ namespace { if (!aPass.empty()) { - aPass.back() = '\0'; - aPass.resize(aPass.size() - 1); + aPass.pop_back(); } } else @@ -161,14 +161,7 @@ namespace #endif // end !WIN32 - void clear(std::string& pass) noexcept - { - //! TODO Call a memory wipe function that hopefully is not optimized out - pass.replace(0, pass.capacity(), pass.capacity(), '\0'); - pass.clear(); - } - - bool read_from_tty(const bool verify, const char *message, std::string& pass1, std::string& pass2) + bool read_from_tty(const bool verify, const char *message, epee::wipeable_string& pass1, epee::wipeable_string& pass2) { while (true) { @@ -184,8 +177,8 @@ namespace if(pass1!=pass2) { std::cout << "Passwords do not match! Please try again." << std::endl; - clear(pass1); - clear(pass2); + pass1.clear(); + pass2.clear(); } else //new password matches return true; @@ -198,7 +191,7 @@ namespace return false; } - bool read_from_file(std::string& pass) + bool read_from_file(epee::wipeable_string& pass) { pass.reserve(tools::password_container::max_password_size); for (size_t i = 0; i < tools::password_container::max_password_size; ++i) @@ -233,7 +226,7 @@ namespace tools password_container::~password_container() noexcept { - clear(m_password); + m_password.clear(); } boost::optional<password_container> password_container::prompt(const bool verify, const char *message) @@ -249,9 +242,8 @@ namespace tools boost::optional<login> login::parse(std::string&& userpass, bool verify, const std::function<boost::optional<password_container>(bool)> &prompt) { login out{}; - password_container wipe{std::move(userpass)}; - const auto loc = wipe.password().find(':'); + const auto loc = userpass.find(':'); if (loc == std::string::npos) { auto result = prompt(verify); @@ -262,10 +254,11 @@ namespace tools } else { - out.password = password_container{wipe.password().substr(loc + 1)}; + out.password = password_container{userpass.substr(loc + 1)}; } - out.username = wipe.password().substr(0, loc); + out.username = userpass.substr(0, loc); + password_container wipe{std::move(userpass)}; return {std::move(out)}; } } diff --git a/src/common/password.h b/src/common/password.h index ba1c30a28..01c6bf05a 100644 --- a/src/common/password.h +++ b/src/common/password.h @@ -32,6 +32,7 @@ #include <string> #include <boost/optional/optional.hpp> +#include "wipeable_string.h" namespace tools { @@ -58,11 +59,10 @@ namespace tools password_container& operator=(const password_container&) = delete; password_container& operator=(password_container&&) = default; - const std::string& password() const noexcept { return m_password; } + const epee::wipeable_string &password() const noexcept { return m_password; } private: - //! TODO Custom allocator that locks to RAM? - std::string m_password; + epee::wipeable_string m_password; }; struct login diff --git a/src/common/util.cpp b/src/common/util.cpp index e8ac61815..de19fec81 100644 --- a/src/common/util.cpp +++ b/src/common/util.cpp @@ -36,9 +36,11 @@ #include "include_base_utils.h" #include "file_io_utils.h" +#include "wipeable_string.h" using namespace epee; #include "util.h" +#include "memwipe.h" #include "cryptonote_config.h" #include "net/http_client.h" // epee::net_utils::... @@ -542,6 +544,10 @@ std::string get_nix_version_display_string() } bool on_startup() { + wipeable_string::set_wipe(&memwipe); + + mlog_configure("", true); + sanitize_locale(); #ifdef __GLIBC__ diff --git a/src/crypto/chacha8.h b/src/crypto/chacha8.h index 80557e9f5..1bf695731 100644 --- a/src/crypto/chacha8.h +++ b/src/crypto/chacha8.h @@ -39,6 +39,7 @@ #if defined(__cplusplus) #include <memory.h> +#include "common/memwipe.h" #include "hash.h" namespace crypto { @@ -54,7 +55,7 @@ namespace crypto { ~chacha8_key() { - memset(data, 0, sizeof(data)); + memwipe(data, sizeof(data)); } }; @@ -75,7 +76,7 @@ namespace crypto { char pwd_hash[HASH_SIZE]; crypto::cn_slow_hash(data, size, pwd_hash); memcpy(&key, pwd_hash, sizeof(key)); - memset(pwd_hash, 0, sizeof(pwd_hash)); + memwipe(pwd_hash, sizeof(pwd_hash)); } inline void generate_chacha8_key(std::string password, chacha8_key& key) { diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index 3c760493f..6759f1826 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -32,6 +32,7 @@ using namespace epee; #include <atomic> +#include "wipeable_string.h" #include "cryptonote_format_utils.h" #include "cryptonote_config.h" #include "crypto/crypto.h" @@ -994,7 +995,7 @@ namespace cryptonote block_hashes_cached = block_hashes_cached_count; } //--------------------------------------------------------------- - crypto::secret_key encrypt_key(crypto::secret_key key, const std::string &passphrase) + crypto::secret_key encrypt_key(crypto::secret_key key, const epee::wipeable_string &passphrase) { crypto::hash hash; crypto::cn_slow_hash(passphrase.data(), passphrase.size(), hash); @@ -1002,7 +1003,7 @@ namespace cryptonote return key; } //--------------------------------------------------------------- - crypto::secret_key decrypt_key(crypto::secret_key key, const std::string &passphrase) + crypto::secret_key decrypt_key(crypto::secret_key key, const epee::wipeable_string &passphrase) { crypto::hash hash; crypto::cn_slow_hash(passphrase.data(), passphrase.size(), hash); diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h index aebeaa6f4..714d80195 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.h +++ b/src/cryptonote_basic/cryptonote_format_utils.h @@ -38,6 +38,11 @@ #include "crypto/hash.h" #include <unordered_map> +namespace epee +{ + class wipeable_string; +} + namespace cryptonote { //--------------------------------------------------------------- @@ -226,8 +231,8 @@ namespace cryptonote bool is_valid_decomposed_amount(uint64_t amount); void get_hash_stats(uint64_t &tx_hashes_calculated, uint64_t &tx_hashes_cached, uint64_t &block_hashes_calculated, uint64_t & block_hashes_cached); - crypto::secret_key encrypt_key(crypto::secret_key key, const std::string &passphrase); - crypto::secret_key decrypt_key(crypto::secret_key key, const std::string &passphrase); + crypto::secret_key encrypt_key(crypto::secret_key key, const epee::wipeable_string &passphrase); + crypto::secret_key decrypt_key(crypto::secret_key key, const epee::wipeable_string &passphrase); #define CHECKED_GET_SPECIFIC_VARIANT(variant_var, specific_type, variable_name, fail_return_val) \ CHECK_AND_ASSERT_MES(variant_var.type() == typeid(specific_type), fail_return_val, "wrong variant type: " << variant_var.type().name() << ", expected " << typeid(specific_type).name()); \ specific_type& variable_name = boost::get<specific_type>(variant_var); diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index dfecab22c..489bbfb4b 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -433,7 +433,7 @@ bool simple_wallet::print_seed(bool encrypted) m_wallet->set_seed_language(mnemonic_language); } - std::string seed_pass; + epee::wipeable_string seed_pass; if (encrypted) { auto pwd_container = tools::password_container::prompt(true, tr("Enter optional seed encryption passphrase, empty to see raw seed")); @@ -1426,7 +1426,7 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) auto pwd_container = tools::password_container::prompt(false, tr("Enter seed encryption passphrase, empty if none")); if (std::cin.eof() || !pwd_container) return false; - std::string seed_pass = pwd_container->password(); + epee::wipeable_string seed_pass = pwd_container->password(); if (!seed_pass.empty()) m_recovery_key = cryptonote::decrypt_key(m_recovery_key, seed_pass); } @@ -2087,12 +2087,12 @@ bool simple_wallet::open_wallet(const boost::program_options::variables_map& vm) fail_msg_writer() << tr("wallet file path not valid: ") << m_wallet_file; return false; } - std::string password; + epee::wipeable_string password; try { auto rc = tools::wallet2::make_from_file(vm, m_wallet_file, password_prompter); m_wallet = std::move(rc.first); - password = std::move(rc.second).password(); + password = std::move(std::move(rc.second).password()); if (!m_wallet) { return false; diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index e29e1051c..fba1ef534 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -613,7 +613,7 @@ bool wallet2::is_deterministic() const return keys_deterministic; } //---------------------------------------------------------------------------------------------------- -bool wallet2::get_seed(std::string& electrum_words, const std::string &passphrase) const +bool wallet2::get_seed(std::string& electrum_words, const epee::wipeable_string &passphrase) const { bool keys_deterministic = is_deterministic(); if (!keys_deterministic) @@ -2150,7 +2150,7 @@ bool wallet2::clear() * \param watch_only true to save only view key, false to save both spend and view keys * \return Whether it was successful. */ -bool wallet2::store_keys(const std::string& keys_file_name, const std::string& password, bool watch_only) +bool wallet2::store_keys(const std::string& keys_file_name, const epee::wipeable_string& password, bool watch_only) { std::string account_data; cryptonote::account_base account = m_account; @@ -2236,7 +2236,7 @@ bool wallet2::store_keys(const std::string& keys_file_name, const std::string& p // Encrypt the entire JSON object. crypto::chacha8_key key; - crypto::generate_chacha8_key(password, key); + crypto::generate_chacha8_key(password.data(), password.size(), key); std::string cipher; cipher.resize(account_data.size()); keys_file_data.iv = crypto::rand<crypto::chacha8_iv>(); @@ -2266,7 +2266,7 @@ namespace * \param keys_file_name Name of wallet file * \param password Password of wallet file */ -bool wallet2::load_keys(const std::string& keys_file_name, const std::string& password) +bool wallet2::load_keys(const std::string& keys_file_name, const epee::wipeable_string& password) { wallet2::keys_file_data keys_file_data; std::string buf; @@ -2277,7 +2277,7 @@ bool wallet2::load_keys(const std::string& keys_file_name, const std::string& pa r = ::serialization::parse_binary(buf, keys_file_data); THROW_WALLET_EXCEPTION_IF(!r, error::wallet_internal_error, "internal error: failed to deserialize \"" + keys_file_name + '\"'); crypto::chacha8_key key; - crypto::generate_chacha8_key(password, key); + crypto::generate_chacha8_key(password.data(), password.size(), key); std::string account_data; account_data.resize(keys_file_data.account_data.size()); crypto::chacha8(keys_file_data.account_data.data(), keys_file_data.account_data.size(), key, keys_file_data.iv, &account_data[0]); @@ -2401,7 +2401,7 @@ bool wallet2::load_keys(const std::string& keys_file_name, const std::string& pa * can be used prior to rewriting wallet keys file, to ensure user has entered the correct password * */ -bool wallet2::verify_password(const std::string& password) const +bool wallet2::verify_password(const epee::wipeable_string& password) const { return verify_password(m_keys_file, password, m_watch_only); } @@ -2418,7 +2418,7 @@ bool wallet2::verify_password(const std::string& password) const * can be used prior to rewriting wallet keys file, to ensure user has entered the correct password * */ -bool wallet2::verify_password(const std::string& keys_file_name, const std::string& password, bool watch_only) +bool wallet2::verify_password(const std::string& keys_file_name, const epee::wipeable_string& password, bool watch_only) { wallet2::keys_file_data keys_file_data; std::string buf; @@ -2429,7 +2429,7 @@ bool wallet2::verify_password(const std::string& keys_file_name, const std::stri r = ::serialization::parse_binary(buf, keys_file_data); THROW_WALLET_EXCEPTION_IF(!r, error::wallet_internal_error, "internal error: failed to deserialize \"" + keys_file_name + '\"'); crypto::chacha8_key key; - crypto::generate_chacha8_key(password, key); + crypto::generate_chacha8_key(password.data(), password.size(), key); std::string account_data; account_data.resize(keys_file_data.account_data.size()); crypto::chacha8(keys_file_data.account_data.data(), keys_file_data.account_data.size(), key, keys_file_data.iv, &account_data[0]); @@ -2466,7 +2466,7 @@ bool wallet2::verify_password(const std::string& keys_file_name, const std::stri * \param two_random Whether it is a non-deterministic wallet * \return The secret key of the generated wallet */ -crypto::secret_key wallet2::generate(const std::string& wallet_, const std::string& password, +crypto::secret_key wallet2::generate(const std::string& wallet_, const epee::wipeable_string& password, const crypto::secret_key& recovery_param, bool recover, bool two_random) { clear(); @@ -2546,7 +2546,7 @@ crypto::secret_key wallet2::generate(const std::string& wallet_, const std::stri * \param password Password of wallet file * \param viewkey view secret key */ -void wallet2::generate(const std::string& wallet_, const std::string& password, +void wallet2::generate(const std::string& wallet_, const epee::wipeable_string& password, const cryptonote::account_public_address &account_public_address, const crypto::secret_key& viewkey) { @@ -2582,7 +2582,7 @@ void wallet2::generate(const std::string& wallet_, const std::string& password, * \param spendkey spend secret key * \param viewkey view secret key */ -void wallet2::generate(const std::string& wallet_, const std::string& password, +void wallet2::generate(const std::string& wallet_, const epee::wipeable_string& password, const cryptonote::account_public_address &account_public_address, const crypto::secret_key& spendkey, const crypto::secret_key& viewkey) { @@ -2616,7 +2616,7 @@ void wallet2::generate(const std::string& wallet_, const std::string& password, * \param wallet_name Name of wallet file (should exist) * \param password Password for wallet file */ -void wallet2::rewrite(const std::string& wallet_name, const std::string& password) +void wallet2::rewrite(const std::string& wallet_name, const epee::wipeable_string& password) { prepare_file_names(wallet_name); boost::system::error_code ignored_ec; @@ -2629,7 +2629,7 @@ void wallet2::rewrite(const std::string& wallet_name, const std::string& passwor * \param wallet_name Base name of wallet file * \param password Password for wallet file */ -void wallet2::write_watch_only_wallet(const std::string& wallet_name, const std::string& password) +void wallet2::write_watch_only_wallet(const std::string& wallet_name, const epee::wipeable_string& password) { prepare_file_names(wallet_name); boost::system::error_code ignored_ec; @@ -2765,7 +2765,7 @@ bool wallet2::generate_chacha8_key_from_secret_keys(crypto::chacha8_key &key) co return true; } //---------------------------------------------------------------------------------------------------- -void wallet2::load(const std::string& wallet_, const std::string& password) +void wallet2::load(const std::string& wallet_, const epee::wipeable_string& password) { clear(); prepare_file_names(wallet_); @@ -2916,10 +2916,10 @@ std::string wallet2::path() const //---------------------------------------------------------------------------------------------------- void wallet2::store() { - store_to("", ""); + store_to("", epee::wipeable_string()); } //---------------------------------------------------------------------------------------------------- -void wallet2::store_to(const std::string &path, const std::string &password) +void wallet2::store_to(const std::string &path, const epee::wipeable_string &password) { trim_hashchain(); diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 866b95853..de1bed90c 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -168,7 +168,7 @@ namespace tools //! Just parses variables. static std::unique_ptr<wallet2> make_dummy(const boost::program_options::variables_map& vm, const std::function<boost::optional<password_container>(const char *, bool)> &password_prompter); - static bool verify_password(const std::string& keys_file_name, const std::string& password, bool watch_only); + static bool verify_password(const std::string& keys_file_name, const epee::wipeable_string& password, bool watch_only); wallet2(bool testnet = false, bool restricted = false) : m_run(true), m_callback(0), m_testnet(testnet), m_always_confirm_transfers(true), m_print_ring_members(false), m_store_tx_info(true), m_default_mixin(0), m_default_priority(0), m_refresh_type(RefreshDefault), m_auto_refresh(true), m_refresh_from_block_height(0), m_confirm_missing_payment_id(true), m_ask_password(true), m_min_output_count(0), m_min_output_value(0), m_merge_destinations(false), m_confirm_backlog(true), m_is_initialized(false), m_restricted(restricted), is_old_file_format(false), m_node_rpc_proxy(m_http_client, m_daemon_rpc_mutex), m_light_wallet(false), m_light_wallet_scanned_block_height(0), m_light_wallet_blockchain_height(0), m_light_wallet_connected(false), m_light_wallet_balance(0), m_light_wallet_unlocked_balance(0) {} @@ -398,7 +398,7 @@ namespace tools * \param two_random Whether it is a non-deterministic wallet * \return The secret key of the generated wallet */ - crypto::secret_key generate(const std::string& wallet, const std::string& password, + crypto::secret_key generate(const std::string& wallet, const epee::wipeable_string& password, const crypto::secret_key& recovery_param = crypto::secret_key(), bool recover = false, bool two_random = false); /*! @@ -408,7 +408,7 @@ namespace tools * \param viewkey view secret key * \param spendkey spend secret key */ - void generate(const std::string& wallet, const std::string& password, + void generate(const std::string& wallet, const epee::wipeable_string& password, const cryptonote::account_public_address &account_public_address, const crypto::secret_key& spendkey, const crypto::secret_key& viewkey); /*! @@ -417,7 +417,7 @@ namespace tools * \param password Password of wallet file * \param viewkey view secret key */ - void generate(const std::string& wallet, const std::string& password, + void generate(const std::string& wallet, const epee::wipeable_string& password, const cryptonote::account_public_address &account_public_address, const crypto::secret_key& viewkey = crypto::secret_key()); /*! @@ -425,23 +425,23 @@ namespace tools * \param wallet_name Name of wallet file (should exist) * \param password Password for wallet file */ - void rewrite(const std::string& wallet_name, const std::string& password); - void write_watch_only_wallet(const std::string& wallet_name, const std::string& password); - void load(const std::string& wallet, const std::string& password); + void rewrite(const std::string& wallet_name, const epee::wipeable_string& password); + void write_watch_only_wallet(const std::string& wallet_name, const epee::wipeable_string& password); + void load(const std::string& wallet, const epee::wipeable_string& password); void store(); /*! * \brief store_to - stores wallet to another file(s), deleting old ones * \param path - path to the wallet file (keys and address filenames will be generated based on this filename) * \param password - password to protect new wallet (TODO: probably better save the password in the wallet object?) */ - void store_to(const std::string &path, const std::string &password); + void store_to(const std::string &path, const epee::wipeable_string &password); std::string path() const; /*! * \brief verifies given password is correct for default wallet keys file */ - bool verify_password(const std::string& password) const; + bool verify_password(const epee::wipeable_string& password) const; cryptonote::account_base& get_account(){return m_account;} const cryptonote::account_base& get_account()const{return m_account;} @@ -466,7 +466,7 @@ namespace tools * \brief Checks if deterministic wallet */ bool is_deterministic() const; - bool get_seed(std::string& electrum_words, const std::string &passphrase = std::string()) const; + bool get_seed(std::string& electrum_words, const epee::wipeable_string &passphrase = epee::wipeable_string()) const; /*! * \brief Checks if light wallet. A light wallet sends view key to a server where the blockchain is scanned. @@ -839,13 +839,13 @@ namespace tools * \param watch_only true to save only view key, false to save both spend and view keys * \return Whether it was successful. */ - bool store_keys(const std::string& keys_file_name, const std::string& password, bool watch_only = false); + bool store_keys(const std::string& keys_file_name, const epee::wipeable_string& password, bool watch_only = false); /*! * \brief Load wallet information from wallet file. * \param keys_file_name Name of wallet file * \param password Password of wallet file */ - bool load_keys(const std::string& keys_file_name, const std::string& password); + bool load_keys(const std::string& keys_file_name, const epee::wipeable_string& password); void process_new_transaction(const crypto::hash &txid, const cryptonote::transaction& tx, const std::vector<uint64_t> &o_indices, uint64_t height, uint64_t ts, bool miner_tx, bool pool, bool double_spend_seen); void process_new_blockchain_entry(const cryptonote::block& b, const cryptonote::block_complete_entry& bche, const crypto::hash& bl_id, uint64_t height, const cryptonote::COMMAND_RPC_GET_BLOCKS_FAST::block_output_indices &o_indices); void detach_blockchain(uint64_t height); diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index c315684de..e790b9954 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -206,7 +206,8 @@ namespace tools } std::fputs(http_login->username.c_str(), rpc_login_file.handle()); std::fputc(':', rpc_login_file.handle()); - std::fputs(http_login->password.c_str(), rpc_login_file.handle()); + const epee::wipeable_string password = http_login->password; + std::fwrite(password.data(), 1, password.size(), rpc_login_file.handle()); std::fflush(rpc_login_file.handle()); if (std::ferror(rpc_login_file.handle())) { diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt index 4073a17dc..e37d34063 100644 --- a/tests/unit_tests/CMakeLists.txt +++ b/tests/unit_tests/CMakeLists.txt @@ -50,6 +50,7 @@ set(unit_tests_sources hashchain.cpp http.cpp main.cpp + memwipe.cpp mnemonics.cpp mul_div.cpp parse_amount.cpp @@ -101,6 +102,8 @@ if (NOT MSVC) COMPILE_FLAGS " -Wno-undef -Wno-sign-compare") endif () +SET_PROPERTY(SOURCE memwipe.cpp PROPERTY COMPILE_FLAGS -Ofast) + add_test( NAME unit_tests COMMAND unit_tests --data-dir "${TEST_DATA_DIR}") diff --git a/tests/unit_tests/http.cpp b/tests/unit_tests/http.cpp index 8d8a0965e..135c0984e 100644 --- a/tests/unit_tests/http.cpp +++ b/tests/unit_tests/http.cpp @@ -211,7 +211,7 @@ std::string get_a1(const http::login& user, const fields& src) { const std::string& realm = src.at(u8"realm"); return boost::join( - std::vector<std::string>{user.username, realm, user.password}, u8":" + std::vector<std::string>{user.username, realm, std::string(user.password.data(), user.password.size())}, u8":" ); } diff --git a/tests/unit_tests/memwipe.cpp b/tests/unit_tests/memwipe.cpp new file mode 100644 index 000000000..b2b19fbf5 --- /dev/null +++ b/tests/unit_tests/memwipe.cpp @@ -0,0 +1,64 @@ +// Copyright (c) 2017, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include "gtest/gtest.h" + +#include <stdint.h> +#include "misc_log_ex.h" +#include "common/memwipe.h" + +// Probably won't catch the optimized out case, but at least we test +// it works in the normal case +static void test(bool wipe) +{ + char *foo = (char*)malloc(4); + ASSERT_TRUE(foo != NULL); + intptr_t foop = (intptr_t)foo; + strcpy(foo, "bar"); + void *bar = wipe ? memwipe(foo, 3) : memset(foo, 0, 3); + ASSERT_EQ(foo, bar); + free(foo); + char *quux = (char*)malloc(4); // same size, just after free, so we're likely to get the same, depending on the allocator + if ((intptr_t)quux == foop) + { + MDEBUG(std::hex << std::setw(8) << std::setfill('0') << *(uint32_t*)quux); + if (wipe) ASSERT_TRUE(!memcmp(quux, "\0\0\0", 3)); + } + else MWARNING("We did not get the same location, cannot check"); + free(quux); +} + +TEST(memwipe, control) +{ + test(false); +} + +TEST(memwipe, works) +{ + test(true); +} |