Allow spending coins from Bitcoin paths if the coin has implemented strong replay protection

core/CHANGELOG.md

@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Print inverted question mark for non-printable characters.
 - Remove pre-fill bar from text rendering functions.  [#1173]
 - Display coin name when signing or verifying messages.  [#1159]
+- Allow spending coins from Bitcoin paths if the coin has implemented strong replay protection via `SIGHASH_FORKID`.  [#1188]
 
 ### Deprecated
 
@@ -269,3 +270,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 [#1159]: https://github.com/trezor/trezor-firmware/issues/1159
 [#1165]: https://github.com/trezor/trezor-firmware/pull/1165
 [#1173]: https://github.com/trezor/trezor-firmware/pull/1173
+[#1188]: https://github.com/trezor/trezor-firmware/issues/1188

core/src/apps/bitcoin/keychain.py

@@ -56,6 +56,16 @@ def get_namespaces_for_coin(coin: coininfo.CoinInfo):
         # m/0x4741b11e/6/pointer
         namespaces.append([0x4741B11E])
 
+    # some wallets such as Electron-Cash (BCH) store coins on Bitcoin paths
+    # we can allow spending these coins from Bitcoin paths if the coin has
+    # implemented strong replay protection via SIGHASH_FORKID
+    if coin.fork_id is not None:
+        namespaces.append([44 | HARDENED, 0 | HARDENED])
+        namespaces.append([48 | HARDENED, 0 | HARDENED])
+        if coin.segwit:
+            namespaces.append([49 | HARDENED, 0 | HARDENED])
+            namespaces.append([84 | HARDENED, 0 | HARDENED])
+
     return namespaces
 
 

core/tests/test_apps.wallet.keychain.py

@@ -87,14 +87,46 @@ def test_bcash(self):
         self.assertFalse(coin.segwit)
         valid_addresses = (
             [44 | HARDENED, 145 | HARDENED],
+            [44 | HARDENED, 0 | HARDENED],
             [45 | HARDENED, 123456],
             [48 | HARDENED, 145 | HARDENED],
+            [48 | HARDENED, 0 | HARDENED],
         )
         invalid_addresses = (
             [43 | HARDENED, 145 | HARDENED],
-            [44 | HARDENED, 0 | HARDENED],
+            [43 | HARDENED, 0 | HARDENED],
             [49 | HARDENED, 145 | HARDENED],
+            [49 | HARDENED, 0 | HARDENED],
             [84 | HARDENED, 145 | HARDENED],
+            [84 | HARDENED, 0 | HARDENED],
+        )
+
+        for addr in valid_addresses:
+            keychain.derive(addr)
+
+        for addr in invalid_addresses:
+            self.assertRaises(wire.DataError, keychain.derive, addr)
+
+    def test_litecoin(self):
+        keychain, coin = await_result(
+            get_keychain_for_coin(wire.DUMMY_CONTEXT, "Litecoin")
+        )
+        self.assertEqual(coin.coin_name, "Litecoin")
+
+        self.assertTrue(coin.segwit)
+        valid_addresses = (
+            [44 | HARDENED, 2 | HARDENED],
+            [45 | HARDENED, 123456],
+            [48 | HARDENED, 2 | HARDENED],
+            [49 | HARDENED, 2 | HARDENED],
+            [84 | HARDENED, 2 | HARDENED],
+        )
+        invalid_addresses = (
+            [43 | HARDENED, 2 | HARDENED],
+            [44 | HARDENED, 0 | HARDENED],
+            [48 | HARDENED, 0 | HARDENED],
+            [49 | HARDENED, 0 | HARDENED],
+            [84 | HARDENED, 0 | HARDENED],
         )
 
         for addr in valid_addresses:

legacy/firmware/CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Changed
 - Print inverted question mark for non-printable characters.
+- Allow spending coins from Bitcoin paths if the coin has implemented strong replay protection via `SIGHASH_FORKID`.  [#1188]
 
 ### Deprecated
 
@@ -331,3 +332,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 [#1030]: https://github.com/trezor/trezor-firmware/issues/1030
 [#1098]: https://github.com/trezor/trezor-firmware/issues/1098
 [#1165]: https://github.com/trezor/trezor-firmware/pull/1165
+[#1188]: https://github.com/trezor/trezor-firmware/issues/1188

legacy/firmware/crypto.c

@@ -508,6 +508,22 @@ int cryptoIdentityFingerprint(const IdentityType *identity, uint8_t *hash) {
   return 1;
 }
 
+static bool check_cointype(const CoinInfo *coin, uint32_t slip44, bool full) {
+#if BITCOIN_ONLY
+  (void)full;
+#else
+  if (!full) {
+    // some wallets such as Electron-Cash (BCH) store coins on Bitcoin paths
+    // we can allow spending these coins from Bitcoin paths if the coin has
+    // implemented strong replay protection via SIGHASH_FORKID
+    if (slip44 == 0x80000000 && coin->has_fork_id) {
+      return true;
+    }
+  }
+#endif
+  return coin->coin_type == slip44;
+}
+
 bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type,
                            uint32_t address_n_count, const uint32_t *address_n,
                            bool full) {
@@ -521,7 +537,7 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type,
     } else {
       valid &= (address_n_count >= 2);
     }
-    valid &= (address_n[1] == coin->coin_type);
+    valid &= check_cointype(coin, address_n[1], full);
     if (full) {
       valid &= (address_n[2] & 0x80000000) == 0x80000000;
       valid &= (address_n[3] & 0x80000000) == 0;
@@ -556,7 +572,7 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type,
     } else {
       valid &= (address_n_count >= 2);
     }
-    valid &= (address_n[1] == coin->coin_type);
+    valid &= check_cointype(coin, address_n[1], full);
     if (full) {
       valid &= (address_n[2] & 0x80000000) == 0x80000000;
       valid &= (address_n[4] & 0x80000000) == 0;
@@ -574,7 +590,7 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type,
     } else {
       valid &= (address_n_count >= 2);
     }
-    valid &= (address_n[1] == coin->coin_type);
+    valid &= check_cointype(coin, address_n[1], full);
     if (full) {
       valid &= (address_n[2] & 0x80000000) == 0x80000000;
       valid &= (address_n[3] & 0x80000000) == 0;
@@ -594,7 +610,7 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type,
     } else {
       valid &= (address_n_count >= 2);
     }
-    valid &= (address_n[1] == coin->coin_type);
+    valid &= check_cointype(coin, address_n[1], full);
     if (full) {
       valid &= (address_n[2] & 0x80000000) == 0x80000000;
       valid &= (address_n[3] & 0x80000000) == 0;

tests/device_tests/test_msg_signtx_invalid_path.py

@@ -0,0 +1,94 @@
+# This file is part of the Trezor project.
+#
+# Copyright (C) 2012-2019 SatoshiLabs and contributors
+#
+# This library is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License version 3
+# as published by the Free Software Foundation.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the License along with this library.
+# If not, see <https://www.gnu.org/licenses/lgpl-3.0.html>.
+
+import pytest
+
+from trezorlib import btc, messages as proto
+from trezorlib.exceptions import TrezorFailure
+from trezorlib.tools import parse_path
+
+from ..tx_cache import TxCache
+
+TX_CACHE_MAINNET = TxCache("Bitcoin")
+TX_CACHE_BCASH = TxCache("Bcash")
+
+TXHASH_8cc1f4 = bytes.fromhex(
+    "8cc1f4adf7224ce855cf535a5104594a0004cb3b640d6714fdb00b9128832dd5"
+)
+TXHASH_d5f65e = bytes.fromhex(
+    "d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882"
+)
+
+
+class TestMsgSigntxInvalidPath:
+
+    # Adapted from TestMsgSigntx.test_one_one_fee,
+    # only changed the coin from Bitcoin to Litecoin.
+    # Litecoin does not have strong replay protection using SIGHASH_FORKID,
+    # spending from Bitcoin path should fail.
+    @pytest.mark.altcoin
+    def test_invalid_path_fail(self, client):
+        # tx: d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882
+        # input 0: 0.0039 BTC
+
+        inp1 = proto.TxInputType(
+            address_n=parse_path("44h/0h/0h/0/0"),
+            # amount=390000,
+            prev_hash=TXHASH_d5f65e,
+            prev_index=0,
+        )
+
+        # address is converted from 1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1 by changing the version
+        out1 = proto.TxOutputType(
+            address="LfWz9wLHmqU9HoDkMg9NqbRosrHvEixeVZ",
+            amount=390000 - 10000,
+            script_type=proto.OutputScriptType.PAYTOADDRESS,
+        )
+
+        with pytest.raises(TrezorFailure) as exc:
+            btc.sign_tx(client, "Litecoin", [inp1], [out1], prev_txes=TX_CACHE_MAINNET)
+
+        if client.features.model == "1":
+            assert exc.value.code == proto.FailureType.ProcessError
+            assert exc.value.message.endswith("Failed to compile input")
+        else:
+            assert exc.value.code == proto.FailureType.DataError
+            assert exc.value.message.endswith("Forbidden key path")
+
+    # Adapted from TestMsgSigntx.test_one_one_fee,
+    # only changed the coin from Bitcoin to Bcash.
+    # Bcash does have strong replay protection using SIGHASH_FORKID,
+    # spending from Bitcoin path should work.
+    @pytest.mark.altcoin
+    def test_invalid_path_pass_forkid(self, client):
+        # tx: 8cc1f4adf7224ce855cf535a5104594a0004cb3b640d6714fdb00b9128832dd5
+        # input 0: 0.0039 BTC
+
+        inp1 = proto.TxInputType(
+            address_n=parse_path("44h/0h/0h/0/0"),
+            amount=390000,
+            prev_hash=TXHASH_8cc1f4,
+            prev_index=0,
+        )
+
+        # address is converted from 1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1 to cashaddr format
+        out1 = proto.TxOutputType(
+            address="bitcoincash:qr0fk25d5zygyn50u5w7h6jkvctas52n0qxff9ja6r",
+            amount=390000 - 10000,
+            script_type=proto.OutputScriptType.PAYTOADDRESS,
+        )
+
+        btc.sign_tx(client, "Bcash", [inp1], [out1], prev_txes=TX_CACHE_BCASH)

tests/txcache/bcash/8cc1f4adf7224ce855cf535a5104594a0004cb3b640d6714fdb00b9128832dd5.json

@@ -0,0 +1,18 @@
+{
+  "bin_outputs": [
+    {
+      "amount": 390000,
+      "script_pubkey": "a9147f844bdb0b8fd54b64e3d16c85dc1170f1ff97c187"
+    }
+  ],
+  "inputs": [
+    {
+      "prev_hash": "54aa5680dea781f45ebb536e53dffc526d68c0eb5c00547e323b2c32382dfba3",
+      "prev_index": 1,
+      "script_sig": "4830450221009e020b0390ccad533b73b552f8a99a9d827212c558e4f755503674d07c92ad4502202d606f7316990e0461c51d4add25054f19c697aa3e3c2ced4d568f0b2c57e62f0121023230848585885f63803a0a8aecdd6538792d5c539215c91698e315bf0253b43d",
+      "sequence": 4294967295
+    }
+  ],
+  "lock_time": 0,
+  "version": 1
+}

tests/ui_tests/fixtures.json

@@ -335,6 +335,8 @@
 "test_msg_signtx_grs.py-test_send_segwit_native_change": "2b9119aea79b50a26bd86cc49fd5421aedee42245b83919bf142d2ca2e50dd16",
 "test_msg_signtx_grs.py-test_send_segwit_p2sh": "e596ff7f2e10633d8de6bd5b98928020dfacf578ce71d6d6e0ca27011ed8bd53",
 "test_msg_signtx_grs.py-test_send_segwit_p2sh_change": "65a33cc9ecf52bdab93d1066bfaff7f30c9908fc5ada8a672c39df57eb6129c2",
+"test_msg_signtx_invalid_path.py-test_invalid_path_fail": "b0f22cba2dbab2cd21c15c002b66ed89b6c728b10daa8d0c0e78abd4164a3912",
+"test_msg_signtx_invalid_path.py-test_invalid_path_pass_forkid": "667dcb09b569e5b4e091e6b1ac7e8e057c0c730c931b22f8c0ee64050f3f467b",
 "test_msg_signtx_komodo.py-test_one_one_fee_sapling": "aa32424c04f65c0e2de57f6eb26830eb037c7b4daaf300ae61d30968affd9193",
 "test_msg_signtx_komodo.py-test_one_one_rewards_claim": "e53f221fda81469027e39e21877a81a8fafbffbece0a45aeda12aae8873b0464",
 "test_msg_signtx_peercoin.py::test_timestamp_included": "825b9bdf5238c5c6415a254a6bae4b2bd9df8fc5cb31f66f0c20145cb4e60bbb",