diff --git a/src/seedsigner/models/psbt_parser.py b/src/seedsigner/models/psbt_parser.py index 044a9ef5..d4fd01d2 100644 --- a/src/seedsigner/models/psbt_parser.py +++ b/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 @@ class PSBTParser(): - 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. + policy with no cosigner information pass as a match between inputs. + Outputs deliberately compare shape alone; see _policy_shape_matches. - 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. + 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 @@ class PSBTParser(): 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 @@ class PSBTParser(): 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. - # empty script by default - sc = script.Script(b"") + # 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 + + # 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 + # 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") - # 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) + # 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) + taproot_entries = list(out.taproot_bip32_derivations.values()) - if sc.data == vout[i].script_pubkey.data: + 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 @@ class PSBTParser(): 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 @@ class PSBTParser(): # 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 @@ class PSBTParser(): 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 @@ class PSBTParser(): 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 @@ class PSBTParser(): 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 @@ class PSBTParser(): 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 @@ class PSBTParser(): 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 @@ class PSBTParser(): _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 @@ class PSBTParser(): 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 @@ class PSBTParser(): 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"] diff --git a/src/seedsigner/views/psbt_views.py b/src/seedsigner/views/psbt_views.py index 77dbb356..dba57fb4 100644 --- a/src/seedsigner/views/psbt_views.py +++ b/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 @@ class PSBTOverviewView(View): 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 @@ class PSBTOverviewView(View): """ 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,6 +340,7 @@ class PSBTChangeDetailsView(View): def run(self): + from embit import bip32 from seedsigner.gui.screens.psbt_screens import PSBTChangeDetailsScreen psbt_parser: PSBTParser = self.controller.psbt_parser @@ -332,32 +348,24 @@ class PSBTChangeDetailsView(View): # 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 @@ class PSBTChangeDetailsView(View): 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 + # The PSBTParser already proves that single sig change outputs are owned by + # this seed. + is_change_addr_verified = True + button_data = [self.NEXT] - 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) + # 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 @@ class PSBTChangeDetailsView(View): 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 @@ class PSBTChangeDetailsView(View): 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 @@ class PSBTSeedCannotSignView(View): 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 @@ class PSBTOutputOwnershipClaimFailedView(View): 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 @@ class PSBTOutputOwnershipClaimFailedView(View): 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,13 +516,99 @@ class PSBTInputOwnershipClaimFailedView(View): 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) @@ -546,23 +616,22 @@ class PSBTInputOwnershipClaimFailedView(View): 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 @@ class PSBTAddressVerificationFailedView(View): 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 @@ class PSBTSignedQRDisplayView(View): ) 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) diff --git a/tests/psbt_testing_util.py b/tests/psbt_testing_util.py index 00ea93d2..85ebe827 100644 --- a/tests/psbt_testing_util.py +++ b/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 diff --git a/tests/screenshot_generator/generator.py b/tests/screenshot_generator/generator.py index 328bf700..5fa52a2c 100644 --- a/tests/screenshot_generator/generator.py +++ b/tests/screenshot_generator/generator.py @@ -449,10 +449,11 @@ def generate_screenshots(locale): 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), diff --git a/tests/test_flows_psbt.py b/tests/test_flows_psbt.py index 18ba21ef..c21a3e05 100644 --- a/tests/test_flows_psbt.py +++ b/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 @@ class TestPSBTFlows(FlowTest): 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 @@ class TestPSBTFlows(FlowTest): 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 @@ class TestPSBTOwnershipClaimRouting(FlowTest): ]) + 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 diff --git a/tests/test_psbt_parser.py b/tests/test_psbt_parser.py index 0b770120..b1ec561a 100644 --- a/tests/test_psbt_parser.py +++ b/tests/test_psbt_parser.py @@ -7,16 +7,18 @@ from unittest.mock import patch 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 @@ class TestPSBTParser: 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 @@ class TestPSBTParserOptimizations: -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 @@ class TestPSBTParserSeedOwnership: 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 @@ class TestPSBTParserSeedOwnership: 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) +