Trezorctl Settings in Uninitialized Devices is Accessible

[wendys-cats]

Bug
1. User is able to access (and exectue) set commands on unitialized devices via trezorctl with the exception of wipe-code and PIN for newest FW (applies down to 1.9.0 and 2.3.0).
2. Older FW, like 1.8.3 and 2.1.8, also allow to set PIN for unitialized device.
After setting PIN for this unitialized device (tried on 2.1.8), user is not able to create new wallet (view video).
2.5 Also, after each try, the (unsuccessful) proccess of loading seed into TT device takes longer and longer (was at 30 something seconds at the end of video, where it cuts).

Used
2.3.2 debug and older (also tried on 2.3.1 revision: 9 bytes b'c6b2580cd', 2.1.8 revision: 8 bytes b'8eb6ce08')
1.9.1 revision: 20 bytes 0xc6b2580cd245ee924507f45e9675f857a3d78768 and older (also tried on 1.8.3 revision: 20 bytes 0xdf0963ec48f01f3d07ffca556e21ff0070cab099, 1.9.0 revision: 20 bytes 0x0b7a8449f8dd003fc415262b05102d113247d3de)
trezorctl 0.12.0
nixOS latest stable
wallet.trezor.io

Steps to Reproduce

1. Load FW into device, no seed
2. Connect it and execute set commands in trezorctl

Expected behavior
User should not be able to manage settings of an unitialized trezor device.

Screenshot - shows the state of device before trying to create a wallet (this part is in the video)

video: https://drive.google.com/file/d/1gCIMpIg5xy7u9HpDiAxwen1RYdKXOT4d/view

Reported as requested by @matejcik . At first I also mentioned that these settings are not saved on unitialized devices, which was a mistake on my part. They are saved and will apply (in instance of soft lock, after seed is loaded and after PIN is set).

[matejcik]

At first I also mentioned that these settings are not saved on unitialized devices, which was a mistake on my part. They are saved and will apply (in instance of soft lock, after seed is loaded and after PIN is set).

This is important! Pointing it out because I completely missed it in bug description.

Now this reduces to a question of whether we want to allow settings in advance (probably not)

[tsusanka]

I am bit confused here.

On an empty device after running trezorctl set passphrase enabled -f the setting is stored because:

$ trezorctl get-features
    ...
    passphrase_always_on_device: True,
    passphrase_protection: True,

but running trezorctl reset-device --skip-backup after that I get:

$ trezorctl get-features
    ...
    passphrase_always_on_device: False,
    passphrase_protection: False,

So in this case the setting does not survive the reset.

I believe it depends. For some settings that are not overridden by the reset_device (or recovery) the setting will stay, for some it will not.

We definitely need to unify the behaviour. I will discuss this in Product which way we would like to go.

[tsusanka]

We have agreed that we will forbid ApplySettings and other "settings messages" in case the device is not initialized.

Also we have agreed no settings will survive the reset/recovery. But since we do what is written above I do not see how that could happen?

[tsusanka]

@wendys-cats please retest this on current master (b6c8cbc) - no settings should be stored until the device is initialized. It is allowed to set PIN during reset/recovery but not before using the trezorctl set command. That should throw "Not initialized" error.

[wendys-cats]

@tsusanka
TEST OK
tested on TT FW 2.3.2 revision b6c8cbc
tested via trezorctl version 0.12.0 (except homescreen on initialized device)
Any action from trezorctl set commands results in "Error: NotInitialized: Device is not initialized".
After generating seed, all the set commands are available (tested PIN, auto-lock-delay, passhprase, wipe code, passphrase always on device, label, homescreen, screen rotation) and are remembered on device.