Bug 203525 – [iOS] Clean up sandbox to group similar rules together

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brent Fulgham]

This revises the sandbox to group related rules together for easier editing in the future, and to reduce the need for so many comments.

There are no functional changes in this code; the expected behavior of the sandbox is identical to the current profile.

Changes include:

1. Creating new functions that add extensions for mach access, preference reading, and file access to support specific features (e.g., media capture, Metal support, etc.)
2. Remove duplicated rules, since the sandbox contains rules that were combined from three separate sources.

[Radar WebKit Bug Importer]

<rdar://problem/56686416>

[Brent Fulgham]

Created attachment 382116 [details]
Patch

[Per Arne Vollan]

Comment on attachment 382116 [details]
Patch

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

Great! R=me.

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:275
> +    (allow iokit-get-properties
> +        (iokit-property "IOGLBundleName")
> +        (iokit-property "IOGLESBundleName")
> +        (iokit-property "IOGLESDefaultUseMetal")
> +        (iokit-property "IOGLESMetalBundleName")
> +        (iokit-property "MetalPluginClassName")
> +        (iokit-property "MetalPluginName")
> +    )

Is this a new rule?

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:433
> +        (global-name "com.apple.frontboard.systemappservices")

This seems to be a duplicate.

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:-520
> -(with-filter (uid 0)
> -    (allow file-read*
> -           (literal "/private/etc/master.passwd")))

Is this a behavior change? Perhaps consider moving this into its own patch in case it is.

[Brent Fulgham]

Comment on attachment 382116 [details]
Patch

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

>> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:275
>> +    )
> 
> Is this a new rule?

It moves code from the overall "iokit-get-properties" allow list to this function. It is an expansion of the regexp, based on feedback from the Sandbox team that it would improve performance to list them separately.

>> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:433
>> +        (global-name "com.apple.frontboard.systemappservices")
> 
> This seems to be a duplicate.

Whoops! That's the whole point of this patch and I missed this one!

>> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:-520
>> -           (literal "/private/etc/master.passwd")))
> 
> Is this a behavior change? Perhaps consider moving this into its own patch in case it is.

No -- we don't allow the WebContent process to run as root, so this rule is meaningless.

[Brent Fulgham]

Created attachment 382195 [details]
Patch for landing

[WebKit Commit Bot]

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

The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

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

imported/w3c/web-platform-tests/websockets/bufferedAmount-unchanged-by-sync-xhr.any.worker.html bug 202003 (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 382195 [details]:

imported/w3c/web-platform-tests/css/css-values/absolute_length_units.html bug 203581 (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 382195 [details]:

The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 382195 [details]
Patch for landing

Clearing flags on attachment: 382195

Committed r251734: <https://trac.webkit.org/changeset/251734>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.