Added support of (open)hardware cryptographic module "Trezor One". The second try. #247

xaionaro · 2018-06-19T16:29:00Z

Implemented the support of Trezor devices.

rfjakob

This looks very good already! Some small changes requested.

One thing I noticed while testing: When the Trezor is in bootloader mode (press both buttons and connect USB), gocryptfs -init -trezorkey hangs.

What happens when two Trezors are connected?

rfjakob · 2018-06-19T20:41:25Z

internal/readpassword/trezor.go

+// This values were generated using command:
+//   dd if=/dev/random of=/dev/stdout bs=48 count=1 2>/dev/null | hexdump -e '8/1 "0x%02x, " "\n"'
+var (
+	TREZOR_DUMMYKEY = []byte{


The problem with a "random" value like this is that you can never know if it has been chosen for a particular reason. Like, maybe this exact value exposes a weakness? No one can tell.

Let's just go with 16 zeros. AES handles that just fine.

I'm afraid of kinda randbow tables (but to optimize of getting encryption keys instead of getting hash function initial values) for zero-filled data. I mean somebody like NSA could have optimized ways of getting keys using known blocks of encrypted and decrypted data where the encrypted data is a zero-filled data. I understand that sounds crazy, but I feel more secure to use random data… On the other hand I agree with your argument (I had the same thought while generating that random numbers) and zero-filled data looks more trustfully. OK :)

We can go for something like "gocryptfs.trezor" or "trezor.trezor.tr" or "tttttt..." if you prefer, I'm fine with that!

Ok, that's sounds more reassuringly, thanks)

rfjakob · 2018-06-19T20:45:02Z

internal/readpassword/trezor.go

+		0xab, 0x05, 0x3b, 0x12, 0xff, 0x4e, 0xa8, 0x8b,
+		0x5b, 0x58, 0x0a, 0x8e, 0x42, 0xcf, 0x5e, 0x20,
+	}
+	TREZOR_NONCE = []byte{


According to slip-0011.md, the IV is optional, and I don't see that it adds security - let's skip it.

rfjakob · 2018-06-19T20:47:15Z

internal/readpassword/trezor.go

+		0xcc, 0xb6, 0xb2, 0xcd, 0xbc, 0x4a, 0xb6, 0xcb,
+	}
+	TREZOR_KEY_NAME            = "gocryptfs"
+	TREZOR_KEY_DERIVATION_PATH = `m/10019'/0'/0'/0'`


These can be const. Also, please follow Go naming like trezorKeyName.

There are a few places in gocryptfs that use this style, like PATH_MAX, but this is only when there is Linux C define copied.

As you suggested, let's use m/10019'/0' for gocryptfs. I see no problem with you using m/10019'/1' and higher.

rfjakob · 2018-06-19T21:13:15Z

Oh, one more thing: I was looking at https://goreportcard.com/report/github.com/xaionaro-go/cryptoWallet :

Please add a LICENSE
golint complains quite a lot about missing function and variable comments. It would be nice to have some documentation about the public functions

xaionaro · 2018-06-20T05:14:46Z

Ok. I'll fix everything soon and will test with two Trezors.

xaionaro

Mentioned problems were fixed.

xaionaro · 2018-06-21T18:08:07Z

All checks have failed

Fixed. Please restart the Travis checks.

rfjakob · 2018-06-21T18:11:20Z

Don't have my computer right now, but you could do

git commit --amend --date=now
git push --force

This only updates the timestamp of the commit, but this changes the git hash, and travis will see a new commit and rebuild.

xaionaro · 2018-06-21T18:47:51Z

git commit --amend --date=now
git push --force

This only updates the timestamp of the commit, but this changes the git hash, and travis will see a new commit and rebuild.

Nice trick. Thanks.

rfjakob · 2018-06-21T20:35:19Z

goreportcard looks excellent now, and you also added a link to godoc, very nice!

The logic for finding out if we have more than one Trezor seems to have a small problem:

$ gocryptfs -init -trezorkey a
Choose a password for protecting your files.
It's more than one Trezor device connected. This case is not implemented, yet. The number of currently connected devices: 2.

I added a little debug output to see what is going on:

$ gocryptfs -init -trezorkey a
Choose a password for protecting your files.
It's more than one Trezor device connected. This case is not implemented, yet. The number of currently connected devices: 2.
trezors=[]cryptoWallet.Wallet{(*trezorOne.trezorOne)(0xc4200ca7d0), (*trezorOne.trezorOne)(0xc4200ca870)}

trezors[0]=&trezorOne.trezorOne{TrezorBase:trezorBase.TrezorBase{Base:satoshilabsWallet.Base{USBHIDWalletBase:internal.USBHIDWalletBase{WalletBase:internal.WalletBase{name:"Trezor One", getPin:(func(string, string, string, string) ([]uint8, error))(nil), getConfirm:(func(string, string, string, string) (bool, error))(nil)}, device:(*hid.usbDevice)(0xc42009d540), vendorID:0x534c, productID:0x1, interfaceID:0x0}}, Client:tesoro.Client{t:(*transport.TransportHID)(0xc4200887b0)}}}

trezors[1]=&trezorOne.trezorOne{TrezorBase:trezorBase.TrezorBase{Base:satoshilabsWallet.Base{USBHIDWalletBase:internal.USBHIDWalletBase{WalletBase:internal.WalletBase{name:"Trezor One", getPin:(func(string, string, string, string) ([]uint8, error))(nil), getConfirm:(func(string, string, string, string) (bool, error))(nil)}, device:(*hid.usbDevice)(0xc42009d600), vendorID:0x534c, productID:0x1, interfaceID:0x1}}, Client:tesoro.Client{t:(*transport.TransportHID)(0xc4200887d0)}}}

Looks like this is because Trezor presents two USB interfaces: interfaceID:0x0 and interfaceID:0x1.

You can also see this in lsusb -t:

$ lsusb -t
                            vvvv Interface 0x0
        |__ Port 2: Dev 47, If 0, Class=Human Interface Device, Driver=, 12M
        |__ Port 2: Dev 47, If 1, Class=Human Interface Device, Driver=, 12M
                            ^^^^ Interface 0x1

xaionaro · 2018-06-22T05:00:10Z

I see. Indeed I forgot to test it after that change. I will handle that today-tomorrow.

xaionaro · 2018-06-22T06:51:09Z

I've just tried to connect one initialized Trezor and one not initialized one and got this:

000 d[09:13:15] [xaionaro@shadow ~/gocode/src/github.com/xaionaro-go/cryptoWallet]$ lsusb 
...
Bus 007 Device 094: ID 534c:0001 SatoshiLabs Bitcoin Wallet [TREZOR]
...
Bus 005 Device 008: ID 534c:0001 SatoshiLabs Bitcoin Wallet [TREZOR]
...
000 d[09:13:48] [xaionaro@shadow ~/gocode/src/github.com/xaionaro-go/cryptoWallet]$ lsusb -t
/:  Bus 07.Port 1: Dev 1, Class=root_hub, Driver=ohci-pci/4p, 12M
    |__ Port 1: Dev 94, If 0, Class=Human Interface Device, Driver=usbhid, 12M
    |__ Port 1: Dev 94, If 1, Class=Human Interface Device, Driver=usbhid, 12M
...
/:  Bus 05.Port 1: Dev 1, Class=root_hub, Driver=ohci-pci/5p, 12M
    |__ ...
    |__ Port 5: Dev 8, If 0, Class=Human Interface Device, Driver=usbhid, 12M
...

The not initialized device has only one interface, the initialized device has two interfaces.

Interface 1 seems to be an U2F interface.

The problem is fixed.

Two connected devices:

$ ./gocryptfs -init -trezorkey /tmp/1
Choose a password for protecting your files.
It's more than one Trezor device connected. This case is not implemented, yet. The number of currently connected devices: 2.

One connected device:

$ ./gocryptfs -init -trezorkey /tmp/1
Choose a password for protecting your files.
PIN: 
Passphrase: 
The gocryptfs filesystem has been created successfully.
You can now mount it using: gocryptfs /tmp/1 MOUNTPOINT

rfjakob · 2018-06-22T18:59:41Z

Maybe changes not pushed? I'm on 06e92a9 and still see the error.

xaionaro · 2018-06-22T19:12:13Z

Maybe changes not pushed? I'm on 06e92a9 and still see the error.

The fix was in cryptoWallet.

rfjakob · 2018-06-22T20:01:19Z

Well, that explains things. Works better now.

Can you reproduce this issue?

One thing I noticed while testing: When the Trezor is in bootloader mode (press both buttons and connect USB), gocryptfs -init -trezorkey hangs.

rfjakob · 2018-06-24T13:26:11Z

Normal mode:

~/go/src/github.com/conejoninja/tesoro/example$ ./example 
Found 1 TREZOR devices connected
>ping 1
1 2
>ping 0
0 2
>init
{"vendor":"bitcointrezor.com","major_version":1,"minor_version":6,"patch_version":1,"device_id":"44FF88209CB15BFDF93BC7A7","pin_protection":false,"passphrase_protection":false,"label":"My TREZOR","coins":[{"coin_name":"Bitcoin","coin_shortcut":"BTC","address_type":0,"maxfee_kb":2000000,"address_type_p2sh":5,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":true,"force_bip143":false},{"coin_name":"Testnet","coin_shortcut":"TEST","address_type":111,"maxfee_kb":10000000,"address_type_p2sh":196,"xpub_magic":70617039,"xprv_magic":70615956,"segwit":true,"force_bip143":false},{"coin_name":"Bcash","coin_shortcut":"BCH","address_type":0,"maxfee_kb":500000,"address_type_p2sh":5,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":false,"forkid":0,"force_bip143":true},{"coin_name":"Namecoin","coin_shortcut":"NMC","address_type":52,"maxfee_kb":10000000,"address_type_p2sh":5,"xpub_magic":27108450,"xprv_magic":27106558,"segwit":false,"force_bip143":false},{"coin_name":"Litecoin","coin_shortcut":"LTC","address_type":48,"maxfee_kb":40000000,"address_type_p2sh":50,"xpub_magic":27108450,"xprv_magic":27106558,"segwit":true,"force_bip143":false},{"coin_name":"Dogecoin","coin_shortcut":"DOGE","address_type":30,"maxfee_kb":1000000000,"address_type_p2sh":22,"xpub_magic":49990397,"xprv_magic":49988504,"segwit":false,"force_bip143":false},{"coin_name":"Dash","coin_shortcut":"DASH","address_type":76,"maxfee_kb":100000,"address_type_p2sh":16,"xpub_magic":50221772,"xprv_magic":50221816,"segwit":false,"force_bip143":false},{"coin_name":"Zcash","coin_shortcut":"ZEC","address_type":7352,"maxfee_kb":1000000,"address_type_p2sh":7357,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":false,"force_bip143":false},{"coin_name":"Bitcoin Gold","coin_shortcut":"BTG","address_type":38,"maxfee_kb":500000,"address_type_p2sh":23,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":true,"forkid":79,"force_bip143":true},{"coin_name":"DigiByte","coin_shortcut":"DGB","address_type":30,"maxfee_kb":500000,"address_type_p2sh":5,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":true,"force_bip143":false},{"coin_name":"Monacoin","coin_shortcut":"MONA","address_type":50,"maxfee_kb":5000000,"address_type_p2sh":55,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":true,"force_bip143":false},{"coin_name":"Fujicoin","coin_shortcut":"FJC","address_type":36,"maxfee_kb":1000000,"address_type_p2sh":16,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":false,"force_bip143":false},{"coin_name":"Vertcoin","coin_shortcut":"VTC","address_type":71,"maxfee_kb":40000000,"address_type_p2sh":5,"xpub_magic":76067358,"xprv_magic":76066276,"segwit":true,"force_bip143":false}],"initialized":true,"revision":"lYjo8nNrYJFvUeRw3rGPVREqbrw=","bootloader_hash":"YzD87BZy+tMLQhtg90+Dmjk5M0Vly3A7K9cYLqLdoBk=","imported":false,"pin_cached":false,"passphrase_cached":false,"needs_backup":true,"flags":0,"model":"1"} 17

Bootloader mode:

~/go/src/github.com/conejoninja/tesoro/example$ ./example 
Found 1 TREZOR devices connected
>init
{"vendor":"bitcointrezor.com","major_version":1,"minor_version":4,"patch_version":0,"bootloader_mode":true,"firmware_present":true} 17

rfjakob · 2018-06-24T14:24:53Z

Quoting https://doc.satoshilabs.com/trezor-tech/api-workflows.html#initialize-features :

Initialize/Features
As first message, the computer should send an empty Initialize packet and expect a Features packet as response. The Initialize packet will cause the device to stop what it is currently doing and should work at any time. Thus, it can also be used to recover from previous errors.

It looks like libWallet does not do this. This is why it does not notice that the Trezor is in bootloader mode ("bootloader_mode":true).

rfjakob · 2018-06-24T14:25:25Z

I have collected some more details at #248 .

This also checks if the device is initialized. Relates to: rfjakob#248

xaionaro · 2018-06-25T18:16:34Z

I have collected some more details at #248 .

Thank you for the details. The problem is fixed.

rfjakob · 2018-06-25T22:16:27Z

Works fine here now, thanks!

In the meantime, I have noticed a flaw in my decrypt-constant-string approach: The unlock key for all filesystems is the same. If an attacker manages to sniff the USB traffic and get the unlock key, hey can unlock all other gocryptfs filesystems that were secured with the same Trezor.

To fix this, I have added a random, per-filesystem TrezerPayload that is saved in the config file, and is used instead of the constant string.

I have squashed and rebased your changes on top of this fix and pushed into the trezorpayload branch. Your changes are in commit a2e1cc7 .

Would you mind giving it a test-drive and see if I have broken something in the rebase?

xaionaro · 2018-06-26T05:06:16Z

In the meantime, I have noticed a flaw in my decrypt-constant-string approach: The unlock key for all filesystems is the same. If an attacker manages to sniff the USB traffic and get the unlock key, hey can unlock all other gocryptfs filesystems that were secured with the same Trezor.

To fix this, I have added a random, per-filesystem TrezerPayload that is saved in the config file, and is used instead of the constant string.

Ok, nice solution.

Would you mind giving it a test-drive and see if I have broken something in the rebase?

Ok, sure.

Also I received a notification that Trezor T have arrived. I'll get it and test with it soon.

xaionaro · 2018-06-28T17:55:09Z

A little offtop:

I've just received "Trezor T". Are you able to get it work? It hangs on my machine on any operation (including trezorctl ping test)

UPD: It appears it was required to just install a new version of python-trezor (it was updated recently).

xaionaro · 2018-06-28T19:21:20Z

It seems that Trezor is going to webusb [and away from usbhid :(]

rfjakob · 2018-06-28T21:07:13Z

The Trezor T seems to have two USB interfaces:

$ lsusb -t
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 480M
    |__ Port 3: Dev 4, If 0, Class=Vendor Specific Class, Driver=, 12M
    |__ Port 3: Dev 4, If 1, Class=Human Interface Device, Driver=usbhid, 12M

Interface 1 is still HID.

rfjakob · 2018-06-28T21:17:44Z

I only ever tested the Trezor T via trezor.io.

I now tried this with the tesoro example:

~/go/src/github.com/conejoninja/tesoro$ git diff
diff --git a/example/main.go b/example/main.go
index 89f7c44..d525bbc 100644
--- a/example/main.go
+++ b/example/main.go
@@ -33,7 +33,7 @@ func main() {
 		// 0x534c : 21324 vendor
 		// 0x0001 : 1     product
 		// 0x00   : Main Trezor Interface
-		if info.Vendor == 21324 && info.Product == 1 && info.Interface == 0 {
+		if info.Vendor == 0x1209 && info.Product == 0x53c1 && info.Interface == 1 {
 			numberDevices++
 			var t transport.TransportHID
 			t.SetDevice(device)

Finds the device, but hangs on init:

~/go/src/github.com/conejoninja/tesoro/example$ ./example 
Found 1 TREZOR devices connected
>init

rfjakob · 2018-07-01T20:02:57Z

Merged into master, but protected by a build tag for now. You can compile gocryptfs with trezor support using

./build.bash -tags enable_trezor

It looks like it does not compile at the moment due to changes in tesoro:

$ ./build.bash -tags enable_trezor
# github.com/xaionaro-go/cryptoWallet/internal/wallets/satoshilabs/trezor
../../xaionaro-go/cryptoWallet/internal/wallets/satoshilabs/trezor/trezor.go:28:13: cannot use device (type "github.com/zserge/hid".Device) as type "github.com/conejoninja/tesoro/vendor/github.com/zserge/hid".Device in argument to t.SetDevice:
	"github.com/zserge/hid".Device does not implement "github.com/conejoninja/tesoro/vendor/github.com/zserge/hid".Device (wrong type for Info method)
		have Info() "github.com/zserge/hid".Info
		want Info() "github.com/conejoninja/tesoro/vendor/github.com/zserge/hid".Info

xaionaro · 2018-07-05T20:35:32Z

./build.bash -tags enable_trezor

Thank you!

It looks like it does not compile at the moment due to changes in tesoro:

Fixed. See xaionaro-go/cryptoWallet#2

xaionaro · 2018-07-14T12:36:16Z

Added support of Trezor T within cryptoWallet. I'll make a new PR to gocryptfs, soon.

xaionaro changed the title ~~Trezor~~ Added support of (open)hardware cryptographic module "Trezor One" Jun 19, 2018

xaionaro changed the title ~~Added support of (open)hardware cryptographic module "Trezor One"~~ Added support of (open)hardware cryptographic module "Trezor One". The second try. Jun 19, 2018

xaionaro mentioned this pull request Jun 19, 2018

Added support of (open)hardware cryptographic module "Trezor One" #243

Closed

Implemented the support of Trezor devices.

3ccd8cf

xaionaro force-pushed the trezor branch from 2021165 to 3ccd8cf Compare June 19, 2018 16:37

rfjakob requested changes Jun 19, 2018

View reviewed changes

xaionaro added 2 commits June 21, 2018 20:18

[trezor] Considered the case if multiple trezors are connected

a2e2e38

[trezor] Fixed the block with trezor constants

ce37f8f

xaionaro commented Jun 21, 2018

View reviewed changes

[trezor] fixed the key derivation path

06e92a9

xaionaro force-pushed the trezor branch from 19ebf79 to 06e92a9 Compare June 21, 2018 18:23

[trezor] Reset trezor device before use

17c90e8

This also checks if the device is initialized. Relates to: rfjakob#248

rfjakob closed this Jul 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support of (open)hardware cryptographic module "Trezor One". The second try. #247

Added support of (open)hardware cryptographic module "Trezor One". The second try. #247

xaionaro commented Jun 19, 2018

rfjakob left a comment

rfjakob Jun 19, 2018

xaionaro Jun 20, 2018 •

edited

Loading

rfjakob Jun 20, 2018

xaionaro Jun 20, 2018

rfjakob Jun 19, 2018

rfjakob Jun 19, 2018

rfjakob commented Jun 19, 2018

xaionaro commented Jun 20, 2018

xaionaro left a comment

xaionaro commented Jun 21, 2018

rfjakob commented Jun 21, 2018

xaionaro commented Jun 21, 2018

rfjakob commented Jun 21, 2018

xaionaro commented Jun 22, 2018

xaionaro commented Jun 22, 2018 •

edited

Loading

rfjakob commented Jun 22, 2018

xaionaro commented Jun 22, 2018

rfjakob commented Jun 22, 2018

rfjakob commented Jun 24, 2018

rfjakob commented Jun 24, 2018

rfjakob commented Jun 24, 2018

xaionaro commented Jun 25, 2018

rfjakob commented Jun 25, 2018

xaionaro commented Jun 26, 2018

xaionaro commented Jun 28, 2018 •

edited

Loading

xaionaro commented Jun 28, 2018

rfjakob commented Jun 28, 2018

rfjakob commented Jun 28, 2018 •

edited

Loading

rfjakob commented Jul 1, 2018

xaionaro commented Jul 5, 2018 •

edited

Loading

xaionaro commented Jul 14, 2018 •

edited

Loading

Added support of (open)hardware cryptographic module "Trezor One". The second try. #247

Added support of (open)hardware cryptographic module "Trezor One". The second try. #247

Conversation

xaionaro commented Jun 19, 2018

rfjakob left a comment

Choose a reason for hiding this comment

rfjakob Jun 19, 2018

Choose a reason for hiding this comment

xaionaro Jun 20, 2018 • edited Loading

Choose a reason for hiding this comment

rfjakob Jun 20, 2018

Choose a reason for hiding this comment

xaionaro Jun 20, 2018

Choose a reason for hiding this comment

rfjakob Jun 19, 2018

Choose a reason for hiding this comment

rfjakob Jun 19, 2018

Choose a reason for hiding this comment

rfjakob commented Jun 19, 2018

xaionaro commented Jun 20, 2018

xaionaro left a comment

Choose a reason for hiding this comment

xaionaro commented Jun 21, 2018

rfjakob commented Jun 21, 2018

xaionaro commented Jun 21, 2018

rfjakob commented Jun 21, 2018

xaionaro commented Jun 22, 2018

xaionaro commented Jun 22, 2018 • edited Loading

rfjakob commented Jun 22, 2018

xaionaro commented Jun 22, 2018

rfjakob commented Jun 22, 2018

rfjakob commented Jun 24, 2018

rfjakob commented Jun 24, 2018

rfjakob commented Jun 24, 2018

xaionaro commented Jun 25, 2018

rfjakob commented Jun 25, 2018

xaionaro commented Jun 26, 2018

xaionaro commented Jun 28, 2018 • edited Loading

xaionaro commented Jun 28, 2018

rfjakob commented Jun 28, 2018

rfjakob commented Jun 28, 2018 • edited Loading

rfjakob commented Jul 1, 2018

xaionaro commented Jul 5, 2018 • edited Loading

xaionaro commented Jul 14, 2018 • edited Loading

xaionaro Jun 20, 2018 •

edited

Loading

xaionaro commented Jun 22, 2018 •

edited

Loading

xaionaro commented Jun 28, 2018 •

edited

Loading

rfjakob commented Jun 28, 2018 •

edited

Loading

xaionaro commented Jul 5, 2018 •

edited

Loading

xaionaro commented Jul 14, 2018 •

edited

Loading