[webkit-reviews] review denied: [Bug 218479] [WebAuthn] [iOS] WebAuthn process doesn't start on iOS devices : [Attachment 413004] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Jiewen Tan
<jiewen_tan at apple.com>'s request for review:
Bug 218479: [WebAuthn] [iOS] WebAuthn process doesn't start on iOS devices
https://bugs.webkit.org/show_bug.cgi?id=218479

Attachment 413004: Patch

https://bugs.webkit.org/attachment.cgi?id=413004&action=review




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 413004
  --> https://bugs.webkit.org/attachment.cgi?id=413004
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