feat(core): Support unverified external inputs.

core/.changelog.d/2144.added

@@ -0,0 +1 @@
+Support unverified external inputs.

core/src/apps/bitcoin/common.py

@@ -161,6 +161,15 @@ def input_is_external(txi: TxInput) -> bool:
     return txi.script_type == InputScriptType.EXTERNAL
 
 
+def input_is_external_unverified(txi: TxInput) -> bool:
+    return (
+        txi.script_type == InputScriptType.EXTERNAL
+        and txi.ownership_proof is None
+        and txi.witness is None
+        and txi.script_sig is None
+    )
+
+
 def tagged_hashwriter(tag: bytes) -> HashWriter:
     tag_digest = sha256(tag).digest()
     ctx = sha256(tag_digest)

core/src/apps/bitcoin/sign_tx/approvers.py

@@ -8,6 +8,7 @@ from trezor.ui.components.common.confirm import INFO
 from apps.common import safety_checks
 
 from ..authorization import FEE_PER_ANONYMITY_DECIMALS
+from ..common import input_is_external_unverified
 from ..keychain import validate_path_against_script_type
 from . import helpers, tx_weight
 from .payment_request import PaymentRequestVerifier
@@ -68,10 +69,16 @@ class Approver:
     def add_external_input(self, txi: TxInput) -> None:
         self.weight.add_input(txi)
         self.total_in += txi.amount
-        self.external_in += txi.amount
         if txi.orig_hash:
             self.orig_total_in += txi.amount
-            self.orig_external_in += txi.amount
+
+        if input_is_external_unverified(txi):
+            if safety_checks.is_strict():
+                raise wire.ProcessError("Unverifiable external input.")
+        else:
+            self.external_in += txi.amount
+            if txi.orig_hash:
+                self.orig_external_in += txi.amount
 
     def _add_output(self, txo: TxOutput, script_pubkey: bytes) -> None:
         self.weight.add_output(script_pubkey)
@@ -351,6 +358,15 @@ class CoinJoinApprover(Approver):
         if not self.authorization.check_sign_tx_input(txi, self.coin):
             raise wire.ProcessError("Unauthorized path")
 
+    def add_external_input(self, txi: TxInput) -> None:
+        super().add_external_input(txi)
+
+        # External inputs should always be verifiable in CoinJoin. This check
+        # is not critical for security, we are just being cautious, because
+        # CoinJoin is automated and this is not a very legitimate use-case.
+        if input_is_external_unverified(txi):
+            raise wire.ProcessError("Unverifiable external input.")
+
     def add_change_output(self, txo: TxOutput, script_pubkey: bytes) -> None:
         super().add_change_output(txo, script_pubkey)
         self.our_weight.add_output(script_pubkey)

core/src/apps/bitcoin/sign_tx/bitcoin.py

@@ -15,6 +15,7 @@ from ..common import (
     bip340_sign,
     ecdsa_sign,
     input_is_external,
+    input_is_external_unverified,
     input_is_segwit,
 )
 from ..ownership import verify_nonownership
@@ -205,8 +206,10 @@ class Bitcoin:
                 progress.advance()
                 txi = await helpers.request_tx_input(self.tx_req, i, self.coin)
                 writers.write_tx_input_check(h_check, txi)
-                assert txi.script_pubkey is not None  # checked in sanitize_tx_input
-                await self.verify_external_input(i, txi, txi.script_pubkey)
+                if not input_is_external_unverified(txi):
+                    assert txi.script_pubkey is not None  # checked in sanitize_tx_input
+                    await self.verify_external_input(i, txi, txi.script_pubkey)
+
             progress.advance(self.tx_info.tx.inputs_count - len(self.external))
         else:
             # There are internal non-Taproot inputs. We need to verify all inputs, because we can't
@@ -232,7 +235,7 @@ class Bitcoin:
                 if script_pubkey != self.input_derive_script(txi):
                     raise wire.DataError("Input does not match scriptPubKey")
 
-                if i in self.external:
+                if i in self.external and not input_is_external_unverified(txi):
                     await self.verify_external_input(i, txi, script_pubkey)
 
         # check that the inputs were the same as those streamed for approval

core/src/apps/bitcoin/writers.py

@@ -15,6 +15,8 @@ from apps.common.writers import (  # noqa: F401
     write_uint64_le,
 )
 
+from .common import input_is_external_unverified
+
 if TYPE_CHECKING:
     from trezor.messages import (
         PrevInput,
@@ -49,6 +51,7 @@ def write_tx_input_check(w: Writer, i: TxInput) -> None:
     write_bytes_fixed(w, i.prev_hash, TX_HASH_SIZE)
     write_uint32(w, i.prev_index)
     write_uint32(w, i.script_type)
+    write_uint8(w, input_is_external_unverified(i))
     write_uint32(w, len(i.address_n))
     for n in i.address_n:
         write_uint32(w, n)

core/tests/test_apps.bitcoin.approver.py

@@ -49,6 +49,7 @@ class TestApprover(unittest.TestCase):
                 script_pubkey=bytes(22),
                 script_type=InputScriptType.EXTERNAL,
                 sequence=0xffffffff,
+                witness="",
             ) for i in range(99)
         ]
 

core/tests/test_apps.bitcoin.sign_tx.writers.py

@@ -43,7 +43,7 @@ class TestWriters(unittest.TestCase):
 
         b = bytearray()
         writers.write_tx_input_check(b, inp)
-        self.assertEqual(len(b), 32 + 4 + 4 + 4 + 4 + 4 + 8 + 26)
+        self.assertEqual(len(b), 32 + 4 + 4 + 1 + 4 + 4 + 4 + 8 + 26)
 
         for bad_prevhash in (b"", b"x", b"hello", b"x" * 33):
             inp.prev_hash = bad_prevhash

docs/common/communication/bitcoin-signing.md

@@ -150,16 +150,28 @@ Full documentation for multisig is TBD.
 
 #### External inputs
 
-Trezor T can include inputs that it will not sign, typically because they are owned by
-another party. Such inputs are of type `EXTERNAL` and the host does not specify a
-derivation path for the key. Instead, these inputs must either already have a valid
-signature or they must come with an ownership proof. If the input already has a valid
-signature, then the host provides the `script_sig` and/or `witness` fields. If the other
-signing party hasn't signed their input yet (i.e., with two Trezors, one must sign first
-so that the other can include a pre-signed input), they can instead provide a
+Trezor can include inputs that it will not sign, typically because they are
+owned by another party. Such inputs are of type `EXTERNAL` and the host does
+not specify a derivation path for the key, but sets the `script_pubkey` field
+to the scriptPubKey of the previous output that is being spent by the external
+input. An external input can either be *verified* or *unverified*. The amounts
+supplied to the transaction by verified external inputs are subtracted from the
+transaction total that the user is asked to confirm, whereas unverified
+external inputs are not subtracted.
+
+Verified external inputs are only supported on the Trezor T. They must either
+already have a valid signature or they must come with an ownership proof. If
+the input already has a valid signature, then the host provides the
+`script_sig` and/or `witness` fields. If the other signing party hasn't signed
+their input yet (i.e., with two Trezors, one must sign first so that the other
+can include a pre-signed input), they can instead provide a
 [SLIP-19](https://github.com/satoshilabs/slips/blob/master/slip-0019.md)
-ownership proof in the `ownership_proof` field, with optional commitment data in
-`commitment_data`. The `script_pubkey` field is required for all external inputs.
+ownership proof in the `ownership_proof` field, with optional commitment data
+in `commitment_data`.
+
+Unverified external inputs are accepted by Trezor only if
+[safety checks](https://wiki.trezor.io/Using_trezorctl_commands_with_Trezor#Safety-checks)
+are disabled on the device.
 
 ### Transaction output