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

Simple files from examples directory do not work on Trezor model T #189

Open
ashleysommer opened this issue Jun 9, 2023 · 8 comments
Open

Comments

@ashleysommer
Copy link

ashleysommer commented Jun 9, 2023

Creating a FIDO2 credential using the FIDO2 python module reference hmac-secret example fails with "Authenticator does not support ClientPin".

This is the corresponding Issue I created for the Trezor Firmware: trezor/trezor-firmware#3068

To Reproduce
Steps to reproduce the behavior:

  1. Plug in Trezor model 2 and input your PIN to connect the device to the PC
  2. Run the reference hmac_secret.py example file.
  3. See error "Authenticator does not support ClientPin".

The error is pretty easy to track down by stepping through the python-fido2 sourcecode. When creating a new credential the client library checks if it needs to ask for Pin Auth from the device. To determine this, it checks for if 'uv' is in ctap info options, and if 'uv==True'. The Trezor model T does advertise "uv=True". On the very next line the fido2 library attempts to initialize the ClientPin verification method then throws an exception because "clientPin" is not in info.options.

Seems like when it comes to 'uv' and 'clientPin', python-fido2 needs both advertised or neither advertised.

If I patch the python-fido2 library to ignore the advertised 'uv==True' and assume no UV, it works normally, asks for my pin on the screen, then creates and registers the credential correctly on the device.

But it fails again further on. When sending the hmac-secret salt for attestation the library needs access to the pin-protocol's shared secret (to encrypt the salt going over the wire). This causes the fido2 library to attempt to initialize the ClientPin module again that leads to the same "Authenticator does not support ClientPin" message.

Patching to ignore the "uv==true" info in this case does not work, because the hmac-secret requires clientPin support regardless.

Note, a lot of this is mentioned already under the "User Verification" heading in the Trezor's webauthn app README. It explains that Trezor does support a very minimal implementation of clientPin, just to the extent that is required for hmac-secret to work.

Additional context
For transparency: I was sent a Trezor Model T free of charge from SatoshiLabs for the purposes of testing and implementing FIDO2 hmac-secret support in a popular opensource application.

@ashleysommer
Copy link
Author

So it seems like there are two issues here, that are potentially solvable in python-fido2.

  1. Not all devices that advertise uv=True necessarily support clientPin. There should be an additional call to ClientPin.is_supported() guarding this line .
  2. When "hmac-secret" extension is advertised, but "clientPin" is not, enter a fallback mode where the client can attempt to create a minimal ClientPin instance without throwing the "Not Supported" exception, just for the purposes of hmac-secret (it needs the connection's shared secret).

@andrewkozlik
Copy link

andrewkozlik commented Jun 10, 2023

I agree with @ashleysommer's assessment. The assumption that uv==True in the authenticatorGetInfo response implies clientPin==True seems incorrect. uv refers to built-in user verification and clientPin refers to the capability of accepting a PIN from the client platform, e.g. from the browser. In my opinion these two are completely independent. For example the CTAP spec explicitly states that "ClientPIN is not considered a built-in user verification method".

The Trezor T supports PIN entry directly on the device's touch screen, which is superior in terms of security to ClientPin, because the PIN cannot be intercepted by a keylogger or other potential malware on the user's machine. Supporting ClientPin on Trezor would reduce the security of the device, because it would encourage users to enter their PIN on their machine rather than on Trezor's built-in touchscreen.

@ashleysommer
Copy link
Author

I created two pull requests (#190 and #191) with fixes for these two issues.

@dainnilsson
Copy link
Member

Just to give a quick response to this: At first glance this looks fine, as do the two PRs. I will need to scrutinize the spec a bit before merging and unfortunately I don't have time to do that right now. Apologies for the delay!

@dainnilsson
Copy link
Member

I have an alternative proposal for changes that I believe should address this, and would love some feedback on it: #193.

As I don't have a Trezor T I haven't been able to test this myself, but looking at the proposed other PRs I believe these changes should address both issues. One difference is that in my PR the get_uv_token method is preferred instead of internal UV if supported and additional permissions are required. I'm assuming that this is supported by the Trezor, but I am not sure. If not then this may need additional changes.

@andrewkozlik
Copy link

"UV token" sounds like something from CTAP 2.1. Trezor currently supports only CTAP 2.0.

@dainnilsson
Copy link
Member

"UV token" sounds like something from CTAP 2.1. Trezor currently supports only CTAP 2.0.

I believe that should still be fine, the code should fall back to pinAuth with internal_uv.

@dainnilsson
Copy link
Member

Version 1.1.2 is now released and should hopefully resolve this.

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

No branches or pull requests

3 participants