From c463f5e23daff71f05486f53ab04510d538dbec7 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Wed, 6 Jul 2022 18:33:08 +0200 Subject: [PATCH] password unification refactor: move methods from wallet to daemon Note in particular that check_password_for_directory was not safe to use while the daemon had wallets loaded, as the same file would have two corresponding Wallet() instances in memory. This was specifically handled in the kivy GUI, on the caller side, by stopping-before and reloading-after the wallets; but it was dirty to have the caller handle this. --- electrum/daemon.py | 94 +++++++++++++++++++++++++++++++- electrum/gui/kivy/main_window.py | 9 +-- electrum/storage.py | 3 + electrum/wallet.py | 77 -------------------------- 4 files changed, 97 insertions(+), 86 deletions(-) diff --git a/electrum/daemon.py b/electrum/daemon.py index 211696948..dbd99f255 100644 --- a/electrum/daemon.py +++ b/electrum/daemon.py @@ -478,6 +478,7 @@ class Daemon(Logger): self.gui_object = None # path -> wallet; make sure path is standardized. self._wallets = {} # type: Dict[str, Abstract_Wallet] + self._wallet_lock = threading.RLock() daemon_jobs = [] # Setup commands server self.commands_server = None @@ -526,12 +527,35 @@ class Daemon(Logger): # not see the exception (especially if the GUI did not start yet). self._stopping_soon_or_errored.set() + def with_wallet_lock(func): + def func_wrapper(self: 'Daemon', *args, **kwargs): + with self._wallet_lock: + return func(self, *args, **kwargs) + return func_wrapper + + @with_wallet_lock def load_wallet(self, path, password, *, manual_upgrades=True) -> Optional[Abstract_Wallet]: path = standardize_path(path) # wizard will be launched if we return if path in self._wallets: wallet = self._wallets[path] return wallet + wallet = self._load_wallet(path, password, manual_upgrades=manual_upgrades, config=self.config) + if wallet is None: + return + wallet.start_network(self.network) + self._wallets[path] = wallet + return wallet + + @staticmethod + def _load_wallet( + path, + password, + *, + manual_upgrades: bool = True, + config: SimpleConfig, + ) -> Optional[Abstract_Wallet]: + path = standardize_path(path) storage = WalletStorage(path) if not storage.file_exists(): return @@ -547,11 +571,10 @@ class Daemon(Logger): return if db.get_action(): return - wallet = Wallet(db, storage, config=self.config) - wallet.start_network(self.network) - self._wallets[path] = wallet + wallet = Wallet(db, storage, config=config) return wallet + @with_wallet_lock def add_wallet(self, wallet: Abstract_Wallet) -> None: path = wallet.storage.path path = standardize_path(path) @@ -561,6 +584,7 @@ class Daemon(Logger): path = standardize_path(path) return self._wallets.get(path) + @with_wallet_lock def get_wallets(self) -> Dict[str, Abstract_Wallet]: return dict(self._wallets) # copy @@ -577,6 +601,7 @@ class Daemon(Logger): fut = asyncio.run_coroutine_threadsafe(self._stop_wallet(path), self.asyncio_loop) return fut.result() + @with_wallet_lock async def _stop_wallet(self, path: str) -> bool: """Returns True iff a wallet was found.""" path = standardize_path(path) @@ -642,3 +667,66 @@ class Daemon(Logger): finally: # app will exit now asyncio.run_coroutine_threadsafe(self.stop(), self.asyncio_loop).result() + + @with_wallet_lock + def _check_password_for_directory(self, *, old_password, new_password=None, wallet_dir: str) -> Tuple[bool, bool]: + """Checks password against all wallets (in dir), returns whether they can be unified and whether they are already. + If new_password is not None, update all wallet passwords to new_password. + """ + assert os.path.exists(wallet_dir), f"path {wallet_dir!r} does not exist" + failed = [] + is_unified = True + for filename in os.listdir(wallet_dir): + path = os.path.join(wallet_dir, filename) + path = standardize_path(path) + if not os.path.isfile(path): + continue + wallet = self.get_wallet(path) + if wallet is None: + try: + wallet = self._load_wallet(path, old_password, manual_upgrades=False, config=self.config) + except util.InvalidPassword: + pass + except Exception: + self.logger.exception(f'failed to load wallet at {path!r}:') + pass + if wallet is None: + failed.append(path) + continue + if not wallet.storage.is_encrypted(): + is_unified = False + try: + wallet.check_password(old_password) + except Exception: + failed.append(path) + continue + if new_password: + self.logger.info(f'updating password for wallet: {path!r}') + wallet.update_password(old_password, new_password, encrypt_storage=True) + can_be_unified = failed == [] + is_unified = can_be_unified and is_unified + return can_be_unified, is_unified + + @with_wallet_lock + def update_password_for_directory( + self, + *, + old_password, + new_password, + wallet_dir: Optional[str] = None, + ) -> bool: + """returns whether password is unified""" + if new_password is None: + # we opened a non-encrypted wallet + return False + if wallet_dir is None: + wallet_dir = os.path.dirname(self.config.get_wallet_path()) + can_be_unified, is_unified = self._check_password_for_directory( + old_password=old_password, new_password=None, wallet_dir=wallet_dir) + if not can_be_unified: + return False + if is_unified and old_password == new_password: + return True + self._check_password_for_directory( + old_password=old_password, new_password=new_password, wallet_dir=wallet_dir) + return True diff --git a/electrum/gui/kivy/main_window.py b/electrum/gui/kivy/main_window.py index 239052914..682f30dc3 100644 --- a/electrum/gui/kivy/main_window.py +++ b/electrum/gui/kivy/main_window.py @@ -12,7 +12,6 @@ from typing import TYPE_CHECKING, Optional, Union, Callable, Sequence from electrum.storage import WalletStorage, StorageReadWriteError from electrum.wallet_db import WalletDB from electrum.wallet import Wallet, InternalAddressCorruption, Abstract_Wallet -from electrum.wallet import update_password_for_directory from electrum.plugin import run_hook from electrum import util @@ -679,7 +678,8 @@ class ElectrumWindow(App, Logger, EventListener): def on_wizard_success(self, storage, db, password): self.password = password if self.electrum_config.get('single_password'): - self._use_single_password = update_password_for_directory(self.electrum_config, password, password) + self._use_single_password = self.daemon.update_password_for_directory( + old_password=password, new_password=password) self.logger.info(f'use single password: {self._use_single_password}') wallet = Wallet(db, storage, config=self.electrum_config) wallet.start_network(self.daemon.network) @@ -1346,10 +1346,7 @@ class ElectrumWindow(App, Logger, EventListener): # called if old_password works on self.wallet self.password = new_password if self._use_single_password: - path = self.wallet.storage.path - self.stop_wallet() - update_password_for_directory(self.electrum_config, old_password, new_password) - self.load_wallet_by_name(path) + self.daemon.update_password_for_directory(old_password=old_password, new_password=new_password) msg = _("Password updated successfully") else: self.wallet.update_password(old_password, new_password) diff --git a/electrum/storage.py b/electrum/storage.py index 17a2f6d59..e80fd1ca3 100644 --- a/electrum/storage.py +++ b/electrum/storage.py @@ -146,6 +146,8 @@ class WalletStorage(Logger): @staticmethod def get_eckey_from_password(password): + if password is None: + password = "" secret = hashlib.pbkdf2_hmac('sha512', password.encode('utf-8'), b'', iterations=1024) ec_key = ecc.ECPrivkey.from_arbitrary_size_secret(secret) return ec_key @@ -160,6 +162,7 @@ class WalletStorage(Logger): raise WalletFileException('no encryption magic for version: %s' % v) def decrypt(self, password) -> None: + """Raises an InvalidPassword exception on invalid password""" if self.is_past_initial_decryption(): return ec_key = self.get_eckey_from_password(password) diff --git a/electrum/wallet.py b/electrum/wallet.py index c2280b29d..e3bcf3502 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -3525,80 +3525,3 @@ def restore_wallet_from_text(text, *, path, config: SimpleConfig, "Start a daemon and use load_wallet to sync its history.") wallet.save_db() return {'wallet': wallet, 'msg': msg} - - -def check_password_for_directory(config: SimpleConfig, old_password, new_password=None) -> Tuple[bool, bool]: - """Checks password against all wallets, returns whether they can be unified and whether they are already. - If new_password is not None, update all wallet passwords to new_password. - """ - dirname = os.path.dirname(config.get_wallet_path()) - failed = [] - is_unified = True - for filename in os.listdir(dirname): - path = os.path.join(dirname, filename) - if not os.path.isfile(path): - continue - basename = os.path.basename(path) - storage = WalletStorage(path) - if not storage.is_encrypted(): - is_unified = False - # it is a bit wasteful load the wallet here, but that is fine - # because we are progressively enforcing storage encryption. - try: - db = WalletDB(storage.read(), manual_upgrades=False) - wallet = Wallet(db, storage, config=config) - except: - _logger.exception(f'failed to load {basename}:') - failed.append(basename) - continue - if wallet.has_keystore_encryption(): - try: - wallet.check_password(old_password) - except: - failed.append(basename) - continue - if new_password: - wallet.update_password(old_password, new_password) - else: - if new_password: - wallet.update_password(None, new_password) - continue - if not storage.is_encrypted_with_user_pw(): - failed.append(basename) - continue - try: - storage.check_password(old_password) - except: - failed.append(basename) - continue - try: - db = WalletDB(storage.read(), manual_upgrades=False) - wallet = Wallet(db, storage, config=config) - except: - _logger.exception(f'failed to load {basename}:') - failed.append(basename) - continue - try: - wallet.check_password(old_password) - except: - failed.append(basename) - continue - if new_password: - wallet.update_password(old_password, new_password) - can_be_unified = failed == [] - is_unified = can_be_unified and is_unified - return can_be_unified, is_unified - - -def update_password_for_directory(config: SimpleConfig, old_password, new_password) -> bool: - " returns whether password is unified " - if new_password is None: - # we opened a non-encrypted wallet - return False - can_be_unified, is_unified = check_password_for_directory(config, old_password, None) - if not can_be_unified: - return False - if is_unified and old_password == new_password: - return True - check_password_for_directory(config, old_password, new_password) - return True