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.

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<std::vector<tools::wallet2::get_outs_entry>> &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.