From a4272de79746c2349a0da8371f45390989cc6b90 Mon Sep 17 00:00:00 2001 From: stoffu Date: Tue, 3 Jul 2018 11:33:11 +0900 Subject: [PATCH] wallet2: unlock keys file before calling verify_password (needed for Windows) Also added notes to WalletManager::verifyWalletPassword (which afaik seems unused by anyone at the moment) regarding the need to unlock the keys file beforehand. --- src/wallet/api/wallet.cpp | 14 +++++++++++ src/wallet/api/wallet.h | 3 +++ src/wallet/api/wallet2_api.h | 10 ++++++++ src/wallet/wallet2.cpp | 49 +++++++++++++++++++++++++++++------- src/wallet/wallet2.h | 5 +++- 5 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index 3f6bfec9e..680da26ce 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -2205,6 +2205,20 @@ void WalletImpl::keyReuseMitigation2(bool mitigation) m_wallet->key_reuse_mitigation2(mitigation); } +bool WalletImpl::lockKeysFile() +{ + return m_wallet->lock_keys_file(); +} + +bool WalletImpl::unlockKeysFile() +{ + return m_wallet->unlock_keys_file(); +} + +bool WalletImpl::isKeysFileLocked() +{ + return m_wallet->is_keys_file_locked(); +} } // namespace namespace Bitmonero = Monero; diff --git a/src/wallet/api/wallet.h b/src/wallet/api/wallet.h index eefb2fe94..58be686fc 100644 --- a/src/wallet/api/wallet.h +++ b/src/wallet/api/wallet.h @@ -188,6 +188,9 @@ public: virtual void segregatePreForkOutputs(bool segregate) override; virtual void segregationHeight(uint64_t height) override; virtual void keyReuseMitigation2(bool mitigation) override; + virtual bool lockKeysFile() override; + virtual bool unlockKeysFile() override; + virtual bool isKeysFileLocked() override; private: void clearStatus() const; diff --git a/src/wallet/api/wallet2_api.h b/src/wallet/api/wallet2_api.h index f54255e91..0cd0ff5cf 100644 --- a/src/wallet/api/wallet2_api.h +++ b/src/wallet/api/wallet2_api.h @@ -900,6 +900,12 @@ struct Wallet //! Initiates a light wallet import wallet request virtual bool lightWalletImportWalletRequest(std::string &payment_id, uint64_t &fee, bool &new_request, bool &request_fulfilled, std::string &payment_address, std::string &status) = 0; + + //! locks/unlocks the keys file; returns true on success + virtual bool lockKeysFile() = 0; + virtual bool unlockKeysFile() = 0; + //! returns true if the keys file is locked + virtual bool isKeysFileLocked() = 0; }; /** @@ -1070,6 +1076,10 @@ struct WalletManager * @param password - password to verify * @param no_spend_key - verify only view keys? * @return - true if password is correct + * + * @note + * This function will fail when the wallet keys file is opened because the wallet program locks the keys file. + * In this case, Wallet::unlockKeysFile() and Wallet::lockKeysFile() need to be called before and after the call to this function, respectively. */ virtual bool verifyWalletPassword(const std::string &keys_file_name, const std::string &password, bool no_spend_key) const = 0; diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 1a1537e62..7433cb9a5 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -2636,7 +2636,7 @@ void wallet2::detach_blockchain(uint64_t height) bool wallet2::deinit() { m_is_initialized=false; - m_keys_file_locker.reset(); + unlock_keys_file(); return true; } //---------------------------------------------------------------------------------------------------- @@ -2803,12 +2803,12 @@ bool wallet2::store_keys(const std::string& keys_file_name, const epee::wipeable crypto::chacha20(account_data.data(), account_data.size(), key, keys_file_data.iv, &cipher[0]); keys_file_data.account_data = cipher; - m_keys_file_locker.reset(); + unlock_keys_file(); std::string buf; r = ::serialization::dump_binary(keys_file_data, buf); r = r && epee::file_io_utils::save_string_to_file(keys_file_name, buf); //and never touch wallet_keys_file again, only read CHECK_AND_ASSERT_MES(r, false, "failed to generate wallet keys file " << keys_file_name); - m_keys_file_locker.reset(new tools::file_locker(m_keys_file)); + lock_keys_file(); return true; } @@ -3029,9 +3029,13 @@ bool wallet2::load_keys(const std::string& keys_file_name, const epee::wipeable_ * can be used prior to rewriting wallet keys file, to ensure user has entered the correct password * */ -bool wallet2::verify_password(const epee::wipeable_string& password) const +bool wallet2::verify_password(const epee::wipeable_string& password) { - return verify_password(m_keys_file, password, m_watch_only || m_multisig, m_account.get_device()); + // this temporary unlocking is necessary for Windows (otherwise the file couldn't be loaded). + unlock_keys_file(); + bool r = verify_password(m_keys_file, password, m_watch_only || m_multisig, m_account.get_device()); + lock_keys_file(); + return r; } /*! @@ -3938,17 +3942,17 @@ void wallet2::load(const std::string& wallet_, const epee::wipeable_string& pass boost::system::error_code e; bool exists = boost::filesystem::exists(m_keys_file, e); THROW_WALLET_EXCEPTION_IF(e || !exists, error::file_not_found, m_keys_file); - m_keys_file_locker.reset(new tools::file_locker(m_keys_file)); - THROW_WALLET_EXCEPTION_IF(!m_keys_file_locker->locked(), error::wallet_internal_error, "internal error: \"" + m_keys_file + "\" is opened by another wallet program"); + lock_keys_file(); + THROW_WALLET_EXCEPTION_IF(!is_keys_file_locked(), error::wallet_internal_error, "internal error: \"" + m_keys_file + "\" is opened by another wallet program"); // this temporary unlocking is necessary for Windows (otherwise the file couldn't be loaded). - m_keys_file_locker.reset(); + unlock_keys_file(); if (!load_keys(m_keys_file, password)) { THROW_WALLET_EXCEPTION_IF(true, error::file_read_error, m_keys_file); } LOG_PRINT_L0("Loaded wallet keys file, with public address: " << m_account.get_public_address_str(m_nettype)); - m_keys_file_locker.reset(new tools::file_locker(m_keys_file)); + lock_keys_file(); //keys loaded ok! //try to load wallet file. but even if we failed, it is not big problem @@ -5989,6 +5993,33 @@ bool wallet2::is_output_blackballed(const crypto::public_key &output) const catch (const std::exception &e) { return false; } } +bool wallet2::lock_keys_file() +{ + if (m_keys_file_locker) + { + MDEBUG(m_keys_file << " is already locked."); + return false; + } + m_keys_file_locker.reset(new tools::file_locker(m_keys_file)); + return true; +} + +bool wallet2::unlock_keys_file() +{ + if (!m_keys_file_locker) + { + MDEBUG(m_keys_file << " is already unlocked."); + return false; + } + m_keys_file_locker.reset(); + return true; +} + +bool wallet2::is_keys_file_locked() const +{ + return m_keys_file_locker->locked(); +} + bool wallet2::tx_add_fake_output(std::vector> &outs, uint64_t global_index, const crypto::public_key& output_public_key, const rct::key& mask, uint64_t real_index, bool unlocked) const { if (!unlocked) // don't add locked outs diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index d33d8258b..c54587693 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -609,7 +609,7 @@ namespace tools /*! * \brief verifies given password is correct for default wallet keys file */ - bool verify_password(const epee::wipeable_string& password) const; + bool verify_password(const epee::wipeable_string& password); cryptonote::account_base& get_account(){return m_account;} const cryptonote::account_base& get_account()const{return m_account;} @@ -1144,6 +1144,9 @@ namespace tools bool unblackball_output(const crypto::public_key &output); bool is_output_blackballed(const crypto::public_key &output) const; + bool lock_keys_file(); + bool unlock_keys_file(); + bool is_keys_file_locked() const; private: /*! * \brief Stores wallet information to wallet file.