Bug 172365 - Enable library validation on WebKit XPC processes
Summary: Enable library validation on WebKit XPC processes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-05-19 10:00 PDT by David Kilzer (:ddkilzer)
Modified: 2018-01-25 10:09 PST (History)
8 users (show)

See Also:


Attachments
Patch (6.24 KB, patch)
2017-05-19 10:00 PDT, David Kilzer (:ddkilzer)
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (535.71 KB, application/zip)
2017-05-19 11:41 PDT, Build Bot
no flags Details
Part 1: Web Content service (23.53 KB, patch)
2018-01-24 10:02 PST, mitz
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2017-05-19 10:00:28 PDT
Need the bug URL (OOPS!).
<rdar://problem/26470661>

Reviewed by NOBODY (OOPS!).

* Configurations/Base.xcconfig:
(WK_LIBRARY_VALIDATION_CODE_SIGN_FLAGS): Set a default value so
that it can be overridden by build commands.
* Configurations/BaseXPCService.xcconfig:
(OTHER_CODE_SIGN_FLAGS): Enable library validation for all SDKs
except for simulator SDKs, based on
WK_LIBRARY_VALIDATION_CODE_SIGN_FLAGS.
* Configurations/DebugRelease.xcconfig:
(OTHER_CODE_SIGN_FLAGS): Disable library validation for
engineering builds.
* Configurations/PluginService.32.xcconfig:
(OTHER_CODE_SIGN_FLAGS): Disable library validation for plugin
process since it loads third-party binaries.
* Configurations/PluginService.64.xcconfig: Ditto.
* Configurations/WebContentService.xcconfig:
(OTHER_CODE_SIGN_FLAGS): Remove since it's now defined in
BaseXPCService.xcconfig.
(WK_XPC_DOMAIN_EXTENSION_CODE_SIGN_FLAGS): Remove since it's no
longer needed.
(WK_XPC_DOMAIN_EXTENSION_CODE_SIGN_FLAGS_YES): Ditto.
---
 7 files changed, 39 insertions(+), 4 deletions(-)
Comment 1 David Kilzer (:ddkilzer) 2017-05-19 10:00:29 PDT
Created attachment 310669 [details]
Patch
Comment 2 David Kilzer (:ddkilzer) 2017-05-19 10:03:36 PDT
<rdar://problem/26470661>
Comment 3 Brent Fulgham 2017-05-19 10:21:09 PDT
Comment on attachment 310669 [details]
Patch

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

> Source/WebKit2/Configurations/Base.xcconfig:109
> +WK_LIBRARY_VALIDATION_CODE_SIGN_FLAGS = -o library;

Could this be gated on target release somehow? I wonder if we could enable it only for builds beyond some target OS to avoid breaking builds targeting older OS's?
Comment 4 Build Bot 2017-05-19 11:41:26 PDT
Comment on attachment 310669 [details]
Patch

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

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2017-05-19 11:41:27 PDT
Created attachment 310680 [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 6 mitz 2018-01-24 10:02:03 PST
Created attachment 332175 [details]
Part 1: Web Content service

This is a revised version of a patch I’ve tested at Apple. Posting to see if the revisions broke it for EWS.
Comment 7 Brent Fulgham 2018-01-24 10:10:24 PST
Comment on attachment 332175 [details]
Part 1: Web Content service

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

> Source/WebKit/UIProcess/mac/WebProcessProxyMac.mm:43
> +    static bool isSystemWebKit = [] {

should we use 'dispatch_once' here?
Comment 8 mitz 2018-01-24 10:12:14 PST
Comment on attachment 332175 [details]
Part 1: Web Content service

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

>> Source/WebKit/UIProcess/mac/WebProcessProxyMac.mm:43
>> +    static bool isSystemWebKit = [] {
> 
> should we use 'dispatch_once' here?

This function shouldn’t be getting called from arbitrary threads, so no.
Comment 9 David Kilzer (:ddkilzer) 2018-01-24 13:37:23 PST
Comment on attachment 332175 [details]
Part 1: Web Content service

r=me
Comment 10 WebKit Commit Bot 2018-01-24 15:30:29 PST
Comment on attachment 332175 [details]
Part 1: Web Content service

Clearing flags on attachment: 332175

Committed r227582: <https://trac.webkit.org/changeset/227582>
Comment 11 WebKit Commit Bot 2018-01-24 15:30:30 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 mitz 2018-01-24 15:39:19 PST
Reopening to track the remaining non-PlugIn services.
Comment 13 mitz 2018-01-25 10:09:57 PST
(In reply to mitz from comment #12)
> Reopening to track the remaining non-PlugIn services.

Actually, going to use bug 173424 for the remaining services.