almindor / etherwall

Ethereum QT5 Wallet
GNU General Public License v3.0
142 stars 59 forks source link

Trezor Model T not detected #99

Closed gkaindl closed 6 years ago

gkaindl commented 6 years ago

My Trezor Model T doesn't get detected by Etherwall 2.2.3 on MacOS 10.12.6. The Trezor works fine in other apps, though, such as Electrum and via WebUSB on MyEtherWallet or the Trezor Wallet.

I've tried plugging it in before and after starting the app, as well as entering the PIN before and after starting the app.

There doesn't seem to be anything trezor/usb related in the logs either.

INFO [06-03|14:41:00] IPC endpoint opened                      url=/Users/XXXXX/Library/Ethereum-geth/geth.ipc

INFO [06-03|14:41:00] Initialised chain configuration          config="{ChainID: 1 Homestead: 1150000 DAO: 1920000 DAOSupport: true EIP150: 2463000 EIP155: 2675000 EIP158: 2675000 Byzantium: 4370000 Constantinople: <nil> Engine: ethash}"
INFO [06-03|14:41:00] Disk storage enabled for ethash caches   dir=/Users/XXXXX/Library/Ethereum-geth/geth/ethash count=3
INFO [06-03|14:41:00] Disk storage enabled for ethash DAGs     dir=/Users/XXXXX/.ethash                           count=2
INFO [06-03|14:41:00] Initialising Ethereum protocol           versions="[63 62]" network=1
INFO [06-03|14:41:00] Loaded most recent local header          number=0 hash=d4e567…cb8fa3 td=17179869184
INFO [06-03|14:41:00] Loaded most recent local full block      number=0 hash=d4e567…cb8fa3 td=17179869184
INFO [06-03|14:41:00] Loaded most recent local fast block      number=0 hash=d4e567…cb8fa3 td=17179869184
INFO [06-03|14:41:00] Loaded local transaction journal         transactions=0 dropped=0
INFO [06-03|14:41:00] Regenerated local transaction journal    transactions=0 accounts=0
INFO [06-03|14:41:00] Starting P2P networking 
INFO [06-03|14:41:00] RLPx listener up                         self="enode://2dabf9bb077d60da00c65cba11fc554466a5aaa531183192cc6eef7c5b6fffb20c95bb034209fe6b11673e4529464c176d04e6d9bf2a8a76932a87648a9d7806@[::]:30303?discport=0"

INFO [06-03|14:41:00] Maximum peer count                       ETH=0 LES=0 total=0
INFO [06-03|14:41:00] Starting peer-to-peer node               instance=Geth/v1.8.8-stable-2688dab4/darwin-amd64/go1.10.1
INFO [06-03|14:41:00] Allocated cache and file handles         database=/Users/XXXXX/Library/Ethereum-geth/geth/chaindata cache=384 handles=1024
almindor commented 6 years ago

Hey,

I don't own a Trezor T yet. Etherwall uses the low-level USB interface directly to talk to Trezor One so it has hard-coded the device-id so it only picks a Trezor One atm.

I need to confirm that Trezor T is 100% compatible on the usb-level communication with how things are done with the Trezor One before enabling it. If you want, you can easily compile a version of Etherwall that will detect the Trezor T by changing this line to allow the enumeration of Trezor One's device ID (second parameter).

The HID documentation at trezor.io currently does not even mention the USB HID id of Trezor T however.

I'll try to get the ID and confirmation that Trezor T is compatible on the HID level with Trezor One from Satoshi Labs. If they confirm I'll enable it in the next release.

almindor commented 6 years ago

It seems Trezor One should be API compatible on the USB level. Found the "vendor/device" ID's in their Java implementation here.

I'm still waiting for a confirmation from the devs but if you're willing please reply here and I'll build a beta release with Trezor One detection enabled.

gkaindl commented 6 years ago

I just tried it (deviceId is 0x53c1 for me, so 0x53c0 might be a preproduction unit or something like this).

With this change, the trezor gets recognized as soon as device has been unlocked by entering the PIN (which makes sense, since model T is only supposed to enable USB after successful PIN entry), but then Etherwall hangs at the "Loading" spinner, so there seems to be some difference there...

almindor commented 6 years ago

Yeah I was afraid that'd be the case. I wish they'd update their docs to reflect these things. I'll talk to Pavol (one of the Satoshi Lab's devs) see if he can give me an idea on what needs to be done. Maybe I can coerce him into sending me a Trezor T :)

Sadly until then this will remain unfixed. I'll update the webpage to make sure people understand that only Trezor One is currently supported.

gkaindl commented 6 years ago

Alright, if you need me to test anything, just update the issue, please! :-)

almindor commented 6 years ago

Sure, if you feel like it we could try and make this work. You could start by adding qDebug() << "whatever\n"; statements all over the Trezor state machine in code. There are a few places in the trezor device manager itself, e.g. here

Then there's the account model response to trezor being initiated here

After which it's mostly communication directly as needed. We first need to find out where things get stuck. It's either somewhere in these two or when it's trying to get the addresses from Trezor.

almindor commented 6 years ago

As a side note: if you decide to go and debug this please make a fork of the repo under your github user so you can easily make a PR if you figure this out :)

gkaindl commented 6 years ago

So actually, it looks as if the first message to trezor (the "Initialize" message) already doesn't return the expected answer.

I've checked their trezor-python libs as well (since they seem to be more up to date), and while trezor-core apparently as a protocol version 2 now, it's not being used, but the trezor-one protocol is still the default (see: here)

Could you do me a favor and try the trezor-t branch on my account? I've added hexdumps of the bytes sent and received on the wire, and that's my output on the model t – it doesn't look right.

WRITE 65 -----
00 3F 23 23 00 00 00 00  00 00 00 00 00 00 00 00  |  .?##............ 
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |  ................ 
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |  ................ 
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |  ................ 
00                                                |  . 
----------
READ 65 -----
3F 23 23 00 BF 00 01 06  00 00 00 00 00 00 00 00  |  ?##............. 
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |  ................ 
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |  ................ 
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |  ................ 
00                                                |  . 
----------

Also, I don't always get the 0x3f 0x23 0x23 "magic bytes" in the reply, sometimes I get 0x3f 0xff 0xff instead.

It'd be helpful to compare that with your output from the Trezor One!

Thanks!

almindor commented 6 years ago

Hmm interesting. I'll do the test tomorrow. I've just pushed a minor fix for exception handling (catch by value vs reference) + updated the trezor-common submodule with the protobuf definitions. See if that maybe changed anything since I haven't upgraded the protobufs in some time. You need to make sure your submodules are updated to latest and then run the ./generate_protobuf.sh in root.

gkaindl commented 6 years ago

Updating the protobufs after merging your latest commit didn't help, unfortunately...

Yeah, it's strange, I consistently get the same garbage data... the overall problem then is that Device::buffer_report() will spin forever in the do-while loop, because the trezor doesn't send anything, while the garbage data makes it expect way more data to read... :-/

almindor commented 6 years ago

This is what I got from the SatoshiLab guys:

matejcik
@matejcik
07:35
@almindor unsure what you mean by "100 % compatible on USB level", but probably not. ATM One communicates over HID, while T is a WebUSB device (with a separate HID interface for U2F)

So it seems they changed the channel. If the protobuf/messages are the same it shouldn't be that hard to implement this but I can't test.

almindor commented 6 years ago

Here's the full communication in case you decide to give it a go:

@matejcik
05:58
@almindor see https://github.com/trezor/python-trezor/blob/master/trezorlib/transport/webusb.py for USB identifiers
matejcik
@matejcik
07:35
@almindor unsure what you mean by "100 % compatible on USB level", but probably not. ATM One communicates over HID, while T is a WebUSB device (with a separate HID interface for U2F)
Aleš Katona
@almindor
09:13
@matejcik yes that's what I meant. Thank for clarification. The protocol messages (defined by the protobuf) are still same for T right?
matejcik
@matejcik
09:15
@almindor yes
@almindor but there are also slight differences in the message chunking format. see the file I linked :)
Aleš Katona
@almindor
09:16
@matejcik thanks. Any chance there's an existing c++ implementation somewhere? :D
matejcik
@matejcik
09:16
btw we're in the process of updating T1 to the webusb format, so then it will be the same again ;)
i wouldn't know. there's a Go one though
@almindor ha, no, i'm wrong! the chunking format is the same. only difference is, something something HID and sending one prefix byte vs not sending it depending on the HID version
which obviously doesn't apply to webusb

So it seems messages and protocol should be the same with the Transport needing to be updated. He also mentioned some initial byte being present in some cases, not sure what that is atm.

I don't have much knowledge of what "webusb" is tho. My understanding is it's some sort of non-finished standard for USB to Browser safe transport. Best I found is this page.

gkaindl commented 6 years ago

ahaa, that sheds some light on it – I didn't realize the HID device exposed by T is just for U2F!

well, yes, it appears they've implemented a "WebUSB" transport in their python lib, which is basically just raw usb access via libusb.

It seems to be really just the transport, otherwise the same protobufs are used, so I think the most straight-forward way would be to implement it directly in wire.cpp, since it'll be just a switch in a couple of places, and factoring this out into its own transport class is probably overkill for this...

almindor commented 6 years ago

Looking at the python code it seems that the only difference is them using hidapi vs libusb directly for the "webUSB" transport.

I'm looking at the hid version here and the web usb version here

We can easily start using libusb for model T (and eventually model One when they update it) and switch to libusb. It seems the hid version is the one with the magic bytes and special handling here.

We also need to make sure we don't miss an interesting hack for windows here

almindor commented 6 years ago

Yup sounds good! Thanks for tackling this. If you need help ping me here. If we need a more direct line I can set up IRC or gitter whichever you prefer.

almindor commented 6 years ago

Fixed by your PR, thanks again! For next steps please #102