Merge pull request #1472

2bddb8eb Refactored password prompting for wallets (Lee Clagett)

5 files changed, 159 insertions, 205 deletions

diff --git a/src/wallet/password_container.cpp b/src/wallet/password_container.cpp
index 5260bbc8b..832b93a1a 100644
--- a/src/wallet/password_container.cpp
+++ b/src/wallet/password_container.cpp
@@ -42,153 +42,17 @@
 #include <unistd.h>
 #endif
 
-namespace tools
+namespace
 {
-  namespace
-  {
-    bool is_cin_tty();
-  }
-  // deleted via private member
-  password_container::password_container()
-    : m_empty(true),m_verify(true)
-  {
-
-  }
-  password_container::password_container(bool verify)
-    : m_empty(true),m_verify(verify)
-  {
-
-  }
-
-  password_container::password_container(std::string&& password)
-    : m_empty(false)
-    , m_password(std::move(password))
-	, m_verify(false)
-  {
-
-  }
-
-
-  password_container::password_container(password_container&& rhs)
-    : m_empty(std::move(rhs.m_empty))
-    , m_password(std::move(rhs.m_password))
-    , m_verify(std::move(rhs.m_verify))
-  {
-  }
-  password_container::~password_container()
-  {
-    clear();
-  }
-
-  void password_container::clear()
-  {
-    if (0 < m_password.capacity())
-    {
-      m_password.replace(0, m_password.capacity(), m_password.capacity(), '\0');
-      m_password.resize(0);
-    }
-    m_empty = true;
-  }
-
-  bool password_container::read_password(const char *message)
-  {
-    clear();
-
-    bool r;
-    if (is_cin_tty())
-    {
-     r = read_from_tty_double_check(message);
-    }
-    else
-    {
-      r = read_from_file();
-    }
-
-    if (r)
-    {
-      m_empty = false;
-    }
-    else
-    {
-      clear();
-    }
-
-    return r;
-  }
-
-  bool password_container::read_from_file()
-  {
-    m_password.reserve(max_password_size);
-    for (size_t i = 0; i < max_password_size; ++i)
-    {
-      char ch = static_cast<char>(std::cin.get());
-      if (std::cin.eof() || ch == '\n' || ch == '\r')
-      {
-        break;
-      }
-      else if (std::cin.fail())
-      {
-        return false;
-      }
-      else
-      {
-        m_password.push_back(ch);
-      }
-    }
-
-    return true;
-  }
-
-bool password_container::read_from_tty_double_check(const char *message) {
-    std::string pass1;
-    std::string pass2;
-    bool match=false;
-    bool doNotVerifyEntry=false;
-    if (m_verify){message = "Enter a password for your new wallet";}
-    do{
-        if (message)
-            std::cout << message <<": ";
-        if (!password_container::read_from_tty(pass1))
-            return false;
-        if (m_verify==true){//double check password; 
-            std::cout << "Confirm Password: ";
-            if (!password_container::read_from_tty(pass2))
-                return false;
-            if(pass1!=pass2){ //new password entered did not match
-
-                std::cout << "Passwords do not match! Please try again." << std::endl;
-                pass1="";
-                pass2="";
-                match=false;
-            }
-            else{//new password matches
-                match=true;
-            }
-        }
-        else
-            doNotVerifyEntry=true; //do not verify
-            //No need to verify password entered at this point in the code 
-            
-    }while(match==false && doNotVerifyEntry==false);
-
-    m_password=pass1;
-    return true;
-  }
-
-
 #if defined(_WIN32)
-
-  namespace
+  bool is_cin_tty() noexcept
   {
-    bool is_cin_tty()
-    {
-      return 0 != _isatty(_fileno(stdin));
-    }
+    return 0 != _isatty(_fileno(stdin));
   }
 
-  bool password_container::read_from_tty(std::string & pass)
+  bool read_from_tty(std::string& pass)
   {
-    const char BACKSPACE = 8;
+    static constexpr const char BACKSPACE = 8;
 
     HANDLE h_cin = ::GetStdHandle(STD_INPUT_HANDLE);
 
@@ -198,8 +62,8 @@ bool password_container::read_from_tty_double_check(const char *message) {
     ::SetConsoleMode(h_cin, mode_new);
 
     bool r = true;
-    pass.reserve(max_password_size);
-    while (pass.size() < max_password_size)
+    pass.reserve(tools::password_container::max_password_size);
+    while (pass.size() < tools::password_container::max_password_size)
     {
       DWORD read;
       char ch;
@@ -235,38 +99,36 @@ bool password_container::read_from_tty_double_check(const char *message) {
     return r;
   }
 
-#else
+#else // end WIN32 
 
-  namespace
+  bool is_cin_tty() noexcept
   {
-    bool is_cin_tty()
-    {
-      return 0 != isatty(fileno(stdin));
-    }
+    return 0 != isatty(fileno(stdin));
+  }
 
-    int getch()
-    {
-      struct termios tty_old;
-      tcgetattr(STDIN_FILENO, &tty_old);
+  int getch() noexcept
+  {
+    struct termios tty_old;
+    tcgetattr(STDIN_FILENO, &tty_old);
 
-      struct termios tty_new;
-      tty_new = tty_old;
-      tty_new.c_lflag &= ~(ICANON | ECHO);
-      tcsetattr(STDIN_FILENO, TCSANOW, &tty_new);
+    struct termios tty_new;
+    tty_new = tty_old;
+    tty_new.c_lflag &= ~(ICANON | ECHO);
+    tcsetattr(STDIN_FILENO, TCSANOW, &tty_new);
 
-      int ch = getchar();
+    int ch = getchar();
 
-      tcsetattr(STDIN_FILENO, TCSANOW, &tty_old);
+    tcsetattr(STDIN_FILENO, TCSANOW, &tty_old);
 
-      return ch;
-    }
+    return ch;
   }
-  bool password_container::read_from_tty(std::string &aPass)
+
+  bool read_from_tty(std::string& aPass)
   {
-    const char BACKSPACE = 127;
+    static constexpr const char BACKSPACE = 127;
 
-    aPass.reserve(max_password_size);
-    while (aPass.size() < max_password_size)
+    aPass.reserve(tools::password_container::max_password_size);
+    while (aPass.size() < tools::password_container::max_password_size)
     {
       int ch = getch();
       if (EOF == ch)
@@ -297,5 +159,90 @@ bool password_container::read_from_tty_double_check(const char *message) {
     return true;
   }
 
-#endif
-}
+#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)
+  {
+    while (true)
+    {
+      if (message)
+        std::cout << message <<": ";
+      if (!read_from_tty(pass1))
+        return false;
+      if (verify)
+      {
+        std::cout << "Confirm Password: ";
+        if (!read_from_tty(pass2))
+          return false;
+        if(pass1!=pass2)
+        {
+          std::cout << "Passwords do not match! Please try again." << std::endl;
+          clear(pass1);
+          clear(pass2);
+        }
+        else //new password matches
+          return true;
+      }
+      else
+        return true;
+        //No need to verify password entered at this point in the code
+    }
+
+    return false;
+  }
+
+  bool read_from_file(std::string& pass)
+  {
+    pass.reserve(tools::password_container::max_password_size);
+    for (size_t i = 0; i < tools::password_container::max_password_size; ++i)
+    {
+      char ch = static_cast<char>(std::cin.get());
+      if (std::cin.eof() || ch == '\n' || ch == '\r')
+      {
+        break;
+      }
+      else if (std::cin.fail())
+      {
+        return false;
+      }
+      else
+      {
+        pass.push_back(ch);
+      }
+    }
+    return true;
+  }
+
+} // anonymous namespace
+
+namespace tools 
+{
+  // deleted via private member
+  password_container::password_container() noexcept : m_password() {}
+  password_container::password_container(std::string&& password) noexcept
+    : m_password(std::move(password)) 
+  {
+  }
+
+  password_container::~password_container() noexcept
+  {
+    clear(m_password);
+  }
+
+  boost::optional<password_container> password_container::prompt(const bool verify, const char *message)
+  {
+    password_container pass1{};
+    password_container pass2{};
+    if (is_cin_tty() ? read_from_tty(verify, message, pass1.m_password, pass2.m_password) : read_from_file(pass1.m_password))
+      return {std::move(pass1)};
+
+    return boost::none;
+  }
+} 
diff --git a/src/wallet/password_container.h b/src/wallet/password_container.h
index 866d170f2..9c6faf9c8 100644
--- a/src/wallet/password_container.h
+++ b/src/wallet/password_container.h
@@ -31,34 +31,37 @@
 #pragma once
 
 #include <string>
-#include <boost/program_options/variables_map.hpp>
+#include <boost/optional/optional.hpp>
 
 namespace tools
 {
   class password_container
   {
   public:
-    static const size_t max_password_size = 1024;
-    password_container(bool verify);
-    password_container(password_container&& rhs);
-    password_container(std::string&& password);
-    ~password_container();
+    static constexpr const size_t max_password_size = 1024;
 
-    void clear();
-    bool empty() const { return m_empty; }
-    const std::string& password() const { return m_password; }
-    void password(std::string&& val) { m_password = std::move(val); m_empty = false; }
-    bool read_password(const char *message = "Password");
+    //! Empty password
+    password_container() noexcept;
 
-  private:
-    //delete constructor with no parameters
-    password_container();
-    bool read_from_file();
-    bool read_from_tty(std::string & pass);
-    bool read_from_tty_double_check(const char *message);
+    //! `password` is used as password
+    password_container(std::string&& password) noexcept;
+
+    //! \return A password from stdin TTY prompt or `std::cin` pipe.
+    static boost::optional<password_container> prompt(bool verify, const char *mesage = "Password");
+
+    password_container(const password_container&) = delete;
+    password_container(password_container&& rhs) = default;
+
+    //! Wipes internal password
+    ~password_container() noexcept;
 
-    bool m_empty;
+    password_container& operator=(const password_container&) = delete;
+    password_container& operator=(password_container&&) = default;
+
+    const std::string& password() const noexcept { return m_password; }
+
+  private:
+    //! TODO Custom allocator that locks to RAM?
     std::string m_password;
-    bool m_verify;
   };
 }
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index e58c31156..ed488b50c 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -172,9 +172,7 @@ boost::optional<tools::password_container> get_password(const boost::program_opt
 
   if (command_line::has_arg(vm, opts.password))
   {
-    tools::password_container pwd(false);
-    pwd.password(command_line::get_arg(vm, opts.password));
-    return {std::move(pwd)};
+    return tools::password_container{command_line::get_arg(vm, opts.password)};
   }
 
   if (command_line::has_arg(vm, opts.password_file))
@@ -190,19 +188,10 @@ boost::optional<tools::password_container> get_password(const boost::program_opt
 
     // Remove line breaks the user might have inserted
     boost::trim_right_if(password, boost::is_any_of("\r\n"));
-    return {tools::password_container(std::move(password))};
+    return {tools::password_container{std::move(password)}};
   }
 
-  //vm is already part of the password container class.  just need to check vm for an already existing wallet
-  //here need to pass in variable map.  This will indicate if the wallet already exists to the read password function
-  tools::password_container pwd(verify);
-  if (pwd.read_password())
-  {
-    return {std::move(pwd)};
-  }
-
-  tools::fail_msg_writer() << tools::wallet2::tr("failed to read wallet password");
-  return boost::none;
+  return tools::wallet2::password_prompt(verify);
 }
 
 std::unique_ptr<tools::wallet2> generate_from_json(const std::string& json_file, bool testnet, bool restricted)
@@ -429,6 +418,18 @@ void wallet2::init_options(boost::program_options::options_description& desc_par
   command_line::add_arg(desc_params, opts.restricted);
 }
 
+boost::optional<password_container> wallet2::password_prompt(const bool is_new_wallet)
+{
+  auto pwd_container = tools::password_container::prompt(
+    is_new_wallet, (is_new_wallet ? tr("Enter a password for your new wallet") : tr("Wallet password"))
+  );
+  if (!pwd_container)
+  {
+    tools::fail_msg_writer() << tr("failed to read wallet password");
+  }
+  return pwd_container;
+}
+
 std::unique_ptr<wallet2> wallet2::make_from_json(const boost::program_options::variables_map& vm, const std::string& json_file)
 {
   const options opts{};
@@ -442,7 +443,7 @@ std::pair<std::unique_ptr<wallet2>, password_container> wallet2::make_from_file(
   auto pwd = get_password(vm, opts, false);
   if (!pwd)
   {
-    return {nullptr, password_container(false)};
+    return {nullptr, password_container{}};
   }
   auto wallet = make_basic(vm, opts);
   if (wallet)
@@ -458,7 +459,7 @@ std::pair<std::unique_ptr<wallet2>, password_container> wallet2::make_new(const
   auto pwd = get_password(vm, opts, true);
   if (!pwd)
   {
-    return {nullptr, password_container(false)};
+    return {nullptr, password_container{}};
   }
   return {make_basic(vm, opts), std::move(*pwd)};
 }
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index 9f4180c0d..54e26008b 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -103,6 +103,9 @@ namespace tools
     static bool has_testnet_option(const boost::program_options::variables_map& vm);
     static void init_options(boost::program_options::options_description& desc_params);
 
+    //! \return Password retrieved from prompt. Logs error on failure.
+    static boost::optional<password_container> password_prompt(const bool is_new_wallet);
+
     //! Uses stdin and stdout. Returns a wallet2 if no errors.
     static std::unique_ptr<wallet2> make_from_json(const boost::program_options::variables_map& vm, const std::string& json_file);
 
diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp
index c34ce4cf2..2a259029d 100644
--- a/src/wallet/wallet_rpc_server.cpp
+++ b/src/wallet/wallet_rpc_server.cpp
@@ -159,9 +159,9 @@ namespace tools
         }
         else
         {
-          tools::password_container pwd(true);
-          pwd.read_password("RPC password");
-          login.password = pwd.password();
+          login.password = tools::password_container::prompt(true, "RPC password").value_or(
+            tools::password_container{}
+          ).password();
         }
 
         if (login.username.empty() || login.password.empty())