From 31f987e9888dd532bf82a7d963e1ef13298f1568 Mon Sep 17 00:00:00 2001 From: Tomas Susanka Date: Thu, 30 Aug 2018 13:45:09 +0200 Subject: [PATCH 1/4] coins: validate derivation paths Based on SLIP-44 ids and other checks. See docs/coins/README for info. --- docs/coins/README.md | 44 ++- docs/coins/bitcoin-path-check.svg | 313 ------------------ src/apps/cardano/address.py | 32 +- src/apps/cardano/get_address.py | 6 +- src/apps/cardano/get_public_key.py | 6 +- src/apps/cardano/sign_message.py | 53 +++ src/apps/cardano/sign_tx.py | 6 +- src/apps/cardano/verify_message.py | 41 +++ src/apps/common/paths.py | 72 ++++ src/apps/ethereum/address.py | 55 +++ src/apps/ethereum/get_address.py | 32 +- src/apps/ethereum/layout.py | 4 +- src/apps/ethereum/sign_message.py | 5 +- src/apps/ethereum/sign_tx.py | 5 +- src/apps/ethereum/verify_message.py | 5 + src/apps/lisk/get_address.py | 6 +- src/apps/lisk/get_public_key.py | 6 +- src/apps/lisk/helpers.py | 18 + src/apps/lisk/sign_message.py | 14 +- src/apps/lisk/sign_tx.py | 6 +- src/apps/monero/get_address.py | 3 + src/apps/monero/get_watch_only.py | 3 + src/apps/monero/key_image_sync.py | 3 + src/apps/monero/misc.py | 19 ++ .../signing/step_01_init_transaction.py | 3 + src/apps/nem/get_address.py | 5 +- src/apps/nem/helpers.py | 27 ++ src/apps/nem/sign_tx.py | 7 +- src/apps/ripple/get_address.py | 4 +- src/apps/ripple/helpers.py | 24 ++ src/apps/ripple/sign_tx.py | 4 +- src/apps/stellar/get_address.py | 4 +- src/apps/stellar/helpers.py | 18 + src/apps/stellar/sign_tx.py | 4 +- src/apps/tezos/get_address.py | 15 +- src/apps/tezos/get_public_key.py | 9 +- src/apps/tezos/helpers.py | 18 + src/apps/tezos/sign_tx.py | 25 +- src/apps/wallet/get_address.py | 9 + src/apps/wallet/get_public_key.py | 7 +- src/apps/wallet/sign_message.py | 11 +- src/apps/wallet/sign_tx/__init__.py | 4 +- src/apps/wallet/sign_tx/addresses.py | 89 ++++- src/apps/wallet/sign_tx/helpers.py | 7 +- src/apps/wallet/sign_tx/signing.py | 14 +- tests/test_apps.cardano.address.py | 39 ++- tests/test_apps.common.paths.py | 46 +++ ...dress.py => test_apps.ethereum.address.py} | 33 +- tests/test_apps.lisk.address.py | 36 ++ tests/test_apps.monero.address.py | 36 ++ tests/test_apps.nem.address.py | 44 ++- tests/test_apps.ripple.address.py | 47 +++ tests/test_apps.ripple.pubkey_to_address.py | 19 -- ...pubkey.py => test_apps.stellar.address.py} | 31 +- tests/test_apps.tezos.address.py | 28 ++ tests/test_apps.wallet.address.py | 129 ++++++++ ...apps.wallet.segwit.signtx.native_p2wpkh.py | 9 +- .../test_apps.wallet.signtx.fee_threshold.py | 7 + tests/test_apps.wallet.signtx.py | 4 + 59 files changed, 1090 insertions(+), 483 deletions(-) delete mode 100644 docs/coins/bitcoin-path-check.svg create mode 100644 src/apps/cardano/sign_message.py create mode 100644 src/apps/cardano/verify_message.py create mode 100644 src/apps/common/paths.py create mode 100644 src/apps/ethereum/address.py create mode 100644 tests/test_apps.common.paths.py rename tests/{test_apps.ethereum.get_address.py => test_apps.ethereum.address.py} (62%) create mode 100644 tests/test_apps.lisk.address.py create mode 100644 tests/test_apps.monero.address.py create mode 100644 tests/test_apps.ripple.address.py delete mode 100644 tests/test_apps.ripple.pubkey_to_address.py rename tests/{test_apps.stellar.address_to_pubkey.py => test_apps.stellar.address.py} (60%) diff --git a/docs/coins/README.md b/docs/coins/README.md index c59dc9199f..9ad5f1d5d4 100644 --- a/docs/coins/README.md +++ b/docs/coins/README.md @@ -4,31 +4,47 @@ Each coin uses [BIP-44](https://github.com/bitcoin/bips/blob/master/bip-0044.med ## List of used derivation paths -| coin | curve | getPublicKey | getAddress | sign | derivation | note | +| coin | curve | getPublicKey | getAddress | sign tx | derivation | note | |----------------|----------------|----------------|------------------|------------------|-----------------|--------------| -| Bitcoin | secp256k | 44'/0'/a' | 44'/0'/a'/y/i | 44'/0'/a'/y/i | BIP-32 | [6](#BitcoinDiagram) | -| Ethereum | secp256k | 44'/60'/0' | 44'/60'/0'/0/i | 44'/60'/0'/0/i | BIP-32 | [1](#Ethereum)| -| Ripple | secp256k | - | 44'/144'/a'/0/0 | 44'/144'/a'/0/0 | BIP-32 | [2](#Ripple) | -| Cardano | ed25519 | 44'/1815'/a' | 44'/1815'/a'/0/i | 44'/1815'/a'/0/i | [Cardano's own](https://cardanolaunch.com/assets/Ed25519_BIP.pdf)[3](#Cardano) | | +| Bitcoin | secp256k | 44'/c'/a' | 44'/c'/a'/y/i | 44'/c'/a'/y/i | BIP-32 | [1](#Bitcoin) | +| Ethereum | secp256k | 44'/60'/0' | 44'/60'/0'/0/i | 44'/60'/0'/0/i | BIP-32 | [2](#Ethereum)| +| Ripple | secp256k | - | 44'/144'/a'/0/0 | 44'/144'/a'/0/0 | BIP-32 | [3](#Ripple) | +| Cardano | ed25519 | 44'/1815'/a' | 44'/1815'/a'/{0,1}/i | 44'/1815'/a'/{0,1}/i | [Cardano's own](https://cardanolaunch.com/assets/Ed25519_BIP.pdf)[4](#Cardano) | | | Stellar | ed25519 | - | 44'/148'/a' | 44'/148'/a' | SLIP-0010 | | | Lisk | ed25519 | 44'/134'/a' | 44'/134'/a' | 44'/134'/a' | SLIP-0010 | | -| NEM | ed25519 | - | 44'/43'/a' | 44'/43'/a' | SLIP-0010 | [4](#NEM) | -| Monero | ed25519 | 44'/128'/a'[5](#Monero) | 44'/128'/a' | 44'/128'/a' | SLIP-0010 | | +| NEM | ed25519 | - | 44'/43'/a' | 44'/43'/a' | SLIP-0010 | [5](#NEM) | +| Monero | ed25519 | 44'/128'/a'[6](#Monero) | 44'/128'/a' | 44'/128'/a' | SLIP-0010 | | +| Tezos | ed25519[7](#Tezos) | 44'/1729'/a' | 44'/1729'/a' | 44'/1729'/a' | SLIP-0010 | | Paths that do not conform to this table are allowed, but user needs to confirm a warning on Trezor. For getPublicKey we do not check if the path is followed by other non-hardened items (anyone can derive those anyway). This is beneficial for Ethereum and its MEW compatibility, which sends `44'/60'/0'/0` for getPublicKey. ## Notes -1. We believe this should be `44'/60'/a'`, because Ethereum is account-based, rather than UTXO-based. Unfortunately, lot of Ethereum tools (MEW, Metamask) do not use such scheme and set `a = 0` and then iterate the address index `i`. Therefore for compatibility reasons we use the same scheme: `44'/60'/0'/0/i` and only the `i` is being iterated. +1. For Bitcoin and its derivatives it is a little bit more complicated. `p` is decided based on the following table: -2. Similar to Ethereum this should be `44'/144'/a'`. But for compatibility with other HW vendors we use `44'/144'/a'/0/0`. + | p | type | input script type | + |-----|--------------|--------------------| + | 44 | legacy | SPENDADDRESS | + | 48 | multisig | SPENDMULTISIG | + | 49 | p2sh segwit | SPENDP2SHWITNESS | + | 84 | native segwit | SPENDWITNESS | -3. Which allows non-hardened derivation on ed25519. +Other `p` are disallowed. `c` has to be equal to the coin's [slip44 id](https://github.com/satoshilabs/slips/blob/master/slip-0044.md) as for every coin. `y` needs to be 0 or 1. -4. NEM's path should be `44'/60'/a'` as per SEP-0005, but we allow `44'/60'/a'/0'/0'` as well for compatibility reasons with NanoWallet. +2. We believe this should be `44'/60'/a'`, because Ethereum is account-based, rather than UTXO-based. Unfortunately, lot of Ethereum tools (MEW, Metamask) do not use such scheme and set `a = 0` and then iterate the address index `i`. Therefore for compatibility reasons we use the same scheme: `44'/60'/0'/0/i` and only the `i` is being iterated. -5. Actually it is GetWatchKey for Monero. +3. Similar to Ethereum this should be `44'/144'/a'`. But for compatibility with other HW vendors we use `44'/144'/a'/0/0`. -6. It is a bit more complicated for Bitcoin-like coins. The following diagram shows how path should be validated for Bitcoin-like coins: +4. Which allows non-hardened derivation on ed25519. -![bitcoin-path-check](bitcoin-path-check.svg) +5. NEM's path should be `44'/60'/a'` as per SEP-0005, but we allow `44'/60'/a'/0'/0'` as well for compatibility reasons with NanoWallet. + +6. Actually it is GetWatchKey for Monero. + +7. Tezos supports multiple curves, but Trezor currently supports ed25519 only. + +Sign message paths are validated in the same way as the sign tx paths are. + +## Allowed values + +For GetPublicKey `a` needs in the interval of \[0, 20]. For GetAddress and signing the longer five-items paths need to have `a` also in range of \[0, 20] and `i` in \[0, 1000000]. If only three-items paths are used (Stellar and Lisk for example), `a` is allowed to be in \[0, 1000000] (because there is no address index `i`). diff --git a/docs/coins/bitcoin-path-check.svg b/docs/coins/bitcoin-path-check.svg deleted file mode 100644 index f2b6c7d141..0000000000 --- a/docs/coins/bitcoin-path-check.svg +++ /dev/null @@ -1,313 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Path = p'/c'/a'/y/x - - - - - - - - c == 0 - - - - - - - - - - uses change - - - - - - - T - - - - - - - - a < 10 - - - - - - - - x < 1000000 - - - - - - - - y == 0 or 1 - - - - - - - - y == 0 - - - - - - - F - - - - - - - - - - if segwit-enabled - - - - - - - - p == 44 - - - - - - - F - - - - - - - - p == 49 or 84 - - - - - - - T - - - - - - - - - - if segwit-enabled - - - - - - - F - - - - - - - T - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - OK! - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - len(path) == 5 - - - - - - - - \ No newline at end of file diff --git a/src/apps/cardano/address.py b/src/apps/cardano/address.py index 95daddac81..1f8a412a08 100644 --- a/src/apps/cardano/address.py +++ b/src/apps/cardano/address.py @@ -1,4 +1,3 @@ -from trezor import wire from trezor.crypto import base58, crc, hashlib from . import cbor @@ -6,14 +5,27 @@ from . import cbor from apps.common import HARDENED, seed -def validate_derivation_path(path: list): - if len(path) < 2 or len(path) > 5: - raise wire.ProcessError("Derivation path must be composed from 2-5 indices") - - if path[0] != HARDENED | 44 or path[1] != HARDENED | 1815: - raise wire.ProcessError("This is not cardano derivation path") - - return path +def validate_full_path(path: list) -> bool: + """ + Validates derivation path to fit 44'/1815'/a'/{0,1}/i, + where `a` is an account number and i an address index. + The max value for `a` is 20, 1 000 000 for `i`. + The derivation scheme v1 allowed a'/0/i only, + but in v2 it can be a'/1/i as well. + """ + if len(path) != 5: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != 1815 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 20 | HARDENED: + return False + if path[3] != 0 and path[3] != 1: + return False + if path[4] > 1000000: + return False + return True def _address_hash(data) -> bytes: @@ -33,8 +45,6 @@ def _get_address_root(node, payload): def derive_address_and_node(root_node, path: list): - validate_derivation_path(path) - derived_node = root_node.clone() address_payload = None diff --git a/src/apps/cardano/get_address.py b/src/apps/cardano/get_address.py index ecf8445c7a..660826c801 100644 --- a/src/apps/cardano/get_address.py +++ b/src/apps/cardano/get_address.py @@ -2,13 +2,15 @@ from trezor import log, ui, wire from trezor.crypto import bip32 from trezor.messages.CardanoAddress import CardanoAddress -from .address import derive_address_and_node +from .address import derive_address_and_node, validate_full_path from .layout import confirm_with_pagination -from apps.common import seed, storage +from apps.common import paths, seed, storage async def get_address(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + mnemonic = storage.get_mnemonic() passphrase = await seed._get_cached_passphrase(ctx) root_node = bip32.from_mnemonic_cardano(mnemonic, passphrase) diff --git a/src/apps/cardano/get_public_key.py b/src/apps/cardano/get_public_key.py index 8e5d0eb2a4..24b7417912 100644 --- a/src/apps/cardano/get_public_key.py +++ b/src/apps/cardano/get_public_key.py @@ -7,10 +7,14 @@ from trezor.messages.HDNodeType import HDNodeType from .address import derive_address_and_node -from apps.common import layout, seed, storage +from apps.common import layout, paths, seed, storage async def get_public_key(ctx, msg): + await paths.validate_path( + ctx, paths.validate_path_for_get_public_key, path=msg.address_n, slip44_id=1815 + ) + mnemonic = storage.get_mnemonic() passphrase = await seed._get_cached_passphrase(ctx) root_node = bip32.from_mnemonic_cardano(mnemonic, passphrase) diff --git a/src/apps/cardano/sign_message.py b/src/apps/cardano/sign_message.py new file mode 100644 index 0000000000..ca2c76834b --- /dev/null +++ b/src/apps/cardano/sign_message.py @@ -0,0 +1,53 @@ +from trezor import log, ui, wire +from trezor.crypto import bip32 +from trezor.crypto.curve import ed25519 +from trezor.messages.CardanoMessageSignature import CardanoMessageSignature + +from .address import derive_address_and_node, validate_full_path +from .layout import confirm_with_pagination + +from apps.common import paths, seed, storage + + +async def sign_message(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + + mnemonic = storage.get_mnemonic() + root_node = bip32.from_mnemonic_cardano(mnemonic) + + try: + signature = _sign_message(root_node, msg.message, msg.address_n) + except ValueError as e: + if __debug__: + log.exception(__name__, e) + raise wire.ProcessError("Signing failed") + mnemonic = None + root_node = None + + if not await confirm_with_pagination( + ctx, msg.message, "Signing message", ui.ICON_RECEIVE, ui.GREEN + ): + raise wire.ActionCancelled("Signing cancelled") + + if not await confirm_with_pagination( + ctx, + paths.break_address_n_to_lines(msg.address_n), + "With address", + ui.ICON_RECEIVE, + ui.GREEN, + ): + raise wire.ActionCancelled("Signing cancelled") + + return signature + + +def _sign_message(root_node, message: str, derivation_path: list): + address, node = derive_address_and_node(root_node, derivation_path) + + signature = ed25519.sign_ext(node.private_key(), node.private_key_ext(), message) + + sig = CardanoMessageSignature() + sig.public_key = seed.remove_ed25519_prefix(node.public_key()) + sig.signature = signature + + return sig diff --git a/src/apps/cardano/sign_tx.py b/src/apps/cardano/sign_tx.py index 962f7cf506..970224d586 100644 --- a/src/apps/cardano/sign_tx.py +++ b/src/apps/cardano/sign_tx.py @@ -6,12 +6,13 @@ from trezor.messages.CardanoTxRequest import CardanoTxRequest from trezor.messages.MessageType import CardanoTxAck from trezor.ui.text import BR -from .address import derive_address_and_node +from .address import derive_address_and_node, validate_full_path from .layout import confirm_with_pagination, progress from apps.cardano import cbor from apps.common import seed, storage from apps.common.layout import address_n_to_str, split_address +from apps.common.paths import validate_path from apps.homescreen.homescreen import display_homescreen @@ -97,6 +98,9 @@ async def sign_tx(ctx, msg): # clear progress bar display_homescreen() + for i in msg.inputs: + await validate_path(ctx, validate_full_path, path=i.address_n) + # sign the transaction bundle and prepare the result transaction = Transaction( msg.inputs, msg.outputs, transactions, root_node, msg.network diff --git a/src/apps/cardano/verify_message.py b/src/apps/cardano/verify_message.py new file mode 100644 index 0000000000..ff505bfeba --- /dev/null +++ b/src/apps/cardano/verify_message.py @@ -0,0 +1,41 @@ +from ubinascii import hexlify + +from trezor import log, ui, wire +from trezor.crypto.curve import ed25519 +from trezor.messages.Failure import Failure +from trezor.messages.Success import Success + +from .address import validate_full_path +from .layout import confirm_with_pagination + +from apps.common import paths + + +async def verify_message(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + + try: + res = _verify_message(msg.public_key, msg.signature, msg.message) + except ValueError as e: + if __debug__: + log.exception(__name__, e) + raise wire.ProcessError("Verifying failed") + + if not res: + return Failure(message="Invalid signature") + + if not await confirm_with_pagination( + ctx, msg.message, "Verifying message", ui.ICON_RECEIVE, ui.GREEN + ): + raise wire.ActionCancelled("Verifying cancelled") + + if not await confirm_with_pagination( + ctx, hexlify(msg.public_key), "With public key", ui.ICON_RECEIVE, ui.GREEN + ): + raise wire.ActionCancelled("Verifying cancelled") + + return Success(message="Message verified") + + +def _verify_message(public_key: bytes, signature: bytes, message: str): + return ed25519.verify(public_key, signature, message) diff --git a/src/apps/common/paths.py b/src/apps/common/paths.py new file mode 100644 index 0000000000..da09189733 --- /dev/null +++ b/src/apps/common/paths.py @@ -0,0 +1,72 @@ +from micropython import const + +from trezor import ui +from trezor.messages import ButtonRequestType +from trezor.ui.text import Text + +from apps.common import HARDENED +from apps.common.confirm import require_confirm + + +async def validate_path(ctx, validate_func, **kwargs): + if not validate_func(**kwargs): + await show_path_warning(ctx, kwargs["path"]) + + +async def show_path_warning(ctx, path: list): + text = Text("Confirm path", ui.ICON_WRONG, icon_color=ui.RED) + text.normal("The path") + text.mono(*break_address_n_to_lines(path)) + text.normal("seems unusual.") + text.normal("Are you sure?") + return await require_confirm( + ctx, text, code=ButtonRequestType.UnknownDerivationPath + ) + + +def validate_path_for_get_public_key(path: list, slip44_id: int) -> bool: + """ + Checks if path has at least three hardened items and slip44 id matches. + The path is allowed to have more than three items, but all the following + items have to be non-hardened. + """ + length = len(path) + if length < 3 or length > 5: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != slip44_id | HARDENED: + return False + if path[2] < HARDENED or path[2] > 20 | HARDENED: + return False + if length > 3 and is_hardened(path[3]): + return False + if length > 4 and is_hardened(path[4]): + return False + return True + + +def is_hardened(i: int) -> bool: + if i & HARDENED: + return True + return False + + +def break_address_n_to_lines(address_n: list) -> list: + def path_item(i: int): + if i & HARDENED: + return str(i ^ HARDENED) + "'" + else: + return str(i) + + lines = [] + path_str = "m/" + "/".join([path_item(i) for i in address_n]) + + per_line = const(17) + while len(path_str) > per_line: + i = path_str[:per_line].rfind("/") + lines.append(path_str[:i]) + path_str = path_str[i:] + lines.append(path_str) + + return lines diff --git a/src/apps/ethereum/address.py b/src/apps/ethereum/address.py new file mode 100644 index 0000000000..4a1043fc0b --- /dev/null +++ b/src/apps/ethereum/address.py @@ -0,0 +1,55 @@ +from apps.common import HARDENED + + +""" +We believe Ethereum should use 44'/60'/a' for everything,because it is +account-based, rather than UTXO-based. Unfortunately, lot of Ethereum +tools (MEW, Metamask) do not use such scheme and set a = 0 and then +iterate the address index i. Therefore for compatibility reasons we use +the same scheme: 44'/60'/0'/0/i and only the i is being iterated. +""" + + +def validate_full_path(path: list) -> bool: + """ + Validates derivation path to equal 44'/60'/0'/0/i, + where `i` is an address index from 0 to 1 000 000. + """ + if len(path) != 5: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != 60 | HARDENED: + return False + if path[2] != 0 | HARDENED: + return False + if path[3] != 0: + return False + if path[4] > 1000000: + return False + return True + + +def ethereum_address_hex(address, network=None): + from ubinascii import hexlify + from trezor.crypto.hashlib import sha3_256 + + rskip60 = network is not None and network.rskip60 + + hx = hexlify(address).decode() + + prefix = str(network.chain_id) + "0x" if rskip60 else "" + hs = sha3_256(prefix + hx, keccak=True).digest() + h = "" + + for i in range(20): + l = hx[i * 2] + if hs[i] & 0x80 and l >= "a" and l <= "f": + l = l.upper() + h += l + l = hx[i * 2 + 1] + if hs[i] & 0x08 and l >= "a" and l <= "f": + l = l.upper() + h += l + + return "0x" + h diff --git a/src/apps/ethereum/get_address.py b/src/apps/ethereum/get_address.py index fb6ecf2a21..e0c7e7b155 100644 --- a/src/apps/ethereum/get_address.py +++ b/src/apps/ethereum/get_address.py @@ -1,3 +1,6 @@ +from .address import ethereum_address_hex, validate_full_path + +from apps.common import paths from apps.common.layout import address_n_to_str, show_address, show_qr from apps.ethereum import networks @@ -8,6 +11,8 @@ async def get_address(ctx, msg): from trezor.crypto.hashlib import sha3_256 from apps.common import seed + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n) seckey = node.private_key() @@ -19,7 +24,7 @@ async def get_address(ctx, msg): network = networks.by_slip44(msg.address_n[1] & 0x7FFFFFFF) else: network = None - hex_addr = _ethereum_address_hex(address, network) + hex_addr = ethereum_address_hex(address, network) desc = address_n_to_str(msg.address_n) while True: if await show_address(ctx, hex_addr, desc=desc): @@ -28,28 +33,3 @@ async def get_address(ctx, msg): break return EthereumAddress(address=address) - - -def _ethereum_address_hex(address, network=None): - from ubinascii import hexlify - from trezor.crypto.hashlib import sha3_256 - - rskip60 = network is not None and network.rskip60 - - hx = hexlify(address).decode() - - prefix = str(network.chain_id) + "0x" if rskip60 else "" - hs = sha3_256(prefix + hx, keccak=True).digest() - h = "" - - for i in range(20): - l = hx[i * 2] - if hs[i] & 0x80 and l >= "a" and l <= "f": - l = l.upper() - h += l - l = hx[i * 2 + 1] - if hs[i] & 0x08 and l >= "a" and l <= "f": - l = l.upper() - h += l - - return "0x" + h diff --git a/src/apps/ethereum/layout.py b/src/apps/ethereum/layout.py index a94b94f496..0608daff92 100644 --- a/src/apps/ethereum/layout.py +++ b/src/apps/ethereum/layout.py @@ -8,12 +8,12 @@ from trezor.utils import chunks, format_amount from apps.common.confirm import require_confirm, require_hold_to_confirm from apps.common.layout import split_address from apps.ethereum import networks, tokens -from apps.ethereum.get_address import _ethereum_address_hex +from apps.ethereum.address import ethereum_address_hex async def require_confirm_tx(ctx, to, value, chain_id, token=None, tx_type=None): if to: - to_str = _ethereum_address_hex(to, networks.by_chain_id(chain_id)) + to_str = ethereum_address_hex(to, networks.by_chain_id(chain_id)) else: to_str = "new contract?" text = Text("Confirm sending", ui.ICON_SEND, icon_color=ui.GREEN, new_lines=False) diff --git a/src/apps/ethereum/sign_message.py b/src/apps/ethereum/sign_message.py index 58b606f7fc..c869f45076 100644 --- a/src/apps/ethereum/sign_message.py +++ b/src/apps/ethereum/sign_message.py @@ -4,7 +4,9 @@ from trezor.messages.EthereumMessageSignature import EthereumMessageSignature from trezor.ui.text import Text from trezor.utils import HashWriter -from apps.common import seed +from .address import validate_full_path + +from apps.common import paths, seed from apps.common.confirm import require_confirm from apps.common.signverify import split_message @@ -19,6 +21,7 @@ def message_digest(message): async def sign_message(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) await require_confirm_sign_message(ctx, msg.message) node = await seed.derive_node(ctx, msg.address_n) diff --git a/src/apps/ethereum/sign_tx.py b/src/apps/ethereum/sign_tx.py index b8acd558e6..436f7a5c40 100644 --- a/src/apps/ethereum/sign_tx.py +++ b/src/apps/ethereum/sign_tx.py @@ -7,7 +7,9 @@ from trezor.messages.EthereumTxRequest import EthereumTxRequest from trezor.messages.MessageType import EthereumTxAck from trezor.utils import HashWriter -from apps.common import seed +from .address import validate_full_path + +from apps.common import paths, seed from apps.ethereum import tokens from apps.ethereum.layout import ( require_confirm_data, @@ -22,6 +24,7 @@ MAX_CHAIN_ID = 2147483629 async def sign_tx(ctx, msg): msg = sanitize(msg) check(msg) + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) data_total = msg.data_length diff --git a/src/apps/ethereum/verify_message.py b/src/apps/ethereum/verify_message.py index b03d662309..41a09418ba 100644 --- a/src/apps/ethereum/verify_message.py +++ b/src/apps/ethereum/verify_message.py @@ -5,6 +5,9 @@ from trezor.crypto.hashlib import sha3_256 from trezor.messages.Success import Success from trezor.ui.text import Text +from .address import validate_full_path + +from apps.common import paths from apps.common.confirm import require_confirm from apps.common.layout import split_address from apps.common.signverify import split_message @@ -12,6 +15,8 @@ from apps.ethereum.sign_message import message_digest async def verify_message(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address) + digest = message_digest(msg.message) sig = bytearray([msg.signature[64]]) + msg.signature[:64] pubkey = secp256k1.verify_recover(sig, digest) diff --git a/src/apps/lisk/get_address.py b/src/apps/lisk/get_address.py index 27b2968cce..9217420c3c 100644 --- a/src/apps/lisk/get_address.py +++ b/src/apps/lisk/get_address.py @@ -1,12 +1,14 @@ from trezor.messages.LiskAddress import LiskAddress -from .helpers import LISK_CURVE, get_address_from_public_key +from .helpers import LISK_CURVE, get_address_from_public_key, validate_full_path -from apps.common import seed +from apps.common import paths, seed from apps.common.layout import address_n_to_str, show_address, show_qr async def get_address(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n, LISK_CURVE) pubkey = node.public_key() pubkey = pubkey[1:] # skip ed25519 pubkey marker diff --git a/src/apps/lisk/get_public_key.py b/src/apps/lisk/get_public_key.py index 6fc35aadff..37c613997f 100644 --- a/src/apps/lisk/get_public_key.py +++ b/src/apps/lisk/get_public_key.py @@ -1,11 +1,13 @@ from trezor.messages.LiskPublicKey import LiskPublicKey -from .helpers import LISK_CURVE +from .helpers import LISK_CURVE, validate_full_path -from apps.common import layout, seed +from apps.common import layout, paths, seed async def get_public_key(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n, LISK_CURVE) pubkey = node.public_key() pubkey = pubkey[1:] # skip ed25519 pubkey marker diff --git a/src/apps/lisk/helpers.py b/src/apps/lisk/helpers.py index 2cca1fdb46..cc32a1f501 100644 --- a/src/apps/lisk/helpers.py +++ b/src/apps/lisk/helpers.py @@ -1,5 +1,7 @@ from trezor.crypto.hashlib import sha256 +from apps.common import HARDENED + LISK_CURVE = "ed25519" @@ -31,3 +33,19 @@ def get_vote_tx_text(votes): def _text_with_plural(txt, value): return "%s %s %s" % (txt, value, ("votes" if value != 1 else "vote")) + + +def validate_full_path(path: list) -> bool: + """ + Validates derivation path to equal 44'/134'/a', + where `a` is an account index from 0 to 1 000 000. + """ + if len(path) != 3: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != 134 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 1000000 | HARDENED: + return False + return True diff --git a/src/apps/lisk/sign_message.py b/src/apps/lisk/sign_message.py index 4be9996c37..8ccb88b72e 100644 --- a/src/apps/lisk/sign_message.py +++ b/src/apps/lisk/sign_message.py @@ -4,9 +4,9 @@ from trezor.messages.LiskMessageSignature import LiskMessageSignature from trezor.ui.text import Text from trezor.utils import HashWriter -from .helpers import LISK_CURVE +from .helpers import LISK_CURVE, validate_full_path -from apps.common import seed +from apps.common import paths, seed from apps.common.confirm import require_confirm from apps.common.signverify import split_message from apps.wallet.sign_tx.signing import write_varint @@ -23,17 +23,15 @@ def message_digest(message): async def sign_message(ctx, msg): - message = msg.message - address_n = msg.address_n + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + await require_confirm_sign_message(ctx, msg.message) - await require_confirm_sign_message(ctx, message) - - node = await seed.derive_node(ctx, address_n, LISK_CURVE) + node = await seed.derive_node(ctx, msg.address_n, LISK_CURVE) seckey = node.private_key() pubkey = node.public_key() pubkey = pubkey[1:] # skip ed25519 pubkey marker - signature = ed25519.sign(seckey, message_digest(message)) + signature = ed25519.sign(seckey, message_digest(msg.message)) return LiskMessageSignature(public_key=pubkey, signature=signature) diff --git a/src/apps/lisk/sign_tx.py b/src/apps/lisk/sign_tx.py index 7c59fdfea0..9466620303 100644 --- a/src/apps/lisk/sign_tx.py +++ b/src/apps/lisk/sign_tx.py @@ -8,12 +8,14 @@ from trezor.messages.LiskSignedTx import LiskSignedTx from trezor.utils import HashWriter from . import layout -from .helpers import LISK_CURVE, get_address_from_public_key +from .helpers import LISK_CURVE, get_address_from_public_key, validate_full_path -from apps.common import seed +from apps.common import paths, seed async def sign_tx(ctx, msg): + await paths.validate_path(ctx, validate_full_path, path=msg.address_n) + pubkey, seckey = await _get_keys(ctx, msg) transaction = _update_raw_tx(msg.transaction, pubkey) diff --git a/src/apps/monero/get_address.py b/src/apps/monero/get_address.py index 93d93c5903..873420d3ca 100644 --- a/src/apps/monero/get_address.py +++ b/src/apps/monero/get_address.py @@ -1,10 +1,13 @@ from trezor.messages.MoneroAddress import MoneroAddress +from apps.common import paths from apps.common.layout import address_n_to_str, show_address, show_qr from apps.monero import misc async def get_address(ctx, msg): + await paths.validate_path(ctx, misc.validate_full_path, path=msg.address_n) + creds = await misc.get_creds(ctx, msg.address_n, msg.network_type) if msg.show_display: diff --git a/src/apps/monero/get_watch_only.py b/src/apps/monero/get_watch_only.py index 8131dde5f9..311a6fc8bd 100644 --- a/src/apps/monero/get_watch_only.py +++ b/src/apps/monero/get_watch_only.py @@ -1,12 +1,15 @@ from trezor.messages.MoneroGetWatchKey import MoneroGetWatchKey from trezor.messages.MoneroWatchKey import MoneroWatchKey +from apps.common import paths from apps.monero import misc from apps.monero.layout import confirms from apps.monero.xmr import crypto async def get_watch_only(ctx, msg: MoneroGetWatchKey): + await paths.validate_path(ctx, misc.validate_full_path, path=msg.address_n) + await confirms.require_confirm_watchkey(ctx) creds = await misc.get_creds(ctx, msg.address_n, msg.network_type) diff --git a/src/apps/monero/key_image_sync.py b/src/apps/monero/key_image_sync.py index fb3455d827..de265b8eaf 100644 --- a/src/apps/monero/key_image_sync.py +++ b/src/apps/monero/key_image_sync.py @@ -7,6 +7,7 @@ from trezor.messages.MoneroKeyImageExportInitAck import MoneroKeyImageExportInit from trezor.messages.MoneroKeyImageSyncFinalAck import MoneroKeyImageSyncFinalAck from trezor.messages.MoneroKeyImageSyncStepAck import MoneroKeyImageSyncStepAck +from apps.common import paths from apps.monero import misc from apps.monero.layout import confirms from apps.monero.xmr import crypto, key_image, monero @@ -46,6 +47,8 @@ class KeyImageSync: async def _init_step(s, ctx, msg): + await paths.validate_path(ctx, misc.validate_full_path, path=msg.address_n) + s.creds = await misc.get_creds(ctx, msg.address_n, msg.network_type) await confirms.require_confirm_keyimage_sync(ctx) diff --git a/src/apps/monero/misc.py b/src/apps/monero/misc.py index b6d32283da..5cdd459c96 100644 --- a/src/apps/monero/misc.py +++ b/src/apps/monero/misc.py @@ -1,3 +1,6 @@ +from apps.common import HARDENED + + async def get_creds(ctx, address_n=None, network_type=None): from apps.common import seed from apps.monero.xmr import crypto, monero @@ -19,3 +22,19 @@ async def get_creds(ctx, address_n=None, network_type=None): creds = AccountCreds.new_wallet(view_sec, spend_sec, network_type) return creds + + +def validate_full_path(path: list) -> bool: + """ + Validates derivation path to equal 44'/128'/a', + where `a` is an account index from 0 to 1 000 000. + """ + if len(path) != 3: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != 128 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 1000000 | HARDENED: + return False + return True diff --git a/src/apps/monero/signing/step_01_init_transaction.py b/src/apps/monero/signing/step_01_init_transaction.py index 2797c0c919..62c7c980f2 100644 --- a/src/apps/monero/signing/step_01_init_transaction.py +++ b/src/apps/monero/signing/step_01_init_transaction.py @@ -19,6 +19,9 @@ async def init_transaction( state: State, address_n: list, network_type: int, tsx_data: MoneroTransactionData ): from apps.monero.signing import offloading_keys + from apps.common import paths + + await paths.validate_path(state.ctx, misc.validate_full_path, path=address_n) state.creds = await misc.get_creds(state.ctx, address_n, network_type) state.fee = state.fee if state.fee > 0 else 0 diff --git a/src/apps/nem/get_address.py b/src/apps/nem/get_address.py index 77100b5c6b..2ca653e552 100644 --- a/src/apps/nem/get_address.py +++ b/src/apps/nem/get_address.py @@ -1,14 +1,17 @@ from trezor.messages.NEMAddress import NEMAddress -from .helpers import NEM_CURVE, get_network_str +from .helpers import NEM_CURVE, check_path, get_network_str from .validators import validate_network from apps.common import seed from apps.common.layout import address_n_to_str, show_address, show_qr +from apps.common.paths import validate_path async def get_address(ctx, msg): network = validate_network(msg.network) + await validate_path(ctx, check_path, path=msg.address_n, network=msg.network) + node = await seed.derive_node(ctx, msg.address_n, NEM_CURVE) address = node.nem_address(network) diff --git a/src/apps/nem/helpers.py b/src/apps/nem/helpers.py index fa367534bb..917a7b1606 100644 --- a/src/apps/nem/helpers.py +++ b/src/apps/nem/helpers.py @@ -1,5 +1,7 @@ from micropython import const +from apps.common import HARDENED + NEM_NETWORK_MAINNET = const(0x68) NEM_NETWORK_TESTNET = const(0x98) NEM_NETWORK_MIJIN = const(0x60) @@ -35,3 +37,28 @@ def get_network_str(network: int) -> str: return "Testnet" elif network == NEM_NETWORK_MIJIN: return "Mijin" + + +def check_path(path: list, network=None) -> bool: + """ + Validates derivation path to fit 44'/43'/a' or 44'/43'/a'/0'/0', + where `a` is an account number. We believe the path should be + 44'/43'/a', but for compatibility reasons with NEM's NanoWallet + we allow 44'/43'/a'/0'/0' as well. + Testnet is also allowed: 44'/1'/a'{/0'/0'} + """ + length = len(path) + if length != 3 and length != 5: + return False + if path[0] != 44 | HARDENED: + return False + if not ( + path[1] == 43 | HARDENED + or (network == NEM_NETWORK_TESTNET and path[1] == 1 | HARDENED) + ): + return False + if path[2] < HARDENED or path[2] > 1000000 | HARDENED: + return False + if length == 5 and (path[3] != 0 | HARDENED or path[4] != 0 | HARDENED): + return False + return True diff --git a/src/apps/nem/sign_tx.py b/src/apps/nem/sign_tx.py index e890ac8230..7ca411f8bb 100644 --- a/src/apps/nem/sign_tx.py +++ b/src/apps/nem/sign_tx.py @@ -3,14 +3,19 @@ from trezor.messages.NEMSignedTx import NEMSignedTx from trezor.messages.NEMSignTx import NEMSignTx from . import mosaic, multisig, namespace, transfer -from .helpers import NEM_CURVE, NEM_HASH_ALG +from .helpers import NEM_CURVE, NEM_HASH_ALG, check_path from .validators import validate from apps.common import seed +from apps.common.paths import validate_path async def sign_tx(ctx, msg: NEMSignTx): validate(msg) + await validate_path( + ctx, check_path, path=msg.transaction.address_n, network=msg.transaction.network + ) + node = await seed.derive_node(ctx, msg.transaction.address_n, NEM_CURVE) if msg.multisig: diff --git a/src/apps/ripple/get_address.py b/src/apps/ripple/get_address.py index 41227ff133..5af586a747 100644 --- a/src/apps/ripple/get_address.py +++ b/src/apps/ripple/get_address.py @@ -3,11 +3,13 @@ from trezor.messages.RippleGetAddress import RippleGetAddress from . import helpers -from apps.common import seed +from apps.common import paths, seed from apps.common.layout import address_n_to_str, show_address, show_qr async def get_address(ctx, msg: RippleGetAddress): + await paths.validate_path(ctx, helpers.validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n) pubkey = node.public_key() address = helpers.address_from_public_key(pubkey) diff --git a/src/apps/ripple/helpers.py b/src/apps/ripple/helpers.py index 36cf1fb2c2..f2d1df68b7 100644 --- a/src/apps/ripple/helpers.py +++ b/src/apps/ripple/helpers.py @@ -4,6 +4,8 @@ from trezor.crypto.hashlib import ripemd160, sha256 from . import base58_ripple +from apps.common import HARDENED + # HASH_TX_ID = const(0x54584E00) # 'TXN' HASH_TX_SIGN = const(0x53545800) # 'STX' # HASH_TX_SIGN_TESTNET = const(0x73747800) # 'stx' @@ -45,3 +47,25 @@ def decode_address(address: str): """Returns so called Account ID""" adr = base58_ripple.decode_check(address) return adr[1:] + + +def validate_full_path(path: list) -> bool: + """ + Validates derivation path to equal 44'/144'/a'/0/0, + where `a` is an account index from 0 to 1 000 000. + Similar to Ethereum this should be 44'/144'/a', but for + compatibility with other HW vendors we use 44'/144'/a'/0/0. + """ + if len(path) != 5: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != 144 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 1000000 | HARDENED: + return False + if path[3] != 0: + return False + if path[4] != 0: + return False + return True diff --git a/src/apps/ripple/sign_tx.py b/src/apps/ripple/sign_tx.py index cd3bb94e55..b35227f78d 100644 --- a/src/apps/ripple/sign_tx.py +++ b/src/apps/ripple/sign_tx.py @@ -8,11 +8,13 @@ from trezor.wire import ProcessError from . import helpers, layout from .serialize import serialize -from apps.common import seed +from apps.common import paths, seed async def sign_tx(ctx, msg: RippleSignTx): validate(msg) + await paths.validate_path(ctx, helpers.validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n) source_address = helpers.address_from_public_key(node.public_key()) diff --git a/src/apps/stellar/get_address.py b/src/apps/stellar/get_address.py index ee115c6dcd..1e15f6b586 100644 --- a/src/apps/stellar/get_address.py +++ b/src/apps/stellar/get_address.py @@ -1,12 +1,14 @@ from trezor.messages.StellarAddress import StellarAddress from trezor.messages.StellarGetAddress import StellarGetAddress -from apps.common import seed +from apps.common import paths, seed from apps.common.layout import address_n_to_str, show_address, show_qr from apps.stellar import helpers async def get_address(ctx, msg: StellarGetAddress): + await paths.validate_path(ctx, helpers.validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n, helpers.STELLAR_CURVE) pubkey = seed.remove_ed25519_prefix(node.public_key()) address = helpers.address_from_public_key(pubkey) diff --git a/src/apps/stellar/helpers.py b/src/apps/stellar/helpers.py index 753ba1817a..0062b7bce8 100644 --- a/src/apps/stellar/helpers.py +++ b/src/apps/stellar/helpers.py @@ -3,6 +3,8 @@ import ustruct from trezor.crypto import base32 from trezor.wire import ProcessError +from apps.common import HARDENED + STELLAR_CURVE = "ed25519" @@ -26,6 +28,22 @@ def address_from_public_key(pubkey: bytes): return base32.encode(address) +def validate_full_path(path: list) -> bool: + """ + Validates derivation path to equal 44'/148'/a', + where `a` is an account index from 0 to 1 000 000. + """ + if len(path) != 3: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != 148 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 1000000 | HARDENED: + return False + return True + + def _crc16_checksum_verify(data: bytes, checksum: bytes): if _crc16_checksum(data) != checksum: raise ProcessError("Invalid address checksum") diff --git a/src/apps/stellar/sign_tx.py b/src/apps/stellar/sign_tx.py index 28bfba7636..f6efa56d70 100644 --- a/src/apps/stellar/sign_tx.py +++ b/src/apps/stellar/sign_tx.py @@ -7,7 +7,7 @@ from trezor.messages.StellarSignTx import StellarSignTx from trezor.messages.StellarTxOpRequest import StellarTxOpRequest from trezor.wire import ProcessError -from apps.common import seed +from apps.common import paths, seed from apps.stellar import consts, helpers, layout, writers from apps.stellar.operations import process_operation @@ -16,6 +16,8 @@ async def sign_tx(ctx, msg: StellarSignTx): if msg.num_operations == 0: raise ProcessError("Stellar: At least one operation is required") + await paths.validate_path(ctx, helpers.validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n, consts.STELLAR_CURVE) pubkey = seed.remove_ed25519_prefix(node.public_key()) diff --git a/src/apps/tezos/get_address.py b/src/apps/tezos/get_address.py index 07ded8153d..9640d2c971 100644 --- a/src/apps/tezos/get_address.py +++ b/src/apps/tezos/get_address.py @@ -1,21 +1,20 @@ from trezor.crypto import hashlib from trezor.messages.TezosAddress import TezosAddress -from apps.common import seed +from apps.common import paths, seed from apps.common.layout import address_n_to_str, show_address, show_qr -from apps.tezos.helpers import ( - TEZOS_CURVE, - TEZOS_ED25519_ADDRESS_PREFIX, - base58_encode_check, -) +from apps.tezos import helpers async def get_address(ctx, msg): - node = await seed.derive_node(ctx, msg.address_n, TEZOS_CURVE) + await paths.validate_path(ctx, helpers.validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n, helpers.TEZOS_CURVE) pk = seed.remove_ed25519_prefix(node.public_key()) pkh = hashlib.blake2b(pk, outlen=20).digest() - address = base58_encode_check(pkh, prefix=TEZOS_ED25519_ADDRESS_PREFIX) + address = helpers.base58_encode_check( + pkh, prefix=helpers.TEZOS_ED25519_ADDRESS_PREFIX + ) if msg.show_display: desc = address_n_to_str(msg.address_n) diff --git a/src/apps/tezos/get_public_key.py b/src/apps/tezos/get_public_key.py index b229a4aa92..c96d468dde 100644 --- a/src/apps/tezos/get_public_key.py +++ b/src/apps/tezos/get_public_key.py @@ -4,16 +4,17 @@ from trezor.messages.TezosPublicKey import TezosPublicKey from trezor.ui.text import Text from trezor.utils import chunks -from apps.common import seed +from apps.common import paths, seed from apps.common.confirm import require_confirm -from apps.tezos.helpers import TEZOS_CURVE, TEZOS_PUBLICKEY_PREFIX, base58_encode_check +from apps.tezos import helpers async def get_public_key(ctx, msg): - node = await seed.derive_node(ctx, msg.address_n, TEZOS_CURVE) + await paths.validate_path(ctx, helpers.validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n, helpers.TEZOS_CURVE) pk = seed.remove_ed25519_prefix(node.public_key()) - pk_prefixed = base58_encode_check(pk, prefix=TEZOS_PUBLICKEY_PREFIX) + pk_prefixed = helpers.base58_encode_check(pk, prefix=helpers.TEZOS_PUBLICKEY_PREFIX) if msg.show_display: await _show_tezos_pubkey(ctx, pk_prefixed) diff --git a/src/apps/tezos/helpers.py b/src/apps/tezos/helpers.py index 4a82a6aa1b..0050b93fd5 100644 --- a/src/apps/tezos/helpers.py +++ b/src/apps/tezos/helpers.py @@ -2,6 +2,8 @@ from micropython import const from trezor.crypto import base58 +from apps.common import HARDENED + TEZOS_CURVE = "ed25519" TEZOS_AMOUNT_DIVISIBILITY = const(6) TEZOS_ED25519_ADDRESS_PREFIX = "tz1" @@ -35,3 +37,19 @@ def base58_decode_check(enc, prefix=None): if prefix is not None: decoded = decoded[len(TEZOS_PREFIX_BYTES[prefix]) :] return decoded + + +def validate_full_path(path: list) -> bool: + """ + Validates derivation path to equal 44'/1729'/a', + where `a` is an account index from 0 to 1 000 000. + """ + if len(path) != 3: + return False + if path[0] != 44 | HARDENED: + return False + if path[1] != 1729 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 1000000 | HARDENED: + return False + return True diff --git a/src/apps/tezos/sign_tx.py b/src/apps/tezos/sign_tx.py index ffb86c5b6e..0bcc8cbc1d 100644 --- a/src/apps/tezos/sign_tx.py +++ b/src/apps/tezos/sign_tx.py @@ -4,19 +4,14 @@ from trezor.crypto.curve import ed25519 from trezor.messages import TezosContractType from trezor.messages.TezosSignedTx import TezosSignedTx -from apps.common import seed +from apps.common import paths, seed from apps.common.writers import write_bytes, write_uint8 -from apps.tezos import layout -from apps.tezos.helpers import ( - TEZOS_CURVE, - TEZOS_ORIGINATED_ADDRESS_PREFIX, - TEZOS_SIGNATURE_PREFIX, - base58_encode_check, -) +from apps.tezos import layout, helpers async def sign_tx(ctx, msg): - node = await seed.derive_node(ctx, msg.address_n, TEZOS_CURVE) + await paths.validate_path(ctx, helpers.validate_full_path, path=msg.address_n) + node = await seed.derive_node(ctx, msg.address_n, helpers.TEZOS_CURVE) if msg.transaction is not None: to = _get_address_from_contract(msg.transaction.destination) @@ -71,9 +66,11 @@ async def sign_tx(ctx, msg): sig_op_contents = opbytes + signature sig_op_contents_hash = hashlib.blake2b(sig_op_contents, outlen=32).digest() - ophash = base58_encode_check(sig_op_contents_hash, prefix="o") + ophash = helpers.base58_encode_check(sig_op_contents_hash, prefix="o") - sig_prefixed = base58_encode_check(signature, prefix=TEZOS_SIGNATURE_PREFIX) + sig_prefixed = helpers.base58_encode_check( + signature, prefix=helpers.TEZOS_SIGNATURE_PREFIX + ) return TezosSignedTx( signature=sig_prefixed, sig_op_contents=sig_op_contents, operation_hash=ophash @@ -85,7 +82,7 @@ def _get_address_by_tag(address_hash): tag = int(address_hash[0]) if 0 <= tag < len(prefixes): - return base58_encode_check(address_hash[1:], prefix=prefixes[tag]) + return helpers.base58_encode_check(address_hash[1:], prefix=prefixes[tag]) raise wire.DataError("Invalid tag in address hash") @@ -94,8 +91,8 @@ def _get_address_from_contract(address): return _get_address_by_tag(address.hash) elif address.tag == TezosContractType.Originated: - return base58_encode_check( - address.hash[:-1], prefix=TEZOS_ORIGINATED_ADDRESS_PREFIX + return helpers.base58_encode_check( + address.hash[:-1], prefix=helpers.TEZOS_ORIGINATED_ADDRESS_PREFIX ) raise wire.DataError("Invalid contract type") diff --git a/src/apps/wallet/get_address.py b/src/apps/wallet/get_address.py index 90c3eb47e2..929aaf9d07 100644 --- a/src/apps/wallet/get_address.py +++ b/src/apps/wallet/get_address.py @@ -3,6 +3,7 @@ from trezor.messages.Address import Address from apps.common import coins, seed from apps.common.layout import address_n_to_str, show_address, show_qr +from apps.common.paths import validate_path from apps.wallet.sign_tx import addresses @@ -10,6 +11,14 @@ async def get_address(ctx, msg): coin_name = msg.coin_name or "Bitcoin" coin = coins.by_name(coin_name) + await validate_path( + ctx, + addresses.validate_full_path, + path=msg.address_n, + coin=coin, + script_type=msg.script_type, + ) + node = await seed.derive_node(ctx, msg.address_n, curve_name=coin.curve_name) address = addresses.get_address(msg.script_type, coin, node, msg.multisig) address_short = addresses.address_short(coin, address) diff --git a/src/apps/wallet/get_public_key.py b/src/apps/wallet/get_public_key.py index d18a6b308a..be8976d6dc 100644 --- a/src/apps/wallet/get_public_key.py +++ b/src/apps/wallet/get_public_key.py @@ -3,7 +3,8 @@ from trezor.messages import InputScriptType from trezor.messages.HDNodeType import HDNodeType from trezor.messages.PublicKey import PublicKey -from apps.common import coins, layout, seed +from apps.common import coins, layout, paths, seed +from apps.wallet.sign_tx.addresses import validate_path_for_bitcoin_public_key async def get_public_key(ctx, msg): @@ -11,6 +12,10 @@ async def get_public_key(ctx, msg): coin = coins.by_name(coin_name) script_type = msg.script_type or InputScriptType.SPENDADDRESS + await paths.validate_path( + ctx, validate_path_for_bitcoin_public_key, path=msg.address_n, coin=coin + ) + curve_name = msg.ecdsa_curve_name if not curve_name: curve_name = coin.curve_name diff --git a/src/apps/wallet/sign_message.py b/src/apps/wallet/sign_message.py index 9128920bd5..23141ec938 100644 --- a/src/apps/wallet/sign_message.py +++ b/src/apps/wallet/sign_message.py @@ -6,8 +6,9 @@ from trezor.ui.text import Text from apps.common import coins, seed from apps.common.confirm import require_confirm +from apps.common.paths import validate_path from apps.common.signverify import message_digest, split_message -from apps.wallet.sign_tx.addresses import get_address +from apps.wallet.sign_tx.addresses import get_address, validate_full_path async def sign_message(ctx, msg): @@ -19,6 +20,14 @@ async def sign_message(ctx, msg): await require_confirm_sign_message(ctx, message) + await validate_path( + ctx, + validate_full_path, + path=msg.address_n, + coin=coin, + script_type=msg.script_type, + ) + node = await seed.derive_node(ctx, address_n, curve_name=coin.curve_name) seckey = node.private_key() diff --git a/src/apps/wallet/sign_tx/__init__.py b/src/apps/wallet/sign_tx/__init__.py index 24d889a6c7..a32c2b8126 100644 --- a/src/apps/wallet/sign_tx/__init__.py +++ b/src/apps/wallet/sign_tx/__init__.py @@ -3,7 +3,7 @@ from trezor.messages.MessageType import TxAck from trezor.messages.RequestType import TXFINISHED from trezor.messages.TxRequest import TxRequest -from apps.common import coins, seed +from apps.common import coins, paths, seed from apps.wallet.sign_tx.helpers import ( UiConfirmFeeOverThreshold, UiConfirmForeignAddress, @@ -50,7 +50,7 @@ async def sign_tx(ctx, msg): res = await layout.confirm_feeoverthreshold(ctx, req.fee, req.coin) progress.report_init() elif isinstance(req, UiConfirmForeignAddress): - res = await layout.confirm_foreign_address(ctx, req.address_n, req.coin) + res = await paths.show_path_warning(ctx, req.address_n) else: raise TypeError("Invalid signing instruction") return req diff --git a/src/apps/wallet/sign_tx/addresses.py b/src/apps/wallet/sign_tx/addresses.py index 677a5b21ee..3a14090bd2 100644 --- a/src/apps/wallet/sign_tx/addresses.py +++ b/src/apps/wallet/sign_tx/addresses.py @@ -5,7 +5,7 @@ from trezor.crypto.hashlib import sha256 from trezor.messages import FailureType, InputScriptType from trezor.utils import ensure -from apps.common import address_type +from apps.common import HARDENED, address_type, paths from apps.common.coininfo import CoinInfo from apps.wallet.sign_tx.multisig import multisig_get_pubkeys, multisig_pubkey_index from apps.wallet.sign_tx.scripts import ( @@ -190,3 +190,90 @@ def address_short(coin: CoinInfo, address: str) -> str: return address[len(coin.cashaddr_prefix) + 1 :] else: return address + + +def validate_full_path( + path: list, coin: CoinInfo, script_type: InputScriptType +) -> bool: + """ + Validates derivation path to fit Bitcoin-like coins. We mostly use + 44', but for segwit-enabled coins we use either 49' (P2WPKH-nested-in-P2SH) + or 84' (native P2WPKH). Electrum uses m/45' for legacy addresses and + m/48' for segwit, so those two are allowed as well. + + See docs/coins for what paths are allowed. Please note that this is not + a comprehensive check, some nuances are omitted for simplification. + """ + if len(path) != 5: + return False + + if not validate_purpose(path[0], coin): + return False + if not validate_purpose_against_script_type(path[0], script_type): + return False + + if path[1] != coin.slip44 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 20 | HARDENED: + return False + if path[3] not in [0, 1]: + return False + if path[4] > 1000000: + return False + return True + + +def validate_purpose(purpose: int, coin: CoinInfo) -> bool: + if purpose not in (44 | HARDENED, 48 | HARDENED, 49 | HARDENED, 84 | HARDENED): + return False + if not coin.segwit and purpose not in (44 | HARDENED, 48 | HARDENED): + return False + return True + + +def validate_purpose_against_script_type( + purpose: int, script_type: InputScriptType +) -> bool: + """ + Validates purpose against provided input's script type: + - 44 for spending address (script_type == SPENDADDRESS) + - 48 for multisig (script_type == SPENDMULTISIG) + - 49 for p2sh-segwit spend (script_type == SPENDP2SHWITNESS) + - 84 for native segwit spend (script_type == SPENDWITNESS) + """ + if purpose == 44 | HARDENED and script_type != InputScriptType.SPENDADDRESS: + return False + if purpose == 48 | HARDENED and script_type != InputScriptType.SPENDMULTISIG: + return False + if ( # p2wsh-nested-in-p2sh + purpose == 49 | HARDENED and script_type != InputScriptType.SPENDP2SHWITNESS + ): + return False + if ( # p2wsh + purpose == 84 | HARDENED and script_type != InputScriptType.SPENDWITNESS + ): + return False + return True + + +def validate_path_for_bitcoin_public_key(path: list, coin: CoinInfo) -> bool: + """ + Validates derivation path to fit Bitcoin-like coins for GetPublicKey. + Script type is omitted here because it is not usually sent. + """ + length = len(path) + if length < 3 or length > 5: + return False + + if not validate_purpose(path[0], coin): + return False + + if path[1] != coin.slip44 | HARDENED: + return False + if path[2] < HARDENED or path[2] > 20 | HARDENED: + return False + if length > 3 and paths.is_hardened(path[3]): + return False + if length > 4 and paths.is_hardened(path[4]): + return False + return True diff --git a/src/apps/wallet/sign_tx/helpers.py b/src/apps/wallet/sign_tx/helpers.py index 837721ea0d..fb3a2884ee 100644 --- a/src/apps/wallet/sign_tx/helpers.py +++ b/src/apps/wallet/sign_tx/helpers.py @@ -39,9 +39,8 @@ class UiConfirmFeeOverThreshold: class UiConfirmForeignAddress: - def __init__(self, address_n: list, coin: CoinInfo): + def __init__(self, address_n: list): self.address_n = address_n - self.coin = coin def confirm_output(output: TxOutputType, coin: CoinInfo): @@ -56,8 +55,8 @@ def confirm_feeoverthreshold(fee: int, coin: CoinInfo): return (yield UiConfirmFeeOverThreshold(fee, coin)) -def confirm_foreign_address(address_n: list, coin: CoinInfo): - return (yield UiConfirmForeignAddress(address_n, coin)) +def confirm_foreign_address(address_n: list): + return (yield UiConfirmForeignAddress(address_n)) def request_tx_meta(tx_req: TxRequest, tx_hash: bytes = None): diff --git a/src/apps/wallet/sign_tx/signing.py b/src/apps/wallet/sign_tx/signing.py index b2a8d43f82..0a67e260dd 100644 --- a/src/apps/wallet/sign_tx/signing.py +++ b/src/apps/wallet/sign_tx/signing.py @@ -107,8 +107,8 @@ async def check_tx_fee(tx: SignTx, root: bip32.HDNode): hash143.add_prevouts(txi) # all inputs are included (non-segwit as well) hash143.add_sequence(txi) - if not address_n_matches_coin(txi.address_n, coin): - await confirm_foreign_address(txi.address_n, coin) + if not validate_full_path(txi.address_n, coin, txi.script_type): + await confirm_foreign_address(txi.address_n) if txi.multisig: multifp.add(txi.multisig) @@ -828,16 +828,6 @@ def node_derive(root: bip32.HDNode, address_n: list) -> bip32.HDNode: return node -def address_n_matches_coin(address_n: list, coin: CoinInfo) -> bool: - if len(address_n) < 2: - return True # path is too short - if address_n[0] not in (44 | 0x80000000, 49 | 0x80000000, 84 | 0x80000000): - return True # path is not BIP44/49/84 - return address_n[1] == ( - coin.slip44 | 0x80000000 - ) # check whether coin_type matches slip44 value - - def ecdsa_sign(node: bip32.HDNode, digest: bytes) -> bytes: sig = secp256k1.sign(node.private_key(), digest) sigder = der.encode_seq((sig[1:33], sig[33:65])) diff --git a/tests/test_apps.cardano.address.py b/tests/test_apps.cardano.address.py index 12454d3db0..23cac3f621 100644 --- a/tests/test_apps.cardano.address.py +++ b/tests/test_apps.cardano.address.py @@ -1,11 +1,11 @@ from common import * from apps.common import seed -from trezor import wire +from apps.common import HARDENED from apps.cardano.address import ( _get_address_root, _address_hash, - validate_derivation_path, + validate_full_path, derive_address_and_node ) from trezor.crypto import bip32 @@ -131,27 +131,32 @@ class TestCardanoAddress(unittest.TestCase): self.assertEqual(result, b'\x1c\xca\xee\xc9\x80\xaf}\xb0\x9a\xa8\x96E\xd6\xa4\xd1\xb4\x13\x85\xb9\xc2q\x1d5/{\x12"\xca') - def test_validate_derivation_path(self): + def test_paths(self): incorrect_derivation_paths = [ - [0x80000000 | 44], - [0x80000000 | 44, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815], - [0x80000000 | 43, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815], - [0x80000000 | 44, 0x80000000 | 1816, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815], + [HARDENED | 44], + [HARDENED | 44, HARDENED | 1815, HARDENED | 1815, HARDENED | 1815, HARDENED | 1815, HARDENED | 1815], + [HARDENED | 43, HARDENED | 1815, HARDENED | 1815, HARDENED | 1815, HARDENED | 1815], + [HARDENED | 44, HARDENED | 1816, HARDENED | 1815, HARDENED | 1815, HARDENED | 1815], + [HARDENED | 44, HARDENED | 1815, 0], + [HARDENED | 44, HARDENED | 1815, 0, 0], + [HARDENED | 44, HARDENED | 1815], + [HARDENED | 44, HARDENED | 1815, HARDENED | 0], + [HARDENED | 44, HARDENED | 1815, HARDENED | 1815, 1, 1], + [HARDENED | 44, HARDENED | 1815, HARDENED | 1815, 0, 0], # a too large ] - correct_derivation_paths = [ - [0x80000000 | 44, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815, 0x80000000 | 1815], - [0x80000000 | 44, 0x80000000 | 1815], - [0x80000000 | 44, 0x80000000 | 1815, 0x80000000], - [0x80000000 | 44, 0x80000000 | 1815, 0], - [0x80000000 | 44, 0x80000000 | 1815, 0, 0], + [HARDENED | 44, HARDENED | 1815, HARDENED | 0, 0, 1], + [HARDENED | 44, HARDENED | 1815, HARDENED | 9, 0, 4], + [HARDENED | 44, HARDENED | 1815, HARDENED | 0, 0, 9], + [HARDENED | 44, HARDENED | 1815, HARDENED | 0, 1, 1], + [HARDENED | 44, HARDENED | 1815, HARDENED | 0, 1, 9], ] - for derivation_path in incorrect_derivation_paths: - self.assertRaises(wire.ProcessError, validate_derivation_path, derivation_path) + for path in incorrect_derivation_paths: + self.assertFalse(validate_full_path(path)) - for derivation_path in correct_derivation_paths: - self.assertEqual(derivation_path, validate_derivation_path(derivation_path)) + for path in correct_derivation_paths: + self.assertTrue(validate_full_path(path)) def test_get_address_root_scheme(self): mnemonic = "all all all all all all all all all all all all" diff --git a/tests/test_apps.common.paths.py b/tests/test_apps.common.paths.py new file mode 100644 index 0000000000..431e82c3ce --- /dev/null +++ b/tests/test_apps.common.paths.py @@ -0,0 +1,46 @@ +from common import * +from apps.common import HARDENED +from apps.common.paths import validate_path_for_get_public_key, is_hardened + + +class TestPaths(unittest.TestCase): + + def test_is_hardened(self): + self.assertTrue(is_hardened(44 | HARDENED)) + self.assertTrue(is_hardened(0 | HARDENED)) + self.assertTrue(is_hardened(99999 | HARDENED)) + + self.assertFalse(is_hardened(44)) + self.assertFalse(is_hardened(0)) + self.assertFalse(is_hardened(99999)) + + def test_path_for_get_public_key(self): + # 44'/41'/0' + self.assertTrue(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0 | HARDENED], 41)) + # 44'/111'/0' + self.assertTrue(validate_path_for_get_public_key([44 | HARDENED, 111 | HARDENED, 0 | HARDENED], 111)) + # 44'/0'/0'/0 + self.assertTrue(validate_path_for_get_public_key([44 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0], 0)) + # 44'/0'/0'/0/0 + self.assertTrue(validate_path_for_get_public_key([44 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0], 0)) + + # 44'/41' + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED], 41)) + # 44'/41'/0 + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0], 41)) + # 44'/41'/0' slip44 mismatch + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0 | HARDENED], 99)) + # # 44'/41'/0'/0' + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0 | HARDENED, 0 | HARDENED], 41)) + # # 44'/41'/0'/0'/0 + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0], 41)) + # # 44'/41'/0'/0'/0' + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], 41)) + # # 44'/41'/0'/0/0/0 + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0 | HARDENED, 0, 0, 0], 41)) + # # 44'/41'/0'/0/0' + self.assertFalse(validate_path_for_get_public_key([44 | HARDENED, 41 | HARDENED, 0 | HARDENED, 0, 0 | HARDENED], 41)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_apps.ethereum.get_address.py b/tests/test_apps.ethereum.address.py similarity index 62% rename from tests/test_apps.ethereum.get_address.py rename to tests/test_apps.ethereum.address.py index 490e04889d..a59d4c7771 100644 --- a/tests/test_apps.ethereum.get_address.py +++ b/tests/test_apps.ethereum.address.py @@ -1,5 +1,6 @@ from common import * -from apps.ethereum.get_address import _ethereum_address_hex +from apps.common.paths import HARDENED +from apps.ethereum.address import ethereum_address_hex, validate_full_path from apps.ethereum.networks import NetworkInfo @@ -20,7 +21,7 @@ class TestEthereumGetAddress(unittest.TestCase): for s in eip55: s = s[2:] b = bytes([int(s[i:i + 2], 16) for i in range(0, len(s), 2)]) - h = _ethereum_address_hex(b) + h = ethereum_address_hex(b) self.assertEqual(h, '0x' + s) def test_ethereum_address_hex_rskip60(self): @@ -41,15 +42,39 @@ class TestEthereumGetAddress(unittest.TestCase): for s in rskip60_chain_30: s = s[2:] b = bytes([int(s[i:i + 2], 16) for i in range(0, len(s), 2)]) - h = _ethereum_address_hex(b, n) + h = ethereum_address_hex(b, n) self.assertEqual(h, '0x' + s) n.chain_id = 31 for s in rskip60_chain_31: s = s[2:] b = bytes([int(s[i:i + 2], 16) for i in range(0, len(s), 2)]) - h = _ethereum_address_hex(b, n) + h = ethereum_address_hex(b, n) self.assertEqual(h, '0x' + s) + def test_paths(self): + # 44'/60'/0'/0/i is correct + incorrect_paths = [ + [44 | HARDENED], + [44 | HARDENED, 60 | HARDENED], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 0, 0], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 1, 0], + [44 | HARDENED, 60 | HARDENED, 1 | HARDENED, 0, 0], + [44 | HARDENED, 160 | HARDENED, 0 | HARDENED, 0, 0], + ] + correct_paths = [ + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 0], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 9], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 9999], + ] + + for path in incorrect_paths: + self.assertFalse(validate_full_path(path)) + + for path in correct_paths: + self.assertTrue(validate_full_path(path)) + if __name__ == '__main__': unittest.main() diff --git a/tests/test_apps.lisk.address.py b/tests/test_apps.lisk.address.py new file mode 100644 index 0000000000..cb91912049 --- /dev/null +++ b/tests/test_apps.lisk.address.py @@ -0,0 +1,36 @@ +from common import * +from apps.common.paths import HARDENED +from apps.lisk.helpers import validate_full_path + + +class TestLiskGetAddress(unittest.TestCase): + + def test_paths(self): + # 44'/134'/a' is correct + incorrect_paths = [ + [44 | HARDENED], + [44 | HARDENED, 134 | HARDENED], + [44 | HARDENED, 134 | HARDENED, 0], + [44 | HARDENED, 134 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 134 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 134 | HARDENED, 0 | HARDENED, 1, 0], + [44 | HARDENED, 134 | HARDENED, 0 | HARDENED, 0, 0], + [44 | HARDENED, 134 | HARDENED, 9999000 | HARDENED], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 0], + [1 | HARDENED, 1 | HARDENED, 1 | HARDENED], + ] + correct_paths = [ + [44 | HARDENED, 134 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 134 | HARDENED, 3 | HARDENED], + [44 | HARDENED, 134 | HARDENED, 9 | HARDENED], + ] + + for path in incorrect_paths: + self.assertFalse(validate_full_path(path)) + + for path in correct_paths: + self.assertTrue(validate_full_path(path)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_apps.monero.address.py b/tests/test_apps.monero.address.py new file mode 100644 index 0000000000..3c3376f321 --- /dev/null +++ b/tests/test_apps.monero.address.py @@ -0,0 +1,36 @@ +from common import * +from apps.common.paths import HARDENED +from apps.monero.misc import validate_full_path + + +class TestMoneroGetAddress(unittest.TestCase): + + def test_paths(self): + # 44'/128'/a' is correct + incorrect_paths = [ + [44 | HARDENED], + [44 | HARDENED, 128 | HARDENED], + [44 | HARDENED, 128 | HARDENED, 0], + [44 | HARDENED, 128 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 128 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 128 | HARDENED, 0 | HARDENED, 1, 0], + [44 | HARDENED, 128 | HARDENED, 0 | HARDENED, 0, 0], + [44 | HARDENED, 128 | HARDENED, 9999000 | HARDENED], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 0], + [1 | HARDENED, 1 | HARDENED, 1 | HARDENED], + ] + correct_paths = [ + [44 | HARDENED, 128 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 128 | HARDENED, 3 | HARDENED], + [44 | HARDENED, 128 | HARDENED, 9 | HARDENED], + ] + + for path in incorrect_paths: + self.assertFalse(validate_full_path(path)) + + for path in correct_paths: + self.assertTrue(validate_full_path(path)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_apps.nem.address.py b/tests/test_apps.nem.address.py index dab6faa8e6..20b61ae723 100644 --- a/tests/test_apps.nem.address.py +++ b/tests/test_apps.nem.address.py @@ -1,7 +1,8 @@ from common import * from ubinascii import unhexlify from trezor.crypto import nem -from apps.nem.helpers import NEM_NETWORK_MAINNET, NEM_NETWORK_TESTNET +from apps.common import HARDENED +from apps.nem.helpers import check_path, NEM_NETWORK_MAINNET, NEM_NETWORK_TESTNET class TestNemAddress(unittest.TestCase): @@ -32,6 +33,47 @@ class TestNemAddress(unittest.TestCase): validity = nem.validate_address('NCUKWDY3J3THKQHAKOK5ALF6ANJQABZHCH7VN6DP', NEM_NETWORK_TESTNET) self.assertFalse(validity) + def test_paths(self): + # 44'/43'/0'/0'/0' + self.assertTrue(check_path([44 | HARDENED, 43 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED])) + # 44'/43'/0'/0'/0' + self.assertTrue(check_path([44 | HARDENED, 43 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0 | HARDENED])) + # 44'/1'/0'/0'/0' testnet + self.assertTrue(check_path([44 | HARDENED, 1 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0 | HARDENED], network=0x98)) + # 44'/43'/0' + self.assertTrue(check_path([44 | HARDENED, 43 | HARDENED, 0 | HARDENED])) + # 44'/43'/2' + self.assertTrue(check_path([44 | HARDENED, 43 | HARDENED, 2 | HARDENED])) + # 44'/1'/0' testnet + self.assertTrue(check_path([44 | HARDENED, 1 | HARDENED, 0 | HARDENED], network=0x98)) + + # 44'/43'/0'/0'/1' + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0 | HARDENED, 0 | HARDENED, 1 | HARDENED])) + # 44'/43'/0'/1'/1' + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0 | HARDENED, 1 | HARDENED, 1 | HARDENED])) + # 44'/43'/0'/1'/0' + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0 | HARDENED, 1 | HARDENED, 0 | HARDENED])) + # 44'/43'/99999'/0'/0' + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 99999000 | HARDENED, 0 | HARDENED, 0 | HARDENED])) + # 44'/99'/0'/0'/0' + self.assertFalse(check_path([44 | HARDENED, 99 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED])) + # 1'/43'/0'/0'/0' + self.assertFalse(check_path([1 | HARDENED, 43 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED])) + # 44'/43'/0'/0/0 + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0 | HARDENED, 0, 0])) + # 44'/43'/0'/0/5 + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0 | HARDENED, 0, 5])) + # 44'/1'/3'/0'/1' testnet + self.assertFalse(check_path([44 | HARDENED, 1 | HARDENED, 3 | HARDENED, 0 | HARDENED, 1 | HARDENED], network=0x98)) + + # 44'/43'/0/0/1 + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0, 0, 1])) + # 44'/43'/0/0/0 + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0, 0, 0])) + # 44'/43'/0/0'/0' + self.assertFalse(check_path([44 | HARDENED, 43 | HARDENED, 0, 0 | HARDENED, 0 | HARDENED])) + + if __name__ == '__main__': unittest.main() diff --git a/tests/test_apps.ripple.address.py b/tests/test_apps.ripple.address.py new file mode 100644 index 0000000000..188769c575 --- /dev/null +++ b/tests/test_apps.ripple.address.py @@ -0,0 +1,47 @@ +from common import * +from apps.common.paths import HARDENED +from apps.ripple.helpers import address_from_public_key, validate_full_path + + +class TestRippleAddress(unittest.TestCase): + + def test_pubkey_to_address(self): + addr = address_from_public_key(unhexlify('ed9434799226374926eda3b54b1b461b4abf7237962eae18528fea67595397fa32')) + self.assertEqual(addr, 'rDTXLQ7ZKZVKz33zJbHjgVShjsBnqMBhmN') + + addr = address_from_public_key(unhexlify('03e2b079e9b09ae8916da8f5ee40cbda9578dbe7c820553fe4d5f872eec7b1fdd4')) + self.assertEqual(addr, 'rhq549rEtUrJowuxQC2WsHNGLjAjBQdAe8') + + addr = address_from_public_key(unhexlify('0282ee731039929e97db6aec242002e9aa62cd62b989136df231f4bb9b8b7c7eb2')) + self.assertEqual(addr, 'rKzE5DTyF9G6z7k7j27T2xEas2eMo85kmw') + + def test_paths(self): + # 44'/144'/a'/0/0 is correct + incorrect_paths = [ + [44 | HARDENED], + [44 | HARDENED, 144 | HARDENED], + [44 | HARDENED, 144 | HARDENED, 0], + [44 | HARDENED, 144 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 144 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 144 | HARDENED, 0 | HARDENED, 1, 0], + [44 | HARDENED, 144 | HARDENED, 0 | HARDENED, 0, 5], + [44 | HARDENED, 144 | HARDENED, 9999 | HARDENED], + [44 | HARDENED, 144 | HARDENED, 9999000 | HARDENED, 0, 0], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 0], + [1 | HARDENED, 1 | HARDENED, 1 | HARDENED], + ] + correct_paths = [ + [44 | HARDENED, 144 | HARDENED, 0 | HARDENED, 0, 0], + [44 | HARDENED, 144 | HARDENED, 3 | HARDENED, 0, 0], + [44 | HARDENED, 144 | HARDENED, 9 | HARDENED, 0, 0], + ] + + for path in incorrect_paths: + self.assertFalse(validate_full_path(path)) + + for path in correct_paths: + self.assertTrue(validate_full_path(path)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_apps.ripple.pubkey_to_address.py b/tests/test_apps.ripple.pubkey_to_address.py deleted file mode 100644 index 7ad5a51018..0000000000 --- a/tests/test_apps.ripple.pubkey_to_address.py +++ /dev/null @@ -1,19 +0,0 @@ -from common import * -from apps.ripple.helpers import address_from_public_key - - -class TestStellarPubkeyToAddress(unittest.TestCase): - - def test_pubkey_to_address(self): - addr = address_from_public_key(unhexlify('ed9434799226374926eda3b54b1b461b4abf7237962eae18528fea67595397fa32')) - self.assertEqual(addr, 'rDTXLQ7ZKZVKz33zJbHjgVShjsBnqMBhmN') - - addr = address_from_public_key(unhexlify('03e2b079e9b09ae8916da8f5ee40cbda9578dbe7c820553fe4d5f872eec7b1fdd4')) - self.assertEqual(addr, 'rhq549rEtUrJowuxQC2WsHNGLjAjBQdAe8') - - addr = address_from_public_key(unhexlify('0282ee731039929e97db6aec242002e9aa62cd62b989136df231f4bb9b8b7c7eb2')) - self.assertEqual(addr, 'rKzE5DTyF9G6z7k7j27T2xEas2eMo85kmw') - - -if __name__ == '__main__': - unittest.main() diff --git a/tests/test_apps.stellar.address_to_pubkey.py b/tests/test_apps.stellar.address.py similarity index 60% rename from tests/test_apps.stellar.address_to_pubkey.py rename to tests/test_apps.stellar.address.py index 2beb1c4e8f..a261def40c 100644 --- a/tests/test_apps.stellar.address_to_pubkey.py +++ b/tests/test_apps.stellar.address.py @@ -1,9 +1,10 @@ from common import * -from apps.stellar.helpers import address_from_public_key, public_key_from_address +from apps.common.paths import HARDENED +from apps.stellar.helpers import address_from_public_key, public_key_from_address, validate_full_path from trezor.wire import ProcessError -class TestStellarAddressToPubkey(unittest.TestCase): +class TestStellarAddress(unittest.TestCase): def test_address_to_pubkey(self): self.assertEqual(public_key_from_address('GBOVKZBEM2YYLOCDCUXJ4IMRKHN4LCJAE7WEAEA2KF562XFAGDBOB64V'), @@ -33,6 +34,32 @@ class TestStellarAddressToPubkey(unittest.TestCase): with self.assertRaises(ProcessError): public_key_from_address('GCN2K2HG53AWX2SP5UHRPMJUUHLJF2XBTGSXROTPWRGAYJCDDP63J2AA') # invalid checksum + def test_paths(self): + # 44'/148'/a' is correct + incorrect_paths = [ + [44 | HARDENED], + [44 | HARDENED, 148 | HARDENED], + [44 | HARDENED, 148 | HARDENED, 0], + [44 | HARDENED, 148 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 148 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 148 | HARDENED, 0 | HARDENED, 1, 0], + [44 | HARDENED, 148 | HARDENED, 0 | HARDENED, 0, 0], + [44 | HARDENED, 148 | HARDENED, 9999000 | HARDENED], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 0], + [1 | HARDENED, 1 | HARDENED, 1 | HARDENED], + ] + correct_paths = [ + [44 | HARDENED, 148 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 148 | HARDENED, 3 | HARDENED], + [44 | HARDENED, 148 | HARDENED, 9 | HARDENED], + ] + + for path in incorrect_paths: + self.assertFalse(validate_full_path(path)) + + for path in correct_paths: + self.assertTrue(validate_full_path(path)) + if __name__ == '__main__': unittest.main() diff --git a/tests/test_apps.tezos.address.py b/tests/test_apps.tezos.address.py index 0640db4ee2..9b093f9e0b 100644 --- a/tests/test_apps.tezos.address.py +++ b/tests/test_apps.tezos.address.py @@ -5,6 +5,8 @@ from trezor.messages import TezosContractType from trezor.messages.TezosContractID import TezosContractID from apps.tezos.sign_tx import _get_address_from_contract +from apps.tezos.helpers import validate_full_path +from apps.common.paths import HARDENED class TestTezosAddress(unittest.TestCase): @@ -38,6 +40,32 @@ class TestTezosAddress(unittest.TestCase): for i, contract in enumerate(contracts): self.assertEqual(_get_address_from_contract(contract), outputs[i]) + def test_paths(self): + # 44'/1729'/a' is correct + incorrect_paths = [ + [44 | HARDENED], + [44 | HARDENED, 1729 | HARDENED], + [44 | HARDENED, 1729 | HARDENED, 0], + [44 | HARDENED, 1729 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 1729 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 1729 | HARDENED, 0 | HARDENED, 1, 0], + [44 | HARDENED, 1729 | HARDENED, 0 | HARDENED, 0, 0], + [44 | HARDENED, 1729 | HARDENED, 9999000 | HARDENED], + [44 | HARDENED, 60 | HARDENED, 0 | HARDENED, 0, 0], + [1 | HARDENED, 1 | HARDENED, 1 | HARDENED], + ] + correct_paths = [ + [44 | HARDENED, 1729 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 1729 | HARDENED, 3 | HARDENED], + [44 | HARDENED, 1729 | HARDENED, 9 | HARDENED], + ] + + for path in incorrect_paths: + self.assertFalse(validate_full_path(path)) + + for path in correct_paths: + self.assertTrue(validate_full_path(path)) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_apps.wallet.address.py b/tests/test_apps.wallet.address.py index 1312258571..7ffdef6b8b 100644 --- a/tests/test_apps.wallet.address.py +++ b/tests/test_apps.wallet.address.py @@ -1,6 +1,8 @@ from common import * from trezor.crypto import bip32, bip39 +from apps.wallet.sign_tx.addresses import validate_full_path, validate_path_for_bitcoin_public_key +from apps.common.paths import HARDENED from apps.common import coins from apps.wallet.sign_tx.signing import * @@ -114,6 +116,133 @@ class TestAddress(unittest.TestCase): # def test_multisig_address_p2wsh(self): # todo couldn't find test data + def test_paths_btc(self): + incorrect_derivation_paths = [ + ([49 | HARDENED], InputScriptType.SPENDP2SHWITNESS), # invalid length + ([49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], InputScriptType.SPENDP2SHWITNESS), # too many HARDENED + ([49 | HARDENED, 0 | HARDENED], InputScriptType.SPENDP2SHWITNESS), # invalid length + ([49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0, 0], InputScriptType.SPENDP2SHWITNESS), # invalid length + ([49 | HARDENED, 123 | HARDENED, 0 | HARDENED, 0, 0, 0], InputScriptType.SPENDP2SHWITNESS), # invalid slip44 + ([49 | HARDENED, 0 | HARDENED, 1000 | HARDENED, 0, 0], InputScriptType.SPENDP2SHWITNESS), # account too high + ([49 | HARDENED, 0 | HARDENED, 1 | HARDENED, 2, 0], InputScriptType.SPENDP2SHWITNESS), # invalid y + ([49 | HARDENED, 0 | HARDENED, 1 | HARDENED, 0, 10000000], InputScriptType.SPENDP2SHWITNESS), # address index too high + ([84 | HARDENED, 0 | HARDENED, 1 | HARDENED, 0, 10000000], InputScriptType.SPENDWITNESS), # address index too high + ([49 | HARDENED, 0 | HARDENED, 1 | HARDENED, 0, 0], InputScriptType.SPENDWITNESS), # invalid input type + ([84 | HARDENED, 0 | HARDENED, 1 | HARDENED, 0, 0], InputScriptType.SPENDP2SHWITNESS), # invalid input type + ([49 | HARDENED, 0 | HARDENED, 5 | HARDENED, 0, 10], InputScriptType.SPENDMULTISIG), # invalid input type + ] + correct_derivation_paths = [ + ([44 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDADDRESS), # btc is segwit coin, but non-segwit paths are allowed as well + ([44 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 1], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 0 | HARDENED, 0 | HARDENED, 1, 0], InputScriptType.SPENDADDRESS), + ([49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDP2SHWITNESS), + ([49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 1, 0], InputScriptType.SPENDP2SHWITNESS), + ([49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 1123], InputScriptType.SPENDP2SHWITNESS), + ([49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 1, 44444], InputScriptType.SPENDP2SHWITNESS), + ([49 | HARDENED, 0 | HARDENED, 5 | HARDENED, 0, 0], InputScriptType.SPENDP2SHWITNESS), + ([84 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDWITNESS), + ([84 | HARDENED, 0 | HARDENED, 5 | HARDENED, 0, 0], InputScriptType.SPENDWITNESS), + ([84 | HARDENED, 0 | HARDENED, 5 | HARDENED, 0, 10], InputScriptType.SPENDWITNESS), + ([48 | HARDENED, 0 | HARDENED, 5 | HARDENED, 0, 10], InputScriptType.SPENDMULTISIG), + ] + coin = coins.by_shortcut('BTC') + for path, input_type in incorrect_derivation_paths: + self.assertFalse(validate_full_path(path, coin, input_type)) + + for path, input_type in correct_derivation_paths: + self.assertTrue(validate_full_path(path, coin, input_type)) + + def test_paths_bch(self): + incorrect_derivation_paths = [ + ([44 | HARDENED], InputScriptType.SPENDADDRESS), # invalid length + ([44 | HARDENED, 145 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], InputScriptType.SPENDADDRESS), # too many HARDENED + ([49 | HARDENED, 145 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDP2SHWITNESS), # bch is not segwit coin so 49' is not allowed + ([84 | HARDENED, 145 | HARDENED, 1 | HARDENED, 0, 1], InputScriptType.SPENDWITNESS), # and neither is 84' + ([44 | HARDENED, 145 | HARDENED], InputScriptType.SPENDADDRESS), # invalid length + ([44 | HARDENED, 145 | HARDENED, 0 | HARDENED, 0, 0, 0], InputScriptType.SPENDADDRESS), # invalid length + ([44 | HARDENED, 123 | HARDENED, 0 | HARDENED, 0, 0, 0], InputScriptType.SPENDADDRESS), # invalid slip44 + ([44 | HARDENED, 145 | HARDENED, 1000 | HARDENED, 0, 0], InputScriptType.SPENDADDRESS), # account too high + ([44 | HARDENED, 145 | HARDENED, 1 | HARDENED, 2, 0], InputScriptType.SPENDADDRESS), # invalid y + ([44 | HARDENED, 145 | HARDENED, 1 | HARDENED, 0, 10000000], InputScriptType.SPENDADDRESS), # address index too high + ([84 | HARDENED, 145 | HARDENED, 1 | HARDENED, 0, 10000000], InputScriptType.SPENDWITNESS), # address index too high + ([44 | HARDENED, 145 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDWITNESS), # input type mismatch + ] + correct_derivation_paths = [ + ([44 | HARDENED, 145 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 145 | HARDENED, 0 | HARDENED, 1, 0], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 145 | HARDENED, 0 | HARDENED, 0, 1123], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 145 | HARDENED, 0 | HARDENED, 1, 44444], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 145 | HARDENED, 5 | HARDENED, 0, 0], InputScriptType.SPENDADDRESS), + ([48 | HARDENED, 145 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDMULTISIG), + ([48 | HARDENED, 145 | HARDENED, 5 | HARDENED, 0, 0], InputScriptType.SPENDMULTISIG), + ([48 | HARDENED, 145 | HARDENED, 5 | HARDENED, 0, 10], InputScriptType.SPENDMULTISIG), + ] + coin = coins.by_shortcut('BCH') # segwit is disabled + for path, input_type in incorrect_derivation_paths: + self.assertFalse(validate_full_path(path, coin, input_type)) + + for path, input_type in correct_derivation_paths: + self.assertTrue(validate_full_path(path, coin, input_type)) + + def test_paths_other(self): + incorrect_derivation_paths = [ + ([44 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDMULTISIG), # input type mismatch + ] + correct_derivation_paths = [ + ([44 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0, 0], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 3 | HARDENED, 0 | HARDENED, 1, 0], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0, 1123], InputScriptType.SPENDADDRESS), + ([44 | HARDENED, 3 | HARDENED, 0 | HARDENED, 1, 44444], InputScriptType.SPENDADDRESS), + ] + coin = coins.by_shortcut('DOGE') # segwit is disabled + for path, input_type in correct_derivation_paths: + self.assertTrue(validate_full_path(path, coin, input_type)) + + for path, input_type in incorrect_derivation_paths: + self.assertFalse(validate_full_path(path, coin, input_type)) + + def test_paths_public_key(self): + incorrect_derivation_paths = [ + [49 | HARDENED], # invalid length + [49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0 | HARDENED], # too many HARDENED + [49 | HARDENED, 0 | HARDENED], # invalid length + [49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0, 0], # invalid length + [49 | HARDENED, 123 | HARDENED, 0 | HARDENED, 0, 0, 0], # invalid slip44 + [49 | HARDENED, 0 | HARDENED, 1000 | HARDENED, 0, 0], # account too high + ] + correct_derivation_paths = [ + [44 | HARDENED, 0 | HARDENED, 0 | HARDENED], # btc is segwit coin, but non-segwit paths are allowed as well + [44 | HARDENED, 0 | HARDENED, 0 | HARDENED, 1, 0], + [49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0], + [49 | HARDENED, 0 | HARDENED, 0 | HARDENED, 1, 0], + [49 | HARDENED, 0 | HARDENED, 5 | HARDENED], + [84 | HARDENED, 0 | HARDENED, 0 | HARDENED, 0, 0], + [84 | HARDENED, 0 | HARDENED, 5 | HARDENED, 0, 0], + [84 | HARDENED, 0 | HARDENED, 5 | HARDENED, 0, 10], + ] + coin = coins.by_shortcut('BTC') + for path in correct_derivation_paths: + self.assertTrue(validate_path_for_bitcoin_public_key(path, coin)) + + for path in incorrect_derivation_paths: + self.assertFalse(validate_path_for_bitcoin_public_key(path, coin)) + + incorrect_derivation_paths = [ + [49 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0, 0], # no segwit + ] + correct_derivation_paths = [ + [44 | HARDENED, 3 | HARDENED, 0 | HARDENED], + [44 | HARDENED, 3 | HARDENED, 1 | HARDENED], + [44 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0], + [44 | HARDENED, 3 | HARDENED, 0 | HARDENED, 0, 0], + ] + coin = coins.by_shortcut('DOGE') # segwit is disabled + for path in correct_derivation_paths: + self.assertTrue(validate_path_for_bitcoin_public_key(path, coin)) + + for path in incorrect_derivation_paths: + self.assertFalse(validate_path_for_bitcoin_public_key(path, coin)) + if __name__ == '__main__': unittest.main() diff --git a/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py b/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py index 6530573d2d..c09f7d48ed 100644 --- a/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py +++ b/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py @@ -61,6 +61,9 @@ class TestSignSegwitTxNativeP2WPKH(unittest.TestCase): TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None)), TxAck(tx=TransactionType(inputs=[inp1])), + signing.UiConfirmForeignAddress(address_n=inp1.address_n), + True, + TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None), serialized=None), TxAck(tx=TransactionType(outputs=[out1])), @@ -156,6 +159,9 @@ class TestSignSegwitTxNativeP2WPKH(unittest.TestCase): TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None)), TxAck(tx=TransactionType(inputs=[inp1])), + signing.UiConfirmForeignAddress(address_n=inp1.address_n), + True, + TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None), serialized=None), TxAck(tx=TransactionType(outputs=[out1])), @@ -212,7 +218,8 @@ class TestSignSegwitTxNativeP2WPKH(unittest.TestCase): def assertEqualEx(self, a, b): # hack to avoid adding __eq__ to signing.Ui* classes if ((isinstance(a, signing.UiConfirmOutput) and isinstance(b, signing.UiConfirmOutput)) or - (isinstance(a, signing.UiConfirmTotal) and isinstance(b, signing.UiConfirmTotal))): + (isinstance(a, signing.UiConfirmTotal) and isinstance(b, signing.UiConfirmTotal)) or + (isinstance(a, signing.UiConfirmForeignAddress) and isinstance(b, signing.UiConfirmForeignAddress))): return self.assertEqual(a.__dict__, b.__dict__) else: return self.assertEqual(a, b) diff --git a/tests/test_apps.wallet.signtx.fee_threshold.py b/tests/test_apps.wallet.signtx.fee_threshold.py index 805c1e12f0..cecaa6da35 100644 --- a/tests/test_apps.wallet.signtx.fee_threshold.py +++ b/tests/test_apps.wallet.signtx.fee_threshold.py @@ -57,8 +57,11 @@ class TestSignTxFeeThreshold(unittest.TestCase): messages = [ None, + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None)), TxAck(tx=TransactionType(inputs=[inp1])), + signing.UiConfirmForeignAddress(address_n=inp1.address_n), + True, TxRequest(request_type=TXMETA, details=TxRequestDetailsType(request_index=None, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), TxAck(tx=ptx1), TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), @@ -121,8 +124,11 @@ class TestSignTxFeeThreshold(unittest.TestCase): messages = [ None, + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None)), TxAck(tx=TransactionType(inputs=[inp1])), + signing.UiConfirmForeignAddress(address_n=inp1.address_n), + True, TxRequest(request_type=TXMETA, details=TxRequestDetailsType(request_index=None, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), TxAck(tx=ptx1), TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), @@ -151,6 +157,7 @@ class TestSignTxFeeThreshold(unittest.TestCase): # hack to avoid adding __eq__ to signing.Ui* classes if ((isinstance(a, signing.UiConfirmOutput) and isinstance(b, signing.UiConfirmOutput)) or (isinstance(a, signing.UiConfirmTotal) and isinstance(b, signing.UiConfirmTotal)) or + (isinstance(a, signing.UiConfirmForeignAddress) and isinstance(b, signing.UiConfirmForeignAddress)) or (isinstance(a, signing.UiConfirmFeeOverThreshold) and isinstance(b, signing.UiConfirmFeeOverThreshold))): return self.assertEqual(a.__dict__, b.__dict__) else: diff --git a/tests/test_apps.wallet.signtx.py b/tests/test_apps.wallet.signtx.py index c51343d49a..1c1869aba3 100644 --- a/tests/test_apps.wallet.signtx.py +++ b/tests/test_apps.wallet.signtx.py @@ -58,8 +58,11 @@ class TestSignTx(unittest.TestCase): messages = [ None, + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None)), TxAck(tx=TransactionType(inputs=[inp1])), + signing.UiConfirmForeignAddress(address_n=inp1.address_n), + True, TxRequest(request_type=TXMETA, details=TxRequestDetailsType(request_index=None, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), TxAck(tx=ptx1), TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), @@ -107,6 +110,7 @@ class TestSignTx(unittest.TestCase): def assertEqualEx(self, a, b): # hack to avoid adding __eq__ to signing.Ui* classes if ((isinstance(a, signing.UiConfirmOutput) and isinstance(b, signing.UiConfirmOutput)) or + (isinstance(a, signing.UiConfirmForeignAddress) and isinstance(b, signing.UiConfirmForeignAddress)) or (isinstance(a, signing.UiConfirmTotal) and isinstance(b, signing.UiConfirmTotal))): return self.assertEqual(a.__dict__, b.__dict__) else: From 8cf1ee5e62a96dfc03e746f1d3f2d5d51ef21dcb Mon Sep 17 00:00:00 2001 From: Tomas Susanka Date: Thu, 8 Nov 2018 14:28:58 +0100 Subject: [PATCH 2/4] paths: temporarily disable GetPublicKey paths checks Until trezor/trezor.js#73 is fixed --- src/apps/wallet/get_public_key.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/apps/wallet/get_public_key.py b/src/apps/wallet/get_public_key.py index be8976d6dc..d18a6b308a 100644 --- a/src/apps/wallet/get_public_key.py +++ b/src/apps/wallet/get_public_key.py @@ -3,8 +3,7 @@ from trezor.messages import InputScriptType from trezor.messages.HDNodeType import HDNodeType from trezor.messages.PublicKey import PublicKey -from apps.common import coins, layout, paths, seed -from apps.wallet.sign_tx.addresses import validate_path_for_bitcoin_public_key +from apps.common import coins, layout, seed async def get_public_key(ctx, msg): @@ -12,10 +11,6 @@ async def get_public_key(ctx, msg): coin = coins.by_name(coin_name) script_type = msg.script_type or InputScriptType.SPENDADDRESS - await paths.validate_path( - ctx, validate_path_for_bitcoin_public_key, path=msg.address_n, coin=coin - ) - curve_name = msg.ecdsa_curve_name if not curve_name: curve_name = coin.curve_name From 2acf0d10bd4e18ec1749ab820c7e97fa829e46cb Mon Sep 17 00:00:00 2001 From: Tomas Susanka Date: Tue, 6 Nov 2018 13:46:45 +0100 Subject: [PATCH 3/4] TEMPORARY: run tests against tsusanka/paths python-trezor branch --- Pipfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pipfile b/Pipfile index 61c5230e25..2876083282 100644 --- a/Pipfile +++ b/Pipfile @@ -4,7 +4,7 @@ name = "pypi" verify_ssl = true [packages] -trezor = {git = "https://github.com/trezor/python-trezor", editable = true, ref = "master"} +trezor = {git = "https://github.com/trezor/python-trezor", editable = true, ref = "tsusanka/paths"} protobuf = "==3.4.0" # python-trezor tests From d83ef07d57191e1e6db958046f535f382e760046 Mon Sep 17 00:00:00 2001 From: Tomas Susanka Date: Thu, 8 Nov 2018 15:27:31 +0100 Subject: [PATCH 4/4] paths: typo, style --- src/apps/common/paths.py | 4 ++-- src/apps/tezos/sign_tx.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/apps/common/paths.py b/src/apps/common/paths.py index da09189733..566171aeb2 100644 --- a/src/apps/common/paths.py +++ b/src/apps/common/paths.py @@ -15,9 +15,9 @@ async def validate_path(ctx, validate_func, **kwargs): async def show_path_warning(ctx, path: list): text = Text("Confirm path", ui.ICON_WRONG, icon_color=ui.RED) - text.normal("The path") + text.normal("Path") text.mono(*break_address_n_to_lines(path)) - text.normal("seems unusual.") + text.normal("is unknown.") text.normal("Are you sure?") return await require_confirm( ctx, text, code=ButtonRequestType.UnknownDerivationPath diff --git a/src/apps/tezos/sign_tx.py b/src/apps/tezos/sign_tx.py index 0bcc8cbc1d..059acefe6a 100644 --- a/src/apps/tezos/sign_tx.py +++ b/src/apps/tezos/sign_tx.py @@ -6,7 +6,7 @@ from trezor.messages.TezosSignedTx import TezosSignedTx from apps.common import paths, seed from apps.common.writers import write_bytes, write_uint8 -from apps.tezos import layout, helpers +from apps.tezos import helpers, layout async def sign_tx(ctx, msg):