Bug 205521 - Network process sandboxes should not include 'common.sb' or 'system.sb'
Summary: Network process sandboxes should not include 'common.sb' or 'system.sb'
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-20 14:32 PST by Brent Fulgham
Modified: 2020-01-08 11:36 PST (History)
6 users (show)

See Also:


Attachments
Patch (28.82 KB, patch)
2019-12-20 14:38 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (29.00 KB, patch)
2020-01-06 16:48 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (28.95 KB, patch)
2020-01-07 12:28 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2019-12-20 14:32:43 PST
The WebKit Networking sandboxes include ‘common.sb’ on iOS, and 'system.sb' on macOS. This enables lots of things we don’t want.

This patch replaces the 'include' with a copy/paste of the contents of the relevant sandbox include file. I removed definitions that were not referenced in the existing Network sandbox, but did not otherwise edit the contents. There are duplicates and redundancies after this patch, which I will remove as a follow-up step once we confirm that this has no regressions.
Comment 1 Brent Fulgham 2019-12-20 14:33:10 PST
<rdar://problem/58095870>
Comment 2 Brent Fulgham 2019-12-20 14:38:29 PST
Created attachment 386257 [details]
Patch
Comment 3 Darin Adler 2019-12-20 15:02:29 PST
Comment on attachment 386257 [details]
Patch

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

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.Networking.sb:585
> -(allow mach-lookup
> +(allow mach-lookup (with report) (with telemetry)

These changes are not mentioned in the change log. Accidentally included in this patch?
Comment 4 Brent Fulgham 2019-12-20 15:28:01 PST
Comment on attachment 386257 [details]
Patch

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

>> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.Networking.sb:585
>> +(allow mach-lookup (with report) (with telemetry)
> 
> These changes are not mentioned in the change log. Accidentally included in this patch?

I should have mentioned them. I wanted to capture telemetry on a few things we are not sure we need.
Comment 5 Brent Fulgham 2020-01-06 16:46:32 PST
The Mac-debug-wk1 test failures cannot be due to this change, since the Sandbox is not used in WK1 builds at all.
Comment 6 Brent Fulgham 2020-01-06 16:48:48 PST
Created attachment 386913 [details]
Patch
Comment 7 Brent Fulgham 2020-01-07 10:21:47 PST
The API test failures cannot be related, because the iOS Simulator does not use or honor sandbox rules. We do not run API tests on device (at least in EWS).
Comment 8 Brent Fulgham 2020-01-07 12:28:05 PST
Created attachment 387018 [details]
Patch
Comment 9 Per Arne Vollan 2020-01-07 13:15:47 PST
Comment on attachment 387018 [details]
Patch

R=me.
Comment 10 WebKit Commit Bot 2020-01-07 16:38:17 PST
Comment on attachment 387018 [details]
Patch

Clearing flags on attachment: 387018

Committed r254174: <https://trac.webkit.org/changeset/254174>
Comment 11 WebKit Commit Bot 2020-01-07 16:38:19 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Truitt Savell 2020-01-08 09:04:43 PST
It looks like the changes in https://trac.webkit.org/changeset/254174/webkit

broke 80 tests on Catalina wk2.

Tracking in https://bugs.webkit.org/show_bug.cgi?id=205932
Comment 13 Brent Fulgham 2020-01-08 09:09:37 PST
(In reply to Truitt Savell from comment #12)
> It looks like the changes in https://trac.webkit.org/changeset/254174/webkit
> 
> broke 80 tests on Catalina wk2.
> 
> Tracking in https://bugs.webkit.org/show_bug.cgi?id=205932

Truitt, can you roll it out while I investigate? These failures make it look like webrtc requires graphics features in the network process, which is unexpected!
Comment 14 Truitt Savell 2020-01-08 09:13:44 PST
Reverted r254174 for reason:

Broke 80 tests on Catalina

Committed r254204: <https://trac.webkit.org/changeset/254204>
Comment 15 Brent Fulgham 2020-01-08 09:36:47 PST
Oh, I'm wrong! This is about sysctl-read values that were allowed by the global access call, but were NOT blocked by the global blocking declaration.

Sandbox: com.apple.WebKit(35422) deny(1) sysctl-read net.routetable.0.0.3.0

I'll see if any others come up.
Comment 16 Brent Fulgham 2020-01-08 11:36:41 PST
Committed r254209: <https://trac.webkit.org/changeset/254209>