andrewkozlik
commented
5 years ago I think it makes sense to differentiate between the two functions for now even though they behave the same. In the future we might want to implement different behavior for each, i.e. we might not want check_pin() to unlock the storage. However, I noticed that during dry run recovery we do not check the PIN if config.has_pin() returns false. We should not trust what config.has_pin() returns, because the stored value is not cryptographically validated.
Changes:
- Removed the code duplication in modtrezorconfig.c.
- Verify the return value of config.has_pin() in dry run recovery.
- Improved UI messages in dry run recovery. See https://github.com/trezor/trezor-core/pull/536
Test cases: TC1: Perform a dry run recovery ("Check recovery seed" in Trezor Wallet) on a device which does not have a PIN set. Verify that the user is not asked to enter PIN and dry run proceeds as expected. TC2: Perform a dry run recovery on a device which has a PIN set. Enter the correct PIN. Verify that dry run proceeds as expected. TC3: Perform a dry run recovery on a device which has a PIN set. Enter an incorrect PIN. Verify that the dry run recovery is aborted.
tsusanka
The check_pin and unlock function are completely the same.
Does it make sense to have both? If needed, then at least call the first from the other one.
@andrewkozlik would you like to look at this?
Reported by @ciny