arno/trezor-firmware
1
0
Fork 0
mirror of https://github.com/trezor/trezor-firmware.git synced 2024-12-24 15:28:10 +00:00
Code Issues Releases Wiki Activity

feat(core): Verify signatures of all original inputs in replacement transactions.

Browse Source
This commit is contained in:
Andrew Kozlik 2022-03-18 20:47:52 +01:00 committed by Martin Milata
parent 9bce153623
commit d16b44bad6
7 changed files with 59 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
core/src/apps/bitcoin/sign_tx/bitcoin.py
View File

@ -122,9 +122,9 @@ class Bitcoin:
# stable device tests.
self.orig_txs: list[OriginalTxInfo] = []
# The digests of the inputs streamed for approval in Step 1. These are used to ensure that
# the inputs streamed for verification in Step 3 are the same as those in Step 1.
self.h_inputs: bytes | None = None
# The digests of the external inputs streamed for approval in Step 1. These are used
# to ensure that the inputs streamed for verification in Step 3 are the same as
# those in Step 1.
self.h_external_inputs: bytes | None = None
# The index of the payment request being processed.
@ -165,11 +165,12 @@ class Bitcoin:
if txi.orig_hash:
await self.process_original_input(txi, script_pubkey)
self.h_inputs = self.tx_info.get_tx_check_digest()
self.tx_info.h_inputs_check = self.tx_info.get_tx_check_digest()
self.h_external_inputs = h_external_inputs_check.get_digest()
# Finalize original inputs.
for orig in self.orig_txs:
orig.h_inputs_check = orig.get_tx_check_digest()
if orig.index != orig.tx.inputs_count:
raise wire.ProcessError("Removal of original inputs is not supported.")
@ -194,7 +195,7 @@ class Bitcoin:
await orig.finalize_tx_hash()
async def step3_verify_inputs(self) -> None:
# should come out the same as h_inputs, checked before continuing
# should come out the same as h_inputs_check, checked before continuing
h_check = HashWriter(sha256())
if self.taproot_only:
@ -220,7 +221,7 @@ class Bitcoin:
# inputs or to falsely claim that a transaction is a replacement of an already approved
# transaction or to construct a valid transaction by combining signatures obtained in
# multiple rounds of the attack.
expected_digest = self.h_inputs
expected_digest = self.tx_info.h_inputs_check
for i in range(self.tx_info.tx.inputs_count):
progress.advance()
txi = await helpers.request_tx_input(self.tx_req, i, self.coin)
@ -406,29 +407,34 @@ class Bitcoin:
async def verify_original_txs(self) -> None:
for orig in self.orig_txs:
if orig.verification_input is None:
raise wire.ProcessError(
"Each original transaction must specify address_n for at least one input."
)
# should come out the same as h_inputs_check, checked before continuing
h_check = HashWriter(sha256())
assert orig.verification_index is not None
txi = orig.verification_input
script_pubkey = self.input_derive_script(txi)
verifier = SignatureVerifier(
script_pubkey, txi.script_sig, txi.witness, self.coin
)
verifier.ensure_hash_type(
(SigHashType.SIGHASH_ALL_TAPROOT, self.get_sighash_type(txi))
)
tx_digest = await self.get_tx_digest(
orig.verification_index,
txi,
orig,
verifier.public_keys,
verifier.threshold,
script_pubkey,
)
verifier.verify(tx_digest)
for i in range(orig.tx.inputs_count):
txi = await helpers.request_tx_input(
self.tx_req, i, self.coin, orig.orig_hash
)
writers.write_tx_input_check(h_check, txi)
script_pubkey = self.input_derive_script(txi)
verifier = SignatureVerifier(
script_pubkey, txi.script_sig, txi.witness, self.coin
)
verifier.ensure_hash_type(
(SigHashType.SIGHASH_ALL_TAPROOT, self.get_sighash_type(txi))
)
tx_digest = await self.get_tx_digest(
i,
txi,
orig,
verifier.public_keys,
verifier.threshold,
script_pubkey,
)
verifier.verify(tx_digest)
# check that the inputs were the same as those streamed for approval
if h_check.get_digest() != orig.h_inputs_check:
raise wire.ProcessError("Transaction has changed during signing")
async def approve_output(
self,

17
core/src/apps/bitcoin/sign_tx/tx_info.py
View File

@ -72,6 +72,11 @@ class TxInfoBase:
# in Steps 1 and 2 and the ones streamed for signing legacy inputs in Step 4.
self.h_tx_check = HashWriter(sha256()) # not a real tx hash
# The digests of the inputs streamed for approval in Step 1. These are used to
# ensure that the inputs streamed for verification in Step 3 are the same as
# those in Step 1.
self.h_inputs_check: bytes | None = None
# BIP-0143 transaction hashing.
self.sig_hasher = signer.create_sig_hasher(tx)
@ -145,22 +150,10 @@ class OriginalTxInfo(TxInfoBase):
signer.write_tx_header(self.h_tx, tx, witness_marker=False)
writers.write_compact_size(self.h_tx, tx.inputs_count)
# The input which will be used for verification and its index in the original transaction.
self.verification_input: TxInput | None = None
self.verification_index: int | None = None
def add_input(self, txi: TxInput, script_pubkey: bytes) -> None:
super().add_input(txi, script_pubkey)
writers.write_tx_input(self.h_tx, txi, txi.script_sig or bytes())
# For verification use the first original non-multisig input that specifies address_n.
# NOTE: Supporting replacement transactions where all internal inputs are multisig would
# require checking the signatures of all of the original internal inputs or not allowing
# unverified external inputs in transactions where multisig inputs are present.
if not self.verification_input and txi.address_n and not txi.multisig:
self.verification_input = txi
self.verification_index = self.index
def add_output(self, txo: TxOutput, script_pubkey: bytes) -> None:
super().add_output(txo, script_pubkey)

6
docs/common/communication/bitcoin-signing.md
View File

@ -286,10 +286,10 @@ set to `tx.extra_data_chunk`.
Trezor sets `request_type` to `TXORIGINPUT`. `request_details.tx_hash` is the
transaction hash of the original transaction.
Host must respond with a `TxAckInput` message. All relevant data must be set in
The host must respond with a `TxAckInput` message. All relevant data must be set in
`tx.input`. The derivation path and `script_type` are mandatory for all original
internal inputs. For each original transaction, one of its original internal inputs must
be accompanied with a valid signature in the `script_sig` and/or `witness` fields.
internal inputs. All original internal inputs must also be accompanied with full
transaction signature data in the `script_sig` and/or `witness` fields.
### Original transaction output

13
tests/device_tests/bitcoin/test_signtx_replacement.py
View File

@ -133,6 +133,7 @@ def test_p2pkh_fee_bump(client: Client):
request_meta(TXHASH_beafc7),
request_input(0, TXHASH_beafc7),
request_output(0, TXHASH_beafc7),
request_orig_input(0, TXHASH_50f6f1),
(tt, request_orig_input(0, TXHASH_50f6f1)),
(tt, request_orig_output(0, TXHASH_50f6f1)),
(tt, request_orig_output(1, TXHASH_50f6f1)),
@ -257,6 +258,8 @@ def test_p2tr_fee_bump(client: Client):
request_output(1),
request_orig_output(1, TXHASH_8e4af7),
messages.ButtonRequest(code=B.SignTx),
request_orig_input(0, TXHASH_8e4af7),
request_orig_input(1, TXHASH_8e4af7),
request_input(0),
request_input(1),
request_output(0),
@ -328,6 +331,7 @@ def test_p2wpkh_finalize(client: Client):
request_output(0, TXHASH_43d273),
request_output(1, TXHASH_43d273),
request_output(2, TXHASH_43d273),
request_orig_input(0, TXHASH_70f987),
request_input(0),
request_output(0),
request_output(1),
@ -464,6 +468,7 @@ def test_p2wpkh_payjoin(
request_input(0, TXHASH_70f987),
request_output(0, TXHASH_70f987),
request_output(1, TXHASH_70f987),
request_orig_input(0, TXHASH_65b768),
request_input(0),
request_input(1),
request_output(0),
@ -538,6 +543,8 @@ def test_p2wpkh_in_p2sh_remove_change(client: Client):
request_meta(TXHASH_efaa41),
request_input(0, TXHASH_efaa41),
request_output(0, TXHASH_efaa41),
request_orig_input(0, TXHASH_334cd7),
request_orig_input(1, TXHASH_334cd7),
request_input(0),
request_input(1),
request_output(0),
@ -618,6 +625,8 @@ def test_p2wpkh_in_p2sh_fee_bump_from_external(client: Client):
request_meta(TXHASH_efaa41),
request_input(0, TXHASH_efaa41),
request_output(0, TXHASH_efaa41),
request_orig_input(0, TXHASH_334cd7),
request_orig_input(1, TXHASH_334cd7),
request_input(0),
request_input(1),
request_output(0),
@ -759,6 +768,10 @@ def test_tx_meld(client: Client):
request_input(1, TXHASH_927784),
request_input(2, TXHASH_927784),
request_output(0, TXHASH_927784),
request_orig_input(0, TXHASH_334cd7),
request_orig_input(1, TXHASH_334cd7),
request_orig_input(0, TXHASH_ed89ac),
request_orig_input(1, TXHASH_ed89ac),
request_input(0),
request_input(1),
request_input(2),

1
tests/txcache/testnet/334cd7ad982b3b15d07dd1c84e939e95efb0803071648048a7f289492e7b4c8a.json
View File

@ -17,6 +17,7 @@
"amount": 998060,
"script_type": "SPENDP2SHWITNESS",
"script_sig": "160014209297fb46272a0b7e05139440dbd39daea3e25a",
"witness":"0247304402206f5b2032601278685921b397416cecd5f70cde48b7203bf2a953283c0cd05f4002205fd21a2c318330f11e8433f8fae6b1c4c257a79da55be9712961214ba2752a1f012103c2c2e65556ca4b7371549324b99390725493c8a6792e093a0bdcbb3e2d7df4ab",
"sequence": 4294967295
}
],

3
tests/txcache/testnet/8e4af74cee65dac2615f7befea76bab4955707cd216011d918b526f5e86d85ba.json
View File

@ -17,7 +17,8 @@
"prev_index": 0,
"script_sig": "",
"script_type": "SPENDTAPROOT",
"sequence": 4294967293
"sequence": 4294967293,
"witness": "0140d961a3fc97561c8a305d0ba78488e8d1efed520ac36b226db76277c38e5ab112236a2f821b6dde82dc4013f74148d1ec6e1e471b952755b5265b7ed594afb723"
}
],
"lock_time": 0,

1
tests/txcache/testnet/ed89acb52cfa438e3653007478e7c7feae89fdde12867943eec91293139730d1.json
View File

@ -27,6 +27,7 @@
"amount": 839318869,
"script_type": "SPENDP2SHWITNESS",
"script_sig": "160014681ea49259abb892460bf3373e8a0b43d877fa18",
"witness": "02473044022039bb1710e041e9a936bd3adf39f61906ac4363a67d1c29b1a264ff8cf5a0c95102202829307d82141792125480abe6491f0f0c7d675c3d1a06daa383d617afcb53380121028cbc37e1816a23086fa738c8415def477e813e20f484dbbd6f5a33a37c322251",
"sequence": 4294967295
}
],