Bug 186788 - Provide a way for Injected Bundles to indicate classes approved for secure encoding/decoding
Summary: Provide a way for Injected Bundles to indicate classes approved for secure en...
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: 189709
  Show dependency treegraph
 
Reported: 2018-06-18 14:06 PDT by Brent Fulgham
Modified: 2018-09-18 13:43 PDT (History)
9 users (show)

See Also:


Attachments
Patch (26.06 KB, patch)
2018-06-19 15:52 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch v2 (Fix 32-bit build) (26.04 KB, patch)
2018-06-20 08:20 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (26.07 KB, patch)
2018-06-20 20:09 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (26.11 KB, patch)
2018-06-20 21:08 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (26.15 KB, patch)
2018-06-20 22:19 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (26.19 KB, patch)
2018-06-21 09:33 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (2.74 MB, application/zip)
2018-06-21 10:44 PDT, Build Bot
no flags Details
Patch (26.19 KB, patch)
2018-06-21 10:50 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.29 MB, application/zip)
2018-06-21 11:41 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews202 for win-future (12.86 MB, application/zip)
2018-06-21 12:33 PDT, Build Bot
no flags Details
Patch (26.21 KB, patch)
2018-06-21 13:50 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.84 MB, application/zip)
2018-06-21 15:12 PDT, Build Bot
no flags Details
Patch for landing (28.88 KB, patch)
2018-06-26 10:37 PDT, 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 2018-06-18 14:06:14 PDT
We want to use NSSecureCoding for data passed over IPC. However, WebKit clients using the Injected Bundle mechanism sometimes want to serialize parameter values that are unique to the WebKit client. WebKit doesn't know anything about these classes, and so cannot enable them for NSSecureCoding.

This patch adds a new WKBundle method, WKBundleExtendClassesForParameterCoder, WKBundleExtendClassesForParameterCoder, which is used to define additional classes that the coder/decoder should permit.
Comment 1 Brent Fulgham 2018-06-19 15:41:58 PDT
<rdar://problem/41094167>
Comment 2 Brent Fulgham 2018-06-19 15:52:05 PDT
Created attachment 343117 [details]
Patch
Comment 3 Brent Fulgham 2018-06-20 08:20:07 PDT
Created attachment 343157 [details]
Patch v2 (Fix 32-bit build)
Comment 4 Brent Fulgham 2018-06-20 20:09:10 PDT
Created attachment 343207 [details]
Patch
Comment 5 Brent Fulgham 2018-06-20 21:08:39 PDT
Created attachment 343209 [details]
Patch
Comment 6 Brent Fulgham 2018-06-20 22:19:31 PDT
Created attachment 343214 [details]
Patch
Comment 7 Brent Fulgham 2018-06-21 09:33:00 PDT
Created attachment 343241 [details]
Patch
Comment 8 Build Bot 2018-06-21 10:44:13 PDT
Comment on attachment 343241 [details]
Patch

Attachment 343241 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8278062

New failing tests:
accessibility/mac/selection-notification-focus-change.html
Comment 9 Build Bot 2018-06-21 10:44:14 PDT
Created attachment 343245 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 10 Brent Fulgham 2018-06-21 10:50:34 PDT
Created attachment 343246 [details]
Patch
Comment 11 Build Bot 2018-06-21 11:41:15 PDT
Comment on attachment 343246 [details]
Patch

Attachment 343246 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8278944

New failing tests:
accessibility/smart-invert-reference.html
imported/w3c/web-platform-tests/streams/piping/error-propagation-forward.html
Comment 12 Build Bot 2018-06-21 11:41:17 PDT
Created attachment 343255 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 13 Build Bot 2018-06-21 12:33:47 PDT
Comment on attachment 343246 [details]
Patch

Attachment 343246 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8279639

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html
Comment 14 Build Bot 2018-06-21 12:33:59 PDT
Created attachment 343256 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 15 Brent Fulgham 2018-06-21 13:50:39 PDT
Created attachment 343271 [details]
Patch
Comment 16 Build Bot 2018-06-21 15:12:25 PDT
Comment on attachment 343271 [details]
Patch

Attachment 343271 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8281808

New failing tests:
http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm
accessibility/mac/selection-notification-focus-change.html
Comment 17 Build Bot 2018-06-21 15:12:27 PDT
Created attachment 343282 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 18 Brent Fulgham 2018-06-21 20:41:41 PDT
performance-api/performance-observer-no-document-leak.html was a flaky timeout, and doesn't seem related to this patch.
Comment 19 Chris Dumez 2018-06-25 08:56:46 PDT
Comment on attachment 343271 [details]
Patch

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

> Source/WebKit/WebProcess/InjectedBundle/API/c/mac/WKBundleMac.h:39
> +WK_EXPORT void WKBundleExtendClassesForParameterCoder(WKBundleRef bundle, WKArrayRef classes);

It would be good to confirm with Brady / Alex / Geoff if we need a corresponding Cocoa SPI for this, and where this SPI should be. I have been told no new C API without Cocoa equivalent.

> Source/WebKit/WebProcess/InjectedBundle/API/c/mac/WKBundleMac.mm:42
> +void WKBundleExtendClassesForParameterCoder(WKBundleRef bundle, WKArrayRef classes)

I think we should return early if classes is null instead of crashing.
Comment 20 Brent Fulgham 2018-06-25 13:27:16 PDT
Comment on attachment 343271 [details]
Patch

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

>> Source/WebKit/WebProcess/InjectedBundle/API/c/mac/WKBundleMac.mm:42
>> +void WKBundleExtendClassesForParameterCoder(WKBundleRef bundle, WKArrayRef classes)
> 
> I think we should return early if classes is null instead of crashing.

OK -- I'll correct that.
Comment 21 Brent Fulgham 2018-06-26 10:37:08 PDT
Created attachment 343616 [details]
Patch for landing
Comment 22 WebKit Commit Bot 2018-06-26 11:17:04 PDT
Comment on attachment 343616 [details]
Patch for landing

Clearing flags on attachment: 343616

Committed r233207: <https://trac.webkit.org/changeset/233207>
Comment 23 WebKit Commit Bot 2018-06-26 11:17:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 Michael Catanzaro 2018-06-27 13:28:38 PDT
Hmm:

[608/829] Building CXX object Source/WebKit/CMakeFiles/WebKit.dir/WebProcess/InjectedBundle/API/c/WKBundle.cpp.o
/home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp:300:5: warning: "WK_API_ENABLED" is not defined, evaluates to 0 [-Wundef]
 #if WK_API_ENABLED
     ^~~~~~~~~~~~~~


I'm guessing this should be:

#if PLATFORM(COCOA) && WK_API_ENABLED
Comment 25 Michael Catanzaro 2018-06-30 11:44:40 PDT
Committed r233400: <https://trac.webkit.org/changeset/233400>