diff --git a/core/src/apps/management/apply_settings.py b/core/src/apps/management/apply_settings.py index a235e7b7be..12cff9f75f 100644 --- a/core/src/apps/management/apply_settings.py +++ b/core/src/apps/management/apply_settings.py @@ -77,7 +77,7 @@ async def require_confirm_change_passphrase_source( text = Text("Passphrase source", ui.ICON_CONFIG) if passphrase_always_on_device: text.normal( - "Do you really want to", "entry passphrase always", "on the device?" + "Do you really want to", "enter passphrase always", "on the device?" ) else: text.normal("Do you want to revoke", "the passphrase on device", "setting?") diff --git a/core/src/storage/device.py b/core/src/storage/device.py index 47f549426a..d028a5c9a0 100644 --- a/core/src/storage/device.py +++ b/core/src/storage/device.py @@ -144,16 +144,13 @@ def no_backup() -> bool: def get_passphrase_always_on_device() -> bool: - b = common.get(_NAMESPACE, _PASSPHRASE_ALWAYS_ON_DEVICE) - # backwards compatible for _PASSPHRASE_SOURCE.HOST = 2 - if b == b"\x02": - return False - # backwards compatible for _PASSPHRASE_SOURCE.DEVICE = 1 - # and also \x01 is TRUE_BYTE so it is future compatible as well - elif b == b"\x01": - return True - else: - return False + """ + This is backwards compatible with _PASSPHRASE_SOURCE: + - If ASK(0) => returns False, the check against b"\x01" in get_bool fails. + - If DEVICE(1) => returns True, the check against b"\x01" in get_bool succeeds. + - If HOST(2) => returns False, the check against b"\x01" in get_bool fails. + """ + return common.get_bool(_NAMESPACE, _PASSPHRASE_ALWAYS_ON_DEVICE) def load_settings( diff --git a/python/src/trezorlib/cli/settings.py b/python/src/trezorlib/cli/settings.py index 5368875462..404711275a 100644 --- a/python/src/trezorlib/cli/settings.py +++ b/python/src/trezorlib/cli/settings.py @@ -139,23 +139,12 @@ def passphrase(): @passphrase.command(name="enabled") -@click.option("-f", "--force-on-device", is_flag=True) -@click.option("-F", "--no-force-on-device", is_flag=True) +@click.option("-f/-F", "--force-on-device/--no-force-on-device", default=None) @click.pass_obj -def passphrase_enable(connect, force_on_device: bool, no_force_on_device: bool): +def passphrase_enable(connect, force_on_device: bool): """Enable passphrase.""" - if force_on_device and no_force_on_device: - raise ValueError( - "Only one option of --force-on-device/-no-force-on-device makes sense." - ) - on_device = None - if force_on_device: - on_device = True - if no_force_on_device: - on_device = False - return device.apply_settings( - connect(), use_passphrase=True, passphrase_always_on_device=on_device + connect(), use_passphrase=True, passphrase_always_on_device=force_on_device ) diff --git a/python/src/trezorlib/cli/trezorctl.py b/python/src/trezorlib/cli/trezorctl.py index 5f184d247c..95b0b235cb 100755 --- a/python/src/trezorlib/cli/trezorctl.py +++ b/python/src/trezorlib/cli/trezorctl.py @@ -141,11 +141,7 @@ def configure_logging(verbose: int): "-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.", + "-P", "--passphrase-on-host", is_flag=True, help="Enter passphrase on host.", ) @click.version_option() @click.pass_context diff --git a/tests/device_tests/test_protection_levels.py b/tests/device_tests/test_protection_levels.py index a8830752db..8e7eb67561 100644 --- a/tests/device_tests/test_protection_levels.py +++ b/tests/device_tests/test_protection_levels.py @@ -63,7 +63,6 @@ class TestProtectionLevels: ) device.change_pin(client) - @pytest.mark.setup_client() def test_ping(self, client): with client: client.set_expected_responses([proto.ButtonRequest(), proto.Success()]) diff --git a/tests/device_tests/test_session_id_and_passphrase.py b/tests/device_tests/test_session_id_and_passphrase.py index 5418ad719e..4ff7e24f3a 100644 --- a/tests/device_tests/test_session_id_and_passphrase.py +++ b/tests/device_tests/test_session_id_and_passphrase.py @@ -17,10 +17,19 @@ import pytest from trezorlib import messages +from trezorlib.messages import FailureType from trezorlib.tools import parse_path +XPUB_PASSPHRASE_A = "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" +XPUB_PASSPHRASE_NONE = "xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy" + def _get_xpub(client, passphrase): + """ + Are calls are intentionally "raw", i.e. we are not using trezorlib + to avoid its implicit session management + (clearing session after settings, hidden handling of session ids, etc.). + """ response = client.call_raw( messages.GetPublicKey(address_n=parse_path("44'/0'/0'"), coin_name="Bitcoin") ) @@ -40,7 +49,6 @@ def _enable_passphrase(client): @pytest.mark.skip_ui -@pytest.mark.setup_client() def test_session_with_passphrase(client): # Turn on passphrase. _enable_passphrase(client) @@ -54,10 +62,7 @@ def test_session_with_passphrase(client): # GetPublicKey requires passphrase and since it is not cached, # Trezor will prompt for it. xpub = _get_xpub(client, passphrase="A") - assert ( - xpub - == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" - ) + assert xpub == XPUB_PASSPHRASE_A # Call Initialize again, this time with the received session id and then call # GetPublicKey. The passphrase should be cached now so Trezor must @@ -65,33 +70,23 @@ def test_session_with_passphrase(client): response = client.call_raw(messages.Initialize(session_id=session_id)) assert isinstance(response, messages.Features) xpub = _get_xpub(client, passphrase=None) - assert ( - xpub - == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" - ) + assert xpub == XPUB_PASSPHRASE_A # 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" - ) + assert xpub == XPUB_PASSPHRASE_A # 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" - ) + assert xpub == XPUB_PASSPHRASE_A @pytest.mark.skip_ui -@pytest.mark.setup_client() def test_session_enable_passphrase(client): # Let's start the communication by calling Initialize. response = client.call_raw(messages.Initialize()) @@ -101,10 +96,7 @@ def test_session_enable_passphrase(client): # Trezor will not prompt for passphrase because it is turned off. xpub = _get_xpub(client, passphrase=None) - assert ( - xpub - == "xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy" - ) + assert xpub == XPUB_PASSPHRASE_NONE # Turn on passphrase. _enable_passphrase(client) @@ -114,25 +106,18 @@ def test_session_enable_passphrase(client): xpub = _get_xpub(client, passphrase=None) assert isinstance(response, messages.Features) assert session_id == response.session_id - assert ( - xpub - == "xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy" - ) + assert xpub == XPUB_PASSPHRASE_NONE # 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" - ) + assert xpub == XPUB_PASSPHRASE_A @pytest.mark.skip_ui @pytest.mark.skip_t1 -@pytest.mark.setup_client() def test_passphrase_always_on_device(client): # Let's start the communication by calling Initialize. response = client.call_raw(messages.Initialize()) @@ -158,10 +143,7 @@ def test_passphrase_always_on_device(client): client.debug.input("") # Input empty passphrase. response = client.call_raw(messages.ButtonAck()) assert isinstance(response, messages.PublicKey) - assert ( - response.xpub - == "xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy" - ) + assert response.xpub == XPUB_PASSPHRASE_NONE # Passphrase will not be prompted. The session id stays the same and the passphrase is cached. response = client.call_raw(messages.Initialize(session_id=session_id)) @@ -170,10 +152,7 @@ def test_passphrase_always_on_device(client): messages.GetPublicKey(address_n=parse_path("44'/0'/0'"), coin_name="Bitcoin") ) assert isinstance(response, messages.PublicKey) - assert ( - response.xpub - == "xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy" - ) + assert response.xpub == XPUB_PASSPHRASE_NONE # In case we want to add a new passphrase we need to send session_id = None. response = client.call_raw(messages.Initialize(session_id=None)) @@ -185,15 +164,11 @@ def test_passphrase_always_on_device(client): client.debug.input("A") # Input empty passphrase. response = client.call_raw(messages.ButtonAck()) assert isinstance(response, messages.PublicKey) - assert ( - response.xpub - == "xpub6CekxGcnqnJ6osfY4Rrq7W5ogFtR54KUvz4H16XzaQuukMFZCGebEpVznfq4yFcKEmYyShwj2UKjL7CazuNSuhdkofF4mHabHkLxCMVvsqG" - ) + assert response.xpub == XPUB_PASSPHRASE_A @pytest.mark.skip_ui @pytest.mark.skip_t2 -@pytest.mark.setup_client() def test_passphrase_on_device_not_possible_on_t1(client): # Let's start the communication by calling Initialize. response = client.call_raw(messages.Initialize()) @@ -206,7 +181,7 @@ def test_passphrase_on_device_not_possible_on_t1(client): response = client.call_raw(messages.ApplySettings(passphrase_always_on_device=True)) assert isinstance(response, messages.Failure) - assert response.code == 3 # DataError + assert response.code == FailureType.DataError response = client.call_raw( messages.GetPublicKey(address_n=parse_path("44'/0'/0'"), coin_name="Bitcoin") @@ -214,7 +189,7 @@ def test_passphrase_on_device_not_possible_on_t1(client): assert isinstance(response, messages.PassphraseRequest) response = client.call_raw(messages.PassphraseAck(on_device=True)) assert isinstance(response, messages.Failure) - assert response.code == 3 # DataError + assert response.code == FailureType.DataError @pytest.mark.skip_ui @@ -226,7 +201,7 @@ def test_passphrase_ack_mismatch(client): assert isinstance(response, messages.PassphraseRequest) response = client.call_raw(messages.PassphraseAck(passphrase="A", on_device=True)) assert isinstance(response, messages.Failure) - assert response.code == 3 # DataError + assert response.code == FailureType.DataError @pytest.mark.skip_ui @@ -238,4 +213,12 @@ def test_passphrase_missing(client): assert isinstance(response, messages.PassphraseRequest) response = client.call_raw(messages.PassphraseAck(passphrase=None)) assert isinstance(response, messages.Failure) - assert response.code == 3 # DataError + assert response.code == FailureType.DataError + + response = client.call_raw( + messages.GetPublicKey(address_n=parse_path("44'/0'/0'"), coin_name="Bitcoin") + ) + assert isinstance(response, messages.PassphraseRequest) + response = client.call_raw(messages.PassphraseAck(passphrase=None, on_device=False)) + assert isinstance(response, messages.Failure) + assert response.code == FailureType.DataError