core/bitcoin: Change CoinJoin round ID length to 32 bytes and remove rate limiting.

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 (
+        return (
-            msg.address_n[:-BIP32_WALLET_DEPTH] != self.address_n
+            len(msg.address_n) >= BIP32_WALLET_DEPTH
-            or msg.coin_name != self.coin.coin_name
+            and msg.address_n[:-BIP32_WALLET_DEPTH] == self.address_n
-            or msg.script_type != self.script_type
+            and msg.coin_name == self.coin.coin_name
-            or len(msg.commitment_data) < _ROUND_ID_LEN
+            and msg.script_type == self.script_type
-            or msg.commitment_data[:-_ROUND_ID_LEN] != self.coordinator.encode()
+            and len(msg.commitment_data) >= _ROUND_ID_LEN
-        ):
+            and 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
 
     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
         )

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"),
+            commitment_data=b"www.example.org" + (1).to_bytes(ROUND_ID_LEN, "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"),
             preauthorized=True,
         )