Bug 217559

Summary:

[WebAuthn] Implement a dummy WebAuthnProcess

Product:

WebKit

Reporter:

Jiewen Tan <jiewen_tan>

Component:

WebKit Misc.

Assignee:

Jiewen Tan <jiewen_tan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, annulen, beidson, bfulgham, ews-watchlist, ggaren, gyuyoung.kim, jiewen_tan, rmondello, ryuan.choi, sam, sergio, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=217401

Bug Depends on:

Bug Blocks:

181943

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	bfulgham: review+
Patch for landing	none

Description Jiewen Tan 2020-10-10 02:32:26 PDT

Implement a dummy WebAuthenticationAgent Part 2.

Comment 1 Radar WebKit Bug Importer 2020-10-10 02:32:57 PDT

<rdar://problem/70168749>

Comment 2 Jiewen Tan 2020-10-10 03:12:10 PDT

Created attachment 411008 [details]
Patch

Comment 3 Jiewen Tan 2020-10-14 15:25:33 PDT

Created attachment 411378 [details]
Patch

Comment 4 Jiewen Tan 2020-10-14 19:45:30 PDT

Created attachment 411396 [details]
Patch

Comment 5 Brent Fulgham 2020-10-16 10:02:48 PDT

Comment on attachment 411396 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411396&action=review

You should have Chris Dumez take a look at the process assertion bits, but this seems to be correct as far as I can tell. You should also file a bug to add relevant entitlements to the new service once you start moving calls to USB/NFC things into this new code path.

> Source/WebKit/ChangeLog:25
> +        This patch therefore focues on making the WebAuthn process happen and removes the WebAuthn daemon.

focus

> Source/WebKit/ChangeLog:45
> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.

introducing

> Source/WebKit/ChangeLog:125
> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.

introducing

> Source/WebKit/ChangeLog:128
> +        The sandbox profile is originally from the GPU Process with IOKit related rules removed. Will tighten it again after the process is functional properly.

"process is fully functional"

> Source/WebKit/ChangeLog:153
> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.

introducing

> Source/WebCore/en.lproj/Localizable.strings:49
> +/* visible name of the GPU process. The argument is the application name. */

This isn't the GPU Process. This is the Web Authentication process!

> Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:81
> +    // On Mac, leak a boost onto the NetworkProcess, GPUProcess and WebAuthnProcess.

Please use the "Oxford Comma":  "NetworkProcess, GPUProcess, and WebAuthnProcess."

Comment 6 Jiewen Tan 2020-10-16 11:44:11 PDT

Comment on attachment 411396 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411396&action=review

Thanks Brent for r+ this patch.

>> Source/WebKit/ChangeLog:25
>> +        This patch therefore focues on making the WebAuthn process happen and removes the WebAuthn daemon.
> 
> focus

Fixed.

>> Source/WebKit/ChangeLog:45
>> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.
> 
> introducing

Fixed.

>> Source/WebKit/ChangeLog:125
>> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.
> 
> introducing

Fixed.

>> Source/WebKit/ChangeLog:128
>> +        The sandbox profile is originally from the GPU Process with IOKit related rules removed. Will tighten it again after the process is functional properly.
> 
> "process is fully functional"

Fixed.

>> Source/WebKit/ChangeLog:153
>> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.
> 
> introducing

Fixed.

>> Source/WebCore/en.lproj/Localizable.strings:49
>> +/* visible name of the GPU process. The argument is the application name. */
> 
> This isn't the GPU Process. This is the Web Authentication process!

Oops. Fixed.

>> Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:81
>> +    // On Mac, leak a boost onto the NetworkProcess, GPUProcess and WebAuthnProcess.
> 
> Please use the "Oxford Comma":  "NetworkProcess, GPUProcess, and WebAuthnProcess."

Fixed.

Comment 7 Jiewen Tan 2020-10-16 11:45:38 PDT

Created attachment 411602 [details]
Patch for landing

Comment 8 EWS 2020-10-16 12:38:53 PDT

Committed r268605: <https://trac.webkit.org/changeset/268605>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411602 [details].