Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
xaionaro wants to merge5 commits intorfjakob:trezorfromxaionaro-go:trezor

Conversation

@xaionaro
Copy link
Contributor

Implemented the support of Trezor devices.

@xaionaroxaionaro changed the titleTrezorAdded support of (open)hardware cryptographic module "Trezor One"Jun 19, 2018
@xaionaroxaionaro changed the titleAdded support of (open)hardware cryptographic module "Trezor One"Added support of (open)hardware cryptographic module "Trezor One". The second try.Jun 19, 2018
Copy link
Owner

@rfjakobrfjakob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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?

// 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{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.

Copy link
ContributorAuthor

@xaionaroxaionaroJun 20, 2018•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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 :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ok, that's sounds more reassuringly, thanks)

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

These can beconst. Also, please follow Go naming liketrezorKeyName.

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

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

@rfjakob
Copy link
Owner

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

  1. Please add a LICENSE

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

@xaionaro
Copy link
ContributorAuthor

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

Copy link
ContributorAuthor

@xaionaroxaionaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Mentioned problems were fixed.

@xaionaro
Copy link
ContributorAuthor

All checks have failed

Fixed. Please restart the Travis checks.

@rfjakob
Copy link
Owner

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
Copy link
ContributorAuthor

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
Copy link
Owner

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 littledebug 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 andinterfaceID:0x1.

You can also see this inlsusb -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
Copy link
ContributorAuthor

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

@xaionaro
Copy link
ContributorAuthor

xaionaro commentedJun 22, 2018•
edited
Loading

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 1seems to be an U2F interface.

The problem is fixed.

Two connected devices:

$ ./gocryptfs -init -trezorkey /tmp/1Choose 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/1Choose 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
Copy link
Owner

Maybe changes not pushed? I'm on06e92a9 and still see the error.

@xaionaro
Copy link
ContributorAuthor

Maybe changes not pushed? I'm on06e92a9 and still see the error.

The fix was incryptoWallet.

@rfjakob
Copy link
Owner

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
Copy link
Owner

Normal mode:

~/go/src/github.com/conejoninja/tesoro/example$ ./example Found 1 TREZOR devices connected>ping 11 2>ping 00 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
Copy link
Owner

Quotinghttps://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
Copy link
Owner

I have collected some more details at#248 .

 This also checks if the device is initialized. Relates to:rfjakob#248
@xaionaro
Copy link
ContributorAuthor

I have collected some more details at#248 .

Thank you for the details. The problem is fixed.

rfjakob reacted with thumbs up emoji

@rfjakob
Copy link
Owner

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-filesystemTrezerPayload 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 thetrezorpayload branch. Your changes are in commita2e1cc7 .

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

@xaionaro
Copy link
ContributorAuthor

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
Copy link
ContributorAuthor

xaionaro commentedJun 28, 2018•
edited
Loading

A little offtop:

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

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

@xaionaro
Copy link
ContributorAuthor

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

@rfjakob
Copy link
Owner

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
Copy link
Owner

rfjakob commentedJun 28, 2018•
edited
Loading

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 diffdiff --git a/example/main.go b/example/main.goindex 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
Copy link
Owner

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".Infowant Info() "github.com/conejoninja/tesoro/vendor/github.com/zserge/hid".Info

@rfjakobrfjakob closed thisJul 1, 2018
@xaionaro
Copy link
ContributorAuthor

xaionaro commentedJul 5, 2018•
edited
Loading

./build.bash -tags enable_trezor

Thank you!

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

Fixed. Seexaionaro-go/cryptoWallet#2

@xaionaro
Copy link
ContributorAuthor

xaionaro commentedJul 14, 2018•
edited
Loading

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

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@rfjakobrfjakobrfjakob requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xaionaro@rfjakob
[8]ページ先頭
©2009-2025 Movatter.jp