Bug 179548 - WebContent sandbox should not include 'system.sb'
Summary: WebContent sandbox should not include 'system.sb'
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks: 179650 179656 179674
  Show dependency treegraph
 
Reported: 2017-11-10 14:28 PST by Brent Fulgham
Modified: 2017-11-14 10:39 PST (History)
6 users (show)

See Also:


Attachments
Patch (10.16 KB, patch)
2017-11-10 14:33 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (76.78 KB, application/zip)
2017-11-10 15:27 PST, Build Bot
no flags Details
Patch (12.84 KB, patch)
2017-11-11 16:27 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (77.25 KB, application/zip)
2017-11-11 17:13 PST, Build Bot
no flags Details
Patch (12.90 KB, patch)
2017-11-13 09:37 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (6.94 KB, patch)
2017-11-13 17:17 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.14 MB, application/zip)
2017-11-13 18:42 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2017-11-10 14:28:52 PST
To support further strengthening of the Safari sandbox, stop including the 'system.sb' sandbox, and instead place relevant rules from that sandbox profile in our WebContent sandbox.

This will allow us to tighten things down further than we could using the global sandbox.

<rdar://problem/35367154>
Comment 1 Brent Fulgham 2017-11-10 14:33:24 PST
Created attachment 326633 [details]
Patch
Comment 2 Build Bot 2017-11-10 15:27:16 PST
Comment on attachment 326633 [details]
Patch

Attachment 326633 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5182796

Number of test failures exceeded the failure limit.
Comment 3 Build Bot 2017-11-10 15:27:17 PST
Created attachment 326641 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 4 Brent Fulgham 2017-11-11 16:27:52 PST
Created attachment 326700 [details]
Patch
Comment 5 Build Bot 2017-11-11 17:13:33 PST
Comment on attachment 326700 [details]
Patch

Attachment 326700 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5195322

Number of test failures exceeded the failure limit.
Comment 6 Build Bot 2017-11-11 17:13:34 PST
Created attachment 326701 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 7 Brent Fulgham 2017-11-13 09:37:48 PST
Created attachment 326764 [details]
Patch
Comment 8 Darin Adler 2017-11-13 09:58:07 PST
Comment on attachment 326764 [details]
Patch

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

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:32
> +;;; Imported from system.sb

We should word this comment differently.

The section below starts out as a copy of system.sb, but eventually it will be different; that’s the point of copying it here rather than doing an import command. So the comment should be worded differently to be forward-looking.

Related: the term "imported" in the comment is unnecessarily slightly confusing since the directive is "import" and idea is that we copied the contents here and did not import it.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:217
>  (allow sysctl-read
>      (sysctl-name
> +        "hw.activecpu"
>          "hw.availcpu"
> -        "hw.ncpu"
> +        "hw.cputype"
> +        "hw.l2cachesize"
> +        "hw.logicalcpu_max"
> +        "hw.physicalcpu_max"
> +        "hw.machine"
> +        "hw.memsize"
>          "hw.model"
> +        "hw.ncpu"
> +        "hw.optional.avx1_0"
> +        "hw.optional.avx2_0"
> +        "hw.optional.sse4_2"
> +        "hw.optional.sse4_1"
> +        "hw.optional.sse3"
> +        "hw.optional.sse2"
> +        "hw.vectorunit"
> +        "kern.hostname"
> +        "kern.maxfilesperproc"
>          "kern.memorystatus_level"
> +        "kern.osrelease"
> +        "kern.ostype"
> +        "kern.osvariant_status"
> +        "kern.safeboot"
> +        "kern.version"
>          "vm.footprint_suspend"))

ChangeLog comment does not mention this change.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:229
> -    (iokit-property-regex #"^AAPL,(DisplayPipe|OpenCLdisabled|alias-policy|boot-display|display-alias|mux-switch-state|ndrv-dev|primary-display|slot-name)")
> +    (iokit-property-regex #"^AAPL,(DisplayPipe|OpenCLdisabled|IOGraphics_LER(|_RegTag_1|_RegTag_0|_Busy_2)|alias-policy|boot-display|display-alias|mux-switch-state|ndrv-dev|primary-display|slot-name)")

ChangeLog comment does not mention this change.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:375
>  (allow mach-lookup
> +    (xpc-service-name "com.apple.PerformanceAnalysis.animationperfd")
>      (xpc-service-name "com.apple.accessibility.mediaaccessibilityd")
>      (xpc-service-name "com.apple.audio.SandboxHelper")
>      (xpc-service-name "com.apple.coremedia.videodecoder")
>      (xpc-service-name "com.apple.coremedia.videoencoder")
>      (xpc-service-name-regex #"\.apple-extension-service$")
>      (xpc-service-name "com.apple.hiservices-xpcservice")
> +    (xpc-service-name "com.apple.ist.ds.appleconnect2.HelperService")
>      (xpc-service-name "com.apple.print.normalizerd")
> +    (xpc-service-name "com.apple.securityd.xpc")
> +    (xpc-service-name "com.apple.signpost.signpost-notificationd")
>  )

ChangeLog comment does not mention this change.
Comment 9 Brent Fulgham 2017-11-13 17:17:29 PST
Created attachment 326830 [details]
Patch
Comment 10 Build Bot 2017-11-13 18:42:19 PST
Comment on attachment 326830 [details]
Patch

Attachment 326830 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5220993

New failing tests:
http/tests/workers/service/service-worker-clear.html
Comment 11 Build Bot 2017-11-13 18:42:21 PST
Created attachment 326836 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6
Comment 12 Brent Fulgham 2017-11-13 18:50:44 PST
Comment on attachment 326830 [details]
Patch

This sandbox is not used for any iOS build, so the test failure is spurious.
Comment 13 WebKit Commit Bot 2017-11-13 18:58:39 PST
Comment on attachment 326830 [details]
Patch

Clearing flags on attachment: 326830

Committed r224799: <https://trac.webkit.org/changeset/224799>
Comment 14 WebKit Commit Bot 2017-11-13 18:58:41 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 Ryosuke Niwa 2017-11-13 23:17:30 PST
This caused WebContent process to crash at launch. See https://webkit.org/b/179656.