diff --git a/core/src/apps/bitcoin/authorization.py b/core/src/apps/bitcoin/authorization.py index c82ec48fed..b5b0d19b28 100644 --- a/core/src/apps/bitcoin/authorization.py +++ b/core/src/apps/bitcoin/authorization.py @@ -1,4 +1,3 @@ -import utime from micropython import const from trezor.messages import MessageType @@ -14,11 +13,8 @@ if False: from apps.common import coininfo from apps.common.seed import Keychain -_ROUND_ID_LEN = const(8) +_ROUND_ID_LEN = const(32) FEE_PER_ANONYMITY_DECIMALS = const(9) -_ROUND_ID_LIFETIME_MS = const( - 10 * 60 * 1000 -) # 10 minutes before a proof of ownership can be generated for another round ID class CoinJoinAuthorization: @@ -26,8 +22,6 @@ class CoinJoinAuthorization: self, msg: AuthorizeCoinJoin, keychain: Keychain, coin: coininfo.CoinInfo ): self.coordinator = msg.coordinator - self.round_id = bytes() - self.round_id_expiry = 0 self.remaining_fee = msg.max_total_fee self.fee_per_anonymity = msg.fee_per_anonymity or 0 self.address_n = msg.address_n @@ -43,31 +37,20 @@ class CoinJoinAuthorization: def check_get_ownership_proof(self, msg: GetOwnershipProof) -> bool: # Check whether the current authorization matches the parameters of the request. - if ( - msg.address_n[:-BIP32_WALLET_DEPTH] != self.address_n - or msg.coin_name != self.coin.coin_name - or msg.script_type != self.script_type - or len(msg.commitment_data) < _ROUND_ID_LEN - or msg.commitment_data[:-_ROUND_ID_LEN] != self.coordinator.encode() - ): - return False - - # Allow changing to a different round ID only after _ROUND_ID_LIFETIME_MS. - round_id = msg.commitment_data[-_ROUND_ID_LEN:] - if round_id == self.round_id: - return True - - if self.round_id_expiry <= utime.ticks_ms(): - self.round_id = round_id - self.round_id_expiry = utime.ticks_ms() + _ROUND_ID_LIFETIME_MS - return True - - return False + return ( + len(msg.address_n) >= BIP32_WALLET_DEPTH + and msg.address_n[:-BIP32_WALLET_DEPTH] == self.address_n + and msg.coin_name == self.coin.coin_name + and msg.script_type == self.script_type + and len(msg.commitment_data) >= _ROUND_ID_LEN + and msg.commitment_data[:-_ROUND_ID_LEN] == self.coordinator.encode() + ) def check_sign_tx_input(self, txi: TxInputType, coin: coininfo.CoinInfo) -> bool: # Check whether the current input matches the parameters of the request. return ( - txi.address_n[:-BIP32_WALLET_DEPTH] == self.address_n + len(txi.address_n) >= BIP32_WALLET_DEPTH + and txi.address_n[:-BIP32_WALLET_DEPTH] == self.address_n and coin.coin_name == self.coin.coin_name and txi.script_type == self.script_type ) diff --git a/tests/device_tests/test_msg_authorize_coinjoin.py b/tests/device_tests/test_msg_authorize_coinjoin.py index 916373b456..cbe189f614 100644 --- a/tests/device_tests/test_msg_authorize_coinjoin.py +++ b/tests/device_tests/test_msg_authorize_coinjoin.py @@ -36,6 +36,7 @@ TXHASH_65b811 = bytes.fromhex( ) PIN = "1234" +ROUND_ID_LEN = 32 pytestmark = pytest.mark.skip_t1 @@ -66,7 +67,7 @@ def test_sign_tx(client): parse_path("84'/1'/0'/1/0"), script_type=messages.InputScriptType.SPENDWITNESS, user_confirmation=True, - commitment_data=b"www.example.com" + (1).to_bytes(8, "big"), + commitment_data=b"www.example.com" + (1).to_bytes(ROUND_ID_LEN, "big"), preauthorized=True, ) @@ -80,7 +81,7 @@ def test_sign_tx(client): parse_path("84'/1'/0'/1/5"), script_type=messages.InputScriptType.SPENDWITNESS, user_confirmation=True, - commitment_data=b"www.example.com" + (1).to_bytes(8, "big"), + commitment_data=b"www.example.com" + (1).to_bytes(ROUND_ID_LEN, "big"), preauthorized=True, ) @@ -297,46 +298,6 @@ def test_wrong_coordinator(client): parse_path("84'/1'/0'/1/0"), script_type=messages.InputScriptType.SPENDWITNESS, user_confirmation=True, - commitment_data=b"www.example.org" + (1).to_bytes(8, "big"), - preauthorized=True, - ) - - -def test_change_round_id(client): - # Ensure that if the round ID changes, then GetOwnershipProof fails. - - btc.authorize_coinjoin( - client, - amount=100000000, - max_fee=50000, - coordinator="www.example.com", - n=parse_path("m/84'/1'/0'"), - coin_name="Testnet", - script_type=messages.InputScriptType.SPENDWITNESS, - ) - - with client: - client.set_expected_responses( - [messages.PreauthorizedRequest(), messages.OwnershipProof()] - ) - btc.get_ownership_proof( - client, - "Testnet", - parse_path("84'/1'/0'/1/0"), - script_type=messages.InputScriptType.SPENDWITNESS, - user_confirmation=True, - commitment_data=b"www.example.com" + (1).to_bytes(8, "big"), - preauthorized=True, - ) - - # GetOwnershipProof with changed round ID. - with pytest.raises(TrezorFailure, match="Unauthorized operation"): - ownership_proof, _ = btc.get_ownership_proof( - client, - "Testnet", - parse_path("84'/1'/0'/1/0"), - script_type=messages.InputScriptType.SPENDWITNESS, - user_confirmation=True, - commitment_data=b"www.example.com" + (2).to_bytes(8, "big"), + commitment_data=b"www.example.org" + (1).to_bytes(ROUND_ID_LEN, "big"), preauthorized=True, )