Verify that change outputs actually pay this seed
What changed, and why it matters
This commit fixes a security flaw in how SeedSigner decides whether a Bitcoin transaction's 'change' output really returns coins to the user's own wallet. Previously, the device trusted too much of what the transaction coordinator (or a malicious wallet app) claimed about output ownership, which could let an attacker trick the user into approving a payment that actually sends change to the attacker. The fix makes the device independently prove ownership of each change output using the seed's own keys, and it now rejects several kinds of contradictory or misleading PSBT annotations as attacks.
Treat this commit as a security fix and include it in the next release. Review the taproot exemption note (PSBT_OUT_TAP_TREE parsing) for follow-up hardening, since honest taproot change with a script tree is currently misclassified as an external spend rather than verified as change.
Security signals we found
Fixes insufficient verification of change output ownership in PSBT parsing
Adds explicit rejection of ownership-claim/scriptPubKey contradictions treated as attacks
Adds rejection of surplus derivation path entries and mixed ecdsa/taproot derivation maps
Removes reliance on coordinator-supplied fingerprints/paths for change classification
Adds new exception classes: PSBTSurplusDerivationPathsError, PSBTMixedDerivationPathTypesError, PSBTOutputOwnershipContradictionError
Updates UI to warn user and discard suspicious PSBTs
Includes extensive regression tests for single-sig, multisig, and taproot deception scenarios
Evidence from the diff
The patch hardens PSBT output verification in seedsigner/models/psbt_parser.py. Before, change detection compared the coordinator-supplied policy (including cosigner fingerprints resolved from global xpubs) and rebuilt the scriptPubKey from the coordinator’s claimed witness/redeem scripts or derivation paths. This allowed a misannotated fingerprint to skip verification entirely, and multisig never required the seed’s key to appear in the committed script. The new code compares policy shape only (type plus m-of-n), then proves ownership for every candidate: single-sig by rebuilding the scriptPubKey from the claimed derivation path, multisig by checking the seed-derived key is present in the committed script. It raises new exceptions for contradictions between the PSBT’s ownership claims and the output’s actual scriptPubKey, surplus derivation paths, and mixed ecdsa/taproot derivation maps. Taproot script-tree outputs remain exempt because the parser cannot yet distinguish an honest tweaked key from a malicious one. UI views were updated to show warning/dire-warning screens and discard the PSBT on these failures. Extensive tests cover the new rejection behaviors.
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 +1390 / −240
### src/seedsigner/models/psbt_parser.py
@@ -51,6 +51,58 @@ class PSBTInputOwnershipClaimError(PSBTVerificationError):
pass
+class PSBTSurplusDerivationPathsError(PSBTVerificationError):
+ """
+ Raised for three similar cases:
+ * single sig: output has more than one derivation path entry.
+ * multisig: an output confirmed to pay this seed claims more derivation path
+ entries than its committed script has keys.
+ * taproot: output has more than one derivation path entry claiming to be its
+ internal key (i.e. claiming no leaf hashes).
+
+ For single sig this is clearly a structural error and is not expected to be seen in
+ the real world but it's worth the sanity check.
+
+ For multisig, this could be an attempt to deceive the user, but we do not try to
+ adjudicate that.
+ """
+ pass
+
+
+class PSBTMixedDerivationPathTypesError(PSBTVerificationError):
+ """
+ An input or output declares derivation paths in both the ecdsa and the taproot key
+ maps (bip32_derivations and taproot_bip32_derivations).
+
+ This is a correctness problem (not expected to be seen in the real world) or
+ potentially a weak form of deception, but we do not try to adjudicate that.
+ """
+ pass
+
+
+class PSBTOutputOwnershipContradictionError(PSBTVerificationError):
+ """
+ The psbt's account of who an output pays contradicts which key(s) the output
+ actually commits to.
+
+ For multisig, it's one of:
+ * The output claims one of our keys, but the script the psbt supplied for it does
+ not match what the output actually commits to (the output's scriptPubKey).
+ * The output claims that one of our keys is part of the multisig that owns the
+ output, but that key is not part of the scriptPubKey commitment.
+ * Our seed owns a key that is part of the scriptPubKey commitment, but the output
+ claims a different seed in our place, by fingerprint or by listed public key.
+
+ Single sig supplies no script, so both contradictions come from the rebuild alone: the
+ output claims our key but commits to another key, or it commits to our key while
+ claiming a different fingerprint in our place.
+
+
+ We treat any of these deceptions as an attack.
+ """
+ pass
+
+
class PSBTSeedCannotSignError(PSBTVerificationError):
"""
The selected seed holds no key that could sign any input.
@@ -186,12 +238,18 @@ def parse(self):
- 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.
+ policy with no cosigner information pass as a match between inputs.
+ Outputs deliberately compare shape alone; see _policy_shape_matches.
+
+ 5. _parse_outputs: organizes the output data (amounts, destination_addresses,
+ etc.) and verifies the ownership of the outputs that come back to this seed
+ via:
+ - single-sig: Rebuild the output script from the seed and match it against
+ the committed scriptPubKey.
+ - multisig: Match the seed's verified key against the pubkeys in the script
+ the output commits to.
+ Every change_data entry after this point will carry a derivation path that
+ our seed provably owns.
Optimization via child_key_derivation_cache:
Parsing traverses a derivation path down to an individual address one level at a
@@ -266,7 +324,55 @@ def _parse_inputs(self, child_key_derivation_cache: dict):
if self.policy != inp_policy:
raise RuntimeError("Mixed inputs in the transaction")
+
def _parse_outputs(self, child_key_derivation_cache: dict):
+ """
+ Sorts each output into change coming back to this seed, an external spend, or
+ OP_RETURN data, and totals the amounts for each. Note that self-transfer/receive
+ outputs are also considered "change".
+
+ Most of the work here is determining which, if any, outputs are verifiably being
+ paid to our seed.
+ """
+
+ """****************** How outputs are verified as change ************************
+ Many outputs are obviously NOT change. An output is only considered possible
+ change if its policy matches the inputs' policy "shape" (script type, plus m-of-n
+ for multisig; see parse()); anything else is recorded as an external spend.
+
+ The psbt will usually annotate which key(s) own a change output (see embit's
+ bip32_derivations and taproot_bip32_derivations), but this is just a claim
+ supplied by the coordinator. These annotations are not authoritative. But such
+ claims are significant; if our checks prove that the claim is false, we consider
+ the deception an attack.
+
+ -- Proving the claim --
+ The output's scriptPubKey determines where the value ACTUALLY goes. But the
+ scriptPubKey only contains a hash of the spending conditions (note: taproot uses a
+ tweaked key instead), so we can't simply inspect the scriptPubKey to determine if
+ the output is ours.
+
+ We must build our own version of the scriptPubKey via:
+ * single sig: derive a key from our seed using the claimed derivation path.
+ * multisig: hash the claimed witness_script or redeem_script.
+
+ That leaves us holding two independent answers about the same output: which key it
+ commits to (our rebuild, matched against the scriptPubKey), and which key the psbt
+ says it commits to (the claim). We evaluate the output on those two facts:
+
+ | claims this seed | doesn't claim this seed
+ -------------------------+----------------------+-------------------------
+ commits to our key | confirmed: is change | contradiction
+ commits to another key | contradiction | presumed external spend
+
+ If our two answers contradict each other, the psbt has been caught in a deception.
+ We raise an exception and reject the psbt.
+
+ (note one exception: no taproot mismatch is rejected. A script tree tweaks our
+ internal key, so an honest taproot change output fails to match too, and we cannot
+ yet tell that apart from an output that claims our key but pays someone else. All
+ taproot mismatches pass as EXTERNAL spends and are never considered "change".)
+ ******************************************************************************"""
self.spend_amount = 0
self.change_amount = 0
self.change_data = []
@@ -283,88 +389,192 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
out_policy = PSBTParser._get_policy(out, vout[i].script_pubkey, self.psbt.xpubs, child_key_derivation_cache)
is_change = False
- # if policy is the same - probably change
- if out_policy == self.policy:
- # double-check that it's change
- # we already checked in get_cosigners and parse_multisig
- # that pubkeys are generated from cosigners,
- # and witness script is corresponding multisig
- # so we only need to check that scriptpubkey is generated from
- # witness script
+ # Is this output change? If this output's policy is superficially similar to
+ # the spending wallet's policy (e.g. they're both 2-of-3 p2wsh), then it's a
+ # candidate for being change.
+ if PSBTParser._policy_shape_matches(out_policy, self.policy):
+ # Begin the extensive work to fully verify whether this output is indeed
+ # change.
+
+ # Each of these is a claim we build our proof from, then keep for the
+ # follow-up check its signature type needs:
+ # * Single sig: the derivation path the seed derives a key at.
+ # * Multisig: the witness or redeem script the coordinator supplied.
+ # Only one of these will be needed, depending on the output type.
+ singlesig_derivation_path = None
+ multisig_script = None
- # empty script by default
- sc = script.Script(b"")
+ # Compared against the output's real scriptPubKey below
+ rebuilt_script_pubkey = script.Script(b"")
# multisig, we know witness script
if self.policy["type"] == "p2wsh":
- sc = script.p2wsh(out.witness_script)
+ multisig_script = out.witness_script
+ rebuilt_script_pubkey = script.p2wsh(multisig_script)
elif self.policy["type"] == "p2sh-p2wsh":
- sc = script.p2sh(script.p2wsh(out.witness_script))
-
+ multisig_script = out.witness_script
+ rebuilt_script_pubkey = script.p2sh(script.p2wsh(multisig_script))
+
# Arbitrary p2sh; includes pre-segwit multisig (m/45')
elif self.policy["type"] == "p2sh":
- sc = script.p2sh(out.redeem_script)
+ multisig_script = out.redeem_script
+ rebuilt_script_pubkey = script.p2sh(multisig_script)
# single-sig: p2pkh, p2sh-p2wpkh, and p2wpkh; taproot handled separately
# below.
elif "pkh" in self.policy["type"]:
- my_pubkey = None
-
- # should be one or zero for single-key addresses
- if len(out.bip32_derivations.values()) > 0:
- der = list(out.bip32_derivations.values())[0].derivation
- my_pubkey = PSBTParser._derive_with_cache(self.root, der, child_key_derivation_cache)
-
- if self.policy["type"] == "p2pkh" and my_pubkey is not None:
- sc = script.p2pkh(my_pubkey)
-
- elif self.policy["type"] == "p2sh-p2wpkh" and my_pubkey is not None:
- sc = script.p2sh(script.p2wpkh(my_pubkey))
-
- elif self.policy["type"] == "p2wpkh" and my_pubkey is not None:
- sc = script.p2wpkh(my_pubkey)
+ # Sanity check; a single sig output shouldn't have multiple derivation
+ # paths.
+ if len(out.bip32_derivations) > 1:
+ raise PSBTSurplusDerivationPathsError("Single-key output claims more than one derivation path")
+
+ # Rebuild the scriptPubKey from the key at the claimed derivation path
+ if len(out.bip32_derivations.values()) == 1:
+ singlesig_derivation_path = list(out.bip32_derivations.values())[0].derivation
+ seed_public_key = PSBTParser._derive_with_cache(self.root, singlesig_derivation_path, child_key_derivation_cache).get_public_key()
+ rebuilt_script_pubkey = PSBTParser._build_singlesig_script(self.policy["type"], seed_public_key)
+ else:
+ # There's nothing for us to verify against so this output will be
+ # considered an external spend.
+ pass
elif "p2tr" in self.policy["type"]:
- my_pubkey = None
- # should have one or zero derivations for single-key addresses
- if len(out.taproot_bip32_derivations.values()) > 0:
- # TODO: Support keys in taptree leaves
- leaf_hashes, derivation = list(out.taproot_bip32_derivations.values())[0]
- der = derivation.derivation
- 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:
+ taproot_entries = list(out.taproot_bip32_derivations.values())
+
+ if len(taproot_entries) == 0:
+ # There's nothing for us to verify against so this output will be
+ # considered an external spend.
+ pass
+ else:
+ # A taproot output has exactly one internal key. So an output
+ # should not claim multiple derivation path entries for the
+ # internal key. However, taproot outputs can have additional
+ # entries for keys in script tree leaves. So we count just the
+ # internal key claims:
+ internal_key_claims = sum(1 for leaf_hashes, _ in taproot_entries if not leaf_hashes)
+ if internal_key_claims > 1:
+ raise PSBTSurplusDerivationPathsError("Taproot output claims more than one internal key")
+
+ if len(taproot_entries) == 1 and internal_key_claims == 1:
+ leaf_hashes, derivation = taproot_entries[0]
+ singlesig_derivation_path = derivation.derivation
+ seed_public_key = PSBTParser._derive_with_cache(self.root, singlesig_derivation_path, child_key_derivation_cache).get_public_key()
+ rebuilt_script_pubkey = PSBTParser._build_singlesig_script(self.policy["type"], seed_public_key)
+ else:
+ # This output has at least one derivation path entry for a key
+ # in a script tree leaf. But since we don't yet parse the
+ # script tree, we can't reconstruct the output's correct
+ # scriptPubKey. So this output will fail to match its
+ # scriptPubKey below, at which point it will be considered an
+ # external spend. This is the best we can do when we cannot
+ # verify ownership.
+ # TODO: Support keys in script tree leaves
+ pass
+
+ verified_derivation_path = self.verified_output_derivation_paths[i]
+
+ if rebuilt_script_pubkey.data == vout[i].script_pubkey.data:
+ # The scriptPubKey we created using our own seed matched what this
+ # output is actually committing to.
+
+ # Remember that "change" is ANY output coming back to our seed
is_change = True
+ if singlesig_derivation_path is not None:
+ if verified_derivation_path is None:
+ # The output pays this seed but the psbt claimed a different
+ # fingerprint here. We treat this deception as an attack.
+ raise PSBTOutputOwnershipContradictionError(f"Output pays this seed at {bip32.path_to_str(singlesig_derivation_path)} but does not claim it there")
+
+ if verified_derivation_path != singlesig_derivation_path:
+ # Shouldn't be able to reach here: the surplus check above
+ # allows only one entry, and the ownership scan refuses a
+ # scope populating both derivation path maps, so the scan can
+ # only have verified this same path.
+ raise RuntimeError(f"Output {i} verified at a path it does not pay")
+
+ elif multisig_script is not None:
+ if verified_derivation_path is None:
+ # No entry claimed this seed's fingerprint, but we already
+ # have everything we need to see if our seed is actually in
+ # the output script.
+ for derivation_path_obj in out.bip32_derivations.values():
+ # Each entry pairs a derivation path with the public key
+ # the coordinator says sits there. Both are its own
+ # claims, so we read only the path and derive the key
+ # ourselves.
+ seed_public_key = PSBTParser._derive_with_cache(self.root, derivation_path_obj.derivation, child_key_derivation_cache).get_public_key()
+
+ if PSBTParser._multisig_script_contains_key(multisig_script, seed_public_key):
+ # The output pays a multisig this seed is part
+ # of, but the psbt did not claim our key there.
+ # We treat this deception as an attack.
+ raise PSBTOutputOwnershipContradictionError(f"Output's committed script holds this seed's key at {bip32.path_to_str(derivation_path_obj.derivation)} but the psbt claims another fingerprint and/or public key there")
+
+ # Every path the psbt supplied has been checked and none puts
+ # this seed in the committed script, so the output is an
+ # external spend.
+ is_change = False
+
+ else:
+ # This output claimed that our seed is part of the receiving
+ # multisig, at a specific path. So now we verify that the key
+ # at that path is in the committed script.
+ seed_public_key = PSBTParser._derive_with_cache(self.root, verified_derivation_path, child_key_derivation_cache).get_public_key()
+ if not PSBTParser._multisig_script_contains_key(multisig_script, seed_public_key):
+ # The psbt said this output was coming back to our seed
+ # at that path, but the key there is not in the committed
+ # script. We treat this deception as an attack.
+ raise PSBTOutputOwnershipContradictionError(f"Output claims this seed at {bip32.path_to_str(verified_derivation_path)} but its committed script does not hold that key")
+
+ # The output should not describe more keys than are actually
+ # used in its script. We check for the more serious deceptions
+ # before this so they can be surfaced first.
+ if len(out.bip32_derivations) > self.policy["n"]:
+ # We don't try to decide if this is an attack or a
+ # mistake. We just abort the parse.
+ raise PSBTSurplusDerivationPathsError("Multisig output claims more derivation paths than its script has keys")
+
+ else:
+ # No handler claimed a matching output, which the branches above
+ # should make impossible. Raise rather than leave is_change True;
+ # that would record change with nothing verified behind it.
+ raise RuntimeError(f"Output {i} matched but no verification handler applies")
+
+ elif verified_derivation_path is not None and "p2tr" not in self.policy["type"]:
+ # The psbt claims one of this seed's keys on this output, yet the
+ # output does NOT pay what that claim describes. We treat this
+ # deception as an attack.
+ # * single sig: verified that this output is not paying our seed at
+ # the claimed derivation path.
+ # * multisig: verified that the output's claimed script is not the
+ # one the output commits to. Note that we haven't verified our
+ # seed's participation in the claimed script; it's irrelevant if
+ # that script isn't committed to in the scriptPubKey.
+ # Taproot is exempt: an output paying our internal key tweaked by
+ # a script tree fails the rebuild above even when the psbt claimed
+ # the seed truthfully, and from here that is indistinguishable
+ # from an output that claims our key and pays someone else.
+ # TODO: Parse PSBT_OUT_TAP_TREE, which embit leaves unparsed in
+ # the scope's `unknown` map. Its merkle root is what separates the
+ # two: a tree that tweaks our key to the committed key makes the
+ # output verifiable change, one that does not is a contradiction to
+ # refuse here, and an output supplying no tree stays exempt, since
+ # an omitted optional field is not a contradiction.
+ raise PSBTOutputOwnershipContradictionError(f"Output claims this seed at {bip32.path_to_str(verified_derivation_path)} but its committed script contradicts that")
+
if vout[i].script_pubkey.data[0] == OPCODES.OP_RETURN:
# The data is written as: OP_RETURN + OP_PUSHDATA1 + len(payload) + payload
self.op_return_data = vout[i].script_pubkey.data[3:]
elif is_change:
addr = vout[i].script_pubkey.address(NETWORKS[SettingsConstants.map_network_to_embit(self.network)])
- claimed_fingerprints = []
- claimed_derivation_paths = []
-
- # extract info from non-taproot outputs
- if len(self.psbt.outputs[i].bip32_derivations) > 0:
- for d, derivation_path in self.psbt.outputs[i].bip32_derivations.items():
- claimed_fingerprints.append(hexlify(derivation_path.fingerprint).decode())
- claimed_derivation_paths.append(bip32.path_to_str(derivation_path.derivation))
-
- # extract info from taproot outputs
- if len(self.psbt.outputs[i].taproot_bip32_derivations) > 0:
- for d, (leaf_hashes, derivation) in self.psbt.outputs[i].taproot_bip32_derivations.items():
- claimed_fingerprints.append(hexlify(derivation.fingerprint).decode())
- claimed_derivation_paths.append(bip32.path_to_str(derivation.derivation))
-
self.change_data.append({
"output_index": i,
"address": addr,
"amount": vout[i].value,
- "claimed_fingerprints": claimed_fingerprints,
- "claimed_derivation_paths": claimed_derivation_paths,
+ "verified_derivation_path": self.verified_output_derivation_paths[i],
})
self.change_amount += vout[i].value
@@ -436,7 +646,7 @@ def _get_policy(scope, scriptpubkey, xpubs, child_key_derivation_cache: dict | N
if script is not None:
m, n, pubkeys = PSBTParser._parse_multisig(script)
-
+
# check pubkeys are derived from cosigners
try:
cosigners = PSBTParser._get_cosigners(pubkeys, scope.bip32_derivations, xpubs, child_key_derivation_cache)
@@ -445,14 +655,55 @@ def _get_policy(scope, scriptpubkey, xpubs, child_key_derivation_cache: dict | N
# 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.
+ # not be rejected outright.
policy.update({"m": m, "n": n})
-
+
return policy
+ @staticmethod
+ def _policy_shape_matches(policy_a: dict, policy_b: dict) -> bool:
+ """
+ Compares two policies on the shape of the script they describe: the script type,
+ plus m-of-n for multisig.
+
+ A policy can also carry the cosigners resolved from the coordinator's global
+ xpubs. Those are never authoritative here, and comparing them would let a psbt
+ decide which of its own outputs get verified: one misannotated fingerprint makes
+ that output's cosigners fail to resolve, and the output then stops matching the
+ inputs' policy. Shape comes from the scriptPubKey and the supplied script, and the
+ caller proves ownership rather than assuming it.
+ """
+ for field in ("type", "m", "n"):
+ if policy_a.get(field) != policy_b.get(field):
+ return False
+
+ return True
+
+
+ @staticmethod
+ def _build_singlesig_script(policy_type: str, public_key: PublicKey) -> script.Script:
+ """
+ Builds the scriptPubKey that pays public_key under the given single-sig
+ policy_type.
+ """
+ if policy_type == "p2pkh":
+ return script.p2pkh(public_key)
+
+ if policy_type == "p2sh-p2wpkh":
+ return script.p2sh(script.p2wpkh(public_key))
+
+ if policy_type == "p2wpkh":
+ return script.p2wpkh(public_key)
+
+ if policy_type == "p2tr":
+ return script.p2tr(public_key)
+
+ # Shouldn't be able to reach here. Just a guard against a future developer calling
+ # this with invalid args.
+ raise RuntimeError(f"Not a single-sig script type: {policy_type}")
+
+
@staticmethod
def _parse_multisig(multisig_script):
"""Takes a script and extracts m,n and pubkeys from it"""
@@ -482,6 +733,15 @@ def _parse_multisig(multisig_script):
return m, n, pubkeys
+ @staticmethod
+ def _multisig_script_contains_key(multisig_script: script.Script, public_key: PublicKey) -> bool:
+ """
+ Determines whether multisig_script includes the provided public_key.
+ """
+ m, n, pubkeys = PSBTParser._parse_multisig(multisig_script)
+ return any(pubkey.sec() == public_key.sec() for pubkey in pubkeys)
+
+
@staticmethod
def _derive_with_cache(parent_key: bip32.HDKey, derivation_path: List[int], child_key_derivation_cache: dict | None = None) -> bip32.HDKey:
"""
@@ -576,7 +836,7 @@ def get_input_fingerprints(psbt: PSBT) -> List[str]:
for pub, (leaf_hashes, derivation_path) in input.taproot_bip32_derivations.items():
# TODO: Support spends from leaves; depends on support in embit
if len(leaf_hashes) > 0:
- raise Exception("Signing keyspends from within a taptree not yet implemented")
+ raise Exception("Signing script path spends is not yet implemented")
fingerprints.add(hexlify(derivation_path.fingerprint).decode())
return list(fingerprints)
@@ -593,7 +853,7 @@ def has_matching_input_fingerprint(psbt: PSBT, seed: Seed, network: str = Settin
instance.
"""
seed_fingerprint = seed.get_fingerprint(network)
-
+
def check_fingerprint_match(public_key: PublicKey, derivation_path_obj: DerivationPath, is_taproot: bool):
"""Check fingerprint match with missing fingerprint fallback"""
@@ -621,7 +881,7 @@ def check_fingerprint_match(public_key: PublicKey, derivation_path_obj: Derivati
for public_key, (leaf_hashes, derivation_path_obj) in input.taproot_bip32_derivations.items():
if check_fingerprint_match(public_key, derivation_path_obj, is_taproot=True):
return True
-
+
return False
@@ -667,6 +927,15 @@ def _get_seed_derivation_path(scope: InputScope | OutputScope, root: bip32.HDKey
Every key in the scope that claims this seed's fingerprint is re-derived and
checked. A false claim raises PSBT[Output|Input]OwnershipClaimError.
+ A further check is then enforced: a scope carrying entries in the
+ bip32_derivations AND taproot_bip32_derivations maps raises
+ PSBTMixedDerivationPathTypesError. Taproot keys are exclusively x-only while
+ non-taproot keys always carry their parity byte; there is no script type that can
+ make use of both types of keys so it is nonsensical for a scope to provide both.
+
+ Note that neither BIP-174 nor BIP-371 forbids the combination. And embit will
+ parse and even sign such a psbt. We disallow it by opinionated choice.
+
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
@@ -700,17 +969,23 @@ def _check_claim(public_key: PublicKey, derivation_path_obj: DerivationPath, is_
_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
+ # TODO: Support keys in script tree leaves
_check_claim(public_key, derivation_path_obj, is_taproot=True)
+ # The derivation path maps cannot both be populated in the same scope. Checked
+ # after the loops so that a false ownership claim, the more serious finding, is
+ # still the one reported.
+ if scope.bip32_derivations and scope.taproot_bip32_derivations:
+ raise PSBTMixedDerivationPathTypesError("Scope declares both ecdsa and taproot derivation paths")
+
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
+ Verifies every derivation path entry that claims 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.
+ derivation path or None (no entry claimed this seed) 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.
@@ -749,7 +1024,7 @@ def _reject_if_seed_cannot_sign(self):
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
+ # An input claims 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.
@@ -760,6 +1035,15 @@ def _reject_if_seed_cannot_sign(self):
raise PSBTSeedCannotSignError()
+ @staticmethod
+ def is_change_branch(derivation_path: List[int]) -> bool:
+ """
+ Returns True if the next-to-last element of the derivation path is the change
+ branch (1).
+ """
+ return len(derivation_path) >= 2 and derivation_path[-2] == 1
+
+
def verify_multisig_output(self, descriptor: Descriptor, change_num: int) -> bool:
change_data = self.get_change_data(change_num)
i = change_data["output_index"]
### src/seedsigner/views/psbt_views.py
@@ -1,11 +1,13 @@
from gettext import gettext as _
from seedsigner.models.psbt_parser import (PSBTInputOwnershipClaimError,
- PSBTOutputOwnershipClaimError, PSBTParser, PSBTSeedCannotSignError)
+ PSBTMixedDerivationPathTypesError, PSBTOutputOwnershipClaimError,
+ PSBTOutputOwnershipContradictionError, PSBTParser, PSBTSeedCannotSignError,
+ PSBTSurplusDerivationPathsError)
from seedsigner.models.settings import SettingsConstants
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
+from seedsigner.views.view import BackStackView, MainMenuView, View, Destination
@@ -103,16 +105,28 @@ def __init__(self):
network=self.settings.get_value(SettingsConstants.SETTING__NETWORK)
)
+ # Note that in almost every exception case, we set clear_history to disable
+ # returning via BACK button in the Destination.
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 PSBTSurplusDerivationPathsError:
+ self.set_redirect(Destination(PSBTSurplusDerivationPathsView, clear_history=True))
+ return
+
+ except PSBTMixedDerivationPathTypesError:
+ self.set_redirect(Destination(PSBTMixedDerivationPathTypesView, clear_history=True))
+ return
+
+ except PSBTOutputOwnershipContradictionError:
+ self.set_redirect(Destination(PSBTOutputOwnershipContradictionView, 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.
@@ -133,17 +147,18 @@ def run(self):
"""
change_data = [
{
+ 'output_index': 0,
'address': 'bc1q............',
'amount': 397621401,
- 'claimed_fingerprints': ['22bde1a9', '73c5da0a'],
- 'claimed_derivation_paths': ['m/48h/1h/0h/2h/1/0', 'm/48h/1h/0h/2h/1/0']
+ 'verified_derivation_path':
+ [2147483696, 2147483649, 2147483648, 2147483650, 1, 0],
}, {},
]
"""
num_change_outputs = 0
num_self_transfer_outputs = 0
for change_output in change_data:
- if change_output["claimed_derivation_paths"][0].split("/")[-2] == "1":
+ if PSBTParser.is_change_branch(change_output["verified_derivation_path"]):
num_change_outputs += 1
else:
num_self_transfer_outputs += 1
@@ -325,39 +340,32 @@ def __init__(self, change_address_num):
def run(self):
+ from embit import bip32
from seedsigner.gui.screens.psbt_screens import PSBTChangeDetailsScreen
psbt_parser: PSBTParser = self.controller.psbt_parser
if not psbt_parser:
# Should not be able to get here
return Destination(MainMenuView)
- # Can we verify this change addr?
change_data = psbt_parser.get_change_data(change_num=self.change_address_num)
"""
change_data:
{
+ 'output_index': 0,
'address': 'bc1q............',
'amount': 397621401,
- 'claimed_fingerprints': ['22bde1a9', '73c5da0a'],
- 'claimed_derivation_paths': ['m/48h/1h/0h/2h/1/0', 'm/48h/1h/0h/2h/1/0']
+ 'verified_derivation_path':
+ [2147483696, 2147483649, 2147483648, 2147483650, 1, 0],
}
"""
-
- # Single-sig verification is easy. We expect to find a single fingerprint
- # and derivation path.
seed_fingerprint = self.controller.psbt_seed.get_fingerprint(self.settings.get_value(SettingsConstants.SETTING__NETWORK))
+ verified_derivation_path = change_data.get("verified_derivation_path")
+ is_change_derivation_path = PSBTParser.is_change_branch(verified_derivation_path)
- if seed_fingerprint not in change_data.get("claimed_fingerprints"):
- # TODO: Something is wrong with this psbt(?). Reroute to warning?
- return Destination(NotYetImplementedView)
-
- i = change_data.get("claimed_fingerprints").index(seed_fingerprint)
- claimed_derivation_path = change_data.get("claimed_derivation_paths")[i]
-
- # 'm/84h/1h/0h/1/0' would be a change addr while 'm/84h/1h/0h/0/0' is a self-receive
- is_change_derivation_path = int(claimed_derivation_path.split("/")[-2]) == 1
- derivation_path_addr_index = int(claimed_derivation_path.split("/")[-1])
+ # TODO: Refuse a path too short to carry a branch and an address index; until then
+ # this can raise on a malformed psbt.
+ derivation_path_addr_index = verified_derivation_path[-1]
if is_change_derivation_path:
# TRANSLATOR_NOTE: The amount you're receiving back from the transaction
@@ -370,66 +378,33 @@ def run(self):
is_change_addr_verified = False
if psbt_parser.is_multisig:
+ # Multisig is verified here rather than during the initial parse because the
+ # descriptor it needs to be checked against does not arrive until mid-flow.
+ # TODO: Verify the multisig change as soon as the descriptor is loaded, rather
+ # than waiting to do it here.
+
# if the known-good multisig descriptor is already onboard:
if self.controller.multisig_wallet_descriptor:
is_change_addr_verified = psbt_parser.verify_multisig_output(self.controller.multisig_wallet_descriptor, change_num=self.change_address_num)
button_data = [self.NEXT]
else:
- # Have the Screen offer to load in the multisig descriptor.
+ # Nothing to check against yet. Have the Screen offer to load in the
+ # multisig descriptor.
button_data = [self.VERIFY_MULTISIG, self.SKIP_VERIFICATION]
else:
- # Single sig
- try:
- from embit import script
- from embit.networks import NETWORKS
-
- if is_change_derivation_path:
- loading_screen_text = _("Verifying Change...")
- else:
- loading_screen_text = _("Verifying Self-Transfer...")
- from seedsigner.gui.screens.screen import LoadingScreenThread
- loading_screen = LoadingScreenThread(text=loading_screen_text)
- loading_screen.start()
-
- # convert change address to script pubkey to get script type
- pubkey = script.address_to_scriptpubkey(change_data["address"])
- script_type = pubkey.script_type()
-
- # extract derivation path to get wallet and change derivation
- change_path = '/'.join(claimed_derivation_path.split("/")[-2:])
- wallet_path = '/'.join(claimed_derivation_path.split("/")[:-2])
-
- xpub = self.controller.psbt_seed.get_xpub(
- wallet_path=wallet_path,
- network=self.settings.get_value(SettingsConstants.SETTING__NETWORK)
- )
-
- # take script type and call script method to generate address from seed / derivation
- xpub_key = xpub.derive(change_path).key
- network = self.settings.get_value(SettingsConstants.SETTING__NETWORK)
- scriptcall = getattr(script, script_type)
- if script_type == "p2sh":
- # single sig only so p2sh is always p2sh-p2wpkh
- calc_address = script.p2sh(script.p2wpkh(xpub_key)).address(
- network=NETWORKS[SettingsConstants.map_network_to_embit(network)]
- )
- else:
- # single sig so this handles p2wpkh and p2wpkh (and p2tr in the future)
- calc_address = scriptcall(xpub_key).address(
- network=NETWORKS[SettingsConstants.map_network_to_embit(network)]
- )
-
- if change_data["address"] == calc_address:
- is_change_addr_verified = True
- button_data = [self.NEXT]
-
- finally:
- loading_screen.stop()
-
- if is_change_addr_verified == False and (not psbt_parser.is_multisig or self.controller.multisig_wallet_descriptor is not None):
- return Destination(PSBTAddressVerificationFailedView, view_args=dict(is_change=is_change_derivation_path, is_multisig=psbt_parser.is_multisig), clear_history=True)
+ # The PSBTParser already proves that single sig change outputs are owned by
+ # this seed.
+ is_change_addr_verified = True
+ button_data = [self.NEXT]
+
+ # TODO: Will be unnecessary once the above update is made to verify multisig
+ # change as soon as the descriptor is loaded.
+ if not is_change_addr_verified and self.controller.multisig_wallet_descriptor is not None:
+ # Verification failed, so this psbt is done.
+ # Set clear_history to disable returning via BACK button.
+ return Destination(PSBTAddressVerificationFailedView, view_args=dict(is_change=is_change_derivation_path), clear_history=True)
selected_menu_num = self.run_screen(
PSBTChangeDetailsScreen,
@@ -439,7 +414,7 @@ def run(self):
amount=change_data.get("amount"),
is_multisig=psbt_parser.is_multisig,
fingerprint=seed_fingerprint,
- derivation_path=claimed_derivation_path,
+ derivation_path=bip32.path_to_str(verified_derivation_path),
is_change_derivation_path=is_change_derivation_path,
derivation_path_addr_index=derivation_path_addr_index,
is_change_addr_verified=is_change_addr_verified,
@@ -470,7 +445,10 @@ def run(self):
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.
+ PSBTSeedCannotSignError).
+
+ We do not view this as an attack; most likely the user simply selected the wrong seed.
+ Routes back to seed selection rather than discarding the psbt.
"""
SELECT_DIFFERENT_SEED = ButtonOption("Select a different seed")
@@ -497,8 +475,11 @@ def run(self):
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.
+ PSBTOutputOwnershipClaimError). Claims on inputs route to
+ PSBTInputOwnershipClaimFailedView instead.
+
+ We view this as an attack. We do not allow the user to continue and give this the
+ "Dire Warning" level.
"""
DISCARD = ButtonOption("Discard transaction")
@@ -507,13 +488,13 @@ def run(self):
DireWarningScreen,
title=_("Suspicious Transaction"),
status_headline=_("Likely an Attack!"),
- text=_("The transaction's change/self-transfer outputs are not going back to your wallet."),
+ text=_("The transaction's change/self-transfer output is 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).
+ # 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)
@@ -522,8 +503,11 @@ def run(self):
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.
+ PSBTInputOwnershipClaimError).
+
+ We do not view this as an attack; a forged input claim only renders the psbt
+ unsignable. We do not allow the user to continue, but only give this the "Warning"
+ level.
"""
DISCARD = ButtonOption("Discard transaction")
@@ -532,37 +516,122 @@ def run(self):
WarningScreen,
title=_("Transaction Problem"),
status_headline=None,
- text=_("This transaction incorrectly claims that its input(s) belong to this seed."),
+ text=_("This transaction incorrectly claims that one of its inputs belongs 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).
+ # 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 PSBTSurplusDerivationPathsView(View):
+ """
+ Reached when an output claims more derivation path entries than its script can use
+ (see PSBTSurplusDerivationPathsError).
+
+ The single sig case is a structural error. The multisig case could be an attempt to
+ deceive but since we don't know for sure, it's sufficient to just use the "Warning"
+ level and stop the user from continuing.
+ """
+ DISCARD = ButtonOption("Discard transaction")
+
+ def run(self):
+ self.run_screen(
+ WarningScreen,
+ title=_("Transaction Problem"),
+ status_headline=None,
+ # TRANSLATOR_NOTE: The transaction/psbt has an error but does not seem to be malicious.
+ text=_("This transaction claims too many keys for one of its outputs."),
+ 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 PSBTMixedDerivationPathTypesView(View):
+ """
+ Reached when an input or output describes its keys in both derivation path maps at
+ once (PSBTMixedDerivationPathTypesError).
+
+ We view this as a strange / buggy psbt and do not try to decide whether it is
+ malicious. We do not allow the user to continue, but only give this the "Warning"
+ level.
+ """
+ DISCARD = ButtonOption("Discard transaction")
+
+ def run(self):
+ self.run_screen(
+ WarningScreen,
+ title=_("Transaction Problem"),
+ status_headline=None,
+ # TRANSLATOR_NOTE: The transaction/psbt has an error but does not seem to be malicious.
+ text=_("This transaction claims taproot and non-taproot keys for the same script."),
+ 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 PSBTOutputOwnershipContradictionView(View):
+ """
+ Reached when the psbt's account of who an output pays contradicts the script that
+ output commits to (see PSBTOutputOwnershipContradictionError).
+
+ We view this as an attack. We do not allow the user to continue and give this the
+ "Dire Warning" level.
+ """
+ DISCARD = ButtonOption("Discard transaction")
+
+ def run(self):
+ self.run_screen(
+ DireWarningScreen,
+ title=_("Suspicious Transaction"),
+ status_headline=_("Likely an Attack!"),
+ # TRANSLATOR_NOTE: The transaction/psbt contains a deception that we consider an attack.
+ text=_("This transaction misrepresents where one of its outputs pays."),
+ 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.
+ Reached from PSBTChangeDetailsView when a multisig change or self-transfer output
+ could not be verified against the descriptor the user supplied.
+
+ We view this as suspicious but stop short of calling it an attack, since the user may
+ have loaded the wrong wallet's descriptor. We do not allow the user to continue and
+ give this the "Dire Warning" level.
"""
- def __init__(self, is_change: bool = True, is_multisig: bool = False):
+ def __init__(self, is_change: bool = True):
super().__init__()
self.is_change = is_change
- self.is_multisig = is_multisig
def run(self):
- if self.is_multisig:
- # TRANSLATOR_NOTE: Variable is either "change" or "self-transfer".
- text = _("Transaction's {} address could not be verified from wallet descriptor.").format(_("change") if self.is_change else _("self-transfer"))
- else:
- # TRANSLATOR_NOTE: Variable is either "change" or "self-transfer".
- text = _("Transaction's {} address could not be generated from your seed.").format(_("change") if self.is_change else _("self-transfer"))
-
+ # TRANSLATOR_NOTE: Variable is either "change" or "self-transfer".
+ text = _("Transaction's {} address could not be verified from wallet descriptor.").format(_("change") if self.is_change else _("self-transfer"))
+
self.run_screen(
DireWarningScreen,
title=_("Suspicious Transaction"),
@@ -572,8 +641,8 @@ def run(self):
show_back_button=False,
)
- # We're done with this PSBT. Route back to MainMenuView, which 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)
@@ -661,8 +730,8 @@ def run(self):
)
self.run_screen(QRDisplayScreen, qr_encoder=qr_encoder)
- # 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).
return Destination(MainMenuView, clear_history=True)
### tests/psbt_testing_util.py
@@ -1,8 +1,9 @@
from binascii import a2b_base64, unhexlify
from io import BytesIO
-from embit import bip32
+from embit import bip32, script
from embit.ec import PublicKey
+from embit.hashes import tagged_hash
from embit.networks import NETWORKS
from embit.psbt import PSBT, DerivationPath, InputScope, OutputScope
@@ -27,6 +28,41 @@ class PSBTTestData:
MULTISIG_NESTED_SEGWIT_1_INPUT = "cHNidP8BADMCAAAAAbtbjTfAfTR/t88dfkGmcZBz0l/uzrLuJEyf0WMHl94vAQAAAAD9////AHEAAABPAQQ1h88EmoSnUYAAAAF+MfPe6kyaEEX91G1HKwksGaixXN6RMBTxIcHKjNYYoQJBzkFqUsm3ttoRsGfx+CMj/77pmyyjitqjWZPG4d3RrxQPuIL/MAAAgAEAAIAAAACAAQAAgE8BBDWHzwRA+hbPgAAAAcRAG6PxSUPbFp8IvKVuNlIQn4W5TK1ceLGdKkxdSfktA8QjWNuOjADhspq0onHkGp117FftE91lAYTev8snQsNRFAPNCiswAACAAQAAgAAAAIABAACATwEENYfPBMtS+WKAAAAB6inmZ+P+TS/JJhI/Fog5q8Rx2Nik9EJVEugTuPSDcI4CbvN4qeJalXMOlJpoKnL9Y64icQtz01M9NG6SOhcG8uQUD4iQRDAAAIABAACAAAAAgAEAAIAAAQBzAgAAAAHCfPUdyeETNxf9pkzBP2oOUfgSISrrZi4uj0boKN2PeQEAAAAA/f///wKwqEIGAQAAABepFOV0lUEDO1EMcpnbm8u5gOS//denhwDh9QUAAAAAF6kUURslNGksU91RDOgpxCM7IQOoQaSHAAAAAAEBIADh9QUAAAAAF6kUURslNGksU91RDOgpxCM7IQOoQaSHAQMEAQAAAAEEIgAgDHlipQ8OV+Wko64bycNh+v4LfTW5d8cTv3T1n4XDS/sBBWlSIQIsxclipQM/Gs3kdO+Mlg1gbcMRf1ukSklJhhQ8iMXpSSEDjvwJUpUWl14h2ma/DH5VeAEhM9PxTz70b6lOAgDiWrshA8FwdXOWiPDUIOtV4aCz/ZxfLr9fwrpHQLCUeAtGctJiU64iBgOO/AlSlRaXXiHaZr8MflV4ASEz0/FPPvRvqU4CAOJauxwPuIL/MAAAgAEAAIAAAACAAQAAgAAAAAAAAAAAIgYCLMXJYqUDPxrN5HTvjJYNYG3DEX9bpEpJSYYUPIjF6UkcA80KKzAAAIABAACAAAAAgAEAAIAAAAAAAAAAACIGA8FwdXOWiPDUIOtV4aCz/ZxfLr9fwrpHQLCUeAtGctJiHA+IkEQwAACAAQAAgAAAAIABAACAAAAAAAAAAAAA"
MULTISIG_LEGACY_P2SH_1_INPUT = "cHNidP8BADMCAAAAAarS4QMzScnPvNBT2B1I3h80UrSoNuKd9vZkjSl7j2WKAQAAAAD9////AHQAAABPAQQ1h88BD7iC/4AAAC16oZcoUbg2ksYGYWMICvKISPS51jTZLJ3tu4schhGxcAJ/o4LdLxGZHyVDceUz5n6ZtABk9X6nBk3yz/f+eXxkpQgPuIL/LQAAgE8BBDWHzwEDzQorgAAALaD15sHGuMCJCZiW09JfCCZEKGcn9WtB395d/8vj/WC7Am982IdFkEBytDXikxzoeLV5q3kpHg+bfmsmj7ncr1lgCAPNCistAACATwEENYfPAQ+IkESAAAAtcUAwLhCloJPpswRwhGdyG3KP1kY0VAetU6FdAmNrqQoDKFgD/CXYCi5ZHqx4HImYJZtoxpjx60Ki7kvK4Ean8FkID4iQRC0AAIAAAQBzAgAAAAFOE/9Gl/ZpjWR8ioftAB2GPhLXScZ3TEa2tIJ+EDwmXQAAAAAA/f///wI+TSQYAQAAABepFE+Jek+WpV0vCjnVpkLqKfoiLvPAhwDh9QUAAAAAF6kUdHD4u+bbshVEAVKkqRQxu+KR8FKHaAAAAAEDBAEAAAABBGlSIQLUf/ihLNEohwIQiCxVNGmPROLzi8t9FnV6DmfCuEhXHCEDK1QlMflvscOGZmSDWJi90FPUQvNFTQUbkkRdowbR0RghA2T1w+njATChE10XcsbguMj0J5RIzJ5TVN7sbVckbzUWU64iBgLUf/ihLNEohwIQiCxVNGmPROLzi8t9FnV6DmfCuEhXHBAPuIL/LQAAgAAAAAAAAAAAIgYDZPXD6eMBMKETXRdyxuC4yPQnlEjMnlNU3uxtVyRvNRYQA80KKy0AAIAAAAAAAAAAACIGAytUJTH5b7HDhmZkg1iYvdBT1ELzRU0FG5JEXaMG0dEYEA+IkEQtAACAAAAAAAAAAAAA"
+ # Unlike the fixtures above, this is a complete psbt: its outputs are intact and it
+ # has its own wallet. Use it wherever a test needs more than one input, or needs
+ # inputs and change at known, differing address indices.
+ #
+ # PSBT Tx and Wallet Details
+ # - Single Sig Wallet P2WPKH (Native Segwit) with no passphrase
+ # - Regtest 394aed14 m/84'/1'/0' tpubDC4rddHCzuinGFEKhiD8A3Ku5GRcNVAz3BitfwL4wn7zKPCya4i2CZLX8uoP8oCdC8VEe4jXb5u9ePCSrRA68YRSgBQra5YzBajyxKFDz4C
+ # - 2 Inputs
+ # - 56,522,834 sats at 1/6
+ # - 1,990,245,069 sats at 1/0
+ # - 4 Outputs
+ # - 1 Output spend to another wallet: 123,456 sats to bcrt1q7cw0wzy8g6mq5qvkpvhnk5gsps5ncy3srp0n2j
+ # - 3 Outputs change
+ # - 1/7 address bcrt1q53j0xwuskuf5gnvynadh0hlazyy8srydlucrhg with amount 123,456 sats
+ # - 1/8 address bcrt1q5gtw3zfp4cx67yk5q42q6j6rfza8aqcwpyyslv with amount 56,399,242 sats
+ # - 1/9 address bcrt1q9rrg7399m43cn0yg4tz0v0ate89jgf2d6kpz7v with amount 1,990,121,477 sats
+ # - Fee 272 sats
+ two_input_seed = Seed("goddess rough corn exclude cream trial fee trumpet million prevent gaze power".split())
+ SINGLE_SIG_NATIVE_SEGWIT_2_INPUTS = "cHNidP8BANgCAAAAAsTXZs3fz/dmGb6M80+jjvJZdYya+cw5bT/dGuhZFdSlAAAAAAD9////qo6xg/UZAvUkcbse1F+C9zbP/FeZNjThx7SCIn6eMCgBAAAAAP3///8EQOIBAAAAAAAWABSkZPM7kLcTRE2En1t33/0RCHgMjQXYnnYAAAAAFgAUKMaPRKXdY4m8iKrE9j+rycskJU1A4gEAAAAAABYAFPYc9wiHRrYKAZYLLztREAwpPBIwipVcAwAAAAAWABSiFuiJIa4NrxLUBVQNS0NIun6DDtoRAABPAQQ1h88DBcQGZIAAAAA+0J+jlNL3dpWwlnBi8Dx+Ipg4e6uvB3HdjzFPX7r9CAOOlAIxgII+/xCcj+XoEenKH7wj5s5wlu7Q7CCZWFLGLhA5Su0UVAAAgAEAAIAAAACAAAEA7QIAAAAEE6njX/fnvn7hbkKIRcxzNYFOSfbCdNeWnd7Fe/1UcQ0BAAAAAP3///8TqeNf9+e+fuFuQohFzHM1gU5J9sJ015ad3sV7/VRxDQMAAAAA/f///xOp41/3575+4W5CiEXMczWBTkn2wnTXlp3exXv9VHENBAAAAAD9////E6njX/fnvn7hbkKIRcxzNYFOSfbCdNeWnd7Fe/1UcQ0GAAAAAP3///8CUnheAwAAAAAWABRCfygPJ+Fjsx4BknYvvm3A3qKn2xJ/XQcAAAAAF6kU1I4TAst5nAj15ey7vwe5cM3OFq+HlhEAAAEBH1J4XgMAAAAAFgAUQn8oDyfhY7MeAZJ2L75twN6ip9sBAwQBAAAAIgYCo7sfm78RQY3B5n0ac/QF8VtMAzFnci+h5D1MtpgRY7oYOUrtFFQAAIABAACAAAAAgAEAAAAGAAAAAAEAcQIAAAABxY7wh0nsfJQfzWrD/9rN9BYsM+iOmPaO6I0ANFgO/PcAAAAAAP3///8CptiUAAAAAAAWABRIm4HhQY/TzOjeWSPRrbuJo9MlW826oHYAAAAAFgAU0z+0L2QSLGtyQTn8FhbCpcI7jbliAQAAAQEfzbqgdgAAAAAWABTTP7QvZBIsa3JBOfwWFsKlwjuNuQEDBAEAAAAiBgITHmebEANk81CraV4xZIpqkNjjw0tIvezl1Ism1NRH3Rg5Su0UVAAAgAEAAIAAAACAAQAAAAAAAAAAIgICuTT7WnuiUTpObjWnZFHzIeEvW9PTB+1LLVFNQJVFeIIYOUrtFFQAAIABAACAAAAAgAEAAAAHAAAAACICAk8f3hpc5C35chgSg+Pe2zZ9IhHREd4aKW2+yAMRIFeqGDlK7RRUAACAAQAAgAAAAIABAAAACQAAAAAAIgIDjt1CjvrnMMnjbmTNKUAYoKEDRbmKjNjbq+6Ppqj3bqQYOUrtFFQAAIABAACAAAAAgAEAAAAIAAAAAA=="
+
+ # Also complete and on the same wallet as the fixture above. Taproot records its
+ # derivation paths in a separate map from the segwit-v0 one, so this is what tests of
+ # that split need.
+ #
+ # PSBT Tx and Wallet Details
+ # - Single Sig Wallet P2TR (Taproot) with no passphrase
+ # - Regtest 394aed14 m/86'/1'/0' tpubDCawGrRg7YdHdFb9p4mmD8GBaZjJegL53FPFRrMkGoLcgLATJfksUs2y1Q7dVzixAkgecazsxEsUuyj3LyDw7eVVYHQyojwrc2hfesK4wXW
+ # - 1 Input
+ # - 3,190,493,401 sats at 1/0
+ # - 2 Outputs
+ # - 1 Output change: 2,871,443,918 sats to bcrt1prz4g6saush37epdwhvwpu78td3q7yfz3xxz37axlx7udck6wracq3rwq30 at 1/1
+ # - 1 Output spend to another wallet: 319,049,328 sats to bcrt1p6p00wazu4nnqac29fvky6vhjnnhku5u2g9njss62rvy7e0yuperq86f5ek
+ # - Fee 155 sats
+ SINGLE_SIG_TAPROOT_WITH_CHANGE = "cHNidP8BAIkCAAAAAf8upuiIWF1VTgC/Q8ZWRrameRigaXpRcQcBe8ye+TK3AQAAAAAXCgAAAs7BJqsAAAAAIlEgGKqNQ7yF4+yFrrscHnjrbEHiJFExhR903ze43FtOH3BwTgQTAAAAACJRINBe93RcrOYO4UVLLE0y8pzvblOKQWcoQ0obCey8nA5GAAAAAE8BBDWHzwNMUx9OgAAAAJdr+WtwWfVa6IPbpKZ4KgRC0clbm11Gl155IPA27n2FAvQCrFGH6Ac2U0Gcy1IH5f5ltgUBDz2+fe8iqL6JzZdgEDlK7RRWAACAAQAAgAAAAIAAAQB9AgAAAAGAKOOUFIzw9pbRDaZ7F0DYhLImrdMn//OSm++ff5VNdAAAAAAAAQAAAAKsjLwAAAAAABYAFKEcuxvXmB3rWHSqSviP5mrKMZoL2RArvgAAAAAiUSBGU0Lg5fx/ECsB1Z4ZUqXQFSLFnlmpm0rm5R2l599h2AAAAAABASvZECu+AAAAACJRIEZTQuDl/H8QKwHVnhlSpdAVIsWeWambSublHaXn32HYAQMEAAAAACEWF7hZVn7pIDR429kAn/WDeQiWjZey1iGHztsL1H83QLMZADlK7RRWAACAAQAAgAAAAIABAAAAAAAAAAEXIBe4WVZ+6SA0eNvZAJ/1g3kIlo2XstYhh87bC9R/N0CzACEHbJdqWyMxF2eOPr6YRXUJmry04HUbgKyeM2IZeG+NI9AZADlK7RRWAACAAQAAgAAAAIABAAAAAQAAAAEFIGyXalsjMRdnjj6+mEV1CZq8tOB1G4CsnjNiGXhvjSPQAAA="
+
SINGLE_SIG_INPUTS = [
SINGLE_SIG_NATIVE_SEGWIT_1_INPUT,
SINGLE_SIG_NESTED_SEGWIT_1_INPUT,
@@ -81,6 +117,13 @@ class PSBTTestData:
MULTISIG_NESTED_SEGWIT_RECEIVE = "01030890d0030000000000010417a9141d3c080d4b05358b8cc439ef12534250563f34a08700"
MULTISIG_LEGACY_P2SH_RECEIVE = "01030890d0030000000000010417a914fa70ebb69e283b493770c5a8fa19ec76da321de68700"
+ # The same output as MULTISIG_NATIVE_SEGWIT_RECEIVE, but populated the way a
+ # coordinator that also holds the recipient's descriptor populates it: the 2-of-3
+ # script the output pays, plus a derivation path entry for each of its cosigners.
+ # Built from recipient_seed, recipient_multisig_key_2 and recipient_multisig_key_3,
+ # each at m/48h/1h/0h/2h/0/0.
+ MULTISIG_NATIVE_SEGWIT_RECEIVE_ANNOTATED = "0101695221027d99f92952795d306ef9b87a9d16c94bedc4b17613042a6f1b80af0e9c7839a32103208d94f8b4df19bcb76c309887904920064ccc417225520790426cb0e40fe6552103d36ac409c6198fee6eae72bf5c38424f3f2e2077a67aaf14d535fd1021b97c4953ae2202027d99f92952795d306ef9b87a9d16c94bedc4b17613042a6f1b80af0e9c7839a31c0f7d3df0300000800100008000000080020000800000000000000000220203208d94f8b4df19bcb76c309887904920064ccc417225520790426cb0e40fe6551ccd063d44300000800100008000000080020000800000000000000000220203d36ac409c6198fee6eae72bf5c38424f3f2e2077a67aaf14d535fd1021b97c491c2f54d8a930000080010000800000008002000080000000000000000001030890d003000000000001042200200936ff1943bbe5c037ad9e9839ca3effe24d8b22fd38cc2601c9007cc0f210a100"
+
ALL_EXTERNAL_OUTPUTS = [
SINGLE_SIG_NATIVE_SEGWIT_RECEIVE,
SINGLE_SIG_NESTED_SEGWIT_RECEIVE,
@@ -158,7 +201,37 @@ def foreign_public_key(derivation_path: str = "m/84h/1h/0h/0/0", seed: Seed = No
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):
+def tapleaf_hash(public_key: PublicKey, leaf_version: int = 0xC0) -> bytes:
+ """
+ The BIP-341 leaf hash of the simplest tapscript (a lone key checksig), built the same
+ way embit builds it when it signs a script path spend.
+ """
+ OP_PUSHBYTES_32 = b"\x20"
+ OP_CHECKSIG = b"\xac"
+ leaf_script = script.Script(OP_PUSHBYTES_32 + public_key.xonly() + OP_CHECKSIG)
+ return tagged_hash("TapLeaf", bytes([leaf_version]) + leaf_script.serialize())
+
+
+# BIP-341's provably unspendable internal key (aka "NUMS" point: "Nothing Up My Sleeve"):
+# the SHA256 of the standard uncompressed encoding of the secp256k1 generator point,
+# lifted to a curve point. A taproot address built for script path spends only uses this
+# as its internal key, so that nobody holds a key path to it.
+NUMS_INTERNAL_KEY = PublicKey.from_xonly(unhexlify("50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0"))
+
+
+def p2tr_with_script_tree(internal_public_key: PublicKey, merkle_root: bytes) -> script.Script:
+ """
+ The scriptPubKey of a taproot output whose internal key is tweaked by a script tree's
+ merkle root, built the same way embit builds the empty-tree form. embit's script.p2tr
+ cannot build this one: it asks the script tree for its own tweak, and embit defines no
+ script tree type to ask.
+ """
+ OP_1 = b"\x51" # segwit version byte
+ OP_PUSHBYTES_32 = b"\x20"
+ return script.Script(OP_1 + OP_PUSHBYTES_32 + internal_public_key.taproot_tweak(merkle_root).xonly())
+
+
+def claim_seed_owns_key(scope: InputScope | OutputScope, claimed_derivation_path: str, public_key: PublicKey, seed: Seed = None, is_taproot: bool = False, leaf_hashes: list = None):
"""
Writes a derivation into the scope claiming that seed owns public_key at
claimed_derivation_path.
@@ -167,6 +240,9 @@ def claim_seed_owns_key(scope: InputScope | OutputScope, claimed_derivation_path
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.
+
+ For taproot, leaf_hashes names the leaf scripts the key is used in. An entry naming
+ none of them is claiming to be the output's internal key; see tapleaf_hash.
"""
if seed is None:
seed = PSBTTestData.seed
@@ -175,6 +251,6 @@ def claim_seed_owns_key(scope: InputScope | OutputScope, claimed_derivation_path
root_for_seed(seed).my_fingerprint, bip32.parse_path(claimed_derivation_path))
if is_taproot:
- scope.taproot_bip32_derivations[public_key] = ([], derivation_path)
+ scope.taproot_bip32_derivations[public_key] = (leaf_hashes or [], derivation_path)
else:
scope.bip32_derivations[public_key] = derivation_path
### tests/screenshot_generator/generator.py
@@ -449,10 +449,11 @@ def mock_version_to_most_recent_release():
ScreenshotConfig(psbt_views.PSBTOverviewView, screenshot_name="PSBTOverviewView_op_return", mock_context_manager=mock_psbt_with_op_return_loaded),
ScreenshotConfig(psbt_views.PSBTOpReturnView, screenshot_name="PSBTOpReturnView_text", mock_context_manager=mock_psbt_with_op_return_loaded),
ScreenshotConfig(psbt_views.PSBTOpReturnView, screenshot_name="PSBTOpReturnView_raw_hex_data", mock_context_manager=mock_psbt_with_op_return_raw_bytes_loaded),
- ScreenshotConfig(psbt_views.PSBTAddressVerificationFailedView, dict(is_change=True, is_multisig=False), screenshot_name="PSBTAddressVerificationFailedView_singlesig_change"),
- 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.PSBTSurplusDerivationPathsView),
+ ScreenshotConfig(psbt_views.PSBTMixedDerivationPathTypesView),
+ ScreenshotConfig(psbt_views.PSBTOutputOwnershipContradictionView),
+ ScreenshotConfig(psbt_views.PSBTAddressVerificationFailedView, dict(is_change=True), screenshot_name="PSBTAddressVerificationFailedView_multisig_change"),
+ ScreenshotConfig(psbt_views.PSBTAddressVerificationFailedView, dict(is_change=False), screenshot_name="PSBTAddressVerificationFailedView_multisig_selftransfer"),
ScreenshotConfig(psbt_views.PSBTOutputOwnershipClaimFailedView),
ScreenshotConfig(psbt_views.PSBTInputOwnershipClaimFailedView),
ScreenshotConfig(psbt_views.PSBTSeedCannotSignView),
### tests/test_flows_psbt.py
@@ -1,10 +1,11 @@
from binascii import a2b_base64
-from embit.psbt import PSBT
+from embit import bip32, script
+from embit.psbt import PSBT, DerivationPath
from base import FlowTest, FlowStep
from psbt_testing_util import (PSBTTestData, claim_seed_owns_key, create_output,
- foreign_public_key)
+ foreign_public_key, root_for_seed)
from seedsigner.controller import Controller
from seedsigner.views.view import MainMenuView
@@ -24,24 +25,7 @@ def test_scan_psbt_first_then_correct_seedqr_flow(self):
since the PSBT is not a self transfer it should enter the PSBTAddressDetailsView flow
"""
def load_psbt_into_decoder(view: scan_views.ScanView):
- """
- PSBT Tx and Wallet Details
- - Single Sig Wallet P2WPKH (Native Segwit) with no passphrase
- - Regtest c751dc07 m/84'/1'/0' tpubDDZBrnxMxbVzqt8EoEiABPxeKzFWma5pra5UEbg3Wst1hrwr6feuvcy7Sov7cpuYx94ypuy1PQ9NDNoQagFs37wGALzLb5Ei3FvyJWPPPKZ
- - 2 Inputs
- - 56,522,834 sats
- - 1,990,245,069 sats
- - 4 Outputs
- - 1 Output to another wallet (bcrt1q7cw0wzy8g6mq5qvkpvhnk5gsps5ncy3srp0n2j) of 123,456 sats
- - 3 Outputs change
- - 3 outputs to emulate a fake mix to increase privacy
- - Change addresses are index 1/7, 1/8, 1/9
- - 1/7 address bcrt1q53j0xwuskuf5gnvynadh0hlazyy8srydlucrhg with amount 123,456 sats
- - 1/8 address bcrt1q5gtw3zfp4cx67yk5q42q6j6rfza8aqcwpyyslv with amount 1,990,121,477 sats
- - 1/9 address bcrt1q9rrg7399m43cn0yg4tz0v0ate89jgf2d6kpz7v with amount 56,399,242 sats
- - Fee 272 sats
- """
- view.decoder.add_data("cHNidP8BANgCAAAAAsTXZs3fz/dmGb6M80+jjvJZdYya+cw5bT/dGuhZFdSlAAAAAAD9////qo6xg/UZAvUkcbse1F+C9zbP/FeZNjThx7SCIn6eMCgBAAAAAP3///8EQOIBAAAAAAAWABSkZPM7kLcTRE2En1t33/0RCHgMjQXYnnYAAAAAFgAUKMaPRKXdY4m8iKrE9j+rycskJU1A4gEAAAAAABYAFPYc9wiHRrYKAZYLLztREAwpPBIwipVcAwAAAAAWABSiFuiJIa4NrxLUBVQNS0NIun6DDtoRAABPAQQ1h88DBcQGZIAAAAA+0J+jlNL3dpWwlnBi8Dx+Ipg4e6uvB3HdjzFPX7r9CAOOlAIxgII+/xCcj+XoEenKH7wj5s5wlu7Q7CCZWFLGLhA5Su0UVAAAgAEAAIAAAACAAAEA7QIAAAAEE6njX/fnvn7hbkKIRcxzNYFOSfbCdNeWnd7Fe/1UcQ0BAAAAAP3///8TqeNf9+e+fuFuQohFzHM1gU5J9sJ015ad3sV7/VRxDQMAAAAA/f///xOp41/3575+4W5CiEXMczWBTkn2wnTXlp3exXv9VHENBAAAAAD9////E6njX/fnvn7hbkKIRcxzNYFOSfbCdNeWnd7Fe/1UcQ0GAAAAAP3///8CUnheAwAAAAAWABRCfygPJ+Fjsx4BknYvvm3A3qKn2xJ/XQcAAAAAF6kU1I4TAst5nAj15ey7vwe5cM3OFq+HlhEAAAEBH1J4XgMAAAAAFgAUQn8oDyfhY7MeAZJ2L75twN6ip9sBAwQBAAAAIgYCo7sfm78RQY3B5n0ac/QF8VtMAzFnci+h5D1MtpgRY7oYOUrtFFQAAIABAACAAAAAgAEAAAAGAAAAAAEAcQIAAAABxY7wh0nsfJQfzWrD/9rN9BYsM+iOmPaO6I0ANFgO/PcAAAAAAP3///8CptiUAAAAAAAWABRIm4HhQY/TzOjeWSPRrbuJo9MlW826oHYAAAAAFgAU0z+0L2QSLGtyQTn8FhbCpcI7jbliAQAAAQEfzbqgdgAAAAAWABTTP7QvZBIsa3JBOfwWFsKlwjuNuQEDBAEAAAAiBgITHmebEANk81CraV4xZIpqkNjjw0tIvezl1Ism1NRH3Rg5Su0UVAAAgAEAAIAAAACAAQAAAAAAAAAAIgICuTT7WnuiUTpObjWnZFHzIeEvW9PTB+1LLVFNQJVFeIIYOUrtFFQAAIABAACAAAAAgAEAAAAHAAAAACICAk8f3hpc5C35chgSg+Pe2zZ9IhHREd4aKW2+yAMRIFeqGDlK7RRUAACAAQAAgAAAAIABAAAACQAAAAAAIgIDjt1CjvrnMMnjbmTNKUAYoKEDRbmKjNjbq+6Ppqj3bqQYOUrtFFQAAIABAACAAAAAgAEAAAAIAAAAAA==")
+ view.decoder.add_data(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_2_INPUTS)
def load_seed_into_decoder(view: scan_views.ScanView):
view.decoder.add_data("080115060387063104071857067618681125136207731354")
@@ -193,9 +177,9 @@ def load_seed_into_decoder(view: scan_views.ScanView):
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.
+ A psbt whose own description of itself does not hold up is refused during parsing,
+ before the user is shown anything about the transaction. These cover what the user
+ meets when that happens: a warning screen that ends the flow, not a crash.
"""
def _load_psbt_for_signing(self, psbt: PSBT, seed: Seed = None):
@@ -250,6 +234,92 @@ def test_forged_input_claim_terminates_signing_flow(self):
])
+ def test_surplus_derivation_paths_terminate_signing_flow(self):
+ """
+ When an output names more derivation paths than its script can use, nothing in
+ the psbt says which key it actually pays, so parsing refuses it.
+
+ The parser tests cover why that shape is refusable. This one covers what the user
+ gets: a warning that ends the flow, rather than a crash, and without first being
+ walked through the details of a transaction about to be discarded.
+ """
+ other_root = root_for_seed(PSBTTestData.recipient_seed)
+
+ psbt = PSBT.parse(a2b_base64(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_2_INPUTS))
+
+ # Output 2 pays a stranger and carries no derivation path entries of its own.
+ # Give it two, so nothing in the psbt says which key it pays.
+ for i in range(2):
+ derivation_path = bip32.parse_path(f"m/84h/1h/0h/0/{i}")
+ psbt.outputs[2].bip32_derivations[other_root.derive(derivation_path).get_public_key()] = \
+ DerivationPath(other_root.my_fingerprint, derivation_path)
+
+ self._load_psbt_for_signing(psbt, seed=PSBTTestData.two_input_seed)
+
+ self.run_sequence([
+ FlowStep(psbt_views.PSBTSelectSeedView, screen_return_value=0),
+ FlowStep(psbt_views.PSBTOverviewView, is_redirect=True),
+ FlowStep(psbt_views.PSBTSurplusDerivationPathsView, button_data_selection=psbt_views.PSBTSurplusDerivationPathsView.DISCARD),
+ FlowStep(MainMenuView),
+ ])
+
+
+ def test_output_ownership_contradiction_terminates_signing_flow(self):
+ """
+ When a psbt marks an output as paying us while its script pays a stranger, the
+ two cannot both be true, so parsing refuses it.
+
+ The parser tests cover which shapes qualify. This one covers what the user gets:
+ a warning that ends the flow, before any transaction detail is rendered.
+ """
+ psbt = PSBT.parse(a2b_base64(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_2_INPUTS))
+
+ # Output 0 is the wallet's own change. Repoint its script at a stranger but leave
+ # its derivation path entry in place, so the psbt still names a key this seed
+ # owns on an output that no longer pays it. Note that psbt.tx is rebuilt on every
+ # access, so the scriptPubKey has to be set on the output scope itself.
+ psbt.outputs[0].script_pubkey = script.p2wpkh(foreign_public_key())
+
+ self._load_psbt_for_signing(psbt, seed=PSBTTestData.two_input_seed)
+
+ self.run_sequence([
+ FlowStep(psbt_views.PSBTSelectSeedView, screen_return_value=0),
+ FlowStep(psbt_views.PSBTOverviewView, is_redirect=True),
+ FlowStep(psbt_views.PSBTOutputOwnershipContradictionView, button_data_selection=psbt_views.PSBTOutputOwnershipContradictionView.DISCARD),
+ FlowStep(MainMenuView),
+ ])
+
+
+ def test_mixed_derivation_path_types_terminate_signing_flow(self):
+ """
+ An input or output filling both derivation path maps at once is refused on that
+ shape alone. The check lives in the ownership scan, which runs over inputs as well
+ as outputs, so this one plants the shape on an input, the side the parser tests do
+ not cover.
+
+ It ends the flow at its own warning before any transaction detail is rendered.
+ """
+ other_root = root_for_seed(PSBTTestData.recipient_seed)
+
+ psbt = PSBT.parse(a2b_base64(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_2_INPUTS))
+
+ # Input 0 already carries a segwit-v0 entry. Add a taproot one beside it, on a
+ # stranger's key so nothing here claims this seed: the refusal is on the shape
+ # alone, not on an ownership claim.
+ derivation_path = bip32.parse_path("m/86h/1h/0h/0/0")
+ psbt.inputs[0].taproot_bip32_derivations[foreign_public_key()] = (
+ [], DerivationPath(other_root.my_fingerprint, derivation_path))
+
+ self._load_psbt_for_signing(psbt, seed=PSBTTestData.two_input_seed)
+
+ self.run_sequence([
+ FlowStep(psbt_views.PSBTSelectSeedView, screen_return_value=0),
+ FlowStep(psbt_views.PSBTOverviewView, is_redirect=True),
+ FlowStep(psbt_views.PSBTMixedDerivationPathTypesView, button_data_selection=psbt_views.PSBTMixedDerivationPathTypesView.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
### tests/test_psbt_parser.py
@@ -7,16 +7,18 @@
from embit import bip32, script
from embit.ec import PublicKey
from embit.networks import NETWORKS
-from embit.psbt import PSBT, DerivationPath
+from embit.psbt import PSBT, DerivationPath, OutputScope
from embit.descriptor import Descriptor
from seedsigner.models.psbt_parser import (PSBTInputOwnershipClaimError,
- PSBTOutputOwnershipClaimError, PSBTParser, PSBTSeedCannotSignError)
+ PSBTMixedDerivationPathTypesError, PSBTOutputOwnershipClaimError,
+ PSBTOutputOwnershipContradictionError, PSBTParser, PSBTSeedCannotSignError,
+ PSBTSurplusDerivationPathsError)
from seedsigner.models.seed import Seed
from seedsigner.models.settings_definition import SettingsConstants
-from psbt_testing_util import (PSBTTestData, claim_seed_owns_key, create_output,
- foreign_public_key, root_for_seed)
+from psbt_testing_util import (NUMS_INTERNAL_KEY, PSBTTestData, claim_seed_owns_key,
+ create_output, foreign_public_key, p2tr_with_script_tree, root_for_seed, tapleaf_hash)
@@ -338,43 +340,31 @@ def test_verify_multisig_output(self):
assert psbt_parser.verify_multisig_output(descriptor, change_num=1) == False
+ def test__is_change_branch__distinguishes_the_change_branch_from_the_receive_branch(self):
+ """
+ A wallet keeps its change addresses on branch 1 and the addresses it hands out to
+ other people on branch 0. The next-to-last element of the derivation path is
+ therefore what separates change from a self-transfer back to our own receive addr.
+ """
+ assert PSBTParser.is_change_branch(bip32.parse_path("m/84h/1h/0h/1/0")) is True
+ assert PSBTParser.is_change_branch(bip32.parse_path("m/84h/1h/0h/0/0")) is False
+
+
# TODO: Refactor all tests to be in the TestPSBTParser class(?)
def test_p2tr_change_detection():
- """ Should successfully detect change in a p2tr to p2tr psbt spend
-
- PSBT Tx and Wallet Details
- - Single Sig Wallet P2TR (Taproot) with no passphrase
- - Regtest 394aed14 m/86'/1'/0' tpubDCawGrRg7YdHdFb9p4mmD8GBaZjJegL53FPFRrMkGoLcgLATJfksUs2y1Q7dVzixAkgecazsxEsUuyj3LyDw7eVVYHQyojwrc2hfesK4wXW
- - 1 Inputs
- - 3,190,493,401 sats
- - 2 Outputs
- - 1 Output spend to another wallet (bcrt1p6p00wazu4nnqac29fvky6vhjnnhku5u2g9njss62rvy7e0yuperq86f5ek) p2tr address
- - 1 Output change
- - addresss bcrt1prz4g6saush37epdwhvwpu78td3q7yfz3xxz37axlx7udck6wracq3rwq30)
- - amount 2,871,443,918 sats
- - Change addresses is index 1/1
- - Fee 155 sats
- """
-
- psbt_base64 = "cHNidP8BAIkCAAAAAf8upuiIWF1VTgC/Q8ZWRrameRigaXpRcQcBe8ye+TK3AQAAAAAXCgAAAs7BJqsAAAAAIlEgGKqNQ7yF4+yFrrscHnjrbEHiJFExhR903ze43FtOH3BwTgQTAAAAACJRINBe93RcrOYO4UVLLE0y8pzvblOKQWcoQ0obCey8nA5GAAAAAE8BBDWHzwNMUx9OgAAAAJdr+WtwWfVa6IPbpKZ4KgRC0clbm11Gl155IPA27n2FAvQCrFGH6Ac2U0Gcy1IH5f5ltgUBDz2+fe8iqL6JzZdgEDlK7RRWAACAAQAAgAAAAIAAAQB9AgAAAAGAKOOUFIzw9pbRDaZ7F0DYhLImrdMn//OSm++ff5VNdAAAAAAAAQAAAAKsjLwAAAAAABYAFKEcuxvXmB3rWHSqSviP5mrKMZoL2RArvgAAAAAiUSBGU0Lg5fx/ECsB1Z4ZUqXQFSLFnlmpm0rm5R2l599h2AAAAAABASvZECu+AAAAACJRIEZTQuDl/H8QKwHVnhlSpdAVIsWeWambSublHaXn32HYAQMEAAAAACEWF7hZVn7pIDR429kAn/WDeQiWjZey1iGHztsL1H83QLMZADlK7RRWAACAAQAAgAAAAIABAAAAAAAAAAEXIBe4WVZ+6SA0eNvZAJ/1g3kIlo2XstYhh87bC9R/N0CzACEHbJdqWyMxF2eOPr6YRXUJmry04HUbgKyeM2IZeG+NI9AZADlK7RRWAACAAQAAgAAAAIABAAAAAQAAAAEFIGyXalsjMRdnjj6+mEV1CZq8tOB1G4CsnjNiGXhvjSPQAAA="
-
- raw = a2b_base64(psbt_base64)
+ """ Should successfully detect change in a p2tr to p2tr psbt spend """
+ raw = a2b_base64(PSBTTestData.SINGLE_SIG_TAPROOT_WITH_CHANGE)
tx = PSBT.parse(raw)
-
- mnemonic = "goddess rough corn exclude cream trial fee trumpet million prevent gaze power".split()
- pw = ""
- seed = Seed(mnemonic, passphrase=pw)
- pp = PSBTParser(p=tx, seed=seed, network=SettingsConstants.REGTEST)
+ pp = PSBTParser(p=tx, seed=PSBTTestData.two_input_seed, network=SettingsConstants.REGTEST)
assert pp.change_data == [
{
'output_index': 0,
'address': 'bcrt1prz4g6saush37epdwhvwpu78td3q7yfz3xxz37axlx7udck6wracq3rwq30',
'amount': 2871443918,
- 'claimed_fingerprints': ['394aed14'],
- 'claimed_derivation_paths': ['m/86h/1h/0h/1/1']}
+ 'verified_derivation_path': bip32.parse_path('m/86h/1h/0h/1/1')}
]
assert pp.spend_amount == 319049328
assert pp.change_amount == 2871443918
@@ -533,8 +523,7 @@ def test_parse_op_return_content():
'output_index': 0,
'address': 'bcrt1qvwkhakqhz7m7kmz6332avatsmdy32m644g86vv',
'amount': 99992296,
- 'claimed_fingerprints': ['0fb882ff'],
- 'claimed_derivation_paths': ["m/84h/1h/0h/0/2"]}
+ 'verified_derivation_path': bip32.parse_path("m/84h/1h/0h/0/2")}
]
assert psbt_parser.spend_amount == 0 # This is a self-spend; no value being spent, other than the tx fee
assert psbt_parser.change_amount == 99992296
@@ -825,10 +814,12 @@ def build_psbt(case: tuple) -> PSBT:
-class TestPSBTParserSeedOwnership:
+class PSBTParserOwnershipTestBase:
"""
- 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.
+ Base class for the two ownership test classes below. Provides:
+ * the signing seed
+ * a psbt made of nothing but that seed's own scopes
+ * the parse call under test.
"""
seed = PSBTTestData.seed
@@ -855,6 +846,12 @@ def _parse(self, psbt: PSBT) -> PSBTParser:
return PSBTParser(psbt, self.seed, network=SettingsConstants.REGTEST)
+
+class TestPSBTParserSeedOwnership(PSBTParserOwnershipTestBase):
+ """
+ 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.
+ """
def test__seed_owns_pubkey__accepts_the_seeds_own_key(self):
"""
seed_owns_pubkey should confirm the simple base case that a pubkey directly
@@ -1238,3 +1235,656 @@ def test_a_psbt_with_no_utxos_is_rejected_rather_than_crashing(self):
with pytest.raises(PSBTSeedCannotSignError):
self._parse(psbt)
+
+
+
+class TestPSBTParserOutputOwnership(PSBTParserOwnershipTestBase):
+ """
+ A psbt annotates its outputs with claims about which keys own them by providing
+ derivation path entries for each key.
+
+ But it's the output's script that actually determines where the funds go.
+
+ These tests cover invalid claims as well as the ways that the claims and the script
+ can disagree and how PSBTParser handles such discrepancies.
+ """
+ def _foreign_multisig_script(self) -> script.Script:
+ """A 2-of-3 built entirely from someone else's keys."""
+ return script.multisig(2, [foreign_public_key(f"m/48h/1h/0h/2h/0/{i}") for i in range(3)])
+
+
+ def _rebuild_around_foreign_keys(self, out: OutputScope):
+ """
+ Replace a multisig output's script to pay to a different quorum made entirely of
+ someone else's keys, but leave the output's original derivation path entries
+ alone. The output now misrepresents who receives its funds.
+ """
+ foreign_script = self._foreign_multisig_script()
+
+ if out.witness_script is not None:
+ out.witness_script = foreign_script
+ inner_script = script.p2wsh(foreign_script)
+ else:
+ inner_script = foreign_script
+
+ if out.redeem_script is not None:
+ out.redeem_script = inner_script
+ out.script_pubkey = script.p2sh(inner_script)
+ else:
+ out.script_pubkey = inner_script
+
+
+ def test__parse__rejects_a_single_key_output_that_claims_more_than_one_path(self):
+ """
+ It is nonsensical for a single sig output to list more than one derivation path
+ entry. Some elaborate deceptions may be possible with extra derivation paths, but
+ we simply reject such psbts by raising PSBTSurplusDerivationPathsError.
+ """
+ decoy_derivation_path = "m/84h/1h/0h/1/9"
+
+ # A normal psbt with change; includes the single derivation path entry that
+ # describes it.
+ psbt = self._psbt_with_change()
+
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt but with a second entry naming a key this seed really does own at
+ # another path.
+ psbt = self._psbt_with_change()
+ decoy_public_key = self._root().derive(decoy_derivation_path).get_public_key()
+ claim_seed_owns_key(psbt.outputs[0], decoy_derivation_path, decoy_public_key)
+
+ with pytest.raises(PSBTSurplusDerivationPathsError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_taproot_output_that_claims_more_than_one_internal_key(self):
+ """
+ Taproot is an exception to the rule that single sig outputs cannot have more than
+ one derivation path entry. A BIP-371 output can legitimately have several entries:
+ the internal key plus one for each key in its script tree.
+
+ But the basic single sig logic holds for the internal key: there can be only one.
+
+ Entries that do name a leaf are legitimate and should be left out of the count.
+ """
+ decoy_derivation_path = "m/86h/1h/0h/1/7"
+ decoy_public_key = self._root().derive(decoy_derivation_path).get_public_key()
+
+ leaf_derivation_path = "m/86h/1h/0h/1/8"
+ leaf_public_key = self._root().derive(leaf_derivation_path).get_public_key()
+
+ # The wallet's own taproot change, carrying the single entry that describes it
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+
+ # The simple case parses and identifies the output as change
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt, with a second entry naming no leaf hashes, which is therefore a
+ # second claim to be the output's one internal key.
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ claim_seed_owns_key(psbt.outputs[0], decoy_derivation_path, decoy_public_key, is_taproot=True)
+
+ with pytest.raises(PSBTSurplusDerivationPathsError):
+ self._parse(psbt)
+
+ # The same two internal key entries, now with a genuine script tree key beside
+ # them. The surplus is still a surplus.
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ claim_seed_owns_key(psbt.outputs[0], decoy_derivation_path, decoy_public_key, is_taproot=True)
+ claim_seed_owns_key(psbt.outputs[0], leaf_derivation_path, leaf_public_key, is_taproot=True,
+ leaf_hashes=[tapleaf_hash(leaf_public_key)])
+
+ with pytest.raises(PSBTSurplusDerivationPathsError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_an_output_with_entries_in_both_derivation_path_maps(self):
+ """
+ The taproot derivation paths (taproot_bip32_derivations) are only relevant for
+ taproot scripts, just as the non-taproot derivation paths (bip32_derivations) are
+ only relevant for non-taproot scripts.
+
+ There is no scenario where both maps could carry valid data for the same output.
+
+ A psbt that includes both is rejected with PSBTMixedDerivationPathTypesError.
+ """
+ # The wallet's own taproot change, with an empty segwit-v0 map beside it
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ assert len(psbt.outputs[0].bip32_derivations) == 0
+ assert len(psbt.outputs[0].taproot_bip32_derivations) == 1
+
+ # Parses successfully as expected
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt, with a truthful entry on a key this seed owns written into the
+ # non-taproot map.
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+
+ extra_derivation_path = "m/84h/1h/0h/1/9"
+ extra_public_key = self._root().derive(extra_derivation_path).get_public_key()
+ claim_seed_owns_key(psbt.outputs[0], extra_derivation_path, extra_public_key)
+
+ # The output now has entries in both maps
+ assert len(psbt.outputs[0].bip32_derivations) == 1
+ assert len(psbt.outputs[0].taproot_bip32_derivations) == 1
+
+ # So it fails as expected
+ with pytest.raises(PSBTMixedDerivationPathTypesError):
+ self._parse(psbt)
+
+ # Set up the same collision, but with a non-taproot output
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_CHANGE)
+ assert len(psbt.outputs[0].bip32_derivations) == 1
+ assert len(psbt.outputs[0].taproot_bip32_derivations) == 0
+
+ # Add a valid taproot derivation path entry
+ taproot_derivation_path = "m/86h/1h/0h/1/9"
+ taproot_public_key = self._root().derive(taproot_derivation_path).get_public_key()
+ claim_seed_owns_key(psbt.outputs[0], taproot_derivation_path, taproot_public_key, is_taproot=True)
+
+ # The output now has entries in both maps
+ assert len(psbt.outputs[0].bip32_derivations) == 1
+ assert len(psbt.outputs[0].taproot_bip32_derivations) == 1
+
+ # Once again fails as expected
+ with pytest.raises(PSBTMixedDerivationPathTypesError):
+ self._parse(psbt)
+
+
+ def test__parse__counts_a_multisig_output_paying_other_people_as_a_spend(self):
+ """
+ Most coordinators will not provide any output derivation paths nor the output
+ script itself for external spends. In most cases the coordinator simply wouldn't
+ know that information for outside parties.
+
+ But even if that information is provided (as Bitcoin Core can do if the recipient
+ is another wallet for which it knows the internal details), the parser should
+ still interpret the output correctly: as an external spend.
+
+ Most of the tests in this class are about what is not allowed. This test gives the
+ parser a scenario that IS allowed, but it touches on many of the areas that the
+ parser's rejection logic depends on.
+ """
+ psbt = PSBT.parse(a2b_base64(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT))
+ recipient_output = create_output(PSBTTestData.MULTISIG_NATIVE_SEGWIT_RECEIVE_ANNOTATED, 10_000)
+ psbt.outputs.append(recipient_output)
+
+ # The coordinator's annotation is honest: the script it supplied is the one the
+ # output really pays, and the keys it names are the recipient's own.
+ assert script.p2wsh(recipient_output.witness_script).data == recipient_output.script_pubkey.data
+ assert len(recipient_output.bip32_derivations) == 3
+
+ # As expected: no errors raised, no deceptions detected
+ psbt_parser = self._parse(psbt)
+
+ # Trivial confirmation: none of the output's three derivation path entries claimed
+ # to belong to this seed.
+ assert psbt_parser.verified_output_derivation_paths[0] is None
+
+ # The parser correctly categorized the output as an external spend
+ assert psbt_parser.change_data == []
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+ # BIP-174 makes the derivation path entries optional, so a coordinator may supply
+ # the output's script and nothing else. Only the entries are dropped here; the
+ # output still commits to the same outside parties' script.
+ recipient_output.bip32_derivations.clear()
+
+ # Still parses successfully; no errors raised, no deceptions detected
+ psbt_parser = self._parse(psbt)
+
+ # Same result. The output was correctly categorized as an external spend.
+ assert psbt_parser.change_data == []
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+
+ def test__parse__counts_multisig_change_with_no_derivation_paths_as_a_spend(self):
+ """
+ The same shape as an outgoing payment, but this output really is our own change:
+ the script it commits to does hold a key of this seed. BIP-174 makes the entries
+ naming that key optional, and this psbt omits them.
+
+ With no entry to derive from there is no key to go looking for in the script, so
+ the output is counted as a spend. That over-reports what is leaving the wallet,
+ and it is a limit on what we can see rather than a detection of anything wrong.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE)
+
+ # Unmodified, this output is the wallet's own change
+ psbt_parser = self._parse(psbt)
+
+ # With the derivation paths present, we verified that the output did name a key
+ # that this seed owns (which also enabled the parser to verify that our key was
+ # indeed part of the script).
+ assert psbt_parser.verified_output_derivation_paths[0] is not None
+
+ # And the output was correctly categorized as change
+ assert psbt_parser.change_amount == 10_000
+ assert psbt_parser.spend_amount == 0
+
+ # Dropping the derivation path entries leaves the coordinator's witness_script
+ # intact, so the output still hashes to the committed scriptPubKey and still
+ # matches the input's policy. Only the path we would derive our key from is gone.
+ psbt.outputs[0].bip32_derivations = {}
+
+ psbt_parser = self._parse(psbt)
+
+ # The output provided no derivation paths to verify (leaving the parser unable to
+ # determine if our seed owns any of the keys in the output's script).
+ assert psbt_parser.verified_output_derivation_paths[0] is None
+
+ # Because we couldn't do proper verification, the parser correctly categorized the
+ # output as an external spend.
+ assert psbt_parser.change_data == []
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+
+ def test__parse__counts_multisig_change_with_no_script_as_a_spend(self):
+ """
+ The same output as the test above, withholding the other half. Here the entries
+ naming this seed are intact and the supplied script is what is missing, which
+ BIP-174 also permits.
+
+ So the psbt claims the output for this seed and gives us nothing to check that
+ claim against. A claim we cannot check is not a contradiction, so the psbt is
+ not rejected, but the output is counted as a spend.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE)
+
+ # The entries claiming this seed are left alone; only the script is withheld
+ psbt.outputs[0].witness_script = None
+
+ psbt_parser = self._parse(psbt)
+
+ # The claim itself still verifies
+ assert psbt_parser.verified_output_derivation_paths[0] is not None
+
+ # But with no script there is no m-of-n to compare, so the output never becomes a
+ # change candidate at all.
+ assert psbt_parser.change_data == []
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+
+ def test__parse__counts_taproot_change_naming_our_internal_key_as_a_spend(self):
+ """
+ SeedSigner would not be used (yet) for a taproot wallet that includes a script
+ tree, but the parser already has to distinguish a derivation path for the internal
+ key vs derivation paths for tapleaf keys. So this test and its sibling that
+ follows verify that the parser correctly handles tapleaf keys when encountered
+ (within the limitations we have due to not parsing the script tree itself).
+
+ This test provides a derivation path for the internal key, which our seed owns.
+ The wallet can spend the output through the key path, so the funds really are this
+ seed's own change. But the address commits to that internal key tweaked by the
+ script tree which we do not yet parse, so the parser's attempts at validating the
+ output's script will fail.
+
+ As a result, we have to treat this output as an external spend. An output cannot
+ be categorized as change if we have not fully verified it.
+ """
+ # TODO: When the script tree is supported, this output should verify as change, by
+ # tweaking our internal key with the tree's merkle root.
+
+ # A taproot address where we control both spending routes: the internal key path
+ # and a one-leaf script tree. The coordinator annotates both keys in the psbt so
+ # the parser will see the derivation path for the internal key and the tapleaf
+ # key.
+ leaf_derivation_path = "m/86h/1h/0h/1/7"
+ leaf_public_key = self._root().derive(leaf_derivation_path).get_public_key()
+ merkle_root = tapleaf_hash(leaf_public_key)
+
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ taproot_output = psbt.outputs[0]
+
+ # The change output's key has already been annotated as the internal key. We just
+ # need to add the tapleaf key's entry.
+ taproot_output.script_pubkey = p2tr_with_script_tree(taproot_output.taproot_internal_key, merkle_root)
+ claim_seed_owns_key(taproot_output, leaf_derivation_path, leaf_public_key, is_taproot=True, leaf_hashes=[merkle_root])
+
+ psbt_parser = self._parse(psbt)
+
+ # Even though the parser verified that our seed owns the internal key...
+ assert psbt_parser.verified_output_derivation_paths[0] is not None
+
+ # ...the parser can't fully verify the output as change, so has to report it as an
+ # external spend.
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+
+ def test__parse__counts_taproot_change_naming_our_script_tree_key_as_a_spend(self):
+ """
+ Sibling to the above test. This time no derivation path is provided for the
+ internal key (BIP-174 says that every entry is optional) and the sole derivation
+ path entry names a tapleaf key instead.
+
+ That is what a real-world script-path-only address looks like: intentionally
+ constructed so that nobody holds the internal key, so there's no derivation path
+ the coordinator could supply for it. Our seed owns the key in the leaf and can
+ spend the output through it, so this too is the wallet's own change. But since
+ embit doesn't yet parse the taproot script tree, the parser cannot verify the
+ output's script. So the output must be reported as a spend.
+ """
+ # TODO: When the script tree is supported this output should verify as change, by
+ # finding our key among the tree's leaves.
+
+ # BIP 341's provably unspendable internal key (the "NUMS" point), over a one-leaf
+ # script tree holding a key this seed does own. With no internal key to describe,
+ # the one entry the coordinator writes is for the leaf key.
+ leaf_derivation_path = "m/86h/1h/0h/1/7"
+ leaf_public_key = self._root().derive(leaf_derivation_path).get_public_key()
+ merkle_root = tapleaf_hash(leaf_public_key)
+
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ taproot_output = psbt.outputs[0]
+ taproot_output.script_pubkey = p2tr_with_script_tree(NUMS_INTERNAL_KEY, merkle_root)
+
+ # Ensure that there's no derivation path entry for the internal key
+ taproot_output.taproot_bip32_derivations.clear()
+
+ # Add the one and only derivation path claim: the tapleaf key
+ claim_seed_owns_key(taproot_output, leaf_derivation_path, leaf_public_key, is_taproot=True, leaf_hashes=[merkle_root])
+
+ psbt_parser = self._parse(psbt)
+
+ # The parser verified that we own the tapleaf key...
+ assert psbt_parser.verified_output_derivation_paths[0] is not None
+
+ # ...but the output still has to be reported as an external spend
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+
+ def test__parse__rejects_an_output_that_claims_this_seed_but_pays_someone_else(self):
+ """
+ The output's derivation path entry claims that the output pays to a key that our
+ seed genuinely owns, but the output's script actually pays out to a different key.
+ We consider this deception an attack.
+ """
+ # First the normal case where it really is our change
+ psbt = self._psbt_with_change()
+ psbt_parser = self._parse(psbt)
+
+ # Confirmed by the parser
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt, but with that output's script repointed at a stranger while its
+ # original derivation path entry is left in place. The output now misrepresents
+ # who receives its funds.
+ psbt = self._psbt_with_change()
+ psbt.outputs[0].script_pubkey = script.p2wpkh(foreign_public_key())
+
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+
+ # Removing the derivation path entry means that there is now no longer any
+ # deception; the output is just an external spend.
+ psbt = self._psbt_with_change()
+ psbt.outputs[0].script_pubkey = script.p2wpkh(foreign_public_key())
+ psbt.outputs[0].bip32_derivations.clear()
+
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+
+ def test__parse__rejects_an_output_that_pays_this_seed_but_claims_someone_else(self):
+ """
+ This is the opposite deception from the previous test. A psbt might mark our own
+ change with a derivation path that claims it belongs to someone else (provides
+ someone else's fingerprint).
+
+ We don't try to decide whether this is an attack or a mistake. Any incorrect
+ claim about ownership is a deception, so the psbt is rejected.
+ """
+ # First the normal case where it really is our change
+ psbt = self._psbt_with_change()
+ psbt_parser = self._parse(psbt)
+
+ # Confirmed by the parser
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt, with the output now claiming a stranger's fingerprint. The key
+ # that the output's script actually pays out to is still our key and the derivation
+ # path is untouched.
+ psbt = self._psbt_with_change()
+ public_key, derivation_path_obj = list(psbt.outputs[0].bip32_derivations.items())[0]
+
+ # Create the deception: Change the fingerprint, but keep the derivation path
+ psbt.outputs[0].bip32_derivations[public_key] = DerivationPath(
+ root_for_seed(PSBTTestData.recipient_seed).my_fingerprint, derivation_path_obj.derivation)
+
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_taproot_change_that_claims_someone_else(self):
+ """
+ The taproot mirror of the test above. The output's scriptPubKey still pays our
+ internal key at the derivation path the psbt supplies, but the entry claims a
+ stranger's fingerprint, so nothing verifies as ours and the psbt is refused.
+
+ Worth its own test because the p2tr branch reaches that refusal by a different
+ route, taking the path from the single internal-key entry rather than from
+ bip32_derivations.
+ """
+ # First the normal case where it really is our change
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ psbt_parser = self._parse(psbt)
+
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt, with only the fingerprint on the internal key's entry replaced.
+ # The scriptPubKey and the derivation path are left alone, so the output still
+ # pays the key that path produces.
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_TAPROOT_1_INPUT, PSBTTestData.SINGLE_SIG_TAPROOT_CHANGE)
+ public_key, (leaf_hashes, derivation_path_obj) = list(psbt.outputs[0].taproot_bip32_derivations.items())[0]
+
+ # The entry has to stay an internal-key claim, so its (empty) leaf hashes are kept
+ assert leaf_hashes == []
+ psbt.outputs[0].taproot_bip32_derivations[public_key] = (
+ leaf_hashes,
+ DerivationPath(root_for_seed(PSBTTestData.recipient_seed).my_fingerprint, derivation_path_obj.derivation))
+
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_multisig_output_built_from_other_peoples_keys(self):
+ """
+ A malicious psbt might try to trick the parser with a multisig output that claims
+ to be the wallet's change, but whose script is built entirely from someone else's
+ keys.
+
+ The output's script type and m-of-n match the inputs', so it is considered as a
+ possible change output. The attacker must also annotate the output with a
+ derivation path entry that names a key our seed really owns in order to pass other
+ parser checks. This creates a false claim that the change output belongs to our
+ seed.
+
+ But a final check prevents this deception from succeeding:
+
+ An output should only be considered change when one of our keys is in the output's
+ script.
+
+ Raise PSBTOutputOwnershipContradictionError if this deception is detected.
+ """
+ # For each multisig script type...
+ for input_base64, change_hex in [
+ (PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NESTED_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_LEGACY_P2SH_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE),
+ ]:
+ # First create the psbt with the wallet's correct change
+ psbt = self._psbt_with_change(input_base64, change_hex)
+
+ # The parse is still successful and raises no alarms
+ psbt_parser = self._parse(psbt)
+
+ assert psbt_parser.change_amount == 10_000
+ assert psbt_parser.spend_amount == 0
+
+ # The same psbt but now make the output pay to someone else's keys
+ psbt = self._psbt_with_change(input_base64, change_hex)
+ self._rebuild_around_foreign_keys(psbt.outputs[0])
+
+ # The output now still claims to pay our seed but the actual output script
+ # says otherwise. The deception is flagged.
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_multisig_output_that_hides_this_seed_behind_another_fingerprint(self):
+ """
+ This scenario leaves a genuine multisig change output completely intact, but
+ relabels the entry describing this seed's key with a different fingerprint.
+
+ The seed's key is still in the script the output commits to, and the psbt still
+ supplies the derivation path that produced our key, so deriving at that path
+ proves this is actually our change.
+
+ But the contradicting fingerprint claimed in the derivation path entry is a
+ deception that we consider an attack. Raises
+ PSBTOutputOwnershipContradictionError.
+ """
+ for input_base64, change_hex in [
+ (PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NESTED_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_LEGACY_P2SH_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE),
+ ]:
+ # Untouched, the output is recognized as change coming back to the seed
+ psbt_parser = self._parse(self._psbt_with_change(input_base64, change_hex))
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt with only the fingerprint on this seed's entry replaced. The
+ # script, the scriptPubKey and every derivation path are left alone.
+ psbt = self._psbt_with_change(input_base64, change_hex)
+ root = self._root()
+ entries = psbt.outputs[0].bip32_derivations
+
+ relabeled = 0
+ for public_key, derivation_path_obj in list(entries.items()):
+ if derivation_path_obj.fingerprint == root.my_fingerprint:
+ entries[public_key] = DerivationPath(b"\x11\x22\x33\x44", derivation_path_obj.derivation)
+ relabeled += 1
+
+ # The tamper only means anything if it actually landed on this seed's entry
+ assert relabeled == 1
+
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+
+
+ def test__parse__refuses_a_multisig_decoy_entry_in_either_position(self):
+ """
+ In this scenario the multisig change output is a legitimate change output that
+ genuinely belongs to our seed, but a decoy derivation path entry is added. The
+ decoy is ALSO a key that our seed owns, but it is not used in the output's script.
+
+ We don't need to decide if such a psbt has malicious intent; the fact that it
+ contradicts itself is unacceptable regardless:
+ * it names a key on an output whose script does not use it.
+ * it names more keys than that script has.
+ Both are provable from the psbt alone, so we reject the psbt.
+
+ This is similar to the single sig test earlier in this class, but is more
+ complicated for multisig since it's the norm for multiple derivation paths to be
+ provided for each multisig change output.
+
+ The derivation path entries are provided in a coordinator-controlled order, so
+ this test covers decoy entries that are listed before or after the seed's actual
+ cosigner entry, across all three multisig script types.
+
+ Both orderings are refused. The ordering only decides which problem we report.
+ We record the first entry that verifies against our seed, so:
+ * When the decoy is listed first, the decoy is what we record and it is not in
+ the script.
+ * When the decoy is listed last, the key we record is our real one and nothing
+ is wrong with it; what gives the decoy away instead is that the output named
+ more keys than its script has.
+ """
+ root = self._root()
+
+ # For each script type...
+ for input_base64, change_hex in [
+ (PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NESTED_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_LEGACY_P2SH_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE),
+ ]:
+ # ...run both versions of the test: decoy listed first and decoy last
+ for decoy_first in [True, False]:
+ psbt = self._psbt_with_change(input_base64, change_hex)
+
+ cosigner_entries = dict(psbt.outputs[0].bip32_derivations)
+
+ # Build the decoy from the cosigners' baseline, then make one minor
+ # derivation path change.
+ genuine_derivation_path = list(cosigner_entries.values())[0].derivation
+ decoy_derivation_path = genuine_derivation_path[:-1] + [genuine_derivation_path[-1] + 1]
+ decoy_public_key = root.derive(decoy_derivation_path).get_public_key()
+ decoy_entry = DerivationPath(root.my_fingerprint, decoy_derivation_path)
+
+ # Add the decoy to the existing 3 derivations
+ entries = psbt.outputs[0].bip32_derivations
+ if decoy_first:
+ entries.clear()
+ entries[decoy_public_key] = decoy_entry
+ entries.update(cosigner_entries)
+ else:
+ entries[decoy_public_key] = decoy_entry
+
+ if decoy_first:
+ # The parser uses the decoy as the comparison against which keys are
+ # actually in the script.
+ expected_error = PSBTOutputOwnershipContradictionError
+ else:
+ # The original cosigner is verified but then the parser detects the
+ # decoy as a surplus derivation path.
+ expected_error = PSBTSurplusDerivationPathsError
+
+ with pytest.raises(expected_error):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_multisig_output_whose_supplied_script_is_not_its_own(self):
+ """
+ This is a change-theft scenario where a malicious coordinator replaces just the
+ scriptPubKey on a multisig change output. As a result, every check on the change
+ output correctly passes, right up until the output's script is hashed and compared
+ to the output's scriptPubKey. The two results are different, proving that the
+ output's funds are not going where the psbt claimed they were.
+ """
+ for input_base64, change_hex in [
+ (PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NESTED_SEGWIT_CHANGE),
+ (PSBTTestData.MULTISIG_LEGACY_P2SH_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE),
+ ]:
+ # First create the psbt with the wallet's correct change
+ psbt = self._psbt_with_change(input_base64, change_hex)
+ psbt_parser = self._parse(psbt)
+
+ # Categorizes the output correctly as change coming back to the seed
+ assert psbt_parser.change_amount == 10_000
+
+ # The same psbt, with only the scriptPubKey repointed at a stranger's
+ # 2-of-3. The psbt's own witness/redeem script and the entries naming this
+ # seed are left untouched, so the psbt still holds its original claims
+ # about who owns the output.
+ psbt = self._psbt_with_change(input_base64, change_hex)
+ out = psbt.outputs[0]
+
+ foreign_script = self._foreign_multisig_script()
+ inner_script = script.p2wsh(foreign_script) if out.witness_script is not None else foreign_script
+ out.script_pubkey = script.p2sh(inner_script) if out.redeem_script is not None else inner_script
+
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+Why this scored 79/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.