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

Add support for FIDO2 tokens #505

Closed
wants to merge 1 commit into from
Closed

Add support for FIDO2 tokens #505

wants to merge 1 commit into from

Conversation

prusnak
Copy link
Contributor

@prusnak prusnak commented Sep 5, 2020

First implementation of #504 - comments are welcome!

This introduces a new initialization option -fido2 which uses a FIDO2 token (or more precisely FIDO2 HMAC Secret extension) instead of stdin to obtain the password.

Two interactions happen with the FIDO2 token during the initialization:

  1. FIDO2 Registration - a token generates a CredentialID
  2. FIDO2 Authentication - gocryptfs generates a random HMACSalt and asks the token to compute the hmacSecret using the salt and credential from the step 1 - this resulting secret is then used instead of the password

Config file contains three extra things when using FIDO2: FeatureFlags.FlagFIDO2, FIDO2.CredentialID and FIDO2.HMACSalt

During the mounting of the FS, only one interaction with the FIDO2 token is needed:

  1. gocryptfs takes FIDO2.CredentialID and FIDO2.HMACSalt from the config and uses this to compue the hmacSecret via the token - this is exactly the same process as the second step in the init step above (so it produces the same secret)

@prusnak
Copy link
Contributor Author

prusnak commented Sep 6, 2020

The build failure in Travis is caused by missing libfido2 (which is not present in Ubuntu Bionic).

@rfjakob
Copy link
Owner

rfjakob commented Sep 6, 2020

Hi! This seems pretty neat! But before going into the code review, I have some questions:

  1. Can you recommend a security token to test this with? Will Trezor Model One work?
  2. I think we don't want to handle device selection inside gocryptfs. The user should pass -fido2 /dev/hidXYZ to us.
  3. What do you think of calling fido2-cred instead of using libfido2? This would avoid the dependency on go-libfido2, which seems to pretty young, and also on libfido2. The stdin/stdout ABI to fido2-cred is well-documented in the man page.

@prusnak
Copy link
Contributor Author

prusnak commented Sep 6, 2020

  1. Will Trezor Model One work?

Trezor One only supports U2F, not FIDO2. Please send me an email with your address+phone number and I'll get a Trezor Model T delivered to you! It's very nice for working with FIDO2 as it shows FIDO2 UserID/RelyingPartyID on its display.

  1. What do you think of calling fido2-cred instead of using libfido2?

If system has fido2-cred/fido2-assert it also has libfido2 installed, so I am not sure the benefit is that strong. Of course, it would be possible to compile gocryptfs with FIDO2 support even on systems with no libfido2 installed.

  1. I think we don't want to handle device selection inside gocryptfs. The user should pass -fido2 /dev/hidXYZ to us.

No strong opinion here. If you want to remove the interactive selection I can do it, but I found it quite neat and not adding too much code actually. I agree that if we decide to drop go-libfido2 dependency and use fido2-cred/fido2-assert via stdin it makes sense to do this.

Let me know what you think and I'll rework the PR to use fido2-cred/fido2-assert via stdin if you want it that way.

@rfjakob
Copy link
Owner

rfjakob commented Sep 8, 2020

I wanted to see if I can compile statically with go-libfido2, but it looks like it does not compile at all here on my Fedora 32:

$ go get github.com/keys-pub/go-libfido2
# github.com/keys-pub/go-libfido2
../../../../pkg/mod/github.com/keys-pub/go-libfido2@v1.4.0/fido2.go:795:10: could not determine kind of name for C.FIDO_CRED_PROT_UV_OPTIONAL
../../../../pkg/mod/github.com/keys-pub/go-libfido2@v1.4.0/fido2.go:797:10: could not determine kind of name for C.FIDO_CRED_PROT_UV_OPTIONAL_WITH_ID
../../../../pkg/mod/github.com/keys-pub/go-libfido2@v1.4.0/fido2.go:799:10: could not determine kind of name for C.FIDO_CRED_PROT_UV_REQUIRED
../../../../pkg/mod/github.com/keys-pub/go-libfido2@v1.4.0/fido2.go:159:16: could not determine kind of name for C.FIDO_EXT_CRED_PROTECT
../../../../pkg/mod/github.com/keys-pub/go-libfido2@v1.4.0/fido2.go:442:14: could not determine kind of name for C.fido_cred_set_prot

Then there is Debian 10 which does not have libfido2 at all ( https://pkgs.org/search/?q=libfido2 ). Not sure about MacOS, but there is libcbor binary checked into the git repo, which I don't like at all.

So yes I would like to avoid the dependency on go-libfido2 :)
The stdin/stout ABI to fido2-cred seems pretty good and straight-forward to me.

fido2 support can then be always-on in gocryptfs, also in the static binaries that I release via github, and fido2 will work on all platforms that have (or will get) fido2-cred.

@prusnak
Copy link
Contributor Author

prusnak commented Sep 8, 2020

Fedora 32 has libfido2 version 1.3.x, go-libfido2 requires 1.4.x.
Debian has libfido2 1.4.0 in testing/unstable, but does not have any version packaged in stable.

I guess the best way how to proceed is to rework the PR to use the command-line tools via stdin/stdout interface. I'll do this later this week. Let's hope the stdin/stdout interface will not change very often. :-)

@rfjakob
Copy link
Owner

rfjakob commented Sep 10, 2020

Btw, about the Trezor Model T, you already sent me one in 2018! I just updated it to the latest firmware and it seems to work just fine:

$ fido2-token -L
/dev/hidraw1: vendor=0x1209, product=0x53c1 (SatoshiLabs TREZOR)

Thanks again!

@prusnak
Copy link
Contributor Author

prusnak commented Sep 10, 2020

Btw, about the Trezor Model T, you already sent me one in 2018! I just updated it to the latest firmware and it seems to work just fine:

Hah, I thought so, but was not 100% sure. 👍

@prusnak
Copy link
Contributor Author

prusnak commented Sep 11, 2020

@rfjakob I reworked the PR to use the stdin/stdout ABI interface instead of go-libfido2. Everything should work as it did before the change.

The only awkward thing is that we need to run fido2-assert twice, because we need to explicitly tell whether PIN should be asked or not. So now I am trying the process without the PIN first and if it fails I try it again with PIN. The fido2-cred is more intelligent in that regard and asks for the PIN only when it is required by the token.

Tested with Trezor Model T and Yubico Security Key with PIN enabled.

@rfjakob
Copy link
Owner

rfjakob commented Sep 12, 2020

Merged with minimal changes as 1e624a4 , thanks!

@rfjakob rfjakob closed this Sep 12, 2020
@prusnak
Copy link
Contributor Author

prusnak commented Sep 12, 2020

Thanks! I went through the changes you made and they make sense.

@prusnak prusnak deleted the fido2 branch September 12, 2020 20:44
@prusnak
Copy link
Contributor Author

prusnak commented Sep 12, 2020

@rfjakob When do you plan to do a next release based on the changes from the gofuse_v2api branch?

@rfjakob
Copy link
Owner

rfjakob commented Sep 24, 2020

So the problem with v2api is that go-fuse has crashes under high load: hanwen/go-fuse#372

Maybe I can get this fixed an release sometime in October

@rfjakob
Copy link
Owner

rfjakob commented Oct 15, 2020

Released as v2.0-beta1

@JokerQyou
Copy link
Contributor

I've just tested the latest master version with a Yubikey 5 on macOS, and it seems -v option passed to fido2-cred is causing trouble. I've also manually tested like this:

$ cat test_cred
YS7vEspph7MWwGGLOiQhpDx+WYyKmS86ROGFPD99AnE=
gocryptfs
test
YS7vEspph7MWwGGLOiQhpDx+WYyKmS86ROGFPD99AnE=
$ fido2-cred -M -h -d -v -i test_cred  'IOService:/AppleACPIPlatformExpert/PCI0@0/AppleACPIPCI/XHC@14/XHC@14000000/HS01@14100000/YubiKey OTP+FIDO+CCID@14100000/IOUSBHostInterface@1/AppleUserUSBHostHIDDevice'
client data hash:
  61 2e ef 12 ca 69 87 b3 16 c0 61 8b 3a 24 21 a4
  3c 7e 59 8c 8a 99 2f 3a 44 e1 85 3c 3f 7d 02 71
relying party id: gocryptfs
user name: test
user id:
  61 2e ef 12 ca 69 87 b3 16 c0 61 8b 3a 24 21 a4
  3c 7e 59 8c 8a 99 2f 3a 44 e1 85 3c 3f 7d 02 71
fido_tx: d=0x7fd4b09044d0, cmd=0x06, buf=0x7fd4b09044d0, count=8
0000: ff a2 89 1d 9f b5 8b 33
fido_rx: d=0x7fd4b09044d0, cmd=0x06, buf=0x7fd4b09044d8, count=17, ms=-1
rx_preamble: initiation frame at 0x7ffee999bc30
0000: ff ff ff ff 86 00 11 ff a2 89 1d 9f b5 8b 33 00
0016: 0c 00 1c 02 05 02 04 05 00 00 00 00 00 00 00 00
0032: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0048: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
rx: payload_len=17
fido_rx: buf=0x7fd4b09044d8, len=17
0000: ff a2 89 1d 9f b5 8b 33 00 0c 00 1c 02 05 02 04
0016: 05
fido_dev_get_cbor_info_tx: dev=0x7fd4b09044d0
fido_tx: d=0x7fd4b09044d0, cmd=0x10, buf=0x7ffee999b470, count=1
0000: 04
fido_dev_get_cbor_info_rx: dev=0x7fd4b09044d0, ci=0x7fd4afc09cd0, ms=-1
fido_rx: d=0x7fd4b09044d0, cmd=0x10, buf=0x7ffee999b470, count=2048, ms=-1
rx_preamble: initiation frame at 0x7ffee999b3f0
0000: 00 0c 00 1c 90 00 c3 00 aa 01 83 66 55 32 46 5f
0016: 56 32 68 46 49 44 4f 5f 32 5f 30 6c 46 49 44 4f
0032: 5f 32 5f 31 5f 50 52 45 02 82 6b 63 72 65 64 50
0048: 72 6f 74 65 63 74 6b 68 6d 61 63 2d 73 65 63 72
rx: payload_len=195
rx: continuation frame at 0x7ffee999b3f0
0000: 00 0c 00 1c 00 65 74 03 50 2f c0 57 9f 81 13 47
0016: ea b1 16 bb 5a 8d b9 20 2a 04 a5 62 72 6b f5 62
0032: 75 70 f5 64 70 6c 61 74 f4 69 63 6c 69 65 6e 74
0048: 50 69 6e f5 75 63 72 65 64 65 6e 74 69 61 6c 4d
rx: continuation frame at 0x7ffee999b3f0
0000: 00 0c 00 1c 01 67 6d 74 50 72 65 76 69 65 77 f5
0016: 05 19 04 b0 06 81 01 07 08 08 18 80 09 82 63 6e
0032: 66 63 63 75 73 62 0a 82 a2 63 61 6c 67 26 64 74
0048: 79 70 65 6a 70 75 62 6c 69 63 2d 6b 65 79 a2 63
rx: continuation frame at 0x7ffee999b3f0
0000: 00 0c 00 1c 02 61 6c 67 27 64 74 79 70 65 6a 70
0016: 75 62 6c 69 63 2d 6b 65 79 00 00 00 00 00 00 00
0032: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0048: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
fido_rx: buf=0x7ffee999b470, len=195
0000: 00 aa 01 83 66 55 32 46 5f 56 32 68 46 49 44 4f
0016: 5f 32 5f 30 6c 46 49 44 4f 5f 32 5f 31 5f 50 52
0032: 45 02 82 6b 63 72 65 64 50 72 6f 74 65 63 74 6b
0048: 68 6d 61 63 2d 73 65 63 72 65 74 03 50 2f c0 57
0064: 9f 81 13 47 ea b1 16 bb 5a 8d b9 20 2a 04 a5 62
0080: 72 6b f5 62 75 70 f5 64 70 6c 61 74 f4 69 63 6c
0096: 69 65 6e 74 50 69 6e f5 75 63 72 65 64 65 6e 74
0112: 69 61 6c 4d 67 6d 74 50 72 65 76 69 65 77 f5 05
0128: 19 04 b0 06 81 01 07 08 08 18 80 09 82 63 6e 66
0144: 63 63 75 73 62 0a 82 a2 63 61 6c 67 26 64 74 79
0160: 70 65 6a 70 75 62 6c 69 63 2d 6b 65 79 a2 63 61
0176: 6c 67 27 64 74 79 70 65 6a 70 75 62 6c 69 63 2d
0192: 6b 65 79
parse_reply_element: cbor type
parse_reply_element: cbor type
fido_dev_open_rx: FIDO_MAXMSG=2048, maxmsgsiz=1200
fido_tx: d=0x7fd4b09044d0, cmd=0x10, buf=0x7fd4b09060f0, count=146
0000: 01 a6 01 58 20 61 2e ef 12 ca 69 87 b3 16 c0 61
0016: 8b 3a 24 21 a4 3c 7e 59 8c 8a 99 2f 3a 44 e1 85
0032: 3c 3f 7d 02 71 02 a1 62 69 64 69 67 6f 63 72 79
0048: 70 74 66 73 03 a2 62 69 64 58 20 61 2e ef 12 ca
0064: 69 87 b3 16 c0 61 8b 3a 24 21 a4 3c 7e 59 8c 8a
0080: 99 2f 3a 44 e1 85 3c 3f 7d 02 71 64 6e 61 6d 65
0096: 64 74 65 73 74 04 81 a2 63 61 6c 67 26 64 74 79
0112: 70 65 6a 70 75 62 6c 69 63 2d 6b 65 79 06 a1 6b
0128: 68 6d 61 63 2d 73 65 63 72 65 74 f5 07 a1 62 75
0144: 76 f5
fido_rx: d=0x7fd4b09044d0, cmd=0x10, buf=0x7ffee999b500, count=2048, ms=-1
rx_preamble: initiation frame at 0x7ffee999b450
0000: 00 0c 00 1c 90 00 01 2b 00 00 00 00 00 00 00 00
0016: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0032: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0048: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
rx: payload_len=1
fido_rx: buf=0x7ffee999b500, len=1
0000: 2b
cbor_parse_reply: blob[0]=0x2b
fido_dev_make_cred_rx: parse_makecred_reply
fido2-cred: fido_dev_make_cred: FIDO_ERR_UNSUPPORTED_OPTION

After some searching through issues of libfido2 repository, it looks like not all FIDO2 tokens are capable of user verification option, or at least Yubikey does not. See also this issue.

Removing the -v option from fido2-cred command call should work, but there is one more issue. If a PIN is required to access the FIDO2 device, fido2-cred will ask the user to type the PIN via stdin, and I don't think gocryptfs could handle that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants