Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pin commands #90

Closed
wants to merge 6 commits into from

Conversation

coinsurenz
Copy link

@coinsurenz coinsurenz commented Oct 10, 2023

Adds the missing prompt_pin and send_pin commands mentioned here (backup appears to have already been implemented FYI).

Adding these commands required some refactoring-
They need to be called on an HWIDevice struct, initialized via get_client, however as things are HWIDevice requires a Fingerprint to initialise it, which is not available when the device is in a locked state. Therefore I have updated its type to Option<Fingerprint>, with None being the value when in a locked state.

This also required some refactoring of the conversion of HWIDeviceInternal to HWIDevice via the TryFrom trait.

when using the python HWI implementation accessing these commands is done by calling:
./hwi.py enumerate
from the command line, which returns:
[{"type": "trezor", "path": "webusb:001:1:3", "label": "My Trezor", "model": "trezor_1", "needs_pin_sent": true, "needs_passphrase_sent": false, "error": "Could not open client or get fingerprint information: Trezor is locked. Unlock by using 'promptpin' and then 'sendpin'.", "code": -12}]
you would then reference the path referenced in this error and enter
./hwi.py -t trezor -d webusb:001:1:2 promptpin
to correctly call the promptpin command.

However in Rust HWI paths are not directly specified, and as things currently are the occurance of all Errors will prevent conversion from HWIDeviceInternal to HWIDevice, and instead return an error which contains only the error message and code ([Err(Hwi("Could not open client or get fingerprint information: Trezor is locked. Unlock by using 'promptpin' and then 'sendpin'.", Some(-12)))] in the example above).

After playing around with a few approaches the cleanest and simplest to my eyes is what I have implemented here- an if statement to let this specific error pass, and then a match statement to either return the error in all other instances, or initialise the struct with the correct Fingerprint value.

@danielabrozzoni
Copy link
Member

Hey, can you try rebasing? We recently fixed the CI. Thanks!

@coinsurenz
Copy link
Author

Appologies for the delay in following up, Ive not been in a position to access the spare trezor I have been using for tests locally. The two tests failing here are

 tests::test_prompt_pin
 tests::test_send_pin

These will fail if a pin has not been enabled on the device. This is troublesome because the docs don't seem to contain a command to toggle pin on and off. Using trezor suite to enable me to toggle pin on and off I believe the solution here is to set the device to have pin enabled, then run these tests first, after which the pin prompt has been passed the device should be in the same state as no pin, where all other tests will pass.

Another issue is that if the pin is prompted but not sent the device remains in the pin prompt state and will not respond to other commands. With a physical device the only way that this seems possible is to remove the usb cable and plug in again, however Im not sure if this is possible via simulator or other means and testing both commands together may therefore be necessary.

A little time poor at the moment but leave this with me and I will come back to this shortly and see if I can find a solution to these issues

@notmandatory
Copy link
Member

Replaced by #106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants