Bug 218479

Summary: [WebAuthn] [iOS] WebAuthn process doesn't start on iOS devices
Product: WebKit Reporter: Jiewen Tan <jiewen_tan>
Component: WebKit Misc.Assignee: Jiewen Tan <jiewen_tan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, jiewen_tan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 181943    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
bfulgham: review+
Patch for landing none

Jiewen Tan
Reported 2020-11-02 22:23:55 PST
WebAuthn process doesn't start on iOS devices.
Attachments
Patch (43.44 KB, patch)
2020-11-02 22:30 PST, Jiewen Tan
no flags
Patch (37.33 KB, patch)
2020-11-05 00:00 PST, Jiewen Tan
no flags
Patch (37.97 KB, patch)
2020-11-05 19:39 PST, Jiewen Tan
bfulgham: review+
Patch for landing (33.10 KB, patch)
2020-11-06 17:10 PST, Jiewen Tan
no flags
Jiewen Tan
Comment 1 2020-11-02 22:24:16 PST
Jiewen Tan
Comment 2 2020-11-02 22:30:20 PST
Brent Fulgham
Comment 3 2020-11-04 14:01:35 PST
Comment on attachment 413004 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413004&action=review This initial sandbox is far too lax. You should pare it down before we land this. I also expect to see macOS sandboxing for this process, is that happening in a separate bug? > Source/WebKit/ChangeLog:12 > + Copied from the GPU process. Will strip it down once the process is fully functional. You will need a sandbox on macOS, too. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:199 > +) I'll bet we do not need media-remote at all. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:219 > +) I doubt we need media-capture at all. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:230 > +) We might need this if we are showing UI through this XPC service, but I suspect that authd actually does that, so we probably don't need this. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:240 > +) I doubt we need media-accessibility-support. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:290 > +) I don't we need the OpenGL support. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:383 > +) I doubt we need speech-synthesis-and-voiceover. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:434 > +) We probably do not need dictionary-support. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:579 > + (global-name "com.apple.tccd")) I don't think we need this, but since it's generating telemetry you can keep it so we can double-check. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:665 > +(play-media) I doubt we need these. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:668 > +(media-remote) Delete this please. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:674 > + (global-name "com.apple.TextInput")) I doubt we need this. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:678 > +(speech-synthesis-and-voiceover) Delete this please. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:681 > + (global-name "com.apple.audio.AudioComponentRegistrar")) Delete this please. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:688 > + "/systemgroup.com.apple.nsurlstoragedresources/Library/dafsaData.bin") Seems very unlikely we need this. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:707 > +) It seems very unlikely we need these. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:724 > + (global-name "com.apple.iconservices")) It seems unlikely we need this stuff. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:738 > +(dictionary-support) Delete this, please. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:748 > +(framebuffer-access) Seems unlikely > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:751 > +(opengl) Seems very unlikely
Brent Fulgham
Comment 4 2020-11-04 16:51:46 PST
Comment on attachment 413004 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413004&action=review > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:544 > + (extension "com.apple.security.exception.shared-preference.read-write")) Please also remove this rule (see Bug 218594). We don't issue this extension to WebKit, ever.
Brent Fulgham
Comment 5 2020-11-04 17:05:36 PST
Comment on attachment 413004 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413004&action=review > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:538 > + (extension "com.apple.security.exception.iokit-user-client-class")) Please also remove this rule (See Bug 218596). We don't issue this extension to WebKit, ever.
Jiewen Tan
Comment 6 2020-11-04 23:58:30 PST
Comment on attachment 413004 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413004&action=review Thanks Brent for reviewing this patch. >> Source/WebKit/ChangeLog:12 >> + Copied from the GPU process. Will strip it down once the process is fully functional. > > You will need a sandbox on macOS, too. The macOS process already has one. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:199 >> +) > > I'll bet we do not need media-remote at all. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:219 >> +) > > I doubt we need media-capture at all. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:230 >> +) > > We might need this if we are showing UI through this XPC service, but I suspect that authd actually does that, so we probably don't need this. Springboard will probably do that for us. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:240 >> +) > > I doubt we need media-accessibility-support. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:290 >> +) > > I don't we need the OpenGL support. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:383 >> +) > > I doubt we need speech-synthesis-and-voiceover. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:434 >> +) > > We probably do not need dictionary-support. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:538 >> + (extension "com.apple.security.exception.iokit-user-client-class")) > > Please also remove this rule (See Bug 218596). We don't issue this extension to WebKit, ever. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:544 >> + (extension "com.apple.security.exception.shared-preference.read-write")) > > Please also remove this rule (see Bug 218594). We don't issue this extension to WebKit, ever. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:579 >> + (global-name "com.apple.tccd")) > > I don't think we need this, but since it's generating telemetry you can keep it so we can double-check. Got you. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:665 >> +(play-media) > > I doubt we need these. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:668 >> +(media-remote) > > Delete this please. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:674 >> + (global-name "com.apple.TextInput")) > > I doubt we need this. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:678 >> +(speech-synthesis-and-voiceover) > > Delete this please. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:681 >> + (global-name "com.apple.audio.AudioComponentRegistrar")) > > Delete this please. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:688 >> + "/systemgroup.com.apple.nsurlstoragedresources/Library/dafsaData.bin") > > Seems very unlikely we need this. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:707 >> +) > > It seems very unlikely we need these. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:724 >> + (global-name "com.apple.iconservices")) > > It seems unlikely we need this stuff. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:738 >> +(dictionary-support) > > Delete this, please. Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:748 >> +(framebuffer-access) > > Seems unlikely Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:751 >> +(opengl) > > Seems very unlikely Removed.
Jiewen Tan
Comment 7 2020-11-05 00:00:08 PST
Jiewen Tan
Comment 8 2020-11-05 19:39:04 PST
Brent Fulgham
Comment 9 2020-11-06 16:20:08 PST
Comment on attachment 413387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413387&action=review Can you please file a radar to audit the sandbox? I think both WebAuthn sandoxes contain tons of stuff we don't need, and we should clean that up before we turn this on for people. But r=me for this work in progress. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:111 > + (xpc-service-name "com.apple.audio.toolbox.reporting.service"))) I'll bet we don't need "play-audio" > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:189 > +) I'll bet we don't need this "play-media" bit at all.
Jiewen Tan
Comment 10 2020-11-06 16:27:26 PST
Comment on attachment 413387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413387&action=review Thanks Brent for r+ this patch. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:111 >> + (xpc-service-name "com.apple.audio.toolbox.reporting.service"))) > > I'll bet we don't need "play-audio" Removed. >> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebAuthn.sb:189 >> +) > > I'll bet we don't need this "play-media" bit at all. Removed.
Jiewen Tan
Comment 11 2020-11-06 17:10:19 PST
Created attachment 413500 [details] Patch for landing
EWS
Comment 12 2020-11-06 17:38:58 PST
Committed r269554: <https://trac.webkit.org/changeset/269554> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413500 [details].
Note You need to log in before you can comment on or make changes to this bug.