Bug 218479

<rdar://problem/70560399>

Created attachment 413004 [details] Patch

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

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.

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.

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.

Created attachment 413264 [details] Patch

Created attachment 413387 [details] Patch

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.

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.

Created attachment 413500 [details] Patch for landing

Committed r269554: <https://trac.webkit.org/changeset/269554> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413500 [details].