Handle authenticators with UV that do not support ClientPin.

[dainnilsson]

This allows instantiation of the ClientPin class on devices which do not support passing a client PIN. This is needed since the addition of getting a UV token allows usage without requiring PIN.

[ashleysommer]

This change does look better than my pair of PRs. My goal was to try to change as few lines of code as possible to keep the footprint of the PRs small and reduce the possibility of unintentionally breaking something else.

I haven't tested if yet, but I think its not quite working as expected yet on the Trezor because pinUvAuthToken is an advertised feature in FIDO 2.1, but the Trezor supports only FIDO 2.0.

So maybe one step further would be, if "uv" is advertised, but not "pinUvAuthToken" then finally fall back to allow_internal_uv like is done in my first PR.

And looks like currently the is_token_supported() check will fail when getting the shared secret in the hmac-secret extension too, even though the call to get the key agreement and shared secret would work on the Trezor without the check.

[dainnilsson]

Yeah, it was my intention to fall back to internal_uv where appropriate, but I may have missed a few cases of this.

EDIT: Are you able to test this code and see how it behaves with the Trezor? I think that it should already do the proper fallback, but if not I'd like to know exactly where it fails.

[dainnilsson]

I just realized that by amending my previous comment with an edit instead of posting a new one, you may not have gotten a notification about it. Just in case you missed it: @ashleysommer. If you did, sorry for the spam!

[ashleysommer]

Hi @dainnilsson Yes, I did see your original edit. Unfortunately I've been very busy and haven't had a chance to fully test this out on the Trezor for you. I will be able to get to it this weekend.

[ashleysommer]

Hi @dainnilsson

In short, yes this PR does fix the issues I was seeing with the Trezor in my test application. The code flow does fall back to internal_uv as you thought.

Stepping though the code, I do have one question about this logic when determining internal_uv (though my understanding may not be correct):

• The internal_uv is only set if pin_token is None (this is fine)
• pin_token is None only if allow_internal_uv is True (this seems logical)
• allow_internal_uv is only True if there are no additional permissions required on the request.

So my understanding is that if any CTAP2 extensions require extra permissions, then allow_internal_uv will be False, so those extensions cannot work on devices that do only internal UV. This may not be a problem though, because from what I can see the only extension that uses the additional permission mechanism is the LargeBlob extension, and Trezor does not support that anyway. It might also be the case that all extensions that request additional permissions are from FIDO 2.1, and compliant FIDO 2.1 devices that have internal UV will also support uvAuthToken so it will use that mechanism instead.

Anyway, I think this PR is good, and can be merged.

[dainnilsson]

@ashleysommer - Perfect, thank you for testing and for verifying the logic!

Your understanding sounds correct to me, and matches my own: Internal UV will be used only if no additional permissions are required. However, if additional permissions are required, this would require a CTAP 2.1 capable device anyway, and the uvAuthToken mechanism should be used in this case. I will merge this PR for inclusion in the next release.