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

xaionaro · 2018-06-07T17:30:50Z

No description provided.

xaionaro · 2018-06-07T18:15:58Z

~~Builds fails because there're dependencies on github.com/rfjakob/gocryptfs/internal/* while new internals are not merged, yet.~~

Builds fails because there's a dependency on https://github.com/rfjakob/eme, but rfjakob/eme#3 is not merged, yet.

xaionaro · 2018-06-08T08:15:00Z

Done some polising. Ready for merging. I can squash it if you want :)

rfjakob · 2018-06-08T09:40:52Z

Hi, and thanks for this pull request!

Some comments:

The crypto algorithm in https://github.com/xaionaro-go/gocryptfs/blob/abdffb029460fcd5f85129d9ba4ac35eb24bc0da/internal/cryptocore/trezor.go#L173 does not look good. What algorithm is used? With Overhead zero, this means the data cannot be authenticated. This means it is not Authenticated Encryption (AEAD)! I suggest to remove on-device encryption and only have masterkey encryption.
The automatic key wiping will be done through autounmount ( autounmount? #128 ). Please get rid of the key wiping inside cryptocore.
You use CipherKeyValue from github.com/conejoninja/tesoro. This function does have documentation: https://godoc.org/github.com/conejoninja/tesoro#Client.CipherKeyValue . What does it do?

rfjakob · 2018-06-08T10:08:42Z

I think we should start much simpler: Using Trezor as a second factor (2FA) in addition to the password. Could work something like this, where the master key is decrypted by Trezor, and then decrypted by gocryptfs with a user password:

User wants to mount filesystem
gocryptfs sends master key "M0" from config file to Trezor for decryption, gets "M1"
gocryptfs asks user for password and decrypts "M1" to "M2".

Note that the order is important: Step 3 has a bulletproof check if the key is valid or has been corrupted in some way due do AES-GCM Authentication.

xaionaro · 2018-06-08T11:13:57Z

User wants to mount filesystem
gocryptfs sends master key "M0" from config file to Trezor for decryption, gets "M1"
gocryptfs asks user for password and decrypts "M1" to "M2".

In my case the third step is skipped because Trezor already requires a password.

xaionaro · 2018-06-08T11:25:54Z

With Overhead zero, this means the data cannot be authenticated. This means it is not Authenticated Encryption (AEAD)!

It has non-zero overhead. I added 14 octets of additionalData (to recheck if the decryption was correct) and two octets to store actual block size (to cut tails, because CipherKeyValue works only with blocks aligned to 16 octets).

I suggest to remove on-device encryption and only have masterkey encryption.

Ok. It is hacky (and I'm weak in cryptography), agreed. I'd like to preserve on-device encryption for myself (to continue experiments with Trezor), but I can support a fork of gocryptfs for that. The most important is master-key encryption, of course.

But if I can improve on-device encryption to get it merged, please let me know.

The automatic key wiping will be done through autounmount ( #128 ). Please get rid of the key wiping inside cryptocore.

I see, ok.

You use CipherKeyValue from github.com/conejoninja/tesoro. This function does have documentation: https://godoc.org/github.com/conejoninja/tesoro#Client.CipherKeyValue . What does it do?

https://github.com/trezor/connect [see "cipherKeyValue"]

It's an API function to a Trezor device (initially used to get master keys) [see SLIP-0011]

xaionaro · 2018-06-09T09:13:47Z

@rfjakob I've fixed the PR.

Trezor is disabled for all platforms except "linux"

xaionaro · 2018-06-09T11:07:06Z

BTW, should I squash this commits?

rfjakob · 2018-06-10T19:50:02Z

Thanks for the update, looks better now! I have ordered a Trezor for development, should arrive next week.

About on-device-encryption: It would be a nice use case, but may be hard to get right. I will look at what crypto APIs can be used on the Trezor.

Squashing not yet neccessary, no.

While looking at the Trezor, I noticed it supports U2F. I'm thinking about whether we can use U2F, this would add support for lots of other USB sticks. Not yet sure if possible from a crypto standpoint, I'll have to read the U2F spec. At least there seems to be a Go package for U2F ( https://github.com/flynn/u2f ). They have several devices tested, but not the Trezor - if you like you could test it and add the Trezor to the table?

prusnak · 2018-06-10T19:57:15Z

Hi! SatoshiLabs (makers of TREZOR) CTO here. Feel free to ask questions or suggest better API when needed. I think that TREZOR integration is much better than using TREZOR as a second factor. (Remember - TREZOR asks for PIN on display - it is already two-factor device). PIN entering might be a problem with TREZOR 1 (because the application would need to provide an interface), but it should work with TREZOR model T with no problem (as you can enter the PIN on the device).

@xaionaro @rfjakob Do you have the new model T? If not, reach me via email at stick@satoshilabs.com and I'll get one to you (both).

rfjakob · 2018-06-10T20:07:03Z

Hi @prusnak , thanks for chiming in! No, I have ordered a Trezor One and would be excited to get a model T as well! I'll email you shortly.

xaionaro · 2018-06-13T09:26:06Z

While looking at the Trezor, I noticed it supports U2F.

Yep. I also use U2F to authenticate on my desktop with Trezor — it really works :)

I'm thinking about whether we can use U2F, this would add support for lots of other USB sticks. Not yet sure if possible from a crypto standpoint, I'll have to read the U2F spec.

I see. Interesting idea. However I'm not sure (yet) when I'll be able to try to implement that.

They have several devices tested, but not the Trezor - if you like you could test it and add the Trezor to the table?

Ok, I'll test that golang library with Trezor One very soon.

@xaionaro @rfjakob Do you have the new model T? If not, reach me via email at stick@satoshilabs.com and I'll get one to you (both).

No. "T" model was too expensive (I bought 7 pieces "Trezor One" and it already was quite expensive). I've emailed you, thank you :)

rfjakob · 2018-06-13T20:44:57Z

U2F won't work because the signature we get back from the U2F stick is randomized. No way to use it as a key.

I have both Trezors now (One and T).

Testing with the Trezor One, I got this, probably because the device was not yet initialized?

$ gocryptfs -init --trezor_encrypt_masterkey a
Choose a password for protecting your files.
2018/06/13 22:39:02 An unexpected behaviour of the trezor device.
panic: An unexpected behaviour of the trezor device.

goroutine 1 [running]:
log.Panic(0xc420153888, 0x1, 0x1)
	/usr/local/go/src/log/log.go:326 +0xc0
github.com/rfjakob/gocryptfs/internal/trezor.(*trezor).Reconnect(0xc42008d590)
	/home/jakob/go/src/github.com/rfjakob/gocryptfs/internal/trezor/trezor_linux.go:46 +0x104
github.com/rfjakob/gocryptfs/internal/trezor.New(0x20)
	/home/jakob/go/src/github.com/rfjakob/gocryptfs/internal/trezor/trezor.go:30 +0xa9
github.com/rfjakob/gocryptfs/internal/configfile.(*ConfFile).EncryptKeyByTrezor(0xc4200dc1e0, 0xc420096180, 0x20, 0x20)
	/home/jakob/go/src/github.com/rfjakob/gocryptfs/internal/configfile/config_file.go:228 +0x26
github.com/rfjakob/gocryptfs/internal/configfile.CreateConfFile(0xc420090180, 0x24, 0x0, 0x0, 0x0, 0xc420096100, 0x10, 0xc420096160, 0x1c, 0x100, ...)
	/home/jakob/go/src/github.com/rfjakob/gocryptfs/internal/configfile/config_file.go:109 +0x357
main.initDir(0xc420126000)
	/home/jakob/go/src/github.com/rfjakob/gocryptfs/init_dir.go:77 +0x176
main.main()
	/home/jakob/go/src/github.com/rfjakob/gocryptfs/main.go:283 +0x684

In any case, we should give a nicer error message to the user.

Considered the case if the Trezor device is not initialized

Conflicts: README.md

xaionaro · 2018-06-14T07:05:35Z

Testing with the Trezor One, I got this, probably because the device was not yet initialized?

I've just tested on uninitilized Trezor One and got the same error, yes.

In any case, we should give a nicer error message to the user.

Fixed.

$ gocryptfs -init --trezor_encrypt_masterkey /tmp/123
2018/06/14 10:05:59 The trezor device seems to be not initialized

xaionaro · 2018-06-14T07:09:16Z

To pass builds (in Travis) you need to restart them (I forgot to push one commit to one of my repositories before pushing a commit to gocryptfs)

rfjakob · 2018-06-14T08:20:26Z

Thanks for the updates, two more high-level comments:

About github.com/xaionaro-go/trezor : I would prefer to keep it in gocryptfs/internal . I believe splitting the code across two repos makes it harder to review and maintain.
The pinentry should be text-only like the password entry is. There are GUI frontends like SiriKali that will show a graphical prompt, but gocryptfs itself should be text-only.

xaionaro · 2018-06-14T15:19:42Z

Yes you can just use readpassword.Once() like mounting without a trezor does.

Done.

Travis failed, but the error is already fixed (after that).

rfjakob · 2018-06-15T07:48:14Z

@xaionaro How do you feel about additionally using a gocryptfs password? Too inconvenient? Optional or mandatory? (On by default or not?)

My concern is that it is about 1000$ to dump all memory out of a Trezor by shipping it to China to a specialized service. The PIN does not help here. See this post for example:

It is not just companies with >1000k equipment. There are companies on a certain popular Chinese website that will crack any ST chip for $750 - $6000 USD (depending on part number). They also crack every other companies' microprocessors as well. (I don't want to post links to encourage anyone)

PS: I have restarted the Travis build: https://travis-ci.org/rfjakob/gocryptfs/builds/392302249

prusnak · 2018-06-15T08:07:30Z

My concern is that it is about 1000$ to dump all memory out of a Trezor by shipping it to China to a specialized service.

I think this article is FUD. While there are some papers about breaking Read Protection (RDP) level 1, so far no one has performed a RDP level 2 hack (this is what we use). Also we encourage people to use passphrase, which is added to the entropy mix, so even if you extract all secrets from the device you still need to passphrase to obtain any key.

rfjakob · 2018-06-15T08:15:34Z

Are you talking about software-based attacks? I'm talking about cracking open the microcontroller and probing the chip. You can order it at http://www.ic-cracker.com/ or http://www.unlock-ic.com/

rfjakob · 2018-06-15T08:22:12Z

On the plus side, another service, https://russiansemiresearch.com/en/service/#read_prot , does not seem to have STM32F2 listed.

prusnak · 2018-06-15T08:28:25Z

Same for the other 2 providers you listed, they list just some really archaic STM chips.

rfjakob · 2018-06-15T08:52:52Z

I have read through the presentation at https://www.aisec.fraunhofer.de/en/FirmwareProtection.html where they downgrade rdp level 2 to rdp level 1 using uv light on an stm32f0. They also say:

Mitigation available
� Check for RDP Level 2 during boot process
� Stop firmware execution if not RDP Level 2, rewrite configuration
� Prevents Cold-Boot Stepping after security downgrade
� Negligible performance+memory overhead

Do you do that?

prusnak · 2018-06-15T08:59:33Z

Yes, I am aware of this attack. We set RDP always on start if it's not already set to level 2.

https://travis-ci.org/rfjakob/gocryptfs/jobs/392302254

xaionaro · 2018-06-16T08:40:02Z

@xaionaro How do you feel about additionally using a gocryptfs password? Too inconvenient? Optional or mandatory? (On by default or not?)

I cannot find any rational reason for that. It already requires 3 factors: a Trezor, a PIN and a passphrase (which, in turn, is not stored on the Trezor). Also I worked with STM32 before and, IMHO, it's not so easy to crack it.

On the other hand, the fact that I don't see a risk doesn't mean that there's no risk :). So I cannot suggest anything here.

PS: I have restarted the Travis build: https://travis-ci.org/rfjakob/gocryptfs/builds/392302249

Fixed.

rfjakob · 2018-06-16T12:37:12Z

Good point about the possibility of having a passphrase on the Trezor.

@xaionaro Pin entry does not seem to work?

$ gocryptfs a b
PIN:
Got an error from a trezor device: string overflow (the trezor device is busy?)

rfjakob · 2018-06-16T12:48:22Z

PS: What is m/3'/14'/15'/93' ?

rfjakob · 2018-06-16T12:55:49Z

Oh dear, the tests of conejoninja/tesoro crap out with compile errors: conejoninja/tesoro#5

To get the code quality of all of this good enough to be merged into gocryptfs is going to be some work.

xaionaro · 2018-06-16T13:31:54Z

@xaionaro Pin entry does not seem to work?

$ gocryptfs a b
PIN:
Got an error from a trezor device: string overflow (the trezor device is busy?)

It works on my computer. How did you get that error?

PS: What is m/3'/14'/15'/93' ?

It's a "derivation path".

I actually badly understand this paths, however it was told to me that:

The paths defines a wallet to be used (different wallets has different keys). It kinda of deterministic hierarchic structure of child keys for a defined master key.
An apostrophe requires a "hardened derivation" to be used.

So I just picked an arbitrary path (I used Pi) to do not intersect with other services (like a bitcoin wallet for example) and added apostrophes everywhere.

rfjakob · 2018-06-16T13:55:08Z

How did you get that error?

Maybe we are on different versions? Care to run ver.sh ? I get this:

$ ./ver.sh 
github.com/rfjakob/gocryptfs                 v1.5-14-g15d0758
github.com/xaionaro-go/cryptoWallet                 39185ff
github.com/zserge/hid                 b5855a5
github.com/conejoninja/tesoro                 1832c48
github.com/conejoninja/hid                 fb96e1d

rfjakob · 2018-06-16T14:14:51Z

The CipherKeyValue function seems to be documented in SLIP-0011 . I have created a pull request to get a link added to https://doc.satoshilabs.com/trezor-tech/api-workflows.html#cipherkeyvalue .

xaionaro · 2018-06-16T14:28:12Z

Maybe we are on different versions? Care to run ver.sh ? I get this:

$ ./ver.sh 
github.com/rfjakob/gocryptfs                 v1.5-14-g15d0758
github.com/xaionaro-go/cryptoWallet                 39185ff
github.com/zserge/hid                 b5855a5
github.com/conejoninja/tesoro                 1832c48
github.com/conejoninja/hid                 fb96e1d

Seems to be the same commits:

$ ./ver.sh
github.com/rfjakob/gocryptfs                 v1.4.4-64-g15d0758
github.com/xaionaro-go/cryptoWallet                 39185ff
github.com/zserge/hid                 b5855a5
github.com/conejoninja/tesoro                 1832c48
github.com/conejoninja/hid                 fb96e1d

Got an error from a trezor device: string overflow (the trezor device is busy?)

BTW, I was able to get a very similar error when a Trezor device was already asking for a PIN code from a previous application (it was displaying a PIN matrix instead of the home screen). That's why I added a comment "(the trezor device is busy?)".

rfjakob · 2018-06-16T15:30:07Z

@prusnak Can you comment here? What should we use as the path for CipherKeyValue?

I saw that
SLIP-0015 uses m/10015'/0', and
SLIP-0016 uses m/10016'/0 (note the missing ').

Should we use m/10017'/0' for gocryptfs?

rfjakob · 2018-06-17T13:37:17Z

@xaionaro , I have noticed that the mount still succeeds when the Trezor is wiped and recreated. But any file returns errors, of course. This is because we had no integrity check for the masterkey.

I would like to re-use the infrastructure we already have for passwords: integrity check, scrypt brute-force protection. The idea is to encrypt a constant string like []byte("tttttttttttttttt") (16 x t) with the Trezor and use that as the password.

I have implemented a skeleton and pushed it to branch "trezor" at https://github.com/rfjakob/gocryptfs/commits/trezor . It should have everything but the actual communication with the Trezor. The Trezor code should go here:

gocryptfs/internal/readpassword/trezor.go

Line 15 in f02fa5c

func Trezor() []byte {

prusnak · 2018-06-18T09:28:04Z

Should we use m/10017'/0' for gocryptfs?

No, please use m/10019'/0' (17 and 18 are reserved according to https://github.com/satoshilabs/slips/, we can reserve 19 for gocryptfs)

xaionaro · 2018-06-18T11:45:03Z

I have implemented a skeleton and pushed it to branch "trezor" at https://github.com/rfjakob/gocryptfs/commits/trezor

I'll be able to take a look only on the weekend because my vacation is over :(

No, please use m/10019'/0' (17 and 18 are reserved according to https://github.com/satoshilabs/slips/, we can reserve 19 for gocryptfs)

Can I also reserve an id for my projects, too?

i2pfs — I'm trying to implement a shared storage that works via I2P and authenticates users using SSH-keys (with native support of Trezor).
trezorLuks — just a very simple wrapper around cryptsetup to encrypt block device using LUKS and a Trezor device.

Should I make a Pull Request for that?

xaionaro · 2018-06-19T10:42:58Z

@rfjakob in my last commits I replaced "Trezor" by "cryptowallet" to be able to add other devices in future. You ignored that in branch "trezor". Should I lock-in into "trezor" or I should rename that? As for me both ways are good enough.

rfjakob · 2018-06-19T11:16:29Z

I have seen the "cryptowallet" rename, but I was not completely sure if it is better. Until we actually support more than one kind of security module, I suggest to keep it simple and stupid, and just call it "trezor".

rfjakob · 2018-06-19T11:20:18Z

Also, the -trezor cli option is nice and short. When we support additional security modules, I'd make that an alias for -hsm.

Then there is the question what to do if multiple security modules are connected at the same time. I think for now it is good enough to detect the case and abort.

prusnak · 2018-06-19T12:00:14Z

@xaionaro Do you need always one key for gocryptfs/i2pfs/trezorLuks? I am thinking about creating just one SLIP and use different derivation paths for various purposes as opposed to creating one separate SLIP for each application.

xaionaro · 2018-06-19T14:24:32Z

@prusnak

Do you need always one key for gocryptfs/i2pfs/trezorLuks? I am thinking about creating just one SLIP and use different derivation paths for various purposes as opposed to creating one separate SLIP for each application.

Yes, I thought to just use different derivation paths for my applications, too. One SLIP is the best way for me. However gocryptfs is not my application, while I'm just trying to push a patch to @rfjakob's application. And if he is not going to write another applications with Trezor in near future then one SLIP is ok :)

UPD:
@rfjakob do you feel comfortable to share the same SLIP between both of us? For example you may use m/10019'/0' and I could use m/10019'/1' (so one of my applications will use m/10019'/1'/0').

xaionaro · 2018-06-19T16:31:07Z

@rfjakob: here's a new pull request: #247

And I think we should close this (#243) request.

rfjakob · 2018-06-19T20:30:33Z

Nice, thanks. Let's continue at #247 .

xaionaro mentioned this pull request Jun 8, 2018

Export an interface EMECipher instead of the struct rfjakob/eme#3

Closed

Added support of encrypting the master key using "Trezor One"

097d1e6

xaionaro force-pushed the master branch from abdffb0 to 097d1e6 Compare June 9, 2018 09:10

Added a reference for function trezor.CipherKeyValue()

8c68be6

xaionaro added 6 commits June 9, 2018 12:23

Fixed building (updated gocryptfs-xray according to the Trezor patch)

67df9ce

Fixed passing of "./test.bash"

23faf1d

[trezor] Fixed crossbuilding

ae127fa

Trezor is disabled for all platforms except "linux"

Added information about Trezor functionality to README.md

3c71e21

[trezor] deduplicated the code of unsupported platforms

79427e4

Fixed: "possible formatting directive in Panic call"

2d2f183

xaionaro added 4 commits June 14, 2018 09:45

Switched to a common library for Trezor routines

d39162f

Considered the case if the Trezor device is not initialized

Don't print "Choose a password ..." if Trezor is used

13b5eed

Considered the case if user canceled PIN entering

a9a2ffb

Merge https://github.com/rfjakob/gocryptfs

c4b1a7c

Conflicts: README.md

[trezor] switched to "github.com/xaionaro-go/cryptoWallet"

632aa54

[trezor] fixing a problem revealed by Travis (a shadowed variable)

15d0758

https://travis-ci.org/rfjakob/gocryptfs/jobs/392302254

rfjakob closed this Jun 19, 2018

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

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

Conversation

xaionaro commented Jun 7, 2018

xaionaro commented Jun 7, 2018 • edited Loading

xaionaro commented Jun 8, 2018 • edited Loading

rfjakob commented Jun 8, 2018

rfjakob commented Jun 8, 2018

xaionaro commented Jun 8, 2018 • edited Loading

xaionaro commented Jun 8, 2018 • edited Loading

xaionaro commented Jun 9, 2018 • edited Loading

xaionaro commented Jun 9, 2018

rfjakob commented Jun 10, 2018

prusnak commented Jun 10, 2018

rfjakob commented Jun 10, 2018

xaionaro commented Jun 13, 2018

rfjakob commented Jun 13, 2018

xaionaro commented Jun 14, 2018 • edited Loading

xaionaro commented Jun 14, 2018

rfjakob commented Jun 14, 2018

xaionaro commented Jun 14, 2018

rfjakob commented Jun 15, 2018

prusnak commented Jun 15, 2018

rfjakob commented Jun 15, 2018

rfjakob commented Jun 15, 2018

prusnak commented Jun 15, 2018

rfjakob commented Jun 15, 2018

prusnak commented Jun 15, 2018 • edited Loading

xaionaro commented Jun 16, 2018

rfjakob commented Jun 16, 2018

rfjakob commented Jun 16, 2018

rfjakob commented Jun 16, 2018 • edited Loading

xaionaro commented Jun 16, 2018 • edited Loading

rfjakob commented Jun 16, 2018

rfjakob commented Jun 16, 2018

xaionaro commented Jun 16, 2018 • edited Loading

rfjakob commented Jun 16, 2018

rfjakob commented Jun 17, 2018 • edited Loading

prusnak commented Jun 18, 2018 • edited Loading

xaionaro commented Jun 18, 2018

xaionaro commented Jun 19, 2018

rfjakob commented Jun 19, 2018

rfjakob commented Jun 19, 2018

prusnak commented Jun 19, 2018

xaionaro commented Jun 19, 2018 • edited Loading

xaionaro commented Jun 19, 2018 • edited Loading

rfjakob commented Jun 19, 2018

xaionaro commented Jun 7, 2018 •

edited

Loading

xaionaro commented Jun 8, 2018 •

edited

Loading

xaionaro commented Jun 8, 2018 •

edited

Loading

xaionaro commented Jun 8, 2018 •

edited

Loading

xaionaro commented Jun 9, 2018 •

edited

Loading

xaionaro commented Jun 14, 2018 •

edited

Loading

prusnak commented Jun 15, 2018 •

edited

Loading

rfjakob commented Jun 16, 2018 •

edited

Loading

xaionaro commented Jun 16, 2018 •

edited

Loading

xaionaro commented Jun 16, 2018 •

edited

Loading

rfjakob commented Jun 17, 2018 •

edited

Loading

prusnak commented Jun 18, 2018 •

edited

Loading

xaionaro commented Jun 19, 2018 •

edited

Loading

xaionaro commented Jun 19, 2018 •

edited

Loading