From a36742648007b39b9ecdcdbe917fde4b03582edb Mon Sep 17 00:00:00 2001 From: matejcik Date: Thu, 23 Jul 2020 13:58:19 +0200 Subject: [PATCH] feat(core/bitcoin): use path schemas --- core/src/apps/bitcoin/addresses.py | 81 +--------- core/src/apps/bitcoin/authorize_coinjoin.py | 13 +- core/src/apps/bitcoin/get_address.py | 7 +- core/src/apps/bitcoin/get_ownership_id.py | 7 +- core/src/apps/bitcoin/get_ownership_proof.py | 7 +- core/src/apps/bitcoin/get_public_key.py | 4 +- core/src/apps/bitcoin/keychain.py | 148 ++++++++++++++----- core/src/apps/bitcoin/multisig.py | 14 +- core/src/apps/bitcoin/sign_message.py | 13 +- core/src/apps/bitcoin/sign_tx/approvers.py | 4 +- 10 files changed, 143 insertions(+), 155 deletions(-) diff --git a/core/src/apps/bitcoin/addresses.py b/core/src/apps/bitcoin/addresses.py index d7bf5721f8..7a767e9318 100644 --- a/core/src/apps/bitcoin/addresses.py +++ b/core/src/apps/bitcoin/addresses.py @@ -4,7 +4,7 @@ from trezor.crypto.hashlib import sha256 from trezor.messages import InputScriptType from trezor.messages.MultisigRedeemScriptType import MultisigRedeemScriptType -from apps.common import HARDENED, address_type +from apps.common import address_type from apps.common.coininfo import CoinInfo from .common import ecdsa_hash_pubkey, encode_bech32_address @@ -155,82 +155,3 @@ 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: EnumTypeInputScriptType, - validate_script_type: bool = True, -) -> 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) not in (4, 5, 6): - return False - - if not validate_purpose(path[0], coin): - return False - if validate_script_type and not validate_purpose_against_script_type( - path[0], script_type - ): - return False - - if path[1] > 20 and path[1] != coin.slip44 | HARDENED: - return False - if (20 < path[2] < HARDENED) or path[2] > 20 | HARDENED: - return False - if path[3] not in (0, 1, 0 | HARDENED, 1 | HARDENED, 2 | HARDENED): - return False - if len(path) > 4 and path[4] > 1000000: - return False - if len(path) > 5 and path[5] > 1000000: - return False - return True - - -def validate_purpose(purpose: int, coin: CoinInfo) -> bool: - if purpose not in ( - 44 | HARDENED, - 45 | HARDENED, - 48 | HARDENED, - 49 | HARDENED, - 84 | HARDENED, - ): - return False - if not coin.segwit and purpose not in (44 | HARDENED, 45 | HARDENED, 48 | HARDENED): - return False - return True - - -def validate_purpose_against_script_type( - purpose: int, script_type: EnumTypeInputScriptType -) -> bool: - """ - Validates purpose against provided input's script type: - - 44 for spending address (script_type == SPENDADDRESS) - - 45, 48 for multisig (script_type == SPENDMULTISIG) - - 49 for p2wsh-nested-in-p2sh spend (script_type == SPENDP2SHWITNESS) - - 84 for p2wsh native segwit spend (script_type == SPENDWITNESS) - """ - if purpose == 44 | HARDENED and script_type != InputScriptType.SPENDADDRESS: - return False - if purpose == 45 | HARDENED and script_type != InputScriptType.SPENDMULTISIG: - return False - if purpose == 48 | HARDENED and script_type not in ( - InputScriptType.SPENDMULTISIG, - InputScriptType.SPENDP2SHWITNESS, - InputScriptType.SPENDWITNESS, - ): - return False - if purpose == 49 | HARDENED and script_type != InputScriptType.SPENDP2SHWITNESS: - return False - if purpose == 84 | HARDENED and script_type != InputScriptType.SPENDWITNESS: - return False - return True diff --git a/core/src/apps/bitcoin/authorize_coinjoin.py b/core/src/apps/bitcoin/authorize_coinjoin.py index 0d0cc3c4ef..e227af27b7 100644 --- a/core/src/apps/bitcoin/authorize_coinjoin.py +++ b/core/src/apps/bitcoin/authorize_coinjoin.py @@ -10,10 +10,9 @@ from apps.base import set_authorization from apps.common.confirm import require_confirm, require_hold_to_confirm from apps.common.paths import validate_path -from . import addresses from .authorization import FEE_PER_ANONYMITY_DECIMALS, CoinJoinAuthorization from .common import BIP32_WALLET_DEPTH -from .keychain import get_keychain_for_coin +from .keychain import get_keychain_for_coin, validate_path_against_script_type if False: from trezor import wire @@ -36,14 +35,14 @@ async def authorize_coinjoin(ctx: wire.Context, msg: AuthorizeCoinJoin) -> Succe if not msg.address_n: raise wire.DataError("Empty path not allowed.") + validation_path = msg.address_n + [0] * BIP32_WALLET_DEPTH await validate_path( ctx, - addresses.validate_full_path, keychain, - msg.address_n + [0] * BIP32_WALLET_DEPTH, - coin.curve_name, - coin=coin, - script_type=msg.script_type, + validation_path, + validate_path_against_script_type( + coin, address_n=validation_path, script_type=msg.script_type + ), ) text = Text("Authorize CoinJoin", ui.ICON_RECOVERY) diff --git a/core/src/apps/bitcoin/get_address.py b/core/src/apps/bitcoin/get_address.py index 0b1f11b472..788d44db7a 100644 --- a/core/src/apps/bitcoin/get_address.py +++ b/core/src/apps/bitcoin/get_address.py @@ -6,7 +6,7 @@ from apps.common.layout import address_n_to_str, show_address, show_qr, show_xpu from apps.common.paths import validate_path from . import addresses -from .keychain import with_keychain +from .keychain import validate_path_against_script_type, with_keychain from .multisig import multisig_pubkey_index if False: @@ -45,12 +45,9 @@ async def get_address( ) -> Address: await validate_path( ctx, - addresses.validate_full_path, keychain, msg.address_n, - coin.curve_name, - coin=coin, - script_type=msg.script_type, + validate_path_against_script_type(coin, msg), ) node = keychain.derive(msg.address_n) diff --git a/core/src/apps/bitcoin/get_ownership_id.py b/core/src/apps/bitcoin/get_ownership_id.py index 4c0266b78a..97c4268664 100644 --- a/core/src/apps/bitcoin/get_ownership_id.py +++ b/core/src/apps/bitcoin/get_ownership_id.py @@ -5,7 +5,7 @@ from trezor.messages.OwnershipId import OwnershipId from apps.common.paths import validate_path from . import addresses, common, scripts -from .keychain import with_keychain +from .keychain import validate_path_against_script_type, with_keychain from .ownership import get_identifier if False: @@ -19,12 +19,9 @@ async def get_ownership_id( ) -> OwnershipId: await validate_path( ctx, - addresses.validate_full_path, keychain, msg.address_n, - coin.curve_name, - coin=coin, - script_type=msg.script_type, + validate_path_against_script_type(coin, msg), ) if msg.script_type not in common.INTERNAL_INPUT_SCRIPT_TYPES: diff --git a/core/src/apps/bitcoin/get_ownership_proof.py b/core/src/apps/bitcoin/get_ownership_proof.py index 530b6d3382..694a8c7023 100644 --- a/core/src/apps/bitcoin/get_ownership_proof.py +++ b/core/src/apps/bitcoin/get_ownership_proof.py @@ -9,7 +9,7 @@ from apps.common.confirm import require_confirm from apps.common.paths import validate_path from . import addresses, common, scripts -from .keychain import with_keychain +from .keychain import validate_path_against_script_type, with_keychain from .ownership import generate_proof, get_identifier if False: @@ -36,12 +36,9 @@ async def get_ownership_proof( else: await validate_path( ctx, - addresses.validate_full_path, keychain, msg.address_n, - coin.curve_name, - coin=coin, - script_type=msg.script_type, + validate_path_against_script_type(coin, msg), ) if msg.script_type not in common.INTERNAL_INPUT_SCRIPT_TYPES: diff --git a/core/src/apps/bitcoin/get_public_key.py b/core/src/apps/bitcoin/get_public_key.py index b576c2dd27..6f57beb1dd 100644 --- a/core/src/apps/bitcoin/get_public_key.py +++ b/core/src/apps/bitcoin/get_public_key.py @@ -3,7 +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 +from apps.common import coins, layout, paths from apps.common.keychain import get_keychain if False: @@ -16,7 +16,7 @@ async def get_public_key(ctx: wire.Context, msg: GetPublicKey) -> PublicKey: coin = coins.by_name(coin_name) curve_name = msg.ecdsa_curve_name or coin.curve_name - keychain = await get_keychain(ctx, curve_name, [[]]) + keychain = await get_keychain(ctx, curve_name, [paths.AlwaysMatchingSchema]) node = keychain.derive(msg.address_n) diff --git a/core/src/apps/bitcoin/keychain.py b/core/src/apps/bitcoin/keychain.py index 76b2f8bc36..c8878b840c 100644 --- a/core/src/apps/bitcoin/keychain.py +++ b/core/src/apps/bitcoin/keychain.py @@ -1,14 +1,18 @@ from trezor import wire +from trezor.messages import InputScriptType as I -from apps.common import HARDENED, coininfo +from apps.common import coininfo from apps.common.keychain import get_keychain +from apps.common.paths import PATTERN_BIP44, PathSchema from .common import BITCOIN_NAMES if False: - from typing import Awaitable, Callable, Optional, Sequence, Tuple, TypeVar + from typing import Awaitable, Callable, Iterable, List, Optional, Tuple, TypeVar from typing_extensions import Protocol + from trezor.messages.TxInputType import EnumTypeInputScriptType + from apps.common.keychain import Keychain, MsgOut, Handler from apps.common.paths import Bip32Path @@ -17,54 +21,124 @@ if False: class MsgWithCoinName(Protocol): coin_name = ... # type: str + class MsgWithAddressScriptType(Protocol): + # XXX should be Bip32Path but that fails + address_n = ... # type: List[int] + script_type = ... # type: EnumTypeInputScriptType + MsgIn = TypeVar("MsgIn", bound=MsgWithCoinName) HandlerWithCoinInfo = Callable[..., Awaitable[MsgOut]] +# common patterns +PATTERN_BIP45 = "m/45'/[0-100]/change/address_index" +PATTERN_PURPOSE48_LEGACY = "m/48'/coin_type'/account'/0'/change/address_index" +PATTERN_PURPOSE48_P2SHSEGWIT = "m/48'/coin_type'/account'/1'/change/address_index" +PATTERN_PURPOSE48_SEGWIT = "m/48'/coin_type'/account'/2'/change/address_index" +PATTERN_BIP49 = "m/49'/coin_type'/account'/change/address_index" +PATTERN_BIP84 = "m/84'/coin_type'/account'/change/address_index" -def get_namespaces_for_coin(coin: coininfo.CoinInfo) -> Sequence[Bip32Path]: - namespaces = [] - slip44_id = coin.slip44 | HARDENED +# compatibility patterns, will be removed in the future +PATTERN_GREENADDRESS_A = "m/[1,4]/address_index" +PATTERN_GREENADDRESS_B = "m/3'/[1-100]'/[1,4]/address_index" +PATTERN_GREENADDRESS_SIGN_A = "m/1195487518" +PATTERN_GREENADDRESS_SIGN_B = "m/1195487518/6/address_index" - # BIP-44 - legacy: m/44'/slip44' (/account'/change/addr) - namespaces.append([44 | HARDENED, slip44_id]) - # BIP-45 - multisig cosigners: m/45' (/cosigner/change/addr) - namespaces.append([45 | HARDENED]) - # "purpose48" - multisig as done by Electrum - # m/48'/slip44' (/account'/script_type'/change/addr) - namespaces.append([48 | HARDENED, slip44_id]) +PATTERN_CASA = "m/49/coin_type/account/change/address_index" - if coin.segwit: - # BIP-49 - p2sh segwit: m/49'/slip44' (/account'/change/addr) - namespaces.append([49 | HARDENED, slip44_id]) - # BIP-84 - native segwit: m/84'/slip44' (/account'/change/addr) - namespaces.append([84 | HARDENED, slip44_id]) +def validate_path_against_script_type( + coin: coininfo.CoinInfo, + msg: MsgWithAddressScriptType = None, + address_n: Bip32Path = None, + script_type: EnumTypeInputScriptType = None, + multisig: bool = False, +) -> bool: + patterns = [] + + if msg is not None: + assert address_n is None and script_type is None + address_n = msg.address_n + script_type = msg.script_type or I.SPENDADDRESS + multisig = bool(getattr(msg, "multisig", False)) + + else: + assert address_n is not None and script_type is not None + + if script_type == I.SPENDADDRESS and not multisig: + patterns.append(PATTERN_BIP44) + if coin.coin_name in BITCOIN_NAMES: + patterns.append(PATTERN_GREENADDRESS_A) + patterns.append(PATTERN_GREENADDRESS_B) + + elif script_type in (I.SPENDADDRESS, I.SPENDMULTISIG) and multisig: + patterns.append(PATTERN_BIP45) + patterns.append(PATTERN_PURPOSE48_LEGACY) + if coin.coin_name in BITCOIN_NAMES: + patterns.append(PATTERN_GREENADDRESS_A) + patterns.append(PATTERN_GREENADDRESS_B) + + elif coin.segwit and script_type == I.SPENDP2SHWITNESS: + patterns.append(PATTERN_BIP49) + if multisig: + patterns.append(PATTERN_PURPOSE48_P2SHSEGWIT) + if coin.coin_name in BITCOIN_NAMES: + patterns.append(PATTERN_GREENADDRESS_A) + patterns.append(PATTERN_GREENADDRESS_B) + patterns.append(PATTERN_CASA) + + elif coin.segwit and script_type == I.SPENDWITNESS: + patterns.append(PATTERN_BIP84) + if multisig: + patterns.append(PATTERN_PURPOSE48_SEGWIT) + if coin.coin_name in BITCOIN_NAMES: + patterns.append(PATTERN_GREENADDRESS_A) + patterns.append(PATTERN_GREENADDRESS_B) + + return any( + PathSchema(pattern, coin.slip44).match(address_n) for pattern in patterns + ) + + +def get_schemas_for_coin(coin: coininfo.CoinInfo) -> Iterable[PathSchema]: + # basic patterns + patterns = [ + PATTERN_BIP44, + PATTERN_BIP45, + PATTERN_PURPOSE48_LEGACY, + ] + + # compatibility patterns if coin.coin_name in BITCOIN_NAMES: - # compatibility namespace for Casa - namespaces.append([49, coin.slip44]) + patterns.extend( + ( + PATTERN_GREENADDRESS_A, + PATTERN_GREENADDRESS_B, + PATTERN_GREENADDRESS_SIGN_A, + PATTERN_GREENADDRESS_SIGN_B, + PATTERN_CASA, + ) + ) - # compatibility namespace for Greenaddress: - # m/branch/address_pointer, for branch in (1, 4) - namespaces.append([1]) - namespaces.append([4]) - # m/3'/subaccount'/branch/address_pointer - namespaces.append([3 | HARDENED]) - # sign msg: - # m/0x4741b11e - # m/0x4741b11e/6/pointer - namespaces.append([0x4741B11E]) + # segwit patterns + if coin.segwit: + patterns.extend( + ( + PATTERN_BIP49, + PATTERN_BIP84, + PATTERN_PURPOSE48_P2SHSEGWIT, + PATTERN_PURPOSE48_SEGWIT, + ) + ) + + schemas = [PathSchema(pattern, coin.slip44) for pattern in patterns] # some wallets such as Electron-Cash (BCH) store coins on Bitcoin paths # we can allow spending these coins from Bitcoin paths if the coin has # implemented strong replay protection via SIGHASH_FORKID if coin.fork_id is not None: - namespaces.append([44 | HARDENED, 0 | HARDENED]) - namespaces.append([48 | HARDENED, 0 | HARDENED]) - if coin.segwit: - namespaces.append([49 | HARDENED, 0 | HARDENED]) - namespaces.append([84 | HARDENED, 0 | HARDENED]) + schemas.extend(PathSchema(pattern, 0) for pattern in patterns) - return namespaces + return schemas def get_coin_by_name(coin_name: Optional[str]) -> coininfo.CoinInfo: @@ -81,9 +155,9 @@ async def get_keychain_for_coin( ctx: wire.Context, coin_name: Optional[str] ) -> Tuple[Keychain, coininfo.CoinInfo]: coin = get_coin_by_name(coin_name) - namespaces = get_namespaces_for_coin(coin) + schemas = get_schemas_for_coin(coin) slip21_namespaces = [[b"SLIP-0019"]] - keychain = await get_keychain(ctx, coin.curve_name, namespaces, slip21_namespaces) + keychain = await get_keychain(ctx, coin.curve_name, schemas, slip21_namespaces) return keychain, coin diff --git a/core/src/apps/bitcoin/multisig.py b/core/src/apps/bitcoin/multisig.py index 65919fc4a5..42e711759e 100644 --- a/core/src/apps/bitcoin/multisig.py +++ b/core/src/apps/bitcoin/multisig.py @@ -5,6 +5,8 @@ from trezor.messages.HDNodeType import HDNodeType from trezor.messages.MultisigRedeemScriptType import MultisigRedeemScriptType from trezor.utils import HashWriter +from apps.common import paths + from .writers import write_bytes_fixed, write_uint32 if False: @@ -42,7 +44,16 @@ def multisig_fingerprint(multisig: MultisigRedeemScriptType) -> bytes: return h.get_digest() +def validate_multisig(multisig: MultisigRedeemScriptType) -> None: + if any(paths.is_hardened(n) for n in multisig.address_n): + raise wire.DataError("Cannot perform hardened derivation from XPUB") + for hd in multisig.pubkeys: + if any(paths.is_hardened(n) for n in hd.address_n): + raise wire.DataError("Cannot perform hardened derivation from XPUB") + + def multisig_pubkey_index(multisig: MultisigRedeemScriptType, pubkey: bytes) -> int: + validate_multisig(multisig) if multisig.nodes: for i, hd_node in enumerate(multisig.nodes): if multisig_get_pubkey(hd_node, multisig.address_n) == pubkey: @@ -54,7 +65,7 @@ def multisig_pubkey_index(multisig: MultisigRedeemScriptType, pubkey: bytes) -> raise wire.DataError("Pubkey not found in multisig script") -def multisig_get_pubkey(n: HDNodeType, p: list) -> bytes: +def multisig_get_pubkey(n: HDNodeType, p: paths.Bip32Path) -> bytes: node = bip32.HDNode( depth=n.depth, fingerprint=n.fingerprint, @@ -68,6 +79,7 @@ def multisig_get_pubkey(n: HDNodeType, p: list) -> bytes: def multisig_get_pubkeys(multisig: MultisigRedeemScriptType) -> List[bytes]: + validate_multisig(multisig) if multisig.nodes: return [multisig_get_pubkey(hd, multisig.address_n) for hd in multisig.nodes] else: diff --git a/core/src/apps/bitcoin/sign_message.py b/core/src/apps/bitcoin/sign_message.py index 2ee9229279..6ea19b1330 100644 --- a/core/src/apps/bitcoin/sign_message.py +++ b/core/src/apps/bitcoin/sign_message.py @@ -6,7 +6,7 @@ from trezor.messages.MessageSignature import MessageSignature from apps.common.paths import validate_path from apps.common.signverify import message_digest, require_confirm_sign_message -from .addresses import get_address, validate_full_path +from .addresses import get_address from .keychain import with_keychain if False: @@ -25,16 +25,7 @@ async def sign_message( script_type = msg.script_type or 0 await require_confirm_sign_message(ctx, coin.coin_shortcut, message) - await validate_path( - ctx, - validate_full_path, - keychain, - msg.address_n, - coin.curve_name, - coin=coin, - script_type=msg.script_type, - validate_script_type=False, - ) + await validate_path(ctx, keychain, msg.address_n) node = keychain.derive(address_n) seckey = node.private_key() diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py index 0ca8041268..bd7771bc50 100644 --- a/core/src/apps/bitcoin/sign_tx/approvers.py +++ b/core/src/apps/bitcoin/sign_tx/approvers.py @@ -5,7 +5,7 @@ from trezor.messages import OutputScriptType from apps.common import safety_checks -from .. import addresses +from .. import keychain from ..authorization import FEE_PER_ANONYMITY_DECIMALS from . import helpers, tx_weight from .tx_info import OriginalTxInfo, TxInfo @@ -90,7 +90,7 @@ class BasicApprover(Approver): self.change_count = 0 # the number of change-outputs async def add_internal_input(self, txi: TxInput) -> None: - if not addresses.validate_full_path(txi.address_n, self.coin, txi.script_type): + if not keychain.validate_path_against_script_type(self.coin, txi): await helpers.confirm_foreign_address(txi.address_n) await super().add_internal_input(txi)