Merge pull request #1013 from kdmukai/psbt_parser_ownership_scan
What changed, and why it matters
This commit hardens SeedSigner's PSBT (Bitcoin transaction) parser so it no longer trusts a transaction's claim that an output belongs to the user's wallet. Previously, a malicious or buggy transaction could mark a payment to an attacker's address as if it were change going back to the user, potentially tricking the user into approving a theft. The fix re-derives every claimed key from the actual seed and rejects the transaction if the claim does not hold up. It also rejects false claims on inputs and clearly tells the user when the wrong seed was selected.
Treat this as a security fix and include it in the next release. Users should upgrade to a version containing this commit before signing PSBTs from untrusted coordinators. Reviewers should verify that the new exception handlers in psbt_views.py cannot be bypassed and that the ownership scan covers all scope types (legacy BIP32, taproot key-path, and future tapscript leaves).
Security signals we found
Fixes fake-change / ownership-spoofing vulnerability in PSBT parsing
Adds explicit rejection of false seed-ownership claims on outputs and inputs
Adds user-facing warning screens for forged ownership claims
Corrects taproot public-key comparison to x-only, avoiding false rejections of valid keys
Adds test fixtures and unit/flow tests for forged single-sig, taproot, multisig, and fingerprint-collision cases
Evidence from the diff
The patch introduces an ownership-verification pass in PSBTParser before any transaction details are shown. It adds PSBTVerificationError subclasses (PSBTOutputOwnershipClaimError, PSBTInputOwnershipClaimError, PSBTSeedCannotSignError), a seed_owns_pubkey() helper that compares derived vs claimed public keys (x-only for taproot, full-key for ECDSA), and _verify_claimed_derivation_paths() which scans every input/output BIP32 and taproot derivation entry that names the seed’s fingerprint. False output claims raise an attack-framed error routed to a dire warning; false input claims raise a milder warning; a seed that cannot sign any input is routed back to seed selection. The change also fixes a taproot parity bug in has_matching_input_fingerprint() and _fill_missing_fingerprints() by comparing x-only public keys, and adds extensive tests including forged-claim, collision, multisig, and cache-coverage cases.
Changed components
src/seedsigner/models/psbt_parser.pysrc/seedsigner/views/psbt_views.pytests/psbt_testing_util.pytests/test_flows_psbt.pytests/test_psbt_parser.pytests/screenshot_generator/generator.pyInspect captured patch +994 / −51
### src/seedsigner/models/psbt_parser.py
@@ -19,14 +19,71 @@ class OPCODES:
+class PSBTVerificationError(Exception):
+ """
+ Base for the checks that reject a psbt outright while it is being parsed.
+
+ A psbt that trips one of these is never shown to the user. The view layer should catch
+ these exceptions and route to a warning (the severity of which and the allowed next
+ steps are determined by the specific subclass error encountered).
+ """
+ pass
+
+
+class PSBTOutputOwnershipClaimError(PSBTVerificationError):
+ """
+ An output scope claims this seed's fingerprint on a key the seed does not derive.
+
+ This is not a psbt that merely fails to be ours. A fingerprint is
+ coordinator-supplied metadata, so this is a psbt asserting that a key belongs to this
+ seed when it does not. On an output that assertion is how a fake change output is
+ dressed up as the user's own, so it should be treated as an attack.
+ """
+ pass
+
+
+class PSBTInputOwnershipClaimError(PSBTVerificationError):
+ """
+ An input scope claims this seed's fingerprint on a key the seed does not derive.
+
+ The same false claim as PSBTOutputOwnershipClaimError, but on an input the threat
+ picture inverts. A forged input claim has no path to losing funds: it cannot produce a
+ signature (embit re-derives the real key and refuses on a mismatch), and it cannot
+ alter the amounts, the fee, or how outputs are classified. The likely causes are
+ instead a psbt assembled for a different wallet, a corrupted entry, or a collaborative
+ spend that happens to include a key whose 4-byte fingerprint collides with ours (1 in
+ 2^32 chance).
+
+ The psbt still fails, deliberately, following embit's lead: sign_with raises on this
+ same condition and abandons the entire signing pass, so tolerating the entry here
+ would only defer the failure to a worse spot. Changing this behavior, if desired,
+ should happen in embit first. Until then the trade-off is accepted: a
+ collaborative-spend counterparty could grief such a transaction into unsignability.
+ """
+ pass
+
+
+class PSBTSeedCannotSignError(PSBTVerificationError):
+ """
+ The selected seed holds no key that could sign any input.
+
+ This is a mismatch rather than an attack: the usual cause is the user picking the
+ wrong seed. It is raised so the flow can say so up front, instead of walking the user
+ through reviewing and approving a transaction that would then produce no signatures.
+ """
+ pass
+
+
+
class PSBTParser():
"""
Reads a psbt on behalf of one seed and works out everything the signing flow shows the
user before they approve: the wallet policy (script type, plus m-of-n and the
cosigners for multisig), the amount coming in, what is being spent, what comes back as
change, the fee, where the spend is going, and any OP_RETURN payload.
- Constructing it with a seed parses immediately.
+ Constructing it with a seed parses immediately; see parse() for what that establishes
+ in what order and which psbts it turns away.
The parse fully processes the psbt, validates what it can, then stores the organized
results in the instance attributes (spend_amount, fee_amount, destination_addresses,
@@ -76,6 +133,12 @@ def __init__(self, p: PSBT, seed: Seed, network: str = SettingsConstants.MAINNET
self.destination_amounts = []
self.op_return_data: bytes = None
+ # Indexed alongside psbt.inputs / psbt.outputs. Each entry is the derivation path
+ # the seed genuinely owns in each scope or None where it owns nothing. Determined
+ # in _verify_claimed_derivation_paths.
+ self.verified_input_derivation_paths: List[List[int] | None] = []
+ self.verified_output_derivation_paths: List[List[int] | None] = []
+
self.root = None
if self.seed is not None:
@@ -111,14 +174,46 @@ def _set_root(self):
def parse(self):
"""
+ Establishes, in order:
+
+ 1. _fill_missing_fingerprints: backfills all-zero fingerprints, but only for
+ scopes the seed provably derives.
+
+ 2. _verify_claimed_derivation_paths: each input and output scope that claims to
+ be controlled by the seed is verified. Raises an Input/Output
+ OwnershipClaimError if a claimed scope fails verification.
+
+ 3. _reject_if_seed_cannot_sign: raises PSBTSeedCannotSignError if none of the
+ inputs can be signed by the seed. A mismatch rather than an attack, caught
+ here so the flow can say so before showing a transaction.
+
+ 4. _parse_inputs: every input must resolve to the same policy otherwise a
+ RuntimeError is raised. TODO: make this a PSBTVerificationError subclass so
+ the view can deliberately catch this scenario and route accordingly.
+
+ A policy is one of:
+ - single-sig: the script type alone. Says nothing about keys.
+ - multisig, cosigners resolved: script type, m-of-n, and the cosigner
+ xpubs that every key in the script was traced back to.
+ - multisig, cosigners unresolved: script type and m-of-n only.
+ _get_policy doesn't propagate cosigner errors, so two such policies match
+ without anything having tied them to the same keys. TODO: don't let a
+ policy with no cosigner information pass as a match.
+
+ 5. _parse_outputs: works out which outputs come back to this seed. For
+ single-sig this proves the output script derives from the seed at the
+ claimed path. TODO: reject outputs at a path the user's wallet would never
+ scan.
+
+ Optimization via child_key_derivation_cache:
Parsing traverses a derivation path down to an individual address one level at a
time, over and over, and where that traversal begins depends on the wallet.
Single-sig traverses the full path down from our own master key, on every OUTPUT
the PSBT claims is ours.
Multisig instead traverses just the last two levels down from each cosigner's
- account xpub — once per cosigner, on every INPUT and on every OUTPUT carrying the
+ account xpub, once per cosigner, on every INPUT and on every OUTPUT carrying the
multisig script.
Deriving each level costs a hash and an elliptic curve operation, and these
@@ -145,6 +240,11 @@ def parse(self):
# Try to fix missing fingerprints before parsing
self._fill_missing_fingerprints(child_key_derivation_cache)
+ # Work out what this seed actually owns before anything below reads the psbt's
+ # claims about it.
+ self._verify_claimed_derivation_paths(child_key_derivation_cache)
+ self._reject_if_seed_cannot_sign()
+
rt = self._parse_inputs(child_key_derivation_cache)
if rt == False:
return False
@@ -232,9 +332,6 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
elif self.policy["type"] == "p2wpkh" and my_pubkey is not None:
sc = script.p2wpkh(my_pubkey)
- if sc.data == vout[i].script_pubkey.data:
- is_change = True
-
elif "p2tr" in self.policy["type"]:
my_pubkey = None
# should have one or zero derivations for single-key addresses
@@ -245,9 +342,6 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
my_pubkey = PSBTParser._derive_with_cache(self.root, der, child_key_derivation_cache)
sc = script.p2tr(my_pubkey)
- if sc.data == vout[i].script_pubkey.data:
- is_change = True
-
if sc.data == vout[i].script_pubkey.data:
is_change = True
@@ -353,6 +447,12 @@ def _get_policy(scope, scriptpubkey, xpubs, child_key_derivation_cache: dict | N
cosigners = PSBTParser._get_cosigners(pubkeys, scope.bip32_derivations, xpubs, child_key_derivation_cache)
policy.update({"m": m, "n": n, "cosigners": cosigners})
except:
+ # TODO: stop swallowing everything here. This also catches bugs in the
+ # cosigner check itself, and cannot tell those apart from the psbt
+ # simply not supplying xpubs to check against, which is valid and must
+ # not be rejected outright. The fallback policy carries no cosigner
+ # information at all, and two of those compare equal on script type
+ # and m-of-n alone. Fix pending with the multisig verification work.
policy.update({"m": m, "n": n})
return policy
@@ -398,7 +498,7 @@ def _derive_with_cache(parent_key: bip32.HDKey, derivation_path: List[int], chil
cache, so a later derivation running through that level picks it up instead of
deriving it a second time.
- Entries are keyed on (id(parent_key), derivation_path_so_far) — the path traversed
+ Entries are keyed on (id(parent_key), derivation_path_so_far), the path traversed
down from that parent to reach this point. id() is the Python built-in for an
object's identity; the parent belongs in the key because a multisig parse runs
these same derivations below each cosigner's xpub in turn.
@@ -499,38 +599,176 @@ def has_matching_input_fingerprint(psbt: PSBT, seed: Seed, network: str = Settin
"""
seed_fingerprint = seed.get_fingerprint(network)
- def check_fingerprint_match(public_key: PublicKey, derivation_path_obj: DerivationPath):
+ def check_fingerprint_match(public_key: PublicKey, derivation_path_obj: DerivationPath, is_taproot: bool):
"""Check fingerprint match with missing fingerprint fallback"""
# If exact fingerprint match
if hexlify(derivation_path_obj.fingerprint).decode() == seed_fingerprint:
return True
-
+
# Missing fingerprint fallback
if derivation_path_obj.fingerprint == b"\x00\x00\x00\x00":
root = bip32.HDKey.from_seed(seed.seed_bytes, version=NETWORKS[SettingsConstants.map_network_to_embit(network)]["xprv"])
try:
- derived_key = root.derive(derivation_path_obj.derivation)
- return derived_key.key.sec() == public_key.sec() # Public keys match
+ return PSBTParser.seed_owns_pubkey(root, derivation_path_obj.derivation, public_key, child_key_derivation_cache=None, is_taproot=is_taproot)
except Exception as e:
logger.debug("Fingerprint fallback derive failed: %s", e, exc_info=True)
return False
-
+
# Check all derivations in all inputs
for input in psbt.inputs:
# Check regular BIP32 derivations
for public_key, derivation_path_obj in input.bip32_derivations.items():
- if check_fingerprint_match(public_key, derivation_path_obj):
+ if check_fingerprint_match(public_key, derivation_path_obj, is_taproot=False):
return True
-
+
# Check Taproot derivations
for public_key, (leaf_hashes, derivation_path_obj) in input.taproot_bip32_derivations.items():
- if check_fingerprint_match(public_key, derivation_path_obj):
+ if check_fingerprint_match(public_key, derivation_path_obj, is_taproot=True):
return True
return False
+ @staticmethod
+ def seed_owns_pubkey(root: bip32.HDKey, claimed_derivation_path: List[int], public_key: PublicKey, child_key_derivation_cache: dict | None, is_taproot: bool = False) -> bool:
+ """
+ Returns True if the signing seed (root) really does derive public_key at
+ claimed_derivation_path.
+
+ This is the canonical ownership check. The fingerprint a psbt or a descriptor
+ carries alongside a key is metadata that whoever wrote the file chose, so it can
+ say anything. Ownership is established here and only here, by deriving the key
+ again from the seed and comparing the actual key material.
+ """
+ derived_public_key = PSBTParser._derive_with_cache(root, claimed_derivation_path, child_key_derivation_cache).get_public_key()
+
+ if is_taproot:
+ # A psbt carries a taproot key as its bare 32-byte x coordinate, but embit
+ # rebuilds a full key from it by just assuming even parity. The key derived
+ # from the seed carries its real parity, so a naive full-key comparison
+ # succeeds only when that real parity happens to be even, wrongly rejecting
+ # roughly half of the keys this seed genuinely owns. Only the x coordinate is
+ # real information: compare x-only.
+ return derived_public_key.xonly() == public_key.xonly()
+
+ # For ecdsa the parity byte IS part of the identity, so compare the full key.
+ # This is deliberately stricter than embit, whose sign_with compares x-only even
+ # for ecdsa. embit would sign a psbt whose entry names the parity-flipped twin of
+ # our real key. The flipped key is still one this seed does NOT derive, and the
+ # signature embit produces under it is one no standard finalizer can use. So this
+ # extra strictness only rejects transactions that could never actually complete.
+ return derived_public_key == public_key
+
+
+ @staticmethod
+ def _get_seed_derivation_path(scope: InputScope | OutputScope, root: bip32.HDKey, child_key_derivation_cache: dict) -> List[int] | None:
+ """
+ Scans the derivation path(s) in the provided input or output scope to determine
+ which, if any, are provably derived from the signing seed (for multisig a path is
+ provided per key; if the seed is part of the multisig, one of the n paths will
+ match). Returns the verified derivation path (as a list of ints) or None.
+
+ Every key in the scope that claims this seed's fingerprint is re-derived and
+ checked. A claim that does not hold up raises
+ PSBT[Output|Input]OwnershipClaimError. This includes fingerprint collisions (two
+ different keys with the same 4-byte fingerprint):
+ * On the output side, a collision is considered an attack.
+ * On the input side it is merely disallowed because it is unsignable by embit.
+
+ One edge case:
+ * A multisig could use this seed in more than one cosigner slot, each
+ at its own derivation path. The scope then carries several entries that all
+ verify against this seed; we return the first but still check the rest.
+
+ The path itself is still whatever the psbt supplied: it can be any length or
+ shape, since any path that derives from the seed will pass. Whether the path is
+ one the user's wallet would ever look at is a separate question, answered
+ elsewhere.
+ """
+ seed_fingerprint = root.my_fingerprint
+ verified_derivation_path = None
+
+ def _check_claim(public_key: PublicKey, derivation_path_obj: DerivationPath, is_taproot: bool):
+ nonlocal verified_derivation_path
+
+ if derivation_path_obj.fingerprint != seed_fingerprint:
+ # Claims to belong to some other key. Nothing to prove or disprove here.
+ return
+
+ if not PSBTParser.seed_owns_pubkey(root, derivation_path_obj.derivation, public_key, child_key_derivation_cache, is_taproot=is_taproot):
+ error_class = (PSBTInputOwnershipClaimError if isinstance(scope, InputScope) else PSBTOutputOwnershipClaimError)
+ raise error_class(f"Key at {bip32.path_to_str(derivation_path_obj.derivation)} claims this seed's fingerprint but does not derive from it")
+
+ # Store only the first verified path
+ if verified_derivation_path is None:
+ verified_derivation_path = derivation_path_obj.derivation
+
+ # Note that both loops check EVERY claim
+ for public_key, derivation_path_obj in scope.bip32_derivations.items():
+ _check_claim(public_key, derivation_path_obj, is_taproot=False)
+
+ for public_key, (leaf_hashes, derivation_path_obj) in scope.taproot_bip32_derivations.items():
+ # TODO: Support keys in taptree leaves
+ _check_claim(public_key, derivation_path_obj, is_taproot=True)
+
+ return verified_derivation_path
+
+
+ def _verify_claimed_derivation_paths(self, child_key_derivation_cache: dict):
+ """
+ Verifies every claimed derivation path that names this seed's fingerprint. The
+ result, stored in verified_[input|output]_derivation_paths, is either the verified
+ derivation path or None (the seed was not named) for each input/output scope.
+
+ The coordinator-supplied fingerprints cannot be trusted as-is. We must derive and
+ verify the ownership of each one that claims to belong to this seed.
+
+ Outputs are verified before inputs; a false claim on an output (e.g. fake-change
+ forgery) is likely an attack whereas a false claim on an input is merely
+ unsignable.
+
+ Raises PSBT[Output|Input]OwnershipClaimError on the first false claim detected.
+ """
+ self.verified_output_derivation_paths = [
+ PSBTParser._get_seed_derivation_path(out, self.root, child_key_derivation_cache)
+ for out in self.psbt.outputs
+ ]
+
+ self.verified_input_derivation_paths = [
+ PSBTParser._get_seed_derivation_path(inp, self.root, child_key_derivation_cache)
+ for inp in self.psbt.inputs
+ ]
+
+
+ def _reject_if_seed_cannot_sign(self):
+ """
+ Rejects the psbt when none of its inputs rely on a key derived by this seed.
+
+ We detect it here, early, so the psbt can be rejected without sending the user
+ through the full verification flow only for signing to fail at the end anyway.
+
+ (embit's sign_with is marginally more permissive: it also signs an input whose
+ script names the master key directly, with no derivation. That runs against how HD
+ wallets are built. The master key is a derivation root, not a spending key, so no
+ standard wallet produces such a psbt. We deliberately ignore this case.
+
+ Similarly, it's not worth the effort to verify that each key is included in its
+ input's script. A psbt that excludes a key in that way would be nonsensical but
+ harmless: the excluded key cannot spend the input, so nothing of this seed's is
+ at risk.)
+ """
+ # An input names a key at a derivation path and _verify_claimed_derivation_paths
+ # proved the seed derives it (single-sig: one such key; multisig: one per
+ # cosigner, ours among them). One verified input path is enough for the psbt to
+ # be signable.
+ if any(path is not None for path in self.verified_input_derivation_paths):
+ return
+
+ # There's nothing for this seed to sign
+ raise PSBTSeedCannotSignError()
+
+
def verify_multisig_output(self, descriptor: Descriptor, change_num: int) -> bool:
change_data = self.get_change_data(change_num)
i = change_data["output_index"]
@@ -558,32 +796,29 @@ def _fill_scope(scope: InputScope | OutputScope):
"""Helper function to fill missing fingerprints in a scope (input/output)"""
# Helper function to check and fix fingerprint
- def _get_updated_fingerprint(public_key: PublicKey, derivation_path_obj: DerivationPath) -> DerivationPath | None:
+ def _get_updated_fingerprint(public_key: PublicKey, derivation_path_obj: DerivationPath, is_taproot: bool) -> DerivationPath | None:
if derivation_path_obj.fingerprint != b"\x00\x00\x00\x00":
return None
-
- # Derive the public key from the currently loaded seed using the derivation
- # contained in the PSBT. If the derived public key exactly matches
- # the PSBT-provided public key, we can be confident that this input/output
- # is owned by the signing seed. In that case we populate the missing (zero)
- # fingerprint with the signing seed's master fingerprint so downstream
- # parsing/signing can treat it as owned by this seed.
- derived_key = PSBTParser._derive_with_cache(
- self.root, derivation_path_obj.derivation, child_key_derivation_cache)
- if derived_key.key.sec() == public_key.sec():
+
+ # If the signing seed really derives the psbt-provided public key at the
+ # claimed derivation path, this input/output is owned by the signing seed.
+ # In that case we populate the missing (zero) fingerprint with the signing
+ # seed's master fingerprint so downstream parsing/signing can treat it as
+ # owned by this seed.
+ if PSBTParser.seed_owns_pubkey(self.root, derivation_path_obj.derivation, public_key, child_key_derivation_cache, is_taproot=is_taproot):
return DerivationPath(self.root.my_fingerprint, derivation_path_obj.derivation)
return None
-
+
# Handle regular BIP32 derivations
for public_key, derivation_path_obj in list(scope.bip32_derivations.items()):
- new_derivation = _get_updated_fingerprint(public_key, derivation_path_obj)
+ new_derivation = _get_updated_fingerprint(public_key, derivation_path_obj, is_taproot=False)
if new_derivation:
scope.bip32_derivations[public_key] = new_derivation
logger.debug(f"Filled missing fingerprint for pubkey {public_key.sec().hex()} derivation {bip32.path_to_str(derivation_path_obj.derivation)}")
-
- # Handle Taproot derivations
+
+ # Handle Taproot derivations
for public_key, (leaf_hashes, derivation_path_obj) in list(scope.taproot_bip32_derivations.items()):
- new_derivation = _get_updated_fingerprint(public_key, derivation_path_obj)
+ new_derivation = _get_updated_fingerprint(public_key, derivation_path_obj, is_taproot=True)
if new_derivation:
scope.taproot_bip32_derivations[public_key] = (leaf_hashes, new_derivation)
logger.debug(f"Filled missing fingerprint for pubkey {public_key.sec().hex()} derivation {bip32.path_to_str(derivation_path_obj.derivation)}")
### src/seedsigner/views/psbt_views.py
@@ -1,9 +1,10 @@
from gettext import gettext as _
-from seedsigner.models.psbt_parser import PSBTParser
+from seedsigner.models.psbt_parser import (PSBTInputOwnershipClaimError,
+ PSBTOutputOwnershipClaimError, PSBTParser, PSBTSeedCannotSignError)
from seedsigner.models.settings import SettingsConstants
-from seedsigner.gui.components import FontAwesomeIconConstants, SeedSignerIconConstants
-from seedsigner.gui.screens.screen import (RET_CODE__BACK_BUTTON, ButtonListScreen, ButtonOption, WarningScreen, DireWarningScreen, QRDisplayScreen)
+from seedsigner.gui.components import FontAwesomeIconConstants, GUIConstants, SeedSignerIconConstants
+from seedsigner.gui.screens.screen import (RET_CODE__BACK_BUTTON, ButtonListScreen, ButtonOption, LargeIconStatusScreen, WarningScreen, DireWarningScreen, QRDisplayScreen)
from seedsigner.views.view import BackStackView, MainMenuView, NotYetImplementedView, View, Destination
@@ -101,9 +102,27 @@ def __init__(self):
seed=self.controller.psbt_seed,
network=self.settings.get_value(SettingsConstants.SETTING__NETWORK)
)
- except Exception as e:
+
+ except PSBTInputOwnershipClaimError:
+ # Set clear_history to disable returning via BACK button
+ self.set_redirect(Destination(PSBTInputOwnershipClaimFailedView, clear_history=True))
+ return
+
+ except PSBTOutputOwnershipClaimError:
+ # Set clear_history to disable returning via BACK button
+ self.set_redirect(Destination(PSBTOutputOwnershipClaimFailedView, clear_history=True))
+ return
+
+ except PSBTSeedCannotSignError:
+ # Not a suspicious psbt, just the wrong seed for it. Send the user back to
+ # pick another rather than clearing the flow.
+ self.controller.psbt_parser = None
+ self.controller.psbt_seed = None
+ self.set_redirect(Destination(PSBTSeedCannotSignView))
+ return
+
+ finally:
self.loading_screen.stop()
- raise e
def run(self):
@@ -129,10 +148,6 @@ def run(self):
else:
num_self_transfer_outputs += 1
- # Everything is set. Stop the loading screen
- if self.loading_screen:
- self.loading_screen.stop()
-
# Run the overview screen
selected_menu_num = self.run_screen(
PSBTOverviewScreen,
@@ -449,10 +464,91 @@ def run(self):
from seedsigner.views.seed_views import LoadMultisigWalletDescriptorView
self.controller.resume_main_flow = Controller.FLOW__PSBT
return Destination(LoadMultisigWalletDescriptorView)
-
+
+
+
+class PSBTSeedCannotSignView(View):
+ """
+ Reached when parsing found this seed can't sign any of the psbt's inputs (see
+ PSBTSeedCannotSignError). Routes back to seed selection so the user can pick another.
+ """
+ SELECT_DIFFERENT_SEED = ButtonOption("Select a different seed")
+
+ def run(self):
+ # This is an informational mismatch, not a warning, so it uses the neutral info
+ # icon and color rather than WarningScreen's alarming yellow edges.
+ # TODO: give this its own InfoScreen (LargeIconStatusScreen with the INFO icon and
+ # color baked in) rather than customizing the base screen at each call site.
+ self.run_screen(
+ LargeIconStatusScreen,
+ title=_("Seed Can't Sign"),
+ status_icon_name=SeedSignerIconConstants.INFO,
+ status_color=GUIConstants.INFO_COLOR,
+ text=_("None of the inputs in this transaction are controlled by this seed."),
+ button_data=[self.SELECT_DIFFERENT_SEED],
+ show_back_button=False,
+ )
+
+ # Set clear_history to disable returning via BACK button.
+ return Destination(PSBTSelectSeedView, clear_history=True)
+
+
+
+class PSBTOutputOwnershipClaimFailedView(View):
+ """
+ Reached when a false ownership claim on an output rejects the psbt (see
+ PSBTOutputOwnershipClaimError). Shows a dire warning and discards the psbt to the main
+ menu. Claims on inputs route to PSBTInputOwnershipClaimFailedView instead.
+ """
+ DISCARD = ButtonOption("Discard transaction")
+
+ def run(self):
+ self.run_screen(
+ DireWarningScreen,
+ title=_("Suspicious Transaction"),
+ status_headline=_("Likely an Attack!"),
+ text=_("The transaction's change/self-transfer outputs are not going back to your wallet."),
+ button_data=[self.DISCARD],
+ show_back_button=False,
+ )
+
+ # We're done with this PSBT. Route back to MainMenuView, which clears all ephemeral
+ # data (except in-memory seeds).
+ # Set clear_history to disable returning via BACK button.
+ return Destination(MainMenuView, clear_history=True)
+
+
+
+class PSBTInputOwnershipClaimFailedView(View):
+ """
+ Reached when a false ownership claim on an input rejects the psbt (see
+ PSBTInputOwnershipClaimError). Shows a plain (not dire) warning and discards the psbt
+ to the main menu.
+ """
+ DISCARD = ButtonOption("Discard transaction")
+
+ def run(self):
+ self.run_screen(
+ WarningScreen,
+ title=_("Transaction Problem"),
+ status_headline=None,
+ text=_("This transaction incorrectly claims that its input(s) belong to this seed."),
+ button_data=[self.DISCARD],
+ show_back_button=False,
+ )
+
+ # We're done with this PSBT. Route back to MainMenuView, which clears all ephemeral
+ # data (except in-memory seeds).
+ # Set clear_history to disable returning via BACK button.
+ return Destination(MainMenuView, clear_history=True)
+
class PSBTAddressVerificationFailedView(View):
+ """
+ Reached when a change or self-transfer output fails address verification. Shows a dire
+ warning and discards the psbt to the main menu.
+ """
def __init__(self, is_change: bool = True, is_multisig: bool = False):
super().__init__()
self.is_change = is_change
@@ -476,8 +572,9 @@ def run(self):
show_back_button=False,
)
- # We're done with this PSBT. Route back to MainMenuView which always
- # clears all ephemeral data (except in-memory seeds).
+ # We're done with this PSBT. Route back to MainMenuView, which clears all ephemeral
+ # data (except in-memory seeds).
+ # Set clear_history to disable returning via BACK button.
return Destination(MainMenuView, clear_history=True)
### tests/psbt_testing_util.py
@@ -1,8 +1,10 @@
from binascii import a2b_base64, unhexlify
from io import BytesIO
+from embit import bip32
+from embit.ec import PublicKey
from embit.networks import NETWORKS
-from embit.psbt import PSBT, OutputScope
+from embit.psbt import PSBT, DerivationPath, InputScope, OutputScope
from seedsigner.models.seed import Seed
@@ -60,6 +62,13 @@ class PSBTTestData:
MULTISIG_LEGACY_P2SH_CHANGE = "0100695221031ce6ddb7a26336264b132c1b45c3e631e8502bc155226d46fc38d73d57f9e4ff21031d9e6e094413dc3a508518f729af23a35b3943ca79bafbc55afab631595e5ccf2103c3a6cb2786d860aa1ccff81fae79c1a973bc8a79938af6ff3c75fc4f54333d7353ae220203c3a6cb2786d860aa1ccff81fae79c1a973bc8a79938af6ff3c75fc4f54333d73100fb882ff2d00008001000000000000002202031d9e6e094413dc3a508518f729af23a35b3943ca79bafbc55afab631595e5ccf1003cd0a2b2d00008001000000000000002202031ce6ddb7a26336264b132c1b45c3e631e8502bc155226d46fc38d73d57f9e4ff100f8890442d0000800100000000000000010308000ff20500000000010417a91402e6f3cb4a98d88fbedf0e43869ce9e1a01e8d9f8700"
MULTISIG_LEGACY_P2SH_SELF_TRANSFER = "01006952210283a03b8fa4cce258e536514416f448cf20aca8ef42492413c149a329029adb4e210346b5d12c00554bce6920e60d647893db6555e0844b8583747bc0ca1ecaba3e1c2103bfc0f7151411ace2d2eef986efca7873dabdc0f6f1bbea7bd3c863be1f2e00de53ae220203bfc0f7151411ace2d2eef986efca7873dabdc0f6f1bbea7bd3c863be1f2e00de100fb882ff2d000080000000000100000022020346b5d12c00554bce6920e60d647893db6555e0844b8583747bc0ca1ecaba3e1c1003cd0a2b2d000080000000000100000022020283a03b8fa4cce258e536514416f448cf20aca8ef42492413c149a329029adb4e100f8890442d000080000000000100000001030890d0030000000000010417a914505efc1adc72f354e827da94b49dec46d80717068700"
+ # Fingerprint-collision pair: two unrelated seeds whose master fingerprints collide,
+ # for tests of the honest-collision case (a genuine foreign key that names our
+ # fingerprint without any forgery involved). Any two seeds collide with probability 1
+ # in 2^32.
+ collision_seed_a = Seed("trend local ecology client torch face wink soup right craft nerve bread".split())
+ collision_seed_b = Seed("oval stadium chimney tone deny catch idea gasp absorb short vibrant tribe".split())
+
# External receive outputs
recipient_seed = Seed("shove album flame dad equal cook spike cheap hollow exit great forest".split())
recipient_multisig_key_2 = Seed("grow curve arrive reflect alarm water black funny comfort match attend tired".split())
@@ -126,3 +135,46 @@ def create_output(output_hex: str, value: int = None) -> OutputScope:
if value is not None:
output.value = value
return output
+
+
+def root_for_seed(seed: Seed) -> bip32.HDKey:
+ """
+ A seed's master key, built the way PSBTParser builds it.
+
+ The network only picks the version bytes, and neither the fingerprint nor any of the
+ key material compared below depends on those, so the fixtures' regtest is used
+ throughout.
+ """
+ return bip32.HDKey.from_seed(seed.seed_bytes, version=NETWORKS["regtest"]["xprv"])
+
+
+def foreign_public_key(derivation_path: str = "m/84h/1h/0h/0/0", seed: Seed = None) -> PublicKey:
+ """
+ A genuine public key belonging to some seed other than the signing seed, for building
+ scopes that name a key the signing seed does not control.
+ """
+ if seed is None:
+ seed = PSBTTestData.recipient_seed
+ return root_for_seed(seed).derive(derivation_path).get_public_key()
+
+
+def claim_seed_owns_key(scope: InputScope | OutputScope, claimed_derivation_path: str, public_key: PublicKey, seed: Seed = None, is_taproot: bool = False):
+ """
+ Writes a derivation into the scope claiming that seed owns public_key at
+ claimed_derivation_path.
+
+ Building one of these needs no key material from the seed, because nothing in a psbt
+ ties a fingerprint to the key written beside it. The fingerprint is all it takes, and
+ that is published in every psbt the seed has ever been sent. So pass a public_key the
+ seed does not own and the result is a psbt asserting ownership that does not exist.
+ """
+ if seed is None:
+ seed = PSBTTestData.seed
+
+ derivation_path = DerivationPath(
+ root_for_seed(seed).my_fingerprint, bip32.parse_path(claimed_derivation_path))
+
+ if is_taproot:
+ scope.taproot_bip32_derivations[public_key] = ([], derivation_path)
+ else:
+ scope.bip32_derivations[public_key] = derivation_path
### tests/screenshot_generator/generator.py
@@ -453,6 +453,9 @@ def mock_version_to_most_recent_release():
ScreenshotConfig(psbt_views.PSBTAddressVerificationFailedView, dict(is_change=False, is_multisig=False), screenshot_name="PSBTAddressVerificationFailedView_singlesig_selftransfer"),
ScreenshotConfig(psbt_views.PSBTAddressVerificationFailedView, dict(is_change=True, is_multisig=True), screenshot_name="PSBTAddressVerificationFailedView_multisig_change"),
ScreenshotConfig(psbt_views.PSBTAddressVerificationFailedView, dict(is_change=False, is_multisig=True), screenshot_name="PSBTAddressVerificationFailedView_multisig_selftransfer"),
+ ScreenshotConfig(psbt_views.PSBTOutputOwnershipClaimFailedView),
+ ScreenshotConfig(psbt_views.PSBTInputOwnershipClaimFailedView),
+ ScreenshotConfig(psbt_views.PSBTSeedCannotSignView),
ScreenshotConfig(psbt_views.PSBTFinalizeView, mock_context_manager=mock_multisig_psbt_loaded),
#ScreenshotConfig(PSBTSignedQRDisplayViewScreenshotConfig),
ScreenshotConfig(psbt_views.PSBTSigningErrorView, mock_context_manager=mock_multisig_psbt_loaded),
### tests/test_flows_psbt.py
@@ -1,8 +1,15 @@
+from binascii import a2b_base64
+
+from embit.psbt import PSBT
+
from base import FlowTest, FlowStep
+from psbt_testing_util import (PSBTTestData, claim_seed_owns_key, create_output,
+ foreign_public_key)
from seedsigner.controller import Controller
from seedsigner.views.view import MainMenuView
from seedsigner.views import scan_views, seed_views, psbt_views
+from seedsigner.models.seed import Seed
from seedsigner.models.settings import SettingsConstants
@@ -181,3 +188,86 @@ def load_seed_into_decoder(view: scan_views.ScanView):
FlowStep(psbt_views.PSBTSignedQRDisplayView),
FlowStep(MainMenuView)
])
+
+
+
+class TestPSBTOwnershipClaimRouting(FlowTest):
+ """
+ A psbt carrying an ownership claim that does not hold up is rejected while it is being
+ parsed, before the user is shown anything about the transaction. These cover the
+ routing that turns that rejection into a warning screen instead of a crash.
+ """
+
+ def _load_psbt_for_signing(self, psbt: PSBT, seed: Seed = None):
+ """
+ Stage the psbt in the Controller and load the signing seed into storage, as if
+ both had just been scanned. Each test's sequence then starts at seed selection;
+ the parse itself runs when PSBTOverviewView is instantiated.
+ """
+ self.settings.set_value(SettingsConstants.SETTING__NETWORK, SettingsConstants.REGTEST)
+ self.controller.psbt = psbt
+ self.controller.storage.set_pending_seed(seed if seed is not None else PSBTTestData.seed)
+ self.controller.storage.finalize_pending_seed()
+
+
+ def _psbt_with_change(self):
+ psbt = PSBT.parse(a2b_base64(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_1_INPUT))
+ psbt.outputs.append(create_output(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_CHANGE, 10_000))
+ return psbt
+
+
+ def test_forged_output_claim_terminates_signing_flow(self):
+ """
+ A forged ownership claim on an output is framed as a potential attack, so it
+ routes to a warning that aborts the signing flow.
+ """
+ psbt = self._psbt_with_change()
+ claim_seed_owns_key(psbt.outputs[0], "m/84h/1h/0h/1/0", foreign_public_key())
+ self._load_psbt_for_signing(psbt)
+
+ self.run_sequence([
+ FlowStep(psbt_views.PSBTSelectSeedView, screen_return_value=0),
+ FlowStep(psbt_views.PSBTOverviewView, is_redirect=True),
+ FlowStep(psbt_views.PSBTOutputOwnershipClaimFailedView, button_data_selection=psbt_views.PSBTOutputOwnershipClaimFailedView.DISCARD),
+ FlowStep(MainMenuView),
+ ])
+
+
+ def test_forged_input_claim_terminates_signing_flow(self):
+ """
+ A false claim on an input is framed as inconsistent data rather than as an attack,
+ but also routes to its own information screen that aborts the signing flow.
+ """
+ psbt = self._psbt_with_change()
+ claim_seed_owns_key(psbt.inputs[0], "m/84h/1h/0h/0/0", foreign_public_key())
+ self._load_psbt_for_signing(psbt)
+
+ self.run_sequence([
+ FlowStep(psbt_views.PSBTSelectSeedView, screen_return_value=0),
+ FlowStep(psbt_views.PSBTOverviewView, is_redirect=True),
+ FlowStep(psbt_views.PSBTInputOwnershipClaimFailedView, button_data_selection=psbt_views.PSBTInputOwnershipClaimFailedView.DISCARD),
+ FlowStep(MainMenuView),
+ ])
+
+
+ def test_wrong_seed_routes_back_to_seed_selection_flow(self):
+ """
+ The wrong seed for a psbt redirects before any transaction detail is rendered and
+ routes back to seed selection with the signing seed cleared so another can be
+ picked.
+ """
+ self._load_psbt_for_signing(self._psbt_with_change(), seed=PSBTTestData.recipient_seed)
+
+ self.run_sequence([
+ FlowStep(psbt_views.PSBTSelectSeedView, screen_return_value=0),
+ FlowStep(psbt_views.PSBTOverviewView, is_redirect=True),
+ FlowStep(psbt_views.PSBTSeedCannotSignView, button_data_selection=psbt_views.PSBTSeedCannotSignView.SELECT_DIFFERENT_SEED),
+ FlowStep(psbt_views.PSBTSelectSeedView),
+ ])
+
+ assert self.controller.psbt_seed is None
+ assert self.controller.psbt_parser is None
+
+ # The psbt itself is kept: the user is choosing a different seed for it, not
+ # starting over
+ assert self.controller.psbt is not None
### tests/test_psbt_parser.py
@@ -4,16 +4,19 @@
from binascii import a2b_base64
from copy import deepcopy
from unittest.mock import patch
-from embit import bip32
+from embit import bip32, script
+from embit.ec import PublicKey
from embit.networks import NETWORKS
from embit.psbt import PSBT, DerivationPath
from embit.descriptor import Descriptor
-from seedsigner.models.psbt_parser import PSBTParser
+from seedsigner.models.psbt_parser import (PSBTInputOwnershipClaimError,
+ PSBTOutputOwnershipClaimError, PSBTParser, PSBTSeedCannotSignError)
from seedsigner.models.seed import Seed
from seedsigner.models.settings_definition import SettingsConstants
-from psbt_testing_util import PSBTTestData, create_output
+from psbt_testing_util import (PSBTTestData, claim_seed_owns_key, create_output,
+ foreign_public_key, root_for_seed)
@@ -209,15 +212,64 @@ def test_missing_fingerprint_handling(self):
from binascii import hexlify
fingerprint_hex = hexlify(derivation.fingerprint).decode()
- # Check if this public key derives from the current seed
+ # Check if this public key derives from the current seed. A psbt
+ # carries a taproot key as its bare 32-byte x coordinate, and embit
+ # rebuilds a full key from it by just assuming even parity. The real
+ # derived key can be odd-parity, so a full-key compare would wrongly
+ # report a mismatch. Only the x coordinate is real data: compare
+ # x-only.
derived_key = parser.root.derive(derivation.derivation)
- if derived_key.key.sec() == pub.sec():
+ if derived_key.xonly() == pub.xonly():
# This pubkey derives from current seed, should have current seed's fingerprint
assert fingerprint_hex == seed_fingerprint, f"Expected {seed_fingerprint}, got {fingerprint_hex} for taproot pubkey that derives from current seed"
else:
# This pubkey doesn't derive from current seed, should remain 00000000
assert fingerprint_hex == "00000000"
+ # All of the above only proves the even-parity case. A psbt carries taproot keys
+ # as bare 32-byte x coordinates and embit rebuilds full keys from them by assuming
+ # even parity; that assumption happens to hold for the fixture's key at
+ # m/86h/1h/0h/0/0. Re-key the taproot input to a path whose key really derives
+ # with odd parity to prove the ownership fallback compares x-only rather than
+ # trusting embit's artificial parity.
+ root = root_for_seed(PSBTTestData.seed)
+ odd_parity_derivation_path = "m/86h/1h/0h/0/1"
+ odd_parity_public_key = root.derive(odd_parity_derivation_path).get_public_key()
+ assert odd_parity_public_key.sec()[0] == 0x03 # odd parity
+
+ psbt = PSBT.parse(a2b_base64(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT))
+ taproot_input = psbt.inputs[0]
+
+ # Present the key the way embit's psbt parsing yields it: rebuilt from just the
+ # x coordinate, carrying the assumed even parity (wrong for this key)
+ x_only_public_key = PublicKey.from_xonly(odd_parity_public_key.xonly())
+ taproot_input.taproot_bip32_derivations.clear()
+ taproot_input.taproot_bip32_derivations[x_only_public_key] = ([], DerivationPath(
+ fingerprint=b"\x00\x00\x00\x00",
+ derivation=bip32.parse_path(odd_parity_derivation_path)
+ ))
+ taproot_input.taproot_internal_key = x_only_public_key
+ taproot_input.witness_utxo.script_pubkey = script.p2tr(x_only_public_key)
+
+ # The zeroed-fingerprint fallback check must recognize this input as the seed's,
+ # even though embit's internal parity byte for the pubkey is wrong. Taproot
+ # pubkeys must be compared by their x-only representation.
+ assert PSBTParser.has_matching_input_fingerprint(psbt, PSBTTestData.seed, SettingsConstants.REGTEST)
+
+ # Comparing x-only looks less strict than the full-key comparison used for
+ # non-taproot keys, but nothing is actually given up: a psbt never carries a
+ # parity byte for a taproot key, so the x coordinate is all the key material
+ # there is to compare. A completely wrong seed will still fail to match.
+ wrong_seed = Seed(["bacon"] * 24)
+ assert not PSBTParser.has_matching_input_fingerprint(psbt, wrong_seed, SettingsConstants.REGTEST)
+
+ # Parsing should successfully fill the fingerprint and verify that the input
+ # belongs to the seed.
+ parser = PSBTParser(p=psbt, seed=PSBTTestData.seed, network=SettingsConstants.REGTEST)
+ (_, filled_derivation) = parser.psbt.inputs[0].taproot_bip32_derivations[x_only_public_key]
+ assert filled_derivation.fingerprint == parser.root.my_fingerprint
+ assert parser.verified_input_derivation_paths == [bip32.parse_path(odd_parity_derivation_path)]
+
def test_trim_and_sig_count(self):
"""
@@ -772,3 +824,417 @@ def build_psbt(case: tuple) -> PSBT:
assert max(capped_sizes) == cap
+
+class TestPSBTParserSeedOwnership:
+ """
+ The ownership scan: what the signing seed provably owns in a psbt, and the rejection
+ of any psbt whose ownership claims do not hold up.
+ """
+ seed = PSBTTestData.seed
+
+ def _root(self) -> bip32.HDKey:
+ return root_for_seed(self.seed)
+
+
+ def _psbt_with_change(self, input_base64: str = None, change_hex: str = None) -> PSBT:
+ """
+ A base psbt plus its own change output. But no external recipient output is added,
+ so all paths are owned by the seed.
+ """
+ if input_base64 is None:
+ input_base64 = PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_1_INPUT
+ if change_hex is None:
+ change_hex = PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_CHANGE
+
+ psbt = PSBT.parse(a2b_base64(input_base64))
+ psbt.outputs.append(create_output(change_hex, 10_000))
+ return psbt
+
+
+ def _parse(self, psbt: PSBT) -> PSBTParser:
+ return PSBTParser(psbt, self.seed, network=SettingsConstants.REGTEST)
+
+
+ def test__seed_owns_pubkey__accepts_the_seeds_own_key(self):
+ """
+ seed_owns_pubkey should confirm the simple base case that a pubkey directly
+ derived from the seed is owned by the seed.
+ """
+ root = self._root()
+ derivation_path = bip32.parse_path("m/84h/1h/0h/0/0")
+ public_key = root.derive(derivation_path).get_public_key()
+
+ assert PSBTParser.seed_owns_pubkey(root, derivation_path, public_key, child_key_derivation_cache=None) is True
+
+
+ def test__seed_owns_pubkey__rejects_a_key_the_seed_does_not_control(self):
+ """
+ seed_owns_pubkey should reject a pubkey that the seed does not control.
+ """
+ root = self._root()
+ derivation_path = bip32.parse_path("m/84h/1h/0h/0/0")
+
+ assert PSBTParser.seed_owns_pubkey(root, derivation_path, foreign_public_key(), child_key_derivation_cache=None) is False
+
+
+ def test__seed_owns_pubkey__rejects_the_seeds_own_key_at_the_wrong_path(self):
+ """
+ The path is as much a part of the claim as the key is. The seed owns this key, but
+ not at the path the claim names so it is still a false claim.
+ """
+ root = self._root()
+ public_key = root.derive(bip32.parse_path("m/84h/1h/0h/0/0")).get_public_key()
+
+ assert PSBTParser.seed_owns_pubkey(root, bip32.parse_path("m/84h/1h/0h/0/1"), public_key, child_key_derivation_cache=None) is False
+
+
+ def test__seed_owns_pubkey__compares_taproot_keys_without_parity(self):
+ """
+ Taproot keys are x-only, so a key the seed genuinely owns routinely differs from
+ the derived key by the parity byte alone. Comparing the full key would read that
+ as a stranger's key and reject the seed's own output.
+ """
+ root = self._root()
+
+ # Address index 1 is the first whose derived key has odd parity, which is the
+ # case where a full comparison and an x-only comparison disagree.
+ derivation_path = bip32.parse_path("m/86h/1h/0h/0/1")
+ public_key = root.derive(derivation_path).get_public_key()
+
+ # Sanity check: the key's first byte is its parity, 0x02 for even, 0x03 for odd
+ assert public_key.sec()[0] == 0x03
+
+ # The psbt carries the key x-only, so reconstruct what it would hold: the same x
+ # coordinate, with the EVEN-parity prefix.
+ as_written_in_psbt = PublicKey.parse(b"\x02" + public_key.xonly())
+
+ # Taproot is x-only so it ignores the now-even parity byte
+ assert PSBTParser.seed_owns_pubkey(root, derivation_path, as_written_in_psbt, child_key_derivation_cache=None, is_taproot=True) is True
+
+ # Non-taproot compares the full key so it rejects the key with the wrong parity byte
+ assert PSBTParser.seed_owns_pubkey(root, derivation_path, as_written_in_psbt, child_key_derivation_cache=None) is False
+
+
+ def test__parse__populates_verified_derivation_paths(self):
+ """
+ The scan writes one verified_*_derivation_paths entry per input and per output
+ (regardless of ownership; not-owned paths are recorded as None) in the psbt's own
+ order.
+ """
+ # The psbt fixture has one input and one output, both of which are owned by the
+ # seed.
+ psbt = self._psbt_with_change()
+ psbt_parser = self._parse(psbt)
+
+ assert len(psbt_parser.verified_input_derivation_paths) == len(psbt.inputs)
+ assert len(psbt_parser.verified_output_derivation_paths) == len(psbt.outputs)
+
+ # Every recorded path is one the seed really does derive the scope's key at
+ for scopes, verified_derivation_paths in [
+ (psbt.inputs, psbt_parser.verified_input_derivation_paths),
+ (psbt.outputs, psbt_parser.verified_output_derivation_paths),
+ ]:
+ for scope, verified_derivation_path in zip(scopes, verified_derivation_paths):
+ assert verified_derivation_path is not None
+ public_key = list(scope.bip32_derivations.keys())[0]
+ assert PSBTParser.seed_owns_pubkey(psbt_parser.root, verified_derivation_path, public_key, child_key_derivation_cache=None) is True
+
+
+ def test__parse__verified_derivation_paths_none_for_not_owned_output(self):
+ """
+ An output paying someone else is not a failure; the seed simply owns nothing
+ there so the matching verified_output_derivation_paths should be None.
+ """
+ psbt = self._psbt_with_change()
+
+ # Add an external recipient at index 0
+ psbt.outputs.insert(0, create_output(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_RECEIVE, 10_000))
+
+ psbt_parser = self._parse(psbt)
+
+ assert psbt_parser.verified_output_derivation_paths[0] is None
+ assert psbt_parser.verified_output_derivation_paths[1] is not None
+
+
+ def test__parse__verified_derivation_paths_none_for_not_owned_input(self):
+ """
+ A collaborative spend also includes an input belonging to another party, in two
+ shapes: a payjoin counterparty's input arrives finalized with no derivation info
+ at all (BIP-78 requires the sender to verify no keypaths appear anywhere), while
+ a coordinator managing every party's wallet writes each party's genuine
+ derivation entry. Neither is a failure; the seed simply owns nothing there.
+ """
+ psbt = self._psbt_with_change()
+
+ # The other party's input: their utxo, carrying no derivation info
+ foreign_input = deepcopy(psbt.inputs[0])
+ foreign_input.bip32_derivations.clear()
+ foreign_input.witness_utxo.script_pubkey = script.p2wpkh(foreign_public_key())
+ psbt.inputs.append(foreign_input)
+
+ # The payjoin shape
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.verified_input_derivation_paths[0] is not None
+ assert psbt_parser.verified_input_derivation_paths[1] is None
+
+ # The coordinated shape: the derivation entry is truthful, naming the other
+ # party's fingerprint and a key that party really controls.
+ claim_seed_owns_key(foreign_input, "m/84h/1h/0h/0/0", foreign_public_key(), seed=PSBTTestData.recipient_seed)
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.verified_input_derivation_paths[0] is not None
+ assert psbt_parser.verified_input_derivation_paths[1] is None
+
+
+ def test__parse__rejects_a_forged_claim_on_an_input(self):
+ """
+ `parse` should raise PSBTInputOwnershipClaimError when a psbt carries a false
+ claim that the seed owns an input that it does not actually control.
+ """
+ psbt = self._psbt_with_change()
+ claim_seed_owns_key(psbt.inputs[0], "m/84h/1h/0h/0/0", foreign_public_key())
+
+ with pytest.raises(PSBTInputOwnershipClaimError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_forged_claim_on_an_output(self):
+ """
+ `parse` should raise PSBTOutputOwnershipClaimError when a psbt carries a false
+ claim that the seed owns an output that it does not actually control. This is
+ the fake-change attack scenario that is more severe than a forged input claim.
+ """
+ psbt = self._psbt_with_change()
+ claim_seed_owns_key(psbt.outputs[0], "m/84h/1h/0h/1/0", foreign_public_key())
+
+ # The output-scope error specifically: the dire, attack-framed rejection
+ with pytest.raises(PSBTOutputOwnershipClaimError):
+ self._parse(psbt)
+
+
+ def test__parse__forged_output_claim_outranks_forged_input_claim(self):
+ """
+ The scan stops at the first false claim, so scan order decides which error the
+ psbt is reported with. A false output claim is the fake-change forgery and must
+ be the one reported even when the same psbt also carries a false input claim --
+ otherwise an attacker could plant a throwaway input claim just to be routed to
+ the milder input-scope warning.
+ """
+ psbt = self._psbt_with_change()
+ claim_seed_owns_key(psbt.inputs[0], "m/84h/1h/0h/0/0", foreign_public_key())
+ claim_seed_owns_key(psbt.outputs[0], "m/84h/1h/0h/1/0", foreign_public_key())
+
+ # The output error is the more severe issue
+ with pytest.raises(PSBTOutputOwnershipClaimError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_forged_taproot_claim(self):
+ """
+ A false taproot claim is rejected the same way as an ecdsa one: the input-scope
+ error on an input, the output-scope (attack) error on an output. Taproot claims
+ live in their own taproot_bip32_derivations dict, so the scan's coverage of that
+ dict is pinned on both scope types.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ claim_seed_owns_key(psbt.inputs[0], "m/86h/1h/0h/0/0", foreign_public_key(), is_taproot=True)
+
+ with pytest.raises(PSBTInputOwnershipClaimError):
+ self._parse(psbt)
+
+ # The same forgery on a taproot output gets the attack classification
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ claim_seed_owns_key(psbt.outputs[0], "m/86h/1h/0h/1/0", foreign_public_key(), is_taproot=True)
+
+ with pytest.raises(PSBTOutputOwnershipClaimError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_forged_claim_on_a_multisig_scope(self):
+ """
+ Multisig scopes legitimately carry one derivation entry per cosigner, but a
+ forged claim naming this seed's fingerprint is rejected there exactly as in
+ single-sig: the input-scope error on an input, the attack-classified error on
+ an output.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE)
+ claim_seed_owns_key(psbt.inputs[0], "m/48h/1h/0h/2h/0/9", foreign_public_key())
+
+ with pytest.raises(PSBTInputOwnershipClaimError):
+ self._parse(psbt)
+
+ # The same forgery on the multisig change output gets the attack classification
+ psbt = self._psbt_with_change(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE)
+ claim_seed_owns_key(psbt.outputs[0], "m/48h/1h/0h/2h/1/9", foreign_public_key())
+
+ with pytest.raises(PSBTOutputOwnershipClaimError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_forged_claim_behind_a_genuine_one(self):
+ """
+ A scope can hold more than one key. Finding the one the seed owns is not a reason
+ to stop looking.
+
+ No honest coordinator writes extra derivation entries on a single-sig scope; this
+ shape arrives only from buggy or adversarial software. The scan copes with it
+ because the same scan serves multisig, where multi-entry scopes are the norm.
+ """
+ psbt = self._psbt_with_change()
+
+ # The scope's own genuine derivation stays exactly where it is...
+ assert len(psbt.inputs[0].bip32_derivations) == 1
+
+ # ...but add a false claim to the bip32_derivations dict
+ claim_seed_owns_key(psbt.inputs[0], "m/84h/1h/0h/0/9", foreign_public_key())
+ assert len(psbt.inputs[0].bip32_derivations) == 2
+ assert list(psbt.inputs[0].bip32_derivations.keys())[-1] == foreign_public_key()
+
+ with pytest.raises(PSBTInputOwnershipClaimError):
+ self._parse(psbt)
+
+
+ def test__parse__accepts_a_key_belonging_to_someone_else(self):
+ """
+ Only a claim on THIS seed's fingerprint is ever checked. A key openly belonging to
+ another wallet says nothing about this seed and must not reject the psbt.
+
+ No honest coordinator writes extra derivation entries on a single-sig scope; this
+ shape arrives only from buggy or adversarial software. The scan copes with it
+ because the same scan serves multisig, where multi-entry scopes are the norm.
+ """
+ psbt = self._psbt_with_change()
+ other_seed = PSBTTestData.recipient_seed
+ claim_seed_owns_key(psbt.inputs[0], "m/84h/1h/0h/0/0", foreign_public_key(), seed=other_seed)
+
+ # Confirm the fixture really does carry someone else's fingerprint
+ claimed_fingerprint = psbt.inputs[0].bip32_derivations[foreign_public_key()].fingerprint
+ assert claimed_fingerprint == root_for_seed(other_seed).my_fingerprint
+ assert claimed_fingerprint != self._root().my_fingerprint
+
+ psbt_parser = self._parse(psbt)
+
+ # The seed still owns its own key in that input, via the scope's genuine
+ # derivation.
+ assert psbt_parser.verified_input_derivation_paths[0] is not None
+
+
+ def test_genuine_fingerprint_collision_is_rejected_like_a_forgery(self):
+ """
+ 4-byte fingerprints collide. The fixture pair were brute-force generated so that
+ they share a master fingerprint with no forgery involved, so a foreign party's
+ honest derivation entry can name our fingerprint on a key we do not control.
+
+ The parser cannot distinguish that from a forged claim -- they are byte-for-byte
+ the same situation -- and so it deliberately fails the psbt as an input-scope
+ inconsistency.
+ """
+ foreign_root = root_for_seed(PSBTTestData.collision_seed_b)
+
+ # The collision is real: same fingerprint, different key material
+ signing_root = root_for_seed(PSBTTestData.collision_seed_a)
+ assert foreign_root.my_fingerprint == signing_root.my_fingerprint
+ assert foreign_root.get_public_key() != signing_root.get_public_key()
+
+ # The foreign party's own genuine entry on their own input: their true
+ # fingerprint beside their true key, no forgery anywhere
+ psbt = self._psbt_with_change()
+ foreign_derivation_path = bip32.parse_path("m/84h/1h/0h/0/0")
+ colliding_public_key = foreign_root.derive(foreign_derivation_path).get_public_key()
+ psbt.inputs[0].bip32_derivations[colliding_public_key] = DerivationPath(foreign_root.my_fingerprint, foreign_derivation_path)
+
+ with pytest.raises(PSBTInputOwnershipClaimError):
+ PSBTParser(psbt, PSBTTestData.collision_seed_a, network=SettingsConstants.REGTEST)
+
+
+ def test_ownership_scan_derives_through_the_cache(self):
+ """
+ Ownership verification does its own deriving and the later parse phases do theirs,
+ but all of it flows through one shared cache: across the whole parse, each unique
+ path level is derived exactly once, no matter how many scopes claim it or how many
+ phases revisit it.
+
+ Note that even the initial pass benefits from the cache by skipping shared
+ levels that have already been derived.
+ """
+ psbt = self._psbt_with_change()
+
+ # Boost to 10 inputs from the same wallet, all sharing the one derivation path
+ for _ in range(9):
+ psbt.inputs.append(deepcopy(psbt.inputs[0]))
+
+ # How deep does the derivation path go?
+ num_levels = len(list(psbt.inputs[0].bip32_derivations.values())[0].derivation)
+
+ # Count every level actually derived over the course of the parse
+ num_derivations = 0
+ uncounted_child = bip32.HDKey.child
+ def counting_child(self, index, hardened=False):
+ nonlocal num_derivations
+ num_derivations += 1
+ return uncounted_child(self, index, hardened)
+
+ with patch.object(bip32.HDKey, "child", counting_child):
+ psbt_parser = self._parse(psbt)
+
+ # Sanity check: the scan really did run over all ten inputs and the change output
+ assert len(psbt_parser.verified_input_derivation_paths) == 10
+ assert all(path is not None for path in psbt_parser.verified_input_derivation_paths)
+ assert psbt_parser.verified_output_derivation_paths[0] is not None
+
+ # The inputs were cloned so they all use the same path with num_levels depth. The
+ # change output differs only in its last two levels. Verify that each of these
+ # levels was derived exactly once.
+ assert num_derivations == num_levels + 2
+
+
+ def test__parse__rejects_a_seed_that_owns_no_input(self):
+ """
+ The wrong seed for a psbt can produce no signature at all, so parsing raises
+ PSBTSeedCannotSignError rather than letting the flow discover it after the user
+ has approved.
+
+ And under the same wrong-seed conditions, embit will produce no signature anyway.
+ """
+ psbt = self._psbt_with_change()
+
+ # Intentionally use the wrong seed
+ with pytest.raises(PSBTSeedCannotSignError):
+ PSBTParser(psbt, PSBTTestData.recipient_seed, network=SettingsConstants.REGTEST)
+
+ # Try to sign with the wrong seed anyway
+ root = root_for_seed(PSBTTestData.recipient_seed)
+ assert psbt.sign_with(root) == 0
+
+
+ def test__parse__accepts_a_cosigner_seed_on_a_multisig(self):
+ """
+ Each cosigner holding a key on the device signs the same psbt in turn, so a seed
+ that owns an input must pass whether or not it is the first one used.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE)
+
+ psbt_parser = PSBTParser(psbt, PSBTTestData.seed, network=SettingsConstants.REGTEST)
+ assert any(path is not None for path in psbt_parser.verified_input_derivation_paths)
+
+ psbt_parser = PSBTParser(psbt, PSBTTestData.multisig_key_2, network=SettingsConstants.REGTEST)
+ assert any(path is not None for path in psbt_parser.verified_input_derivation_paths)
+
+ psbt_parser = PSBTParser(psbt, PSBTTestData.multisig_key_3, network=SettingsConstants.REGTEST)
+ assert any(path is not None for path in psbt_parser.verified_input_derivation_paths)
+
+
+ def test_a_psbt_with_no_utxos_is_rejected_rather_than_crashing(self):
+ """
+ A psbt with no inputs is malformed and cannot be signed. `parse` should raise
+ PSBTSeedCannotSignError. Note that diagnosing the malformation is not this check's
+ job.
+ """
+ psbt = self._psbt_with_change()
+ for psbt_input in psbt.inputs:
+ psbt_input.witness_utxo = None
+ psbt_input.non_witness_utxo = None
+ psbt_input._utxo = None
+ psbt_input.bip32_derivations.clear()
+
+ with pytest.raises(PSBTSeedCannotSignError):
+ self._parse(psbt)Why this scored 78/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.