Bug 206906 - Limit access to 'com.apple.SecurityService' and 'com.apple.ocspd' to systems that require it
Summary: Limit access to 'com.apple.SecurityService' and 'com.apple.ocspd' to systems ...
Status: RESOLVED WONTFIX
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: 206832
Blocks:
  Show dependency tree / graph
 
Reported: 2020-01-28 14:16 PST by Brent Fulgham
Modified: 2020-01-29 10:16 PST (History)
4 users (show)

See Also:

Attachments
Patch (4.11 KB, patch)
2020-01-28 15:25 PST, Brent Fulgham 		no flags Details | Formatted Diff | Diff
Patch (4.14 KB, patch)
2020-01-28 18:24 PST, Brent Fulgham 		ap: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2020-01-28 14:16:08 PST 
Make use of the Sandbox parameter features to allow WebKit to hint to the sandbox that it is running on a version of macOS that can benefit from updated system components.
Comment 1 Radar WebKit Bug Importer 2020-01-28 15:21:29 PST 
<rdar://problem/58971886>
Comment 2 Brent Fulgham 2020-01-28 15:25:00 PST 
Created attachment 389077 [details]
Patch
Comment 3 Per Arne Vollan 2020-01-28 15:38:44 PST 
Comment on attachment 389077 [details]
Patch

R=me.
Comment 4 Alexey Proskuryakov 2020-01-28 18:05:21 PST 
Comment on attachment 389077 [details]
Patch

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

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:622
> +        if (osVersionParts.size() < 3 || osVersionParts[2].toInt() <= 3)

The logic looks wrong here. It seems exceeding unlikely that you are getting the right set of versions here. 

This means that 10.14.3 and 10.15.3 both need the parameter, but .4 updates do not.

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:623
> +            sandboxParameters.addParameter("_OS_NEEDS_EME_QUIRK", "YES");

We control all parameter names, what is the need for the leading underscore?
Comment 5 Brent Fulgham 2020-01-28 18:24:32 PST 
Created attachment 389100 [details]
Patch
Comment 6 Brent Fulgham 2020-01-28 18:33:39 PST 
Comment on attachment 389077 [details]
Patch

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

>> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:622
>> +        if (osVersionParts.size() < 3 || osVersionParts[2].toInt() <= 3)
> 
> The logic looks wrong here. It seems exceeding unlikely that you are getting the right set of versions here. 
> 
> This means that 10.14.3 and 10.15.3 both need the parameter, but .4 updates do not.

Doh!

>> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:623
>> +            sandboxParameters.addParameter("_OS_NEEDS_EME_QUIRK", "YES");
> 
> We control all parameter names, what is the need for the leading underscore?

Consistency with the other parameters that work like this. _OS_VERSION
Comment 7 Alexey Proskuryakov 2020-01-28 19:56:59 PST 
Comment on attachment 389100 [details]
Patch

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

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:621
> +    if (osVersionParts[0].toInt() == 10 && osVersionParts[1].toInt() <= 15) {

Can you put this into an #if block too, so that it’s easier to remove in the future?

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:674
> +(if (equal? (param "_OS_NEEDS_EME_QUIRK") "YES")

Did you check how this behaves on old OS versions? I vaguely remember that profile may fail to compile when an undefined param is used.
Comment 8 Brent Fulgham 2020-01-29 10:16:01 PST 
After talking with a few other people, we're going to do this slightly differently.