Bug 238230 - Migrate manifest version content security policy filtering for extensions into WebKit
Summary: Migrate manifest version content security policy filtering for extensions int...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-22 15:33 PDT by Kate Cheney
Modified: 2022-03-30 18:49 PDT (History)
5 users (show)

See Also:

Attachments
WIP patch (77.66 KB, patch)
2022-03-22 15:58 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Patch (165.65 KB, patch)
2022-03-25 11:29 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Patch (163.95 KB, patch)
2022-03-28 11:37 PDT, Kate Cheney 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (163.93 KB, patch)
2022-03-28 12:43 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Patch (163.86 KB, patch)
2022-03-28 15:13 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Patch (163.86 KB, patch)
2022-03-28 17:10 PDT, Kate Cheney 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (164.73 KB, patch)
2022-03-29 07:37 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Patch for landing (164.75 KB, patch)
2022-03-30 09:13 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Show Obsolete (7) 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 Kate Cheney 2022-03-22 15:33:22 PDT 
Migrate manifest version content security policy filtering for extensions into WebKit
Comment 1 Kate Cheney 2022-03-22 15:58:43 PDT 
Created attachment 455443 [details]
WIP patch
Comment 2 Kate Cheney 2022-03-22 15:59:53 PDT 
rdar://60081492
Comment 3 Kate Cheney 2022-03-25 11:29:09 PDT 
Created attachment 455787 [details]
Patch
Comment 4 Kate Cheney 2022-03-28 11:37:12 PDT 
Created attachment 455932 [details]
Patch
Comment 5 Kate Cheney 2022-03-28 12:43:57 PDT 
Created attachment 455944 [details]
Patch
Comment 6 Kate Cheney 2022-03-28 15:13:25 PDT 
Created attachment 455959 [details]
Patch
Comment 7 Kate Cheney 2022-03-28 17:10:02 PDT 
Created attachment 455973 [details]
Patch
Comment 8 Kate Cheney 2022-03-29 07:37:47 PDT 
Created attachment 456023 [details]
Patch
Comment 9 pascoe@apple.com 2022-03-29 10:49:15 PDT 
Comment on attachment 456023 [details]
Patch

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

(not a reviewer, just informal comments)

> Source/WebCore/page/PageConfiguration.h:161
> +    WebCore::ContentSecurityPolicyModeForExtension contentSecurityPolicyModeForExtension { WebCore::ContentSecurityPolicyModeForExtension::None };

nit: no need for WebCore:: here

> Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:207
> +    hostIsPublicSuffix = isPublicSuffix(parsedSource.host.value.toString());

nit: may be able to use toStringWithoutCopying here

> Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:230
> +        if (!schemeIsInHttpFamily(parsedSource.scheme.toString()) || !SecurityOrigin::isLocalHostOrLoopbackIPAddress(parsedSource.host.value))

ditto
Comment 10 Timothy Hatcher 2022-03-29 11:17:10 PDT 
Comment on attachment 456023 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:1373
> +    return WebKit::toWKContentSecurityPolicyModeForExtension(_pageConfiguration->contentSecurityPolicyModeForExtension());

Can you drop "WebKit::" here?
Comment 11 Kate Cheney 2022-03-29 12:11:29 PDT 
Thanks for the review comments! I'll make all of these changes. Waiting on the final EWS bots before uploading a PFL.
Comment 12 Kate Cheney 2022-03-29 12:28:37 PDT 
(In reply to Timothy Hatcher from comment #10)
> Comment on attachment 456023 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456023&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:1373
> > +    return WebKit::toWKContentSecurityPolicyModeForExtension(_pageConfiguration->contentSecurityPolicyModeForExtension());
> 
> Can you drop "WebKit::" here?

This actually seems to be needed, I think because we don't have namespace declarations in ObjC files?
Comment 13 Kate Cheney 2022-03-30 09:13:08 PDT 
Created attachment 456129 [details]
Patch for landing
Comment 14 EWS 2022-03-30 18:49:35 PDT 
Committed r292134 (249041@main): <https://commits.webkit.org/249041@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456129 [details].