Bug 204111

Summary: [WebAuthn] User Verification (UV) option present on a CTAP2 authenticatorMakeCredential while the authenticator has not advertised support for it
Product: WebKit Reporter: kostas
Component: WebCore Misc.Assignee: Jiewen Tan <jiewen_tan>
Status: RESOLVED FIXED    
Severity: Normal CC: alex.gaynor, bfulgham, commit-queue, jiewen_tan, jonathan, kostas, loginllama, paul.l.kehrer, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 13   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 181943    
Attachments:
Description Flags
Patch none

Description kostas 2019-11-12 05:16:02 PST
The latest public FIDO2 CTAP spec mandates authenticators to terminate any ongoing procedures with CTAP2_ERR_UNSUPPORTED_OPTION if any of the options passed are not supported by the authenticator. In this case, a CTAP2 authenticatorMakeCredential from Safari sets the UV option to FALSE even if the authenticator never advertised support for it in authenticatorGetInfo response. The expected behavior here would be to not include the UV option at all since it is not supported (and not set it to FALSE or TRUE). See chapter 5.3 in latest FIDO2 CTAP spec, step 3 in the procedure of authenticatorMakeCredential and step 5 in the procedure for authenticatorGetAssertion. Please note, that the above is applicable to the authenticatorGetAssertion requests as well.

Latest public spec: https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html
Comment 1 login Llama 2020-01-03 12:03:42 PST
It seems bug (198408) has landed in iOS 13.3.1 and for desktop.

Good but that makes this bug much worse.  

Now almost no CTAP 2 keys are working, unless they support builtin UV. 

I have tested that and they work. 

There are some authenticators that for whatever reason don't properly check like some of the early Yubico 5ci with software 5.2.3.  All of the shipping ones with 5.2.4 fail to work with Safari because of this bug.  I tested other CTAP 2 authenticators and they also don't work almost certainly for the same reason.

So basically all CTAP2 authenticators without built in UV are currently broken.
Comment 2 Radar WebKit Bug Importer 2020-01-06 08:46:30 PST Comment hidden (obsolete)
Comment 3 Jiewen Tan 2020-01-15 18:19:11 PST
<rdar://problem/57019604>
Comment 4 Jiewen Tan 2020-01-15 19:20:19 PST
Created attachment 387887 [details]
Patch
Comment 5 Brent Fulgham 2020-01-16 13:16:07 PST
Comment on attachment 387887 [details]
Patch

r=me
Comment 6 Jiewen Tan 2020-01-16 14:11:03 PST
Comment on attachment 387887 [details]
Patch

Thanks Brent for r+ the patch.
Comment 7 WebKit Commit Bot 2020-01-16 14:54:08 PST
Comment on attachment 387887 [details]
Patch

Clearing flags on attachment: 387887

Committed r254710: <https://trac.webkit.org/changeset/254710>
Comment 8 WebKit Commit Bot 2020-01-16 14:54:09 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Jiewen Tan 2020-01-16 14:57:27 PST
(In reply to kostas from comment #0)
> The latest public FIDO2 CTAP spec mandates authenticators to terminate any
> ongoing procedures with CTAP2_ERR_UNSUPPORTED_OPTION if any of the options
> passed are not supported by the authenticator. In this case, a CTAP2
> authenticatorMakeCredential from Safari sets the UV option to FALSE even if
> the authenticator never advertised support for it in authenticatorGetInfo
> response. The expected behavior here would be to not include the UV option
> at all since it is not supported (and not set it to FALSE or TRUE). See
> chapter 5.3 in latest FIDO2 CTAP spec, step 3 in the procedure of
> authenticatorMakeCredential and step 5 in the procedure for
> authenticatorGetAssertion. Please note, that the above is applicable to the
> authenticatorGetAssertion requests as well.
> 
> Latest public spec:
> https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-
> authenticator-protocol-v2.0-ps-20190130.html

Thanks for reporting the issue, please follow our STP release to verify the fix or you could use our nightly to do that.
Comment 10 login Llama 2020-01-17 06:39:27 PST
Thanks Jiewen

When will this land in the STP?

I think Kostas is on vacation, and I am traveling but will have a computer to test on Monday.

I will let you know once I can test.
Comment 11 Jiewen Tan 2020-01-17 13:00:19 PST
(In reply to login Llama from comment #10)
> Thanks Jiewen
> 
> When will this land in the STP?
> 
> I think Kostas is on vacation, and I am traveling but will have a computer
> to test on Monday.
> 
> I will let you know once I can test.

Even for (In reply to login Llama from comment #10)
> Thanks Jiewen
> 
> When will this land in the STP?
> 
> I think Kostas is on vacation, and I am traveling but will have a computer
> to test on Monday.
> 
> I will let you know once I can test.

Sorry, even for STP, we don't usually comment on its future release. But you can definitely follow webkit.org's release note to see if the next one contains the fix.
Comment 12 login Llama 2020-01-21 07:51:22 PST
I tested with build 254850 from today.

The UV bug for make credential seems to be fixed.

For Get Assertion I am still seeing a problem.

Is there any debugging like Chrome's chrome://device-log/ That will let me see the messages?
Comment 13 login Llama 2020-01-21 08:27:55 PST
If option clientPin=0 then it works for makeCradential and getAssertion.

If clientPin=1 then both makeCradential and getAssertion fail.

I expect that for makeCredential because pintoken is required to be able to make the credential over CTAP2.

getAssertion should work as long as WebAuthn has UV unset or discouraged.

Given I cant see the cbor message going to the authenticator I cant say if this is the same bug or another one related to Pin triggered when getInfo contains clientPin=1.
Comment 14 Jiewen Tan 2020-01-21 12:00:24 PST
(In reply to login Llama from comment #13)
> If option clientPin=0 then it works for makeCradential and getAssertion.
> 
> If clientPin=1 then both makeCradential and getAssertion fail.
> 
> I expect that for makeCredential because pintoken is required to be able to
> make the credential over CTAP2.
> 
> getAssertion should work as long as WebAuthn has UV unset or discouraged.
> 
> Given I cant see the cbor message going to the authenticator I cant say if
> this is the same bug or another one related to Pin triggered when getInfo
> contains clientPin=1.

It should be a bug about our immature ClientPIN support. Bug 206547.
Comment 15 login Llama 2020-01-21 13:22:29 PST
While there may be a pin bug, I don't think this is that.

I modified a authenticator to ignore the extra UV option in the request and that works for getAssertion with no "uv" option advertised in getInfo and clientPin=1

I think there is still a bug with getAssertion.  Are you perhaps sending options with uv=0 for iterating through the allow list or something?
Comment 16 Jiewen Tan 2020-01-21 13:56:29 PST
(In reply to login Llama from comment #15)
> While there may be a pin bug, I don't think this is that.
> 
> I modified a authenticator to ignore the extra UV option in the request and
> that works for getAssertion with no "uv" option advertised in getInfo and
> clientPin=1
> 
> I think there is still a bug with getAssertion.  Are you perhaps sending
> options with uv=0 for iterating through the allow list or something?

Maybe you could tell me the model of the key you are using and the parameters you pass to the API? So, I can try to reproduce the bug. Otherwise, I'm not seeing any problems with our current code.
Comment 17 login Llama 2020-01-22 15:40:51 PST
I think I must have just been having trouble with the version from the build archives.

STP 99 with webkit 15610 seems to be working fine.

With and without pin configured as long as you don't send UV required.

Thanks big improvement in STP 99
Comment 18 Jiewen Tan 2020-01-22 15:47:13 PST
(In reply to login Llama from comment #17)
> I think I must have just been having trouble with the version from the build
> archives.
> 
> STP 99 with webkit 15610 seems to be working fine.
> 
> With and without pin configured as long as you don't send UV required.
> 
> Thanks big improvement in STP 99

I don't think STP 99 contains this fix. It will be great if you could redo your test with the next one, which will probably have the fix.