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

Transport layer encryption #13

Open
1 task
szszszsz opened this issue Mar 26, 2020 · 5 comments
Open
1 task

Transport layer encryption #13

szszszsz opened this issue Mar 26, 2020 · 5 comments

Comments

@szszszsz
Copy link
Member

In #11 (comment) there was a suggestion to use encrypted channel to transport data against USB sniffing. For the FIDO2 encryption layer is already provided, and here it would be redundant. It would be surely useful in case of the automatic protocol downgrade to the FIDO U2F (e.g. for nonsupporting browser, FIDO U2F-only device or when using old API instead of Webauthn).
Perhaps best of two worlds would be to detect access path, and add encryption on-the-fly.
In case of the other JS applications, the session token should be sufficient to protect access, as all data have to be requested from the device.

  • To measure the overhead of the additional encryption layer. In case its marginal, enable encryption in all cases.

To decide / discuss further.

cc @onlykey

@onlykey
Copy link

onlykey commented Mar 26, 2020

@szszszsz So when you say FIDO2 has an encryption layer, I would need to look into it more to see what threat models apply, I am a bit skeptical... I guess the fact that they use AES with an IV of all 0s makes me wonder 😟 https://github.com/Yubico/python-fido2/blob/7376b989a4008955cdd11926b3c22279c7b5481d/fido2/ctap2.py#L854

The way we do transport layer encryption is the web app generates a NACL key pair, the OnlyKey generates a NACL key pair and then the information between the two is encrypted with a NACL shared secret. The threat model we were looking to protect against, and the same one here would be:

  • Malicious local app with full USB access should not be able to see request/response data
  • Malicious web app/browser should not be able to see request/response data
  • If another tab/browser tries to inject in any way the NACL key on device would be destroyed, so legitimate tab/browser would get an error

For overhead, for the way we do it the ciphertext is the same length as the plaintext (AES-256) so no overhead there. There is of course some overhead for the initial key exchange (32 bytes to/from for NACL public keys. Currently send one FIDO2 message to initialize NACL keys and it also exchanges device firmware version, browser, OS and then sends a second message to perform the actual encryption/signing. Everything is encoded in the FIDO2 keyhandle. We do have the option to enable or disable the transport layer encryption as its not needed for some things - https://github.com/trustcrypto/libraries/blob/7a37d0b52bf30b1e4597a75ea41a30e6aef6ce5b/fido2/ok_extension.cpp#L92

@szszszsz
Copy link
Member Author

Thank you for the insight!
I have just checked this against FIDO2 CTAP specification, and the only thing that is encrypted is the new user's PIN to be set on the device, and for that single use zeroed IV should suffice (as long as the shared secret is recomputed). There is no other AES use at all (excluding hmac-secret extension), specifically transport layer encryption. Sorry for confusion.

In that case I believe that your suggestion to use the initial keys exchange, and later encrypting the traffic is right. I agree with the threat model you are proposing. I would add potential radio eavesdropping to complete the list, since it is possible to use FIDO2 over NFC and Bluetooth (whenever implemented). For the NFC it might be required to limit the encryption due to energy limits, but this is another problem.
Regarding overhead, I initially thought rather about performance lost from possible doubled AES encryption, but that is solved now.

Concluding, transport layer encryption should be added to the design.

@szszszsz szszszsz added this to the Basic firmware capabilities milestone Mar 28, 2020
@jans23
Copy link
Member

jans23 commented Apr 2, 2020

Without looking much into detail, I understand the threat "Malicious web app/browser should not be able to see request/response data" as well as a different browser or tab, both would be mitigated by a session token already. Right?

@szszszsz
Copy link
Member Author

szszszsz commented Apr 2, 2020

This is correct. Establishing the transport layer encryption will allow to omit the session token though, since without properly encrypted data with a session generated transport keys it will not be possible to either read or write into given session. And the keys will be destroyed on the another connection attempt (with optional timeout).

@jans23
Copy link
Member

jans23 commented Apr 2, 2020

In this case transport layer encryption is required to address the threat "malicious local app with full USB access or eavesdropper of NFC and Bluetooth communication should not be able to see request/response data". I think it would be good to mitigate this threat but not relevant for most users and therefore not a high-priority.

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

No branches or pull requests

3 participants