Bug 203419 – [iOS] Fix sandbox violations seen while running layout tests

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Per Arne Vollan]

Deny mach lookup to 'com.apple.logd' and 'com.apple.logd.events' and suppress logs, since these are believed to be unneeded in the WebContent process.
Allow sysctl write to 'vm.footprint_suspend' internally.
Deny mach lookup to 'com.apple.system.notification_center' and suppress logs, since allowing this is not believed to be needed in the WebContent process.

[Per Arne Vollan]

Created attachment 381930 [details]
Patch

[Brent Fulgham]

Comment on attachment 381930 [details]
Patch

r=me

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #2)
> Comment on attachment 381930 [details]
> Patch
> 
> r=me

Thanks for reviewing!

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 381930 [details]:

imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html bug 203394 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 381930 [details]:

The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 381930 [details]:

imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html bug 203394 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 381930 [details]
Patch

Clearing flags on attachment: 381930

Committed r251612: <https://trac.webkit.org/changeset/251612>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56637619>

[Alexey Proskuryakov]

Comment on attachment 381930 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +        Deny mach lookup to 'com.apple.logd' and 'com.apple.logd.events' and suppress logs, since these are
> +        believed to be unneeded in the WebContent process. Allow sysctl write to 'vm.footprint_suspend'.
> +        Deny mach lookup to 'com.apple.system.notification_center' and suppress logs, since allowing this
> +        is not believed to be needed in the WebContent process.

Where can stack traces for these violations be seen? It seems quite counter-intuitive that we do not need these connections.

Can you document these decisions in the associated radar, if they cannot be documented here? Right now, there is nearly zero paper trail for why these dangerous changes were made.

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:482
> +(with-filter (system-attribute apple-internal)
> +    (allow sysctl-read sysctl-write
> +           (sysctl-name "vm.footprint_suspend")))

"system-attribute apple-internal" is not what it seems. This change allows vm.footprint_suspend on some AppleInternal installs, but not on others, and it's unlikely that this is what you were after. You can see <rdar://problem/55853605> for some discussion of the differences.

But also, footprint is a publicly shipping tool. What specifically about vm.footprint_suspend makes it AppleInternal only?

[Per Arne Vollan]

(In reply to Alexey Proskuryakov from comment #10)
> Comment on attachment 381930 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381930&action=review
> 
> > Source/WebKit/ChangeLog:11
> > +        Deny mach lookup to 'com.apple.logd' and 'com.apple.logd.events' and suppress logs, since these are
> > +        believed to be unneeded in the WebContent process. Allow sysctl write to 'vm.footprint_suspend'.
> > +        Deny mach lookup to 'com.apple.system.notification_center' and suppress logs, since allowing this
> > +        is not believed to be needed in the WebContent process.
> 
> Where can stack traces for these violations be seen? It seems quite
> counter-intuitive that we do not need these connections.
> 
> Can you document these decisions in the associated radar, if they cannot be
> documented here? Right now, there is nearly zero paper trail for why these
> dangerous changes were made.
> 

You are absolutely right. I have reverted most of these changes in https://bugs.webkit.org/show_bug.cgi?id=203505, except for access to 'com.apple.logd.events', which is not being looked up during layout tests.

> > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:482
> > +(with-filter (system-attribute apple-internal)
> > +    (allow sysctl-read sysctl-write
> > +           (sysctl-name "vm.footprint_suspend")))
> 
> "system-attribute apple-internal" is not what it seems. This change allows
> vm.footprint_suspend on some AppleInternal installs, but not on others, and
> it's unlikely that this is what you were after. You can see
> <rdar://problem/55853605> for some discussion of the differences.
> 
> But also, footprint is a publicly shipping tool. What specifically about
> vm.footprint_suspend makes it AppleInternal only?

See comment in <rdar://problem/56637619>.

Thanks for reviewing!