From 2bddb8ebee4b25f2f73e6476dd1019459d8a1aca Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Sat, 17 Dec 2016 18:07:15 -0500 Subject: [PATCH] Refactored password prompting for wallets --- src/simplewallet/simplewallet.cpp | 221 ++++++---------------- src/simplewallet/simplewallet.h | 3 + src/wallet/password_container.cpp | 293 ++++++++++++------------------ src/wallet/password_container.h | 41 +++-- src/wallet/wallet2.cpp | 33 ++-- src/wallet/wallet2.h | 3 + src/wallet/wallet_rpc_server.cpp | 6 +- 7 files changed, 225 insertions(+), 375 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index b46447975..793a93e22 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -335,7 +335,6 @@ bool simple_wallet::seed(const std::vector &args/* = std::vector &args/* = std::vector()*/) { - bool success = false; if (m_wallet->watch_only()) { fail_msg_writer() << tr("wallet is watch-only and has no seed"); @@ -347,87 +346,49 @@ bool simple_wallet::seed_set_language(const std::vector &args/* = s return true; } - tools::password_container pwd_container(m_wallet_file.empty()); - success = pwd_container.read_password(); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("failed to read wallet password"); - return true; - } + std::string mnemonic_language = get_mnemonic_language(); + if (mnemonic_language.empty()) + return true; - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) - { - fail_msg_writer() << tr("invalid password"); - return true; + m_wallet->set_seed_language(std::move(mnemonic_language)); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - std::string mnemonic_language = get_mnemonic_language(); - if (mnemonic_language.empty()) - return true; - m_wallet->set_seed_language(mnemonic_language); - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } bool simple_wallet::set_always_confirm_transfers(const std::vector &args/* = std::vector()*/) { - bool success = false; - tools::password_container pwd_container(m_wallet_file.empty()); - success = pwd_container.read_password(); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("failed to read wallet password"); - return true; + m_wallet->always_confirm_transfers(is_it_true(args[1])); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) - { - fail_msg_writer() << tr("invalid password"); - return true; - } - - m_wallet->always_confirm_transfers(is_it_true(args[1])); - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } bool simple_wallet::set_store_tx_info(const std::vector &args/* = std::vector()*/) { - bool success = false; if (m_wallet->watch_only()) { fail_msg_writer() << tr("wallet is watch-only and cannot transfer"); return true; } - tools::password_container pwd_container(m_wallet_file.empty()); - success = pwd_container.read_password(); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("failed to read wallet password"); - return true; + m_wallet->store_tx_info(is_it_true(args[1])); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) - { - fail_msg_writer() << tr("invalid password"); - return true; - } - - m_wallet->store_tx_info(is_it_true(args[1])); - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } bool simple_wallet::set_default_mixin(const std::vector &args/* = std::vector()*/) { - bool success = false; if (m_wallet->watch_only()) { fail_msg_writer() << tr("wallet is watch-only and cannot transfer"); @@ -449,25 +410,12 @@ bool simple_wallet::set_default_mixin(const std::vector &args/* = s if (mixin == 0) mixin = DEFAULT_MIX; - tools::password_container pwd_container(m_wallet_file.empty()); - - success = pwd_container.read_password(); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("failed to read wallet password"); - return true; + m_wallet->default_mixin(mixin); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) - { - fail_msg_writer() << tr("invalid password"); - return true; - } - - m_wallet->default_mixin(mixin); - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } catch(const boost::bad_lexical_cast &) @@ -484,7 +432,6 @@ bool simple_wallet::set_default_mixin(const std::vector &args/* = s bool simple_wallet::set_default_priority(const std::vector &args/* = std::vector()*/) { - bool success = false; int priority = 0; if (m_wallet->watch_only()) { @@ -512,24 +459,12 @@ bool simple_wallet::set_default_priority(const std::vector &args/* } } - tools::password_container pwd_container(m_wallet_file.empty()); - success = pwd_container.read_password(); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("failed to read wallet password"); - return true; + m_wallet->set_default_priority(priority); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) - { - fail_msg_writer() << tr("invalid password"); - return true; - } - - m_wallet->set_default_priority(priority); - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } catch(const boost::bad_lexical_cast &) @@ -546,93 +481,52 @@ bool simple_wallet::set_default_priority(const std::vector &args/* bool simple_wallet::set_auto_refresh(const std::vector &args/* = std::vector()*/) { - - tools::password_container pwd_container(m_wallet_file.empty()); - - bool success = pwd_container.read_password(); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("failed to read wallet password"); - return true; + const bool auto_refresh = is_it_true(args[1]); + m_wallet->auto_refresh(auto_refresh); + m_idle_mutex.lock(); + m_auto_refresh_enabled.store(auto_refresh, std::memory_order_relaxed); + m_idle_cond.notify_one(); + m_idle_mutex.unlock(); + + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) - { - fail_msg_writer() << tr("invalid password"); - return true; - } - - bool auto_refresh = is_it_true(args[1]); - m_wallet->auto_refresh(auto_refresh); - m_idle_mutex.lock(); - m_auto_refresh_enabled.store(auto_refresh, std::memory_order_relaxed); - m_idle_cond.notify_one(); - m_idle_mutex.unlock(); - - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } bool simple_wallet::set_refresh_type(const std::vector &args/* = std::vector()*/) { - bool success = false; - tools::wallet2::RefreshType refresh_type; if (!parse_refresh_type(args[1], refresh_type)) { return true; } - tools::password_container pwd_container(m_wallet_file.empty()); - success = pwd_container.read_password(); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("failed to read wallet password"); - return true; + m_wallet->set_refresh_type(refresh_type); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) - { - fail_msg_writer() << tr("invalid password"); - return true; - } - - m_wallet->set_refresh_type(refresh_type); - - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } bool simple_wallet::set_confirm_missing_payment_id(const std::vector &args/* = std::vector()*/) { - bool success = false; if (m_wallet->watch_only()) { fail_msg_writer() << tr("wallet is watch-only and cannot transfer"); return true; } - tools::password_container pwd_container(m_wallet_file.empty()); - success = pwd_container.read_password(); - if (!success) - { - fail_msg_writer() << tr("failed to read wallet password"); - return true; - } - /* verify password before using so user doesn't accidentally set a new password for rewritten wallet */ - success = m_wallet->verify_password(pwd_container.password()); - if (!success) + const auto pwd_container = get_and_verify_password(); + if (pwd_container) { - fail_msg_writer() << tr("invalid password"); - return true; + m_wallet->confirm_missing_payment_id(is_it_true(args[1])); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); } - - m_wallet->confirm_missing_payment_id(is_it_true(args[1])); - m_wallet->rewrite(m_wallet_file, pwd_container.password()); return true; } @@ -1298,7 +1192,20 @@ std::string simple_wallet::get_mnemonic_language() } return language_list[language_number]; } +//---------------------------------------------------------------------------------------------------- +boost::optional simple_wallet::get_and_verify_password() const +{ + auto pwd_container = tools::wallet2::password_prompt(m_wallet_file.empty()); + if (!pwd_container) + return boost::none; + if (!m_wallet->verify_password(pwd_container->password())) + { + fail_msg_writer() << tr("invalid password"); + return boost::none; + } + return pwd_container; +} //---------------------------------------------------------------------------------------------------- bool simple_wallet::new_wallet(const boost::program_options::variables_map& vm, const crypto::secret_key& recovery_key, bool recover, bool two_random, const std::string &old_language) @@ -1532,29 +1439,15 @@ bool simple_wallet::save(const std::vector &args) //---------------------------------------------------------------------------------------------------- bool simple_wallet::save_watch_only(const std::vector &args/* = std::vector()*/) { - bool success = false; - tools::password_container pwd_container(m_wallet_file.empty()); + const auto pwd_container = tools::password_container::prompt(true, tr("Password for new watch-only wallet")); - success = pwd_container.read_password(tr("Password for the new watch-only wallet")); - if (!success) + if (!pwd_container) { fail_msg_writer() << tr("failed to read wallet password"); return true; } - std::string password = pwd_container.password(); - success = pwd_container.read_password(tr("Enter new password again")); - if (!success) - { - fail_msg_writer() << tr("failed to read wallet password"); - return true; - } - if (password != pwd_container.password()) - { - fail_msg_writer() << tr("passwords do not match"); - return true; - } - m_wallet->write_watch_only_wallet(m_wallet_file, pwd_container.password()); + m_wallet->write_watch_only_wallet(m_wallet_file, pwd_container->password()); return true; } diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index 420597699..c3e14a8cc 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -81,6 +81,9 @@ namespace cryptonote void wallet_idle_thread(); + //! \return Prompts user for password and verifies against local file. Logs on error and returns `none` + boost::optional get_and_verify_password() const; + bool new_wallet(const boost::program_options::variables_map& vm, const crypto::secret_key& recovery_key, bool recover, bool two_random, const std::string &old_language); bool new_wallet(const boost::program_options::variables_map& vm, const cryptonote::account_public_address& address, 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 #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(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)); - } - - int getch() - { - 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); - - int ch = getchar(); - - tcsetattr(STDIN_FILENO, TCSANOW, &tty_old); - - return ch; - } + return 0 != isatty(fileno(stdin)); } - bool password_container::read_from_tty(std::string &aPass) - { - const char BACKSPACE = 127; - aPass.reserve(max_password_size); - while (aPass.size() < max_password_size) + 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); + + int ch = getchar(); + + tcsetattr(STDIN_FILENO, TCSANOW, &tty_old); + + return ch; + } + + bool read_from_tty(std::string& aPass) + { + static constexpr const char BACKSPACE = 127; + + 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(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::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 -#include +#include 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; + + //! `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 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; + + password_container& operator=(const password_container&) = delete; + password_container& operator=(password_container&&) = default; + + const std::string& password() const noexcept { return m_password; } 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); - - bool m_empty; + //! 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 db4fae557..712f08adc 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -174,9 +174,7 @@ boost::optional 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)) @@ -192,19 +190,10 @@ boost::optional 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 generate_from_json(const std::string& json_file, bool testnet, bool restricted) @@ -431,6 +420,18 @@ void wallet2::init_options(boost::program_options::options_description& desc_par command_line::add_arg(desc_params, opts.restricted); } +boost::optional 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::make_from_json(const boost::program_options::variables_map& vm, const std::string& json_file) { const options opts{}; @@ -444,7 +445,7 @@ std::pair, 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) @@ -460,7 +461,7 @@ std::pair, 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 c3381730b..40c339e67 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -104,6 +104,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_prompt(const bool is_new_wallet); + //! Uses stdin and stdout. Returns a wallet2 if no errors. static std::unique_ptr 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())