From 90d5cdfd5be242f04a87a0576d762710b9e8d213 Mon Sep 17 00:00:00 2001 From: Tomas Susanka Date: Fri, 8 Nov 2019 08:43:32 +0000 Subject: [PATCH] all: rework passphrase The `on_device` field is being moved to PassphraseAck, State messages are removed. Features newly contain `session_id`. --- common/protob/messages-common.proto | 21 +-- common/protob/messages-management.proto | 4 +- common/protob/messages.proto | 4 +- core/src/apps/cardano/seed.py | 42 +++--- core/src/apps/common/passphrase.py | 55 ++++++++ core/src/apps/common/request_passphrase.py | 86 ------------ core/src/apps/common/seed.py | 7 +- core/src/apps/debug/__init__.py | 5 +- core/src/apps/homescreen/__init__.py | 17 ++- core/src/storage/cache.py | 68 +++------- core/src/storage/device.py | 2 +- core/src/trezor/messages/Features.py | 3 + core/src/trezor/messages/Initialize.py | 9 +- core/src/trezor/messages/MessageType.py | 2 - core/src/trezor/messages/PassphraseAck.py | 6 +- core/src/trezor/messages/PassphraseRequest.py | 6 +- .../src/trezor/messages/PassphraseStateAck.py | 14 -- .../trezor/messages/PassphraseStateRequest.py | 26 ---- docs/communication/index.md | 9 ++ docs/communication/passphrase.md | 64 +++++++++ docs/communication/sessions.md | 36 +++++ python/src/trezorlib/cli/trezorctl.py | 15 ++- python/src/trezorlib/client.py | 40 +++--- python/src/trezorlib/messages/Features.py | 3 + python/src/trezorlib/messages/Initialize.py | 9 +- python/src/trezorlib/messages/MessageType.py | 2 - .../src/trezorlib/messages/PassphraseAck.py | 6 +- .../trezorlib/messages/PassphraseRequest.py | 6 +- .../trezorlib/messages/PassphraseStateAck.py | 14 -- .../messages/PassphraseStateRequest.py | 26 ---- python/src/trezorlib/messages/__init__.py | 2 - tests/device_tests/test_basic.py | 2 +- .../test_msg_cardano_sign_tx_slip39_basic.py | 5 +- tests/device_tests/test_session_id.py | 123 ++++++++++++++++++ 34 files changed, 403 insertions(+), 336 deletions(-) create mode 100644 core/src/apps/common/passphrase.py delete mode 100644 core/src/apps/common/request_passphrase.py delete mode 100644 core/src/trezor/messages/PassphraseStateAck.py delete mode 100644 core/src/trezor/messages/PassphraseStateRequest.py create mode 100644 docs/communication/index.md create mode 100644 docs/communication/passphrase.md create mode 100644 docs/communication/sessions.md delete mode 100644 python/src/trezorlib/messages/PassphraseStateAck.py delete mode 100644 python/src/trezorlib/messages/PassphraseStateRequest.py create mode 100644 tests/device_tests/test_session_id.py diff --git a/common/protob/messages-common.proto b/common/protob/messages-common.proto index cb89d8a9a..127be8d6c 100644 --- a/common/protob/messages-common.proto +++ b/common/protob/messages-common.proto @@ -110,31 +110,16 @@ message PinMatrixAck { * @next PassphraseAck */ message PassphraseRequest { - optional bool on_device = 1; // passphrase is being entered on the device + optional bool _deprecated_on_device = 1; // moved to PassphraseAck but left for backwards compatibility } /** * Request: Send passphrase back - * @next PassphraseStateRequest + * @auxend */ message PassphraseAck { optional string passphrase = 1; - optional bytes state = 2; // expected device state -} - -/** - * Response: Device awaits passphrase state - * @next PassphraseStateAck - */ -message PassphraseStateRequest { - optional bytes state = 1; // actual device state -} - -/** - * Request: Send passphrase state back - * @auxend - */ -message PassphraseStateAck { + optional bool on_device = 2; // user wants to enter passphrase on the device } /** diff --git a/common/protob/messages-management.proto b/common/protob/messages-management.proto index 8b7f0b215..487093f13 100644 --- a/common/protob/messages-management.proto +++ b/common/protob/messages-management.proto @@ -20,8 +20,7 @@ enum BackupType { * @next Features */ message Initialize { - optional bytes state = 1; // assumed device state, clear session if set and different - optional bool skip_passphrase = 2; // this session should always assume empty passphrase + optional bytes session_id = 1; // assumed device session id; Trezor clears caches if it is different or empty } /** @@ -88,6 +87,7 @@ message Features { optional bool sd_card_present = 32; // is SD card present optional bool sd_protection = 33; // is SD Protect enabled optional bool wipe_code_protection = 34; // is wipe code protection enabled + optional bytes session_id = 35; } /** diff --git a/common/protob/messages.proto b/common/protob/messages.proto index a9c74fd63..2d973010b 100644 --- a/common/protob/messages.proto +++ b/common/protob/messages.proto @@ -54,8 +54,8 @@ enum MessageType { MessageType_EntropyAck = 36 [(wire_in) = true]; MessageType_PassphraseRequest = 41 [(wire_out) = true]; MessageType_PassphraseAck = 42 [(wire_in) = true, (wire_tiny) = true, (wire_no_fsm) = true]; - MessageType_PassphraseStateRequest = 77 [(wire_out) = true]; - MessageType_PassphraseStateAck = 78 [(wire_in) = true, (wire_tiny) = true, (wire_no_fsm) = true]; +// PassphraseStateRequest = 77 DEPRECATED +// PassphraseStateAck = 78 DEPRECATED MessageType_RecoveryDevice = 45 [(wire_in) = true]; MessageType_WordRequest = 46 [(wire_out) = true]; MessageType_WordAck = 47 [(wire_in) = true]; diff --git a/core/src/apps/cardano/seed.py b/core/src/apps/cardano/seed.py index 672daf778..a0d96abff 100644 --- a/core/src/apps/cardano/seed.py +++ b/core/src/apps/cardano/seed.py @@ -1,11 +1,15 @@ import storage -import storage.cache from trezor import wire from trezor.crypto import bip32 from apps.cardano import CURVE, SEED_NAMESPACE from apps.common import mnemonic -from apps.common.request_passphrase import protect_by_passphrase +from apps.common.passphrase import get as get_passphrase + +if False: + from typing import Optional + +_cached_root = None # type: Optional[bytes] class Keychain: @@ -30,34 +34,26 @@ class Keychain: return node -async def _get_passphrase(ctx: wire.Context) -> bytes: - passphrase = storage.cache.get_passphrase() - if passphrase is None: - passphrase = await protect_by_passphrase(ctx) - storage.cache.set_passphrase(passphrase) - - return passphrase - - async def get_keychain(ctx: wire.Context) -> Keychain: + global _cached_root + if not storage.is_initialized(): raise wire.NotInitialized("Device is not initialized") - if mnemonic.is_bip39(): - # derive the root node from mnemonic and passphrase - passphrase = await _get_passphrase(ctx) - root = bip32.from_mnemonic_cardano(mnemonic.get_secret().decode(), passphrase) - else: - seed = storage.cache.get_seed() - if seed is None: - passphrase = await _get_passphrase(ctx) + if _cached_root is None: + passphrase = await get_passphrase(ctx) + if mnemonic.is_bip39(): + # derive the root node from mnemonic and passphrase + _cached_root = bip32.from_mnemonic_cardano( + mnemonic.get_secret().decode(), passphrase + ) + else: seed = mnemonic.get_seed(passphrase) - storage.cache.set_seed(seed) - root = bip32.from_seed(seed, "ed25519 cardano seed") + _cached_root = bip32.from_seed(seed, "ed25519 cardano seed") # derive the namespaced root node for i in SEED_NAMESPACE: - root.derive_cardano(i) + _cached_root.derive_cardano(i) - keychain = Keychain(SEED_NAMESPACE, root) + keychain = Keychain(SEED_NAMESPACE, _cached_root) return keychain diff --git a/core/src/apps/common/passphrase.py b/core/src/apps/common/passphrase.py new file mode 100644 index 000000000..492e821bc --- /dev/null +++ b/core/src/apps/common/passphrase.py @@ -0,0 +1,55 @@ +from micropython import const + +import storage.device +from trezor import wire +from trezor.messages.PassphraseAck import PassphraseAck +from trezor.messages.PassphraseRequest import PassphraseRequest +from trezor.ui.passphrase import CANCELLED, PassphraseKeyboard + +if __debug__: + from apps.debug import input_signal + +_MAX_PASSPHRASE_LEN = const(50) + + +def is_enabled() -> bool: + return storage.device.is_passphrase_enabled() + + +async def get(ctx: wire.Context) -> str: + if is_enabled(): + return await request_from_user(ctx) + else: + return "" + + +async def request_from_user(ctx: wire.Context) -> str: + request = PassphraseRequest() + ack = await ctx.call(request, PassphraseAck) + if ack.on_device: + if ack.passphrase is not None: + raise wire.ProcessError("Passphrase provided when it should not be") + passphrase = await request_from_user_on_device(ctx) + else: + if ack.passphrase is None: + raise wire.ProcessError("Passphrase not provided") + passphrase = ack.passphrase + + if len(passphrase) > _MAX_PASSPHRASE_LEN: + raise wire.DataError("Maximum passphrase length is %d" % _MAX_PASSPHRASE_LEN) + + return passphrase + + +async def request_from_user_on_device(ctx: wire.Context) -> str: + keyboard = PassphraseKeyboard("Enter passphrase", _MAX_PASSPHRASE_LEN) + if __debug__: + passphrase = await ctx.wait(keyboard, input_signal()) + else: + passphrase = await ctx.wait(keyboard) + if passphrase is CANCELLED: + raise wire.ActionCancelled("Passphrase entry cancelled") + + assert isinstance(passphrase, str) + + return passphrase diff --git a/core/src/apps/common/request_passphrase.py b/core/src/apps/common/request_passphrase.py deleted file mode 100644 index 476d149a6..000000000 --- a/core/src/apps/common/request_passphrase.py +++ /dev/null @@ -1,86 +0,0 @@ -from micropython import const - -import storage.device -from storage import cache -from trezor import ui, wire -from trezor.messages import ButtonRequestType, PassphraseSourceType -from trezor.messages.ButtonAck import ButtonAck -from trezor.messages.ButtonRequest import ButtonRequest -from trezor.messages.PassphraseAck import PassphraseAck -from trezor.messages.PassphraseRequest import PassphraseRequest -from trezor.messages.PassphraseStateAck import PassphraseStateAck -from trezor.messages.PassphraseStateRequest import PassphraseStateRequest -from trezor.ui.passphrase import CANCELLED, PassphraseKeyboard, PassphraseSource -from trezor.ui.popup import Popup -from trezor.ui.text import Text - -if __debug__: - from apps.debug import input_signal - -_MAX_PASSPHRASE_LEN = const(50) - - -async def protect_by_passphrase(ctx: wire.Context) -> str: - if storage.device.has_passphrase(): - return await request_passphrase(ctx) - else: - return "" - - -async def request_passphrase(ctx: wire.Context) -> str: - source = storage.device.get_passphrase_source() - if source == PassphraseSourceType.ASK: - source = await request_passphrase_source(ctx) - passphrase = await request_passphrase_ack( - ctx, source == PassphraseSourceType.DEVICE - ) - if len(passphrase) > _MAX_PASSPHRASE_LEN: - raise wire.DataError("Maximum passphrase length is %d" % _MAX_PASSPHRASE_LEN) - return passphrase - - -async def request_passphrase_source(ctx: wire.Context) -> int: - req = ButtonRequest(code=ButtonRequestType.PassphraseType) - await ctx.call(req, ButtonAck) - - text = Text("Enter passphrase", ui.ICON_CONFIG) - text.normal("Where do you want to", "enter your passphrase?") - source = PassphraseSource(text) - - response = await ctx.wait(source) - assert isinstance(response, int) - return response - - -async def request_passphrase_ack(ctx: wire.Context, on_device: bool) -> str: - if not on_device: - text = Text("Passphrase entry", ui.ICON_CONFIG) - text.normal("Please type your", "passphrase on the", "connected host.") - await Popup(text) - - passphrase_request = PassphraseRequest(on_device=on_device) - ack = await ctx.call(passphrase_request, PassphraseAck) - - if on_device: - if ack.passphrase is not None: - raise wire.ProcessError("Passphrase provided when it should not be") - - keyboard = PassphraseKeyboard("Enter passphrase", _MAX_PASSPHRASE_LEN) - if __debug__: - passphrase = await ctx.wait(keyboard, input_signal()) - else: - passphrase = await ctx.wait(keyboard) - if passphrase is CANCELLED: - raise wire.ActionCancelled("Passphrase cancelled") - else: - if ack.passphrase is None: - raise wire.ProcessError("Passphrase not provided") - passphrase = ack.passphrase - - assert isinstance(passphrase, str) - - state = cache.get_state(prev_state=ack.state, passphrase=passphrase) - state_request = PassphraseStateRequest(state=state) - await ctx.call(state_request, PassphraseStateAck) - - return passphrase diff --git a/core/src/apps/common/seed.py b/core/src/apps/common/seed.py index 8b74880e9..375da4170 100644 --- a/core/src/apps/common/seed.py +++ b/core/src/apps/common/seed.py @@ -5,7 +5,7 @@ from trezor.crypto import bip32, hashlib, hmac from trezor.crypto.curve import secp256k1 from apps.common import HARDENED, mnemonic -from apps.common.request_passphrase import protect_by_passphrase +from apps.common.passphrase import get as get_passphrase if False: from typing import List, Union @@ -114,10 +114,7 @@ async def get_keychain(ctx: wire.Context, namespaces: list) -> Keychain: raise wire.NotInitialized("Device is not initialized") seed = storage.cache.get_seed() if seed is None: - passphrase = storage.cache.get_passphrase() - if passphrase is None: - passphrase = await protect_by_passphrase(ctx) - storage.cache.set_passphrase(passphrase) + passphrase = await get_passphrase(ctx) seed = mnemonic.get_seed(passphrase) storage.cache.set_seed(seed) keychain = Keychain(seed, namespaces) diff --git a/core/src/apps/debug/__init__.py b/core/src/apps/debug/__init__.py index 4f1004f98..fc75629e0 100644 --- a/core/src/apps/debug/__init__.py +++ b/core/src/apps/debug/__init__.py @@ -97,13 +97,12 @@ if __debug__: ctx: wire.Context, msg: DebugLinkGetState ) -> DebugLinkState: from trezor.messages.DebugLinkState import DebugLinkState - from storage.device import has_passphrase - from apps.common import mnemonic + from apps.common import mnemonic, passphrase m = DebugLinkState() m.mnemonic_secret = mnemonic.get_secret() m.mnemonic_type = mnemonic.get_type() - m.passphrase_protection = has_passphrase() + m.passphrase_protection = passphrase.is_enabled() m.reset_entropy = reset_internal_entropy if msg.wait_layout or current_content is None: diff --git a/core/src/apps/homescreen/__init__.py b/core/src/apps/homescreen/__init__.py index db711fd11..0e7128139 100644 --- a/core/src/apps/homescreen/__init__.py +++ b/core/src/apps/homescreen/__init__.py @@ -34,8 +34,8 @@ def get_features() -> Features: f.initialized = storage.is_initialized() f.pin_protection = config.has_pin() f.pin_cached = config.has_pin() - f.passphrase_protection = storage.device.has_passphrase() - f.passphrase_cached = cache.has_passphrase() + f.passphrase_protection = storage.device.is_passphrase_enabled() + # f.passphrase_cached = cache.has_passphrase() # TODO f.needs_backup = storage.device.needs_backup() f.unfinished_backup = storage.device.unfinished_backup() f.no_backup = storage.device.no_backup() @@ -71,14 +71,13 @@ def get_features() -> Features: f.sd_card_present = io.SDCard().present() f.sd_protection = storage.sd_salt.is_enabled() f.wipe_code_protection = config.has_wipe_code() + f.session_id = cache.get_session_id() return f async def handle_Initialize(ctx: wire.Context, msg: Initialize) -> Features: - if msg.state is None or msg.state != cache.get_state(prev_state=bytes(msg.state)): + if msg.session_id is None or msg.session_id != cache.get_session_id(): cache.clear() - if msg.skip_passphrase: - cache.set_passphrase("") return get_features() @@ -91,7 +90,7 @@ async def handle_Cancel(ctx: wire.Context, msg: Cancel) -> NoReturn: async def handle_ClearSession(ctx: wire.Context, msg: ClearSession) -> Success: - cache.clear(keep_passphrase=True) + cache.clear() return Success(message="Session cleared") @@ -102,10 +101,10 @@ async def handle_Ping(ctx: wire.Context, msg: Ping) -> Success: from trezor.ui.text import Text await require_confirm(ctx, Text("Confirm"), ProtectCall) - if msg.passphrase_protection: - from apps.common.request_passphrase import protect_by_passphrase + if msg.passphrase_protection: # TODO + from apps.common import passphrase - await protect_by_passphrase(ctx) + await passphrase.get(ctx) return Success(message=msg.message) diff --git a/core/src/storage/cache.py b/core/src/storage/cache.py index 0e4f15b0e..42cf02dbb 100644 --- a/core/src/storage/cache.py +++ b/core/src/storage/cache.py @@ -1,75 +1,41 @@ -from storage.device import get_device_id -from trezor.crypto import hashlib, hmac, random +from trezor.crypto import random if False: from typing import Optional _cached_seed = None # type: Optional[bytes] -_cached_seed_without_passphrase = None # type: Optional[bytes] -_cached_passphrase = None # type: Optional[str] -_cached_passphrase_fprint = b"\x00\x00\x00\x00" # type: bytes +_cached_seed_without_passphrase = None # type: Optional[bytes] # Needed for SLIP-21 +_cached_session_id = None # type: Optional[bytes] -def get_state(prev_state: bytes = None, passphrase: str = None) -> Optional[bytes]: - if prev_state is None: - salt = random.bytes(32) # generate a random salt if no state provided - else: - salt = prev_state[:32] # use salt from provided state - if len(salt) != 32: - return None # invalid state - if passphrase is None: - if _cached_passphrase is None: - return None # we don't have any passphrase to compute the state - else: - passphrase = _cached_passphrase # use cached passphrase - return _compute_state(salt, passphrase) +def get_session_id() -> Optional[bytes]: + global _cached_session_id + if not _cached_session_id: + _cached_session_id = random.bytes(32) + return _cached_session_id -def _compute_state(salt: bytes, passphrase: str) -> bytes: - # state = HMAC(passphrase, salt || device_id) - message = salt + get_device_id().encode() - state = hmac.new(passphrase.encode(), message, hashlib.sha256).digest() - return salt + state +def set_seed(seed: Optional[bytes]) -> None: + global _cached_seed + _cached_seed = seed def get_seed() -> Optional[bytes]: return _cached_seed -def get_seed_without_passphrase() -> Optional[bytes]: - return _cached_seed_without_passphrase - - -def get_passphrase() -> Optional[str]: - return _cached_passphrase - - -def get_passphrase_fprint() -> bytes: - return _cached_passphrase_fprint - - -def has_passphrase() -> bool: - return _cached_passphrase is not None - - -def set_seed(seed: Optional[bytes]) -> None: - global _cached_seed - _cached_seed = seed - - def set_seed_without_passphrase(seed: Optional[bytes]) -> None: global _cached_seed_without_passphrase _cached_seed_without_passphrase = seed -def set_passphrase(passphrase: Optional[str]) -> None: - global _cached_passphrase, _cached_passphrase_fprint - _cached_passphrase = passphrase - _cached_passphrase_fprint = _compute_state(b"FPRINT", passphrase or "")[:4] +def get_seed_without_passphrase() -> Optional[bytes]: + return _cached_seed_without_passphrase + +def clear() -> None: + global _cached_session_id + _cached_session_id = None -def clear(keep_passphrase: bool = False) -> None: set_seed(None) set_seed_without_passphrase(None) - if not keep_passphrase: - set_passphrase(None) diff --git a/core/src/storage/device.py b/core/src/storage/device.py index 224910b20..adbbf78d9 100644 --- a/core/src/storage/device.py +++ b/core/src/storage/device.py @@ -101,7 +101,7 @@ def get_backup_type() -> EnumTypeBackupType: return backup_type # type: ignore -def has_passphrase() -> bool: +def is_passphrase_enabled() -> bool: return common.get_bool(_NAMESPACE, _USE_PASSPHRASE) diff --git a/core/src/trezor/messages/Features.py b/core/src/trezor/messages/Features.py index 17586cc24..bec33f393 100644 --- a/core/src/trezor/messages/Features.py +++ b/core/src/trezor/messages/Features.py @@ -50,6 +50,7 @@ class Features(p.MessageType): sd_card_present: bool = None, sd_protection: bool = None, wipe_code_protection: bool = None, + session_id: bytes = None, ) -> None: self.vendor = vendor self.major_version = major_version @@ -84,6 +85,7 @@ class Features(p.MessageType): self.sd_card_present = sd_card_present self.sd_protection = sd_protection self.wipe_code_protection = wipe_code_protection + self.session_id = session_id @classmethod def get_fields(cls) -> Dict: @@ -121,4 +123,5 @@ class Features(p.MessageType): 32: ('sd_card_present', p.BoolType, 0), 33: ('sd_protection', p.BoolType, 0), 34: ('wipe_code_protection', p.BoolType, 0), + 35: ('session_id', p.BytesType, 0), } diff --git a/core/src/trezor/messages/Initialize.py b/core/src/trezor/messages/Initialize.py index 3e4176999..73dd3fdc9 100644 --- a/core/src/trezor/messages/Initialize.py +++ b/core/src/trezor/messages/Initialize.py @@ -15,15 +15,12 @@ class Initialize(p.MessageType): def __init__( self, - state: bytes = None, - skip_passphrase: bool = None, + session_id: bytes = None, ) -> None: - self.state = state - self.skip_passphrase = skip_passphrase + self.session_id = session_id @classmethod def get_fields(cls) -> Dict: return { - 1: ('state', p.BytesType, 0), - 2: ('skip_passphrase', p.BoolType, 0), + 1: ('session_id', p.BytesType, 0), } diff --git a/core/src/trezor/messages/MessageType.py b/core/src/trezor/messages/MessageType.py index 87fdc57a3..e6f5426e1 100644 --- a/core/src/trezor/messages/MessageType.py +++ b/core/src/trezor/messages/MessageType.py @@ -29,8 +29,6 @@ EntropyRequest = 35 # type: Literal[35] EntropyAck = 36 # type: Literal[36] PassphraseRequest = 41 # type: Literal[41] PassphraseAck = 42 # type: Literal[42] -PassphraseStateRequest = 77 # type: Literal[77] -PassphraseStateAck = 78 # type: Literal[78] RecoveryDevice = 45 # type: Literal[45] WordRequest = 46 # type: Literal[46] WordAck = 47 # type: Literal[47] diff --git a/core/src/trezor/messages/PassphraseAck.py b/core/src/trezor/messages/PassphraseAck.py index 3e965b0e2..d8fc4ded2 100644 --- a/core/src/trezor/messages/PassphraseAck.py +++ b/core/src/trezor/messages/PassphraseAck.py @@ -16,14 +16,14 @@ class PassphraseAck(p.MessageType): def __init__( self, passphrase: str = None, - state: bytes = None, + on_device: bool = None, ) -> None: self.passphrase = passphrase - self.state = state + self.on_device = on_device @classmethod def get_fields(cls) -> Dict: return { 1: ('passphrase', p.UnicodeType, 0), - 2: ('state', p.BytesType, 0), + 2: ('on_device', p.BoolType, 0), } diff --git a/core/src/trezor/messages/PassphraseRequest.py b/core/src/trezor/messages/PassphraseRequest.py index 21395653a..27806cb9e 100644 --- a/core/src/trezor/messages/PassphraseRequest.py +++ b/core/src/trezor/messages/PassphraseRequest.py @@ -15,12 +15,12 @@ class PassphraseRequest(p.MessageType): def __init__( self, - on_device: bool = None, + _deprecated_on_device: bool = None, ) -> None: - self.on_device = on_device + self._deprecated_on_device = _deprecated_on_device @classmethod def get_fields(cls) -> Dict: return { - 1: ('on_device', p.BoolType, 0), + 1: ('_deprecated_on_device', p.BoolType, 0), } diff --git a/core/src/trezor/messages/PassphraseStateAck.py b/core/src/trezor/messages/PassphraseStateAck.py deleted file mode 100644 index 3846fc5f8..000000000 --- a/core/src/trezor/messages/PassphraseStateAck.py +++ /dev/null @@ -1,14 +0,0 @@ -# Automatically generated by pb2py -# fmt: off -import protobuf as p - -if __debug__: - try: - from typing import Dict, List # noqa: F401 - from typing_extensions import Literal # noqa: F401 - except ImportError: - pass - - -class PassphraseStateAck(p.MessageType): - MESSAGE_WIRE_TYPE = 78 diff --git a/core/src/trezor/messages/PassphraseStateRequest.py b/core/src/trezor/messages/PassphraseStateRequest.py deleted file mode 100644 index 5dbac3b33..000000000 --- a/core/src/trezor/messages/PassphraseStateRequest.py +++ /dev/null @@ -1,26 +0,0 @@ -# Automatically generated by pb2py -# fmt: off -import protobuf as p - -if __debug__: - try: - from typing import Dict, List # noqa: F401 - from typing_extensions import Literal # noqa: F401 - except ImportError: - pass - - -class PassphraseStateRequest(p.MessageType): - MESSAGE_WIRE_TYPE = 77 - - def __init__( - self, - state: bytes = None, - ) -> None: - self.state = state - - @classmethod - def get_fields(cls) -> Dict: - return { - 1: ('state', p.BytesType, 0), - } diff --git a/docs/communication/index.md b/docs/communication/index.md new file mode 100644 index 000000000..4eb8a2880 --- /dev/null +++ b/docs/communication/index.md @@ -0,0 +1,9 @@ +# Communication + +_Note: In this section we describe the internal functioning of the communication protocol. If you wish to implement Trezor support you should use [Connect](https://github.com/trezor/connect/) or [python-trezor](https://pypi.org/project/trezor/), which will do all this hard work for you._ + +We use [Protobuf v2](https://developers.google.com/protocol-buffers/) for host-device communication. The communication cycle is very simple, Trezor receives a message (request), acts on it and responds with another one (response). Trezor on its own is incapable of initiating the communication. + +## Definitions + +Protobuf messages are defined in the [Common](https://github.com/trezor/trezor-firmware/tree/master/common) project, which is part of this monorepo. This repository is also exported to [trezor/trezor-common](https://github.com/trezor/trezor-common) to be used by third parties, which prefer not to include the whole monorepo. That copy is read-only mirror and all changes are happening in this monorepo. diff --git a/docs/communication/passphrase.md b/docs/communication/passphrase.md new file mode 100644 index 000000000..8e6391546 --- /dev/null +++ b/docs/communication/passphrase.md @@ -0,0 +1,64 @@ +# Passphrase + +As of \[versions TBD\] we have changed how [passphrase](https://wiki.trezor.io/Passphrase) is communicated between the Host and the Device. + +Passphrase is very tightly coupled with _sessions_. The reader is encouraged to read on that topic first in the [sessions.md](sessions.md) section. + +## Scheme + +As soon as Trezor needs the passphrase to do BIP-39/SLIP-39 derivations it prompts the user for passphrase. + +``` +GetAddress(...) +---------> PassphraseRequest() + <--------- +PassphraseAck +(str passphrase, bool on_device) +---------> Address(...) + <--------- +``` + +In the default Trezor setting, the passphrase is obtained from the Host. Trezor sends a PassphraseRequest message and awaits PassphraseAck as a response. This message contains field `passphrase` to transmit it or it has `on_device` boolean flag to indicate that the user wishes to enter the passphrase on Trezor instead. Setting both `passphrase` and `on_device` to true is forbidden. + +Note that this has changed as of TBD. In previous firmware versions the `on_device` flag was in the PassphraseRequest message, since this decision has been made on Trezor. We also had two additional messages PassphraseStateRequest and PassphraseStateAck which were removed. + +## Example + +On an initialized device with passphrase enabled a common communication starts like this: + +``` +Initialize() +---------> Features(..., session_id) + <--------- +GetAddress(...) +---------> PassphraseRequest() + <--------- +PassphraseAck(...) +---------> Address(...) + <--------- +``` + +The device requested the passphrase since the BIP-39/SLIP-39 seed is not yet cached. After this workflow the seed is cached and the passphrase will therefore never be requested again unless the session is cleared*. + +Since we do not have sessions, the Host can not be sure that someone else has not used the device and applied another session id (e.g. changed the Passphrase). To work around this we send the session id again on every subsequent message. See more on that in [session.md](). + +``` +Initialize(session_id) +---------> Features(..., session_id) + <--------- +GetPublicKey(...) +---------> PublicKey(...) + <--------- +``` + +As long as the session_id in `Initialize` is the same as the one Trezor stores internally, Trezor guarantees the same passphrase is being used. + +---- + +\* There is one exception and that is Cardano. Because Cardano has a different BIP-39/SLIP-39 derivation scheme for passphrase we can not use the cached seed. As a workaround we prompt for the passphrase again in such case and cache the cardano seed in the cardano app directly. + +## Passphrase always on device + +User might want to enforce the passphrase entry on the device every time without the hassle of instructing the Host to do so. + +For such cases the user may apply the *Passphrase always on device* setting TODO: Name. As the name suggests, with this setting the passphrase is prompted on the device right away and no PassphraseRequest/PassphraseAck messages are exchanged. diff --git a/docs/communication/sessions.md b/docs/communication/sessions.md new file mode 100644 index 000000000..963f8b472 --- /dev/null +++ b/docs/communication/sessions.md @@ -0,0 +1,36 @@ +# Sessions (the lack of them) + +Currently the communication protocol lacks sessions, which are planned to be introduced in the near future (see [#79](https://github.com/trezor/trezor-firmware/issues/79)). + +To ensure the device is in the expected state we use something called _session_id_. Session id is a 32 bytes long random blob which identifies the internal device state (mainly its caches). This is primarily useful for passphrase to make sure the same passphrase is cached in the device as the one the user entered a few minutes ago. See [passphrase.md](passphrase.md) for more on that. + +On first initialization the Host does not have a session id and starts the communication with an empty Initialize: + +``` +Initialize() +---------> Features(..., session_id) + <--------- +``` + +After the first Features message is received the Host might store the session_id. To ensure the device state Host must send the Initialize message again including that particular session_id: + +``` +Initialize(session_id) +---------> Features(..., session_id) + <--------- +Request +---------> Response + <--------- +``` + +So to make sure the device state has not changed, the Host must send the Initialize message with the correctly stored session_id before each request. Yes, this is stupid. + +As mentioned, sessions will be introduced soon™ and fix that. We will probably take the first few bytes of the session_id, declare it a session id, and the rest will remain, without the annoying requirement of sending Initialize before every message. + +---- + +The session is terminated and therefore the caches are cleared if: +- Initialize.session_id is empty. +- Initialize.session_id is different then the one cached in Trezor. +- Trezor is replugged (session is not persistent). +- ClearSession is received. diff --git a/python/src/trezorlib/cli/trezorctl.py b/python/src/trezorlib/cli/trezorctl.py index 95878fd9d..f90071e0f 100755 --- a/python/src/trezorlib/cli/trezorctl.py +++ b/python/src/trezorlib/cli/trezorctl.py @@ -141,9 +141,16 @@ def configure_logging(verbose: int): @click.option( "-j", "--json", "is_json", is_flag=True, help="Print result as JSON object" ) +@click.option( + "-P", + "--passphrase-on-host", + "passphrase_on_host", + is_flag=True, + help="Enter passphrase on host.", +) @click.version_option() @click.pass_context -def cli(ctx, path, verbose, is_json): +def cli(ctx, path, verbose, is_json, passphrase_on_host): configure_logging(verbose) def get_device(): @@ -157,13 +164,15 @@ def cli(ctx, path, verbose, is_json): if path is not None: click.echo("Using path: {}".format(path)) sys.exit(1) - return TrezorClient(transport=device, ui=ui.ClickUI()) + return TrezorClient( + transport=device, ui=ui.ClickUI(), passphrase_on_host=passphrase_on_host + ) ctx.obj = get_device @cli.resultcallback() -def print_result(res, path, verbose, is_json): +def print_result(res, path, verbose, is_json, passphrase_on_host): if is_json: if isinstance(res, protobuf.MessageType): click.echo(json.dumps({res.__class__.__name__: res.__dict__})) diff --git a/python/src/trezorlib/client.py b/python/src/trezorlib/client.py index 24b4ccbb4..baff68087 100644 --- a/python/src/trezorlib/client.py +++ b/python/src/trezorlib/client.py @@ -107,15 +107,21 @@ class TrezorClient: - passphrase request (ask the user to enter a passphrase) See `trezorlib.ui` for details. - You can supply a `state` you saved in the previous session. If you do, - the user might not need to enter their passphrase again. + You can supply a `session_id` you might have saved in the previous session. + If you do, the user might not need to enter their passphrase again. + + Set `passphrase_on_host` to True if you want to enter passphrase on host directly + instead of on Trezor. """ - def __init__(self, transport, ui=_NO_UI_OBJECT, state=None): + def __init__( + self, transport, ui=_NO_UI_OBJECT, session_id=None, passphrase_on_host=False + ): LOG.info("creating client instance for device: {}".format(transport.get_path())) self.transport = transport self.ui = ui - self.state = state + self.session_id = session_id + self.passphrase_on_host = passphrase_on_host # XXX remove when old Electrum has been cycled out. # explanation: We changed the API in 0.11 and this broke older versions @@ -177,10 +183,9 @@ class TrezorClient: else: return resp - def _callback_passphrase(self, msg): - if msg.on_device: - passphrase = None - else: + def _callback_passphrase(self, msg: messages.PassphraseRequest): + if self.passphrase_on_host: + on_device = False try: passphrase = self.ui.get_passphrase() except exceptions.Cancelled: @@ -191,16 +196,13 @@ class TrezorClient: if len(passphrase) > MAX_PASSPHRASE_LENGTH: self.call_raw(messages.Cancel()) raise ValueError("Passphrase too long") + else: + on_device = True + passphrase = None - resp = self.call_raw( - messages.PassphraseAck(passphrase=passphrase, state=self.state) + return self.call_raw( + messages.PassphraseAck(passphrase=passphrase, on_device=on_device) ) - if isinstance(resp, messages.PassphraseStateRequest): - # TODO report to the user that the passphrase has changed? - self.state = resp.state - return self.call_raw(messages.PassphraseStateAck()) - else: - return resp def _callback_button(self, msg): __tracebackhide__ = True # for pytest # pylint: disable=W0612 @@ -229,7 +231,7 @@ class TrezorClient: @tools.session def init_device(self): - resp = self.call_raw(messages.Initialize(state=self.state)) + resp = self.call_raw(messages.Initialize(session_id=self.session_id)) if not isinstance(resp, messages.Features): raise exceptions.TrezorException("Unexpected initial response") else: @@ -245,6 +247,8 @@ class TrezorClient: self.features.patch_version, ) self.check_firmware_version(warn_only=True) + if self.version >= (2, 1, 9): + self.session_id = self.features.session_id def is_outdated(self): if self.features.bootloader_mode: @@ -295,7 +299,7 @@ class TrezorClient: def clear_session(self): resp = self.call_raw(messages.ClearSession()) if isinstance(resp, messages.Success): - self.state = None + self.session_id = None self.init_device() return resp.message else: diff --git a/python/src/trezorlib/messages/Features.py b/python/src/trezorlib/messages/Features.py index 23720aa22..4a2951f24 100644 --- a/python/src/trezorlib/messages/Features.py +++ b/python/src/trezorlib/messages/Features.py @@ -50,6 +50,7 @@ class Features(p.MessageType): sd_card_present: bool = None, sd_protection: bool = None, wipe_code_protection: bool = None, + session_id: bytes = None, ) -> None: self.vendor = vendor self.major_version = major_version @@ -84,6 +85,7 @@ class Features(p.MessageType): self.sd_card_present = sd_card_present self.sd_protection = sd_protection self.wipe_code_protection = wipe_code_protection + self.session_id = session_id @classmethod def get_fields(cls) -> Dict: @@ -121,4 +123,5 @@ class Features(p.MessageType): 32: ('sd_card_present', p.BoolType, 0), 33: ('sd_protection', p.BoolType, 0), 34: ('wipe_code_protection', p.BoolType, 0), + 35: ('session_id', p.BytesType, 0), } diff --git a/python/src/trezorlib/messages/Initialize.py b/python/src/trezorlib/messages/Initialize.py index dacf75889..3d8ca3810 100644 --- a/python/src/trezorlib/messages/Initialize.py +++ b/python/src/trezorlib/messages/Initialize.py @@ -15,15 +15,12 @@ class Initialize(p.MessageType): def __init__( self, - state: bytes = None, - skip_passphrase: bool = None, + session_id: bytes = None, ) -> None: - self.state = state - self.skip_passphrase = skip_passphrase + self.session_id = session_id @classmethod def get_fields(cls) -> Dict: return { - 1: ('state', p.BytesType, 0), - 2: ('skip_passphrase', p.BoolType, 0), + 1: ('session_id', p.BytesType, 0), } diff --git a/python/src/trezorlib/messages/MessageType.py b/python/src/trezorlib/messages/MessageType.py index f66649b06..abe7754eb 100644 --- a/python/src/trezorlib/messages/MessageType.py +++ b/python/src/trezorlib/messages/MessageType.py @@ -27,8 +27,6 @@ EntropyRequest = 35 # type: Literal[35] EntropyAck = 36 # type: Literal[36] PassphraseRequest = 41 # type: Literal[41] PassphraseAck = 42 # type: Literal[42] -PassphraseStateRequest = 77 # type: Literal[77] -PassphraseStateAck = 78 # type: Literal[78] RecoveryDevice = 45 # type: Literal[45] WordRequest = 46 # type: Literal[46] WordAck = 47 # type: Literal[47] diff --git a/python/src/trezorlib/messages/PassphraseAck.py b/python/src/trezorlib/messages/PassphraseAck.py index 65f7be27b..3a95f6bc6 100644 --- a/python/src/trezorlib/messages/PassphraseAck.py +++ b/python/src/trezorlib/messages/PassphraseAck.py @@ -16,14 +16,14 @@ class PassphraseAck(p.MessageType): def __init__( self, passphrase: str = None, - state: bytes = None, + on_device: bool = None, ) -> None: self.passphrase = passphrase - self.state = state + self.on_device = on_device @classmethod def get_fields(cls) -> Dict: return { 1: ('passphrase', p.UnicodeType, 0), - 2: ('state', p.BytesType, 0), + 2: ('on_device', p.BoolType, 0), } diff --git a/python/src/trezorlib/messages/PassphraseRequest.py b/python/src/trezorlib/messages/PassphraseRequest.py index 94a5bdbac..a2abc1f9d 100644 --- a/python/src/trezorlib/messages/PassphraseRequest.py +++ b/python/src/trezorlib/messages/PassphraseRequest.py @@ -15,12 +15,12 @@ class PassphraseRequest(p.MessageType): def __init__( self, - on_device: bool = None, + _deprecated_on_device: bool = None, ) -> None: - self.on_device = on_device + self._deprecated_on_device = _deprecated_on_device @classmethod def get_fields(cls) -> Dict: return { - 1: ('on_device', p.BoolType, 0), + 1: ('_deprecated_on_device', p.BoolType, 0), } diff --git a/python/src/trezorlib/messages/PassphraseStateAck.py b/python/src/trezorlib/messages/PassphraseStateAck.py deleted file mode 100644 index 1faedc1c5..000000000 --- a/python/src/trezorlib/messages/PassphraseStateAck.py +++ /dev/null @@ -1,14 +0,0 @@ -# Automatically generated by pb2py -# fmt: off -from .. import protobuf as p - -if __debug__: - try: - from typing import Dict, List # noqa: F401 - from typing_extensions import Literal # noqa: F401 - except ImportError: - pass - - -class PassphraseStateAck(p.MessageType): - MESSAGE_WIRE_TYPE = 78 diff --git a/python/src/trezorlib/messages/PassphraseStateRequest.py b/python/src/trezorlib/messages/PassphraseStateRequest.py deleted file mode 100644 index 6cbcbe58b..000000000 --- a/python/src/trezorlib/messages/PassphraseStateRequest.py +++ /dev/null @@ -1,26 +0,0 @@ -# Automatically generated by pb2py -# fmt: off -from .. import protobuf as p - -if __debug__: - try: - from typing import Dict, List # noqa: F401 - from typing_extensions import Literal # noqa: F401 - except ImportError: - pass - - -class PassphraseStateRequest(p.MessageType): - MESSAGE_WIRE_TYPE = 77 - - def __init__( - self, - state: bytes = None, - ) -> None: - self.state = state - - @classmethod - def get_fields(cls) -> Dict: - return { - 1: ('state', p.BytesType, 0), - } diff --git a/python/src/trezorlib/messages/__init__.py b/python/src/trezorlib/messages/__init__.py index bd1bfa8e3..f829461b6 100644 --- a/python/src/trezorlib/messages/__init__.py +++ b/python/src/trezorlib/messages/__init__.py @@ -194,8 +194,6 @@ from .NEMTransfer import NEMTransfer from .NextU2FCounter import NextU2FCounter from .PassphraseAck import PassphraseAck from .PassphraseRequest import PassphraseRequest -from .PassphraseStateAck import PassphraseStateAck -from .PassphraseStateRequest import PassphraseStateRequest from .PinMatrixAck import PinMatrixAck from .PinMatrixRequest import PinMatrixRequest from .Ping import Ping diff --git a/tests/device_tests/test_basic.py b/tests/device_tests/test_basic.py index 6a503d143..ff59c181f 100644 --- a/tests/device_tests/test_basic.py +++ b/tests/device_tests/test_basic.py @@ -20,7 +20,7 @@ from trezorlib import device, messages class TestBasic: def test_features(self, client): f0 = client.features - f1 = client.call(messages.Initialize()) + f1 = client.call(messages.Initialize(f0.session_id)) assert f0 == f1 def test_ping(self, client): diff --git a/tests/device_tests/test_msg_cardano_sign_tx_slip39_basic.py b/tests/device_tests/test_msg_cardano_sign_tx_slip39_basic.py index 79aca76bf..75e5d62c0 100644 --- a/tests/device_tests/test_msg_cardano_sign_tx_slip39_basic.py +++ b/tests/device_tests/test_msg_cardano_sign_tx_slip39_basic.py @@ -119,10 +119,7 @@ def test_cardano_sign_tx( inputs = [cardano.create_input(i) for i in inputs] outputs = [cardano.create_output(o) for o in outputs] - expected_responses = [ - messages.PassphraseRequest(), - messages.PassphraseStateRequest(), - ] + expected_responses = [messages.PassphraseRequest()] expected_responses += [ messages.CardanoTxRequest(tx_index=i) for i in range(len(transactions)) ] diff --git a/tests/device_tests/test_session_id.py b/tests/device_tests/test_session_id.py new file mode 100644 index 000000000..5b0595189 --- /dev/null +++ b/tests/device_tests/test_session_id.py @@ -0,0 +1,123 @@ +# This file is part of the Trezor project. +# +# Copyright (C) 2012-2019 SatoshiLabs and contributors +# +# This library is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License version 3 +# as published by the Free Software Foundation. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the License along with this library. +# If not, see . + +import pytest + +from trezorlib import messages +from trezorlib.tools import parse_path + + +def _get_xpub(client, passphrase): + response = client.call_raw( + messages.GetPublicKey(address_n=parse_path("44'/0'/0'"), coin_name="Bitcoin") + ) + if passphrase is not None: + assert isinstance(response, messages.PassphraseRequest) + response = client.call_raw(messages.PassphraseAck(passphrase=passphrase)) + assert isinstance(response, messages.PublicKey) + return response.xpub + + +@pytest.mark.skip_t1 # TODO +@pytest.mark.setup_client(passphrase=True) +def test_session_with_passphrase(client): + # Let's start the communication by calling Initialize. + response = client.call_raw(messages.Initialize()) + assert isinstance(response, messages.Features) + session_id = response.session_id + assert len(session_id) == 32 + + # GetPublicKey requires passphrase and since it is not cached, + # Trezor will prompt for it. + xpub = _get_xpub(client, passphrase="A") + assert ( + xpub + == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" + ) + + # Call Initialize again, this time with the received session id and then call + # GetPublicKey. The passphrase should be cached now so Trezor must + # not ask for it again, whilst returning the same xpub. + response = client.call_raw(messages.Initialize(session_id=session_id)) + assert isinstance(response, messages.Features) + xpub = _get_xpub(client, passphrase=None) + assert ( + xpub + == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" + ) + + # If we set session id in Initialize to None, the cache will be cleared + # and Trezor will ask for the passphrase again. + response = client.call_raw(messages.Initialize(session_id=None)) + assert isinstance(response, messages.Features) + xpub = _get_xpub(client, passphrase="A") + assert ( + xpub + == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" + ) + + # Unknown session id is the same as setting it to None. + response = client.call_raw(messages.Initialize(session_id=b"X" * 32)) + assert isinstance(response, messages.Features) + xpub = _get_xpub(client, passphrase="A") + assert ( + xpub + == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" + ) + + +@pytest.mark.skip_t1 # TODO +@pytest.mark.setup_client() +def test_session_enable_passphrase(client): + # Let's start the communication by calling Initialize. + response = client.call_raw(messages.Initialize()) + assert isinstance(response, messages.Features) + session_id = response.session_id + assert len(session_id) == 32 + + # Trezor will not prompt for passphrase because it is turned off. + xpub = _get_xpub(client, passphrase=None) + assert ( + xpub + == "xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy" + ) + + # Turn on passphrase. + response = client.call_raw(messages.ApplySettings(use_passphrase=True)) + assert isinstance(response, messages.ButtonRequest) # confirm dialog + client.debug.press_yes() + response = client.call_raw(messages.ButtonAck()) + assert isinstance(response, messages.Success) + + # The session id is unchanged, therefore we do not prompt for the passphrase. + response = client.call_raw(messages.Initialize(session_id=session_id)) + xpub = _get_xpub(client, passphrase=None) + assert isinstance(response, messages.Features) + assert session_id == response.session_id + assert ( + xpub + == "xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy" + ) + + # We clear the session id now, so the passphrase should be asked. + response = client.call_raw(messages.Initialize()) + xpub = _get_xpub(client, passphrase="A") + assert isinstance(response, messages.Features) + assert session_id != response.session_id + assert ( + xpub + == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" + )