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)

6 files changed, 177 insertions, 25 deletions

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__