Bug 191516 - [WebAuthn] Support CTAP Client Pin
Summary: [WebAuthn] Support CTAP Client Pin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
: 205222 (view as bug list)
Depends on:
Blocks: 181943
  Show dependency treegraph
 
Reported: 2018-11-11 17:04 PST by Jiewen Tan
Modified: 2020-01-14 12:23 PST (History)
9 users (show)

See Also:


Attachments
Patch (78.02 KB, patch)
2020-01-06 16:51 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (83.55 KB, patch)
2020-01-07 10:12 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (83.55 KB, patch)
2020-01-07 11:25 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (79.83 KB, patch)
2020-01-07 14:05 PST, Jiewen Tan
bfulgham: review+
Details | Formatted Diff | Diff
Patch for landing (80.47 KB, patch)
2020-01-10 17:02 PST, Jiewen Tan
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Build fix (1.68 KB, patch)
2020-01-13 17:30 PST, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Radar WebKit Bug Importer 2019-10-23 15:54:19 PDT
<rdar://problem/56558558>
Comment 2 login Llama 2019-12-13 09:50:23 PST
Yes for keys with PIN set you cant create new credentials without the user inputting the PIN in CTAP2.   The workaround that Firefox and Chrome use when UV is set to discouraged is to make the credential using U2F on keys that support both CTAP1 and CTAP2.  For a CTAP2 only key or if you are making a resident credential there is no way to do it without PIN support.

I will note that currently CTAP2.1 will allow the creation of non resident credentials via CTAP2, but there are millions of authenticators in the market currently that will only support CTAP2.0.
Comment 3 Jiewen Tan 2020-01-06 08:48:06 PST
*** Bug 205222 has been marked as a duplicate of this bug. ***
Comment 4 login Llama 2020-01-06 10:49:11 PST
My first choice is supporting client pin. 
Second choice is using the U2F make credential workaround that Google used in Chrome.

On the other hand bug 204111 is the highest priority as it is breaking all CTAP 2 authenticators without builtin UV.  That is about 98%.  Until that gets fixed we cant really test this.

Thanks
Comment 5 Jiewen Tan 2020-01-06 16:51:21 PST
Created attachment 386914 [details]
Patch
Comment 6 Jiewen Tan 2020-01-07 10:12:25 PST
Created attachment 386984 [details]
Patch
Comment 7 Jiewen Tan 2020-01-07 11:25:57 PST
Created attachment 387005 [details]
Patch
Comment 8 Jiewen Tan 2020-01-07 14:05:32 PST
Created attachment 387028 [details]
Patch
Comment 9 Brent Fulgham 2020-01-10 15:23:07 PST
Comment on attachment 387028 [details]
Patch

r=me
Comment 10 Jiewen Tan 2020-01-10 15:38:42 PST
Thanks Brent for r+ this patch.
Comment 11 Jiewen Tan 2020-01-10 17:02:30 PST
Created attachment 387399 [details]
Patch for landing
Comment 12 WebKit Commit Bot 2020-01-10 18:47:49 PST
Comment on attachment 387399 [details]
Patch for landing

Rejecting attachment 387399 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 387399, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=387399&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=191516&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Updating working directory
Processing patch 387399 from bug 191516.
Fetching: https://bugs.webkit.org/attachment.cgi?id=387399
Authentication required
Authentication required
Authentication required
Updating OpenSource
From https://git.webkit.org/git/WebKit
   8cd2c8b4c7e..e4cf8500860  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 254386 = 8cd2c8b4c7e20fe5b4f3240a3e719722a80784d9
r254387 = e4cf8500860fe817dd55f3a86a67f2a8ab032ead
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: https://webkit-queues.webkit.org/results/13302981
Comment 13 Jiewen Tan 2020-01-13 10:54:09 PST
Committed r254439: <https://trac.webkit.org/changeset/254439>
Comment 14 Don Olmstead 2020-01-13 17:26:26 PST
Reopening for build fix.

Commit queue cancelled my patch because of the following

In file included from /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource55.cpp:1:
In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/WebAuthentication/fido/FidoService.cpp:31:
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/WebAuthentication/fido/CtapAuthenticator.h:66:70: error: unknown type name 'CryptoKeyEC'; did you mean 'WebCore::CryptoKeyEC'?
    void continueGetPinTokenAfterRequestPin(const String& pin, const CryptoKeyEC&);
                                                                     ^~~~~~~~~~~
                                                                     WebCore::CryptoKeyEC
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/WebAuthentication/fido/CtapAuthenticator.h:40:7: note: 'WebCore::CryptoKeyEC' declared here
class CryptoKeyEC;
      ^
1 error generated.

From https://webkit-queues.webkit.org/results/13303609 which came from https://bugs.webkit.org/show_bug.cgi?id=205964
Comment 15 Don Olmstead 2020-01-13 17:30:37 PST
Created attachment 387598 [details]
Build fix
Comment 16 Don Olmstead 2020-01-13 17:51:27 PST
Comment on attachment 387598 [details]
Build fix

Affected builds seem happy
Comment 17 WebKit Commit Bot 2020-01-13 22:42:02 PST
Comment on attachment 387598 [details]
Build fix

Clearing flags on attachment: 387598

Committed r254495: <https://trac.webkit.org/changeset/254495>
Comment 18 Jiewen Tan 2020-01-14 11:06:39 PST
(In reply to Don Olmstead from comment #16)
> Comment on attachment 387598 [details]
> Build fix
> 
> Affected builds seem happy

Thanks for fixing the issue.
Comment 19 Don Olmstead 2020-01-14 12:23:52 PST
(In reply to Jiewen Tan from comment #18)
> (In reply to Don Olmstead from comment #16)
> > Comment on attachment 387598 [details]
> > Build fix
> > 
> > Affected builds seem happy
> 
> Thanks for fixing the issue.

👍