Bug 203525

Summary: [iOS] Clean up sandbox to group similar rules together
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit2Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Brent Fulgham
Reported 2019-10-28 14:47:58 PDT
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.
Attachments
Patch (19.68 KB, patch)
2019-10-28 15:03 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch for landing (19.58 KB, patch)
2019-10-29 11:18 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-10-28 15:01:44 PDT
<rdar://problem/56686416>
Brent Fulgham
Comment 2 2019-10-28 15:03:48 PDT
Created attachment 382116 [details] Patch
Per Arne Vollan
Comment 3 2019-10-28 16:40:50 PDT
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 4 2019-10-28 16:50:38 PDT
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
Comment 5 2019-10-29 11:18:08 PDT
Created attachment 382195 [details] Patch for landing
WebKit Commit Bot
Comment 6 2019-10-29 12:04:17 PDT
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 7 2019-10-29 12:04:24 PDT
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: youennf@gmail.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 8 2019-10-29 12:59:45 PDT
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: simon.fraser@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 9 2019-10-29 12:59:46 PDT
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 10 2019-10-29 14:22:30 PDT
Comment on attachment 382195 [details] Patch for landing Clearing flags on attachment: 382195 Committed r251734: <https://trac.webkit.org/changeset/251734>
WebKit Commit Bot
Comment 11 2019-10-29 14:22:31 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.