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.
<rdar://problem/56686416>
Created attachment 382116 [details] Patch
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.
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.
Created attachment 382195 [details] Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 382195 [details]: The commit-queue is continuing to process your patch.
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.
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.
Comment on attachment 382195 [details] Patch for landing Clearing flags on attachment: 382195 Committed r251734: <https://trac.webkit.org/changeset/251734>
All reviewed patches have been landed. Closing bug.