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

sendpin doesn't work on 1.2.1 #447

Closed
hugohn opened this issue Feb 5, 2021 · 11 comments
Closed

sendpin doesn't work on 1.2.1 #447

hugohn opened this issue Feb 5, 2021 · 11 comments

Comments

@hugohn
Copy link
Contributor

hugohn commented Feb 5, 2021

I am not sure if this feature has worked before, but promptpin/sendpin is not working on 1.2.1. sendpin fails to unlock the Trezor One.

You can unlock the Trezor One using the Trezor web app (trezor.js):
[trezor.js] [call] Received PinMatrixRequest {type: "PinMatrixRequestType_Current"}type: "PinMatrixRequestType_Current"
[trezor.js] [call] Sending PinMatrixAck {pin: "6666"}

But when using HWI:
./hwi --testnet -t trezor -p webusb:020:5:4:3 promptpin

Use 'sendpin' to provide the number positions for the PIN as displayed on your device's screen Use the numeric keypad to describe number positions. The layout is: 7 8 9 4 5 6 1 2 3 {"success": true}

./hwi --testnet --debug -t trezor -p webusb:020:5:4:3 sendpin 8888

DEBUG:hwilib.devices.trezorlib.transport:Enumerating WebUsbTransport: found 1 devices DEBUG:hwilib.devices.trezorlib.transport:Enumerating HidTransport: found 0 devices DEBUG:hwilib.devices.trezorlib.transport:Enumerating UdpTransport: found 0 devices DEBUG:hwilib.devices.trezorlib.transport:looking for device by full path: webusb:020:5:4:3 DEBUG:hwilib.devices.trezorlib.client:creating client instance for device: webusb:020:5:4:3 DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: GetFeatures DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Failure DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: Initialize DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Features DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: Initialize DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Features DEBUG:hwilib.devices.trezorlib.transport:Enumerating WebUsbTransport: found 1 devices DEBUG:hwilib.devices.trezorlib.transport:Enumerating HidTransport: found 0 devices DEBUG:hwilib.devices.trezorlib.transport:Enumerating UdpTransport: found 0 devices DEBUG:hwilib.devices.trezorlib.transport:looking for device by full path: webusb:020:5:4:3 DEBUG:hwilib.devices.trezorlib.client:creating client instance for device: webusb:020:5:4:3 DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: PinMatrixAck DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Failure DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: GetFeatures DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Features {"success": false}

*Note: PIN was set to 1111, "6666" and "8888" are relative positions on the blind matrix.

@hugohn
Copy link
Contributor Author

hugohn commented Feb 5, 2021

cc @prusnak

@achow101
Copy link
Member

achow101 commented Feb 5, 2021

Does this fail on master too?

I'm wondering if I've been experiencing this on master and just chalking it up to actually forgetting the pin I set on my test device.

@hugohn
Copy link
Contributor Author

hugohn commented Feb 6, 2021

Yeah, I've tested 1.2.0, 1.2.1 and master. All failed to unlock.

@prusnak
Copy link
Collaborator

prusnak commented Feb 6, 2021

This is caused because HWI sends GetFeatures and Initialize before sendpin as can be seen from the log above:

DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: GetFeatures
DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Failure
DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: Initialize
DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Features
DEBUG:hwilib.devices.trezorlib.transport.protocol:sending message: Initialize
DEBUG:hwilib.devices.trezorlib.transport.protocol:received message: Features

This aborts the PIN workflow in the device.

Investigating further ...

@prusnak
Copy link
Collaborator

prusnak commented Feb 6, 2021

So it seems the following happens:

  File "/Users/stick/work/bitcoin-core/hwi/hwilib/cli.py", line 280, in main
    result = process_commands(sys.argv[1:])
  File "/Users/stick/work/bitcoin-core/hwi/hwilib/cli.py", line 257, in process_commands
    client = find_device(args.password, args.device_type, args.fingerprint, args.expert)
  File "/Users/stick/work/bitcoin-core/hwi/hwilib/commands.py", line 73, in find_device
    devices = enumerate(password)
  File "/Users/stick/work/bitcoin-core/hwi/hwilib/commands.py", line 66, in enumerate
    result.extend(imported_dev.enumerate(password))
  File "/Users/stick/work/bitcoin-core/hwi/hwilib/devices/trezor.py", line 604, in enumerate
    client.client.init_device()

HWI tries to enumerate the devices on sendpin which causes that client.client.init_device() is called on the device during enumeration - which sends GetFeatures aborting the PIN workflow.

@prusnak
Copy link
Collaborator

prusnak commented Feb 6, 2021

It would be ideal if there was a way how to join promptpin and sendpin into a single command called unlockpin.

Having bi-directional stdin/stdout communication interface #450 would allow us to not restart HWI all the time, just once per session leading to an easy fix of the issue.

@achow101
Copy link
Member

achow101 commented Feb 6, 2021

Oh, I remember now. The way to use promptpin is to also specify the device path so it doesn't do the whole enumerate "guess my device" thing.

@prusnak
Copy link
Collaborator

prusnak commented Feb 6, 2021

Oh, I remember now. The way to use promptpin is to also specify the device path so it doesn't do the whole enumerate "guess my device" thing.

  1. I think you mean sendpin not promptpin, right?
  2. The original report contains both the device type and the device path for both promptpin and sendpin calls. I confirm I see the same behaviour when I try to reproduce on my machine. Providing the path does not skip the enumeration (which sends GetFeatures) to the device.

@achow101
Copy link
Member

achow101 commented Feb 6, 2021

  1. I think you mean sendpin not promptpin, right?

Oops yes.

  1. The original report contains both the device type and the device path for both promptpin and sendpin calls. I confirm I see the same behaviour when I try to reproduce on my machine. Providing the path does not skip the enumeration (which sends GetFeatures) to the device.

Looking more closely, it's because the -d is the switch to use for specifying the path, not -p.

When I tested this prior to commenting, I had used the correct switch and it all worked fine. In my first comment, I was forgetting -d entirely.

@hugohn
Copy link
Contributor Author

hugohn commented Feb 7, 2021

Looking more closely, it's because the -d is the switch to use for specifying the path, not -p.

Oh wow, confirmed that changing to -d works!!! So it was user (mine) error...

@prusnak
Copy link
Collaborator

prusnak commented Feb 7, 2021

Ah :-) I did the same mistake, because I copied the code from the OP.

I confirm that it works once I changed -p to -d

Closing then.

@prusnak prusnak closed this as completed Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants