wallet: encrypt the keystore before adding it to db
What the developer wrote
Authored by ThomasV
wallet: encrypt the keystore before adding it to db
adding keystore to db may result in a disk write, and we should
never write unencrypted keys to disk (except lightning_xprv)
What changed, and why it matters
This commit fixes a bug where Electrum could write unencrypted private keys to disk while creating or restoring a wallet, even when the user asked for password protection. The change rearranges the wallet setup so that the keystore is encrypted before it is placed into the wallet database, and the database storage is encrypted before any writes happen. It also moves the address-import step to happen after the keystore is saved, reducing the chance of plaintext key material touching disk.
Users who created or restored password-protected Electrum wallets with versions prior to this fix should consider rotating any funds associated with those wallets and, if the wallet file may have been stored on shared or backed-up media, treat the file as potentially containing plaintext key material. Developers should verify that no other code paths persist a keystore before encryption is enabled, and review partial-write behavior of WalletStorage.
Security signals we found
Plaintext private key material could be written to persistent storage before encryption was applied
Wallet creation and restoration paths reordered to encrypt before database insertion
Imported private key path now encrypts keystore before adding derived addresses and saving
Assertion added that storage file does not exist too early during restore
Lightning xprv now derived with the user password instead of no password in create_new_wallet
Evidence from the diff
The patch reorders create_new_wallet() and restore_wallet_from_text() so that storage.set_password() and db.set_keystore_encryption() are called before db.put(‘keystore’, k.dump()) and before any save_db(). The keystore’s update_password(None, password) is now invoked before dumping it into the WalletDB. For imported private-key wallets, import_private_keys() was split: self.keystore.import_private_keys() encrypts keys first, save_keystore() persists the encrypted keystore, and only then are derived addresses added. A new helper _add_imported_addresses() separates address metadata insertion from key import. The old flow called wallet.update_password() after the database had already been populated and potentially flushed, which could leave plaintext xprv/imported keys in the wallet file or in partial-write artifacts.
Changed components
electrum/wallet.pyelectrum/keystore.pyImported_KeyStore.import_private_keysImported_Wallet.import_private_keyscreate_new_walletrestore_wallet_from_textInspect captured patch +41 / −24
diff --git a/electrum/keystore.py b/electrum/keystore.py
index e162185..0d7fd8e 100644
--- a/electrum/keystore.py
+++ b/electrum/keystore.py
@@ -304,6 +304,21 @@ class Imported_KeyStore(Software_KeyStore):
self.keypairs[pubkey] = pw_encode(serialized_privkey, password, version=self.pw_hash_version)
return txin_type, pubkey
+ def import_private_keys(self, keys: Sequence[str], password: Optional[str]):
+ good_inputs = [] # type: List[Tuple[str, bytes]]
+ bad_keys = [] # type: List[Tuple[str, str]]
+ for key in keys:
+ try:
+ txin_type, pubkey = self.import_privkey(key, password)
+ except Exception as e:
+ bad_keys.append((key, 'invalid private key' + f': {e}'))
+ continue
+ if txin_type not in ('p2pkh', 'p2wpkh', 'p2wpkh-p2sh'):
+ bad_keys.append((key, 'not implemented type' + f': {txin_type}'))
+ continue
+ good_inputs.append((txin_type, pubkey))
+ return good_inputs, bad_keys
+
def delete_imported_key(self, key: str) -> None:
self.keypairs.pop(key)
diff --git a/electrum/wallet.py b/electrum/wallet.py
index 90a0040..0de4221 100644
--- a/electrum/wallet.py
+++ b/electrum/wallet.py
@@ -3904,26 +3904,20 @@ class Imported_Wallet(Simple_Wallet):
x = self.db.get_imported_address(address)
return x.get('pubkey') if x else None
- def import_private_keys(self, keys: Sequence[str], password: Optional[str], *,
- write_to_disk=True) -> Tuple[List[str], List[Tuple[str, str]]]:
- good_addr = [] # type: List[str]
- bad_keys = [] # type: List[Tuple[str, str]]
- for key in keys:
- try:
- txin_type, pubkey = self.keystore.import_privkey(key, password)
- except Exception as e:
- bad_keys.append((key, _('invalid private key') + f': {e}'))
- continue
- if txin_type not in ('p2pkh', 'p2wpkh', 'p2wpkh-p2sh'):
- bad_keys.append((key, _('not implemented type') + f': {txin_type}'))
- continue
+ def _add_imported_addresses(self, good_inputs):
+ for txin_type, pubkey in good_inputs:
addr = bitcoin.pubkey_to_address(txin_type, pubkey)
- good_addr.append(addr)
self.db.add_imported_address(addr, {'type': txin_type, 'pubkey': pubkey})
self.adb.add_address(addr)
+
+ def import_private_keys(self, keys: Sequence[str], password: Optional[str], *,
+ write_to_disk=True) -> Tuple[List[str], List[Tuple[str, str]]]:
+ good_inputs, bad_keys = self.keystore.import_private_keys(keys, password)
self.save_keystore()
+ self._add_imported_addresses(good_inputs)
if write_to_disk:
self.save_db()
+ good_addr = [bitcoin.pubkey_to_address(txin_type, pubkey) for txin_type, pubkey in good_inputs]
return good_addr, bad_keys
def import_private_key(self, key: str, password: Optional[str]) -> str:
@@ -4415,20 +4409,22 @@ def create_new_wallet(
storage = WalletStorage(path, allow_partial_writes=config.WALLET_PARTIAL_WRITES)
if storage.file_exists():
raise UserFacingException("Remove the existing wallet first!")
+ if encrypt_file:
+ storage.set_password(password, StorageEncryptionVersion.USER_PASSWORD)
db = WalletDB('', storage=storage, upgrade=True)
-
seed = Mnemonic('en').make_seed(seed_type=seed_type)
k = keystore.from_seed(seed, passphrase=passphrase)
+ k.update_password(None, password)
db.put('keystore', k.dump())
db.put('wallet_type', 'standard')
if k.can_have_deterministic_lightning_xprv():
- db.put('lightning_xprv', k.get_lightning_xprv(None))
+ db.put('lightning_xprv', k.get_lightning_xprv(password))
if gap_limit is not None:
db.put('gap_limit', gap_limit)
if gap_limit_for_change is not None:
db.put('gap_limit_for_change', gap_limit_for_change)
+ db.set_keystore_encryption(bool(password))
wallet = Wallet(db, config=config)
- wallet.update_password(old_pw=None, new_pw=password, encrypt_storage=encrypt_file)
wallet.synchronize()
msg = "Please keep your seed in a safe place; if you lose it, you will not be able to restore your wallet."
wallet.save_db()
@@ -4450,15 +4446,18 @@ def restore_wallet_from_text(
"""Restore a wallet from text. Text can be a seed phrase, a master
public key, a master private key, a list of bitcoin addresses
or bitcoin private keys."""
+ if encrypt_file is None:
+ encrypt_file = True
if path is None: # create wallet in-memory
storage = None
else:
storage = WalletStorage(path, allow_partial_writes=config.WALLET_PARTIAL_WRITES)
if storage.file_exists():
raise UserFacingException("Remove the existing wallet first!")
- if encrypt_file is None:
- encrypt_file = True
+ if encrypt_file:
+ storage.set_password(password, StorageEncryptionVersion.USER_PASSWORD)
db = WalletDB('', storage=storage, upgrade=True)
+ db.set_keystore_encryption(bool(password))
text = text.strip()
if keystore.is_address_list(text):
wallet = Imported_Wallet(db, config=config)
@@ -4468,14 +4467,16 @@ def restore_wallet_from_text(
if not good_inputs:
raise UserFacingException("None of the given addresses can be imported")
elif keystore.is_private_key_list(text, allow_spaces_inside_key=False):
- k = keystore.Imported_KeyStore({})
- db.put('keystore', k.dump())
- wallet = Imported_Wallet(db, config=config)
keys = keystore.get_private_keys(text, allow_spaces_inside_key=False)
- good_inputs, bad_inputs = wallet.import_private_keys(keys, None, write_to_disk=False)
+ k = keystore.Imported_KeyStore({})
+ good_inputs, bad_inputs = k.import_private_keys(keys, None)
# FIXME tell user about bad_inputs
if not good_inputs:
raise UserFacingException("None of the given privkeys can be imported")
+ k.update_password(None, password)
+ db.put('keystore', k.dump())
+ wallet = Imported_Wallet(db, config=config)
+ wallet._add_imported_addresses(good_inputs)
else:
if keystore.is_master_key(text):
k = keystore.from_master_key(text)
@@ -4485,6 +4486,8 @@ def restore_wallet_from_text(
db.put('lightning_xprv', k.get_lightning_xprv(None))
else:
raise UserFacingException("Seed or key not recognized")
+ if not k.is_watching_only():
+ k.update_password(None, password)
db.put('keystore', k.dump())
db.put('wallet_type', 'standard')
if gap_limit is not None:
@@ -4494,7 +4497,6 @@ def restore_wallet_from_text(
wallet = wallet_factory(db, config=config)
if db.storage:
assert not db.storage.file_exists(), "file was created too soon! plaintext keys might have been written to disk"
- wallet.update_password(old_pw=None, new_pw=password, encrypt_storage=encrypt_file)
wallet.synchronize()
msg = ("This wallet was restored offline. It may contain more addresses than displayed. "
"Start a daemon and use load_wallet to sync its history.")
Why this scored 71/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.