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

Trezor and Keepkey require user interaction #83

Closed
achow101 opened this issue Dec 29, 2018 · 6 comments
Closed

Trezor and Keepkey require user interaction #83

achow101 opened this issue Dec 29, 2018 · 6 comments

Comments

@achow101
Copy link
Member

The Trezor and Keepkey both require that the user respond to prompts and to enter things into the keyboard which are then sent to the device over USB. This is not ideal for us since HWI is intended to be a utility where everything the user needs to do is done in one command. I don't see any way around this though as all of the prompts require seeing something on the device's display which we are unable to see with software.

@instagibbs
Copy link
Collaborator

instagibbs commented Dec 29, 2018 via email

@achow101
Copy link
Member Author

Transaction signing prompts require the user to interact with the device itself. These prompts require the user to interact with the computer the device is connected to.

@instagibbs
Copy link
Collaborator

instagibbs commented Dec 29, 2018 via email

@achow101 achow101 mentioned this issue Jan 6, 2019
12 tasks
@achow101
Copy link
Member Author

achow101 commented Jan 6, 2019

If trezor/trezor-mcu#442 is accepted, the Trezor would let users enter the pin on device which fixes this issue for us for the Trezor.

@kewde
Copy link

kewde commented Jan 11, 2019

I can attest to the difficulty of integrating it with bitcoin-core.
I've written the implementation primarily to support our integration (C++) into particl-core.

Essentially, as a workaround, I've opted to write a custom Open() handler.
When for example a signing request tries to open a handle to the usb device, it will initiate a full unlock of the device (if necessary).
The tricky thing is, during the user interaction, it can not maintain the handle, it has to release it and "poll the device at regular intervals".
The device caches the PIN and passphrase.
particl/particl-core#52

function Sign() {
    usb = OpenWithUnlock(); // get handle on usb device
   ...
}

function OpenWithUnlock() {
    sendPingMessageToTrezor(); // triggers unlock procedure
    stop = false;
    while (!stop) {
        usb = Open(); // Opens a real handle
        msg = Read(usb);
        if (msg === Success || Failure) {
            stop = true;
        } 
        Close(usb);
        Sleep();
    }
    return Open();
}

function Unlock() {
    usb = Open(); // Opens a real handle
    Send(usb, pin);
     // Do not Read() at this step, or the unlock loop will catch it.
}

@keepkeyjon
Copy link

trezorctl and go-keepkey solve this nicely by taking over the terminal & sending ansi escape sequences as appropriate to make the 'ui' work:

$ go-keepkey changePin
DebugLink established over WebUSB
Connected to 1 devices
  -- Serial#: , Label: 
Awaiting user button press
sending debug press
Enter your pin using the corresponding positions shown on your device
7 | 8 | 9
4 | 5 | 6
1 | 2 | 3

✔ Pin: ********█

If trezor/trezor-mcu#442 is accepted,

We're a bit limited since we've only got one button, which makes on-device PIN entry a pain in the ass. I've prototyped this for KeepKey, but the UX is awful. Passphrase would be even worse.

When for example a signing request tries to open a handle to the usb device, it will initiate a full unlock of the device (if necessary).... The device caches the PIN and passphrase.

Our device will fall for the same trick. Any command that involves key derivation will work (GetAddress without showing on the display is a good one). IMO though, it would have been better if there was a SessionRequest message that the device could use to prompt for it, preventing many of this kind of 'hack'... it should be difficult to do what you're trying to do here.

For Trezor's you need to avoid re-sending Initialize unless you're careful about maintaining the session bytes: https://github.com/trezor/trezor-mcu/blob/7dce7bd92d65726e08e0311f9821f6cc966b44a1/firmware/fsm_msg_common.h#L26. We don't have that yet, but should.

achow101 added a commit that referenced this issue Jan 30, 2019
97b1d35 Add promptpin and sendpin commands (Andrew Chow)
bb88cc5 Introduce and implement prompt_pin and send_pin methods (Andrew Chow)

Pull request description:

  This PR introduces two new commands, `promptpin` and `sendpin`, which only work on Trezors and KeepKeys. `promptpin` triggers the device to display the prompt for entering the PIN. `sendpin` takes the scrambled PIN as an argument and sends it to the device. The device will then cache the pin if it is correct.

  The way that the libraries setup the client classes prevents us from doing this. To work around that, I made subclasses of those client classes which do not do the initialization call that breaks this process. Instead, the initialization call will be done by each function individually if it needs it.

  Closes #83

Tree-SHA512: 0aa9d135eeceef63b41c1e20efbf7439d5bf9a26f84b66da3c0602952894aa9a659bb5a9b2780b0506578a2b61049233db588cabfc0da8ed10dacd91d97c7f99
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

No branches or pull requests

4 participants