Merge pull request #10872 from f321x/trezor_always_on_device_passphrase
What changed, and why it matters
This commit fixes how Electrum's Trezor hardware wallet plugin handles passphrases. Previously, the code could prompt the user to enter their passphrase on the computer even when the Trezor device was configured to ask for the passphrase on the device itself. This could make users type their passphrase twice or, worse, enter a sensitive passphrase into the computer when they expected to enter it only on the device. The patch also makes the settings screen more robust when the device is locked. It is a usability and security-hardening fix rather than a remote-exploitable vulnerability.
Users who use Trezor devices with passphrase protection, especially with on-device passphrase entry, should upgrade to a version containing this commit. Reviewers should verify that PassphraseSetting.ON_DEVICE is correctly propagated to the Trezor library and that no fallback path reintroduces host-side passphrase prompts when on-device entry is configured.
Security signals we found
Passphrase entry mode mismatch between software and hardware wallet
Potential unintended exposure of passphrase to host computer
Device feature access after unpairing could yield None values
UI crash/undefined behavior when device is locked
Evidence from the diff
The change refactors Trezor session creation in clientbase.py. It now calls ensure_unlocked() first so device features are populated, then checks features.passphrase_protection and the new features.passphrase_always_on_device flag. If passphrase_always_on_device is true, it uses PassphraseSetting.ON_DEVICE instead of calling get_passphrase(), which would prompt the user in Electrum. This prevents double passphrase entry and ensures the passphrase is entered only on the device when that mode is enabled. The qt.py changes handle cases where client.features may be None after unpairing or when the device is locked, avoiding crashes in the settings UI.
Changed components
electrum/plugins/trezor/clientbase.pyelectrum/plugins/trezor/qt.pyTrezor hardware wallet integrationPassphrase handling flowInspect captured patch +22 / −21
### electrum/plugins/trezor/clientbase.py
@@ -110,25 +110,21 @@ def session(self):
if self._session is None:
assert self.handler is not None, "No UI handler for session"
self.pair_if_needed()
-
- # If needed, unlock the device (triggering PIN entry dialog for legacy model).
- with self.client.get_session(passphrase=PassphraseSetting.STANDARD_WALLET) as session:
- session.ensure_unlocked()
-
- passphrase = PassphraseSetting.STANDARD_WALLET # (empty passphrase)
- if self.client.features.passphrase_protection:
- passphrase = self.get_passphrase(Capability.PassphraseEntry in self.client.features.capabilities)
-
- # Then, derive a session for this wallet (possibly with a passphrase)
- if passphrase == PassphraseSetting.STANDARD_WALLET:
- self._session = session # reuse the session above to avoid re-derivation
- self.logger.info("Opened standard %s", self._session)
+ self.client.ensure_unlocked() # unlock device so features are populated and we know about passphrase
+
+ features = self.client.features
+ if not features.passphrase_protection:
+ passphrase = PassphraseSetting.STANDARD_WALLET # (empty passphrase)
+ elif features.passphrase_always_on_device:
+ # the device asks for the passphrase itself, prompting in electrum as well would make the user enter it twice
+ passphrase = PassphraseSetting.ON_DEVICE
else:
- self._session = self.client.get_session(passphrase)
- self.logger.info("Re-opened passphrase %s", self._session)
+ passphrase = self.get_passphrase(Capability.PassphraseEntry in features.capabilities)
- return self._session
+ self._session = self.client.get_session(passphrase=passphrase)
+ self.logger.info(f"Opened {self._session} ({features.passphrase_protection=}, on_device={passphrase is PassphraseSetting.ON_DEVICE})")
+ return self._session
def run_flow(self, message=None, creating_wallet=False):
if self.in_flow:
### electrum/plugins/trezor/qt.py
@@ -506,9 +506,12 @@ def task():
raise RuntimeError("Device not connected")
if method:
getattr(client, method)(*args, **kw_args)
- if unpair_after:
- devmgr.unpair_id(device_id)
- return client.features
+ try:
+ features = client.features # some features are set None after unpairing, so store them first
+ finally: # always clean up
+ if unpair_after:
+ devmgr.unpair_id(device_id)
+ return features
thread.add(task, on_success=update)
@@ -531,7 +534,8 @@ def update(features):
device_label.setText(features.label)
pin_set_label.setText(noyes[features.pin_protection])
- passphrases_label.setText(disen[features.passphrase_protection])
+ passphrase_protection = features.passphrase_protection # might be None if device is locked
+ passphrases_label.setText(disen[passphrase_protection] if passphrase_protection is not None else _("Unknown"))
bl_hash_label.setText(bl_hash)
label_edit.setText(features.label)
device_id_label.setText(features.device_id)
@@ -541,7 +545,8 @@ def update(features):
clear_pin_warning.setVisible(features.pin_protection)
pin_button.setText(setchange[features.pin_protection])
pin_msg.setVisible(not features.pin_protection)
- passphrase_button.setText(endis[features.passphrase_protection])
+ passphrase_button.setText(endis[bool(passphrase_protection)])
+ passphrase_button.setEnabled(passphrase_protection is not None)
language_label.setText(features.language)
def set_label_enabled():Why this scored 29/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.