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

Support for Trezor session pass-through #326

Open
matejcik opened this issue May 4, 2020 · 10 comments
Open

Support for Trezor session pass-through #326

matejcik opened this issue May 4, 2020 · 10 comments

Comments

@matejcik
Copy link
Contributor

matejcik commented May 4, 2020

related: #151, #319 (and #319 (comment)), trezor/trezor-firmware#923

Starting with Trezor T and currently also implemented on Trezor One, the passphrase is cached only for the current session. The host must provide a valid session ID to the Initialize call, otherwise the user needs to enter passphrase again.

HWI currently has no capability to retrieve / retain / pass-through this session ID. This leads to frustrating behavior, where the user would need to enter passphrase every time they execute a HWI command; more often in case some sort of end-user application is using HWI as a library and emitting commands frequently.

The current solution implemented in HWI is to (almost) never call Initialize, and implicitly rely on the fact that sessions stay open indefinitely. This might not be true in the future: we have USB-level encrypted sessions on the roadmap.
(N.b., when implemented, this will probably also break the promptpin/sendpin song and dance.)

Also when accessing Trezor by a USB path and not a fingerprint, the caller would be susceptible to something else on the host touching Trezor and, e.g., opening a session with a different passphrase. If I'm not mistaken, this would not be detected by the HWI caller, unless they periodically enumerate devices and notice a change in the XPUB fingerprint?

Of course, this is only a problem when the passphrase is entered on the device; when entering passphrase on the host, the caller can always repeat the passphrase and don't need to care about sessions and passphrase caching at all.


The architecture and design of Trezor and HWI are fundamentally incompatible in this regard. This is problematic when a fully stateful app like Wasabi is using HWI as a fully stateless bridge.

HWI's preferred (it seems) solution was raised in trezor/trezor-firmware#923, but we are moving away from this, not towards it. Our security model is locking Trezor to a particular application, as opposed to a cryptographic server freely available to anything on the host.
(which seems to be what some other HW wallets are doing?)

From Trezor's point of view, the obvious solution is for HWI to obtain the session ID (during enumeration, presumably? or maybe via "ensure-unlocked" command?), and have the caller pass this as a token to subsequent commands. Either as a parameter or via an environment variable.
It seems that there is a possible incompatibility of such feature, and specifying the device via a fingerprint. I don't currently have a good answer for that, hence this open-ended issue.

Adding session ID to path as suggested at some point in #319 is a bad idea as it makes the path transient.

cc @nopara73, @NicolasDorier, @molnard, for the "stateful app" viewpoint

@NicolasDorier
Copy link

As far as BTCPay Server is concerned, supporting Trezor T has been very difficult even before the breaking change. I am not motivated to change btcpayserver own code to accomodate any change they did, so any solution that hwi can handle transparently (ie, without me changing my code) is fine.

Else, I invite Trezor developers to fix BTCPay Server source code to improve the UX of their user.

I can only say that we are not supporting "password on host".

Also, I quickly tested the latest release of HWI on Trezor T, I don't remember any UX issue. If there is a need to enter password for each command, I don't think that it is huge deal, as our user rarely need to do several commands.

@molnard
Copy link
Contributor

molnard commented May 6, 2020

Also when accessing Trezor by a USB path and not a fingerprint, the caller would be susceptible to something else on the host touching Trezor and, e.g., opening a session with a different passphrase. If I'm not mistaken, this would not be detected by the HWI caller, unless they periodically enumerate devices and notice a change in the XPUB fingerprint?

Yes. Wasabi is using this XPUB fingerprint to identify the correct device. Enumerate gives back a list of devices. So I need to have the correct fingerprint among the results of the enumerate. What @NicolasDorier mentioned that the user has to enter the passphrase on every command seems to be OK for me too.
Basically this will only require:

  • Finding the device and add it to the Wasabi (first time and one time).
  • Signing transactions.
  • Show address on the device, etc - rarely used.

However as far as I can understand that would also imply some kind of session ID. That could be internally handled by HWI.

The advantage of this that HWI can remain stateless which is huge, as having a unified interface among all hardware wallets. The disadvantage that the user has to enter his passphrase on every send.

@prusnak
Copy link
Collaborator

prusnak commented Feb 4, 2021

The only thing that HWI would need to store the dict of device_ids and session_ids. This could be in a form of a .json file stored in ~/.hwi or alternative location. It would typically look like this:

{
"99513eef22a57cf1c7b0a060": "34d53ec7b0288afbbf1fc744bd6d538a",
"ee25fd67ec651d086d95bbc6": "bed421aa11b477a054de1e3bab322d28",
"518d63cad652e94711084394": "ebffda63e458fcc7b918652e806cf404"
}

If the file is lost, there is no problem, because a new session would be created next time.

@achow101 Would this be acceptable? If yes, we'll contribute a pull request with the change.

This will be also required in the future, once Trezor will switch to encrypted sessions.

@benma I understand bitbox02 has encrypted sessions already. How is pairing implemented for bitbox02? Does user have to pair the device for each operation separately?

@benma
Copy link
Contributor

benma commented Feb 4, 2021

The BitBox02 is paired with a host public key, and an encrypted channel is established automatically for each session once the pairing code was accepted. Normally we do one pubkey per application (BitBoxApp, Electrum, etc.). Since there was no infrastructure to perform our pairing workflow in HWI, we made a shortcut and ask users to pair in the BitBoxApp, and reuse that pairing in HWI.

This is obviously not ideal. It would be great to have first class support for pairing in HWI. I am not sure how to achieve that though, as it requires both output (pairing code) and input (accept/reject the pairing code). This is especially tricky on the CLI, which is used by downstream wallets directly.

@matejcik
Copy link
Contributor Author

matejcik commented Feb 5, 2021

and reuse that pairing in HWI.

I'm interested in this step. How do you configure HWI to use the pairing? I'm assuming there must be some sort of key file or string to supply?

@benma
Copy link
Contributor

benma commented Feb 5, 2021

@matejcik the pairing info is stored in a file by the BitBoxApp and reused by HWI. On Linux, it's at ~/.config/bitbox/bitbox02/bitbox02.json. It contains the host static keypair and a list of device pubkeys (one for each paired device).

As I said, reuse by HWI is not a good solution and mostly a workaround. Ideally HWI would make and store its own keypair and use that to pair, not depending on the BitBoxApp.

@benma
Copy link
Contributor

benma commented Feb 5, 2021

@matejcik If you need a pairing workflow, it would be helpful to create a design that works for Trezor and BitBox02 at the same time, so downstream wallets have an easier time and do not have to implement multiple workflows.

@matejcik
Copy link
Contributor Author

matejcik commented Feb 5, 2021

@achow101 Would this be acceptable? If yes, we'll contribute a pull request with the change.

in particular re @prusnak's request, we might implement the session storage in trezorlib, and HWI would only have to point to an appropriate file location, maybe reusing trezorctl's default

@matejcik If you need a pairing workflow, it would be helpful to create a design that works for Trezor and BitBox02 at the same time, so downstream wallets have an easier time and do not have to implement multiple workflows.

Have you looked at the HWI flow for request/enter PIN on the T1? It's pretty bad but presumably something along these lines could also work for pairing?

@achow101
Copy link
Member

achow101 commented Feb 5, 2021

I don't think that HWI should implement session caching itself. I don't want to have filesystem handling code within HWI's code as I have found that maintaining filesystem code for multiple platforms to be rather painful. However, if the underlying libraries want to store state on disk (as Bitbox02 does) and provide some API that HWI can use to access this state to maintain the same session on device, then that is fine. Even better would be for the libraries to do this transparently without HWI having many (or any) changes necessary other than updating the local copies.

@prusnak
Copy link
Collaborator

prusnak commented Feb 6, 2021

If we don't want to store session on filesystem, we can have them in memory. We'd just need to have bi-directional stdin/stdout communication interface #450, so we don't have to restart HWI all the time and we'll start it only once per session

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

6 participants