Bug 231751 - CSP: Implement src-elem and src-attr directives
Summary: CSP: Implement src-elem and src-attr directives
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: 2021-10-14 10:58 PDT by Kate Cheney
Modified: 2021-10-15 10:39 PDT (History)
3 users (show)

See Also:


Attachments
Patch (140.89 KB, patch)
2021-10-14 10:59 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch (151.51 KB, patch)
2021-10-14 14:06 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kate Cheney 2021-10-14 10:58:10 PDT
CSP: Implement src-elem and src-attr directives
Comment 1 Kate Cheney 2021-10-14 10:59:34 PDT
Created attachment 441240 [details]
Patch
Comment 2 Brent Fulgham 2021-10-14 11:43:53 PDT
Comment on attachment 441240 [details]
Patch

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

> Source/WebCore/page/csp/ContentSecurityPolicyDirective.cpp:33
> +ContentSecurityPolicyDirective::~ContentSecurityPolicyDirective()

Do we need this dummy implementation if the declaration in the header is for a pure virtual? Or maybe (if this is the only destructor you really need) the declaration should just be for a virtual method (not pure virtual).
Comment 3 Brent Fulgham 2021-10-14 11:45:55 PDT
Comment on attachment 441240 [details]
Patch

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

r=me

> LayoutTests/TestExpectations:-911
> -imported/w3c/web-platform-tests/content-security-policy/style-src-attr-elem/ [ Skip ]

Nice!

> LayoutTests/http/tests/security/contentSecurityPolicy/report-uri-scheme-relative-expected.txt:9
> +{"csp-report":{"document-uri":"http://127.0.0.1:8000/security/contentSecurityPolicy/report-uri-scheme-relative.py","referrer":"","violated-directive":"script-src-elem","effective-directive":"script-src","original-policy":"script-src 'self'; report-uri //127.0.0.1:8080/security/contentSecurityPolicy/resources/save-report.py","blocked-uri":"inline","status-code":200}}

I wonder if we could consider removing any of these WebKit-specific tests now that we pass the official WPT tests?

> LayoutTests/imported/w3c/web-platform-tests/content-security-policy/script-src-attr-elem/script-src-attr-allowed-src-blocked-expected.txt:3
> +PASS Should not fire a security policy violation event

So good to see these timeouts go away. This will help speed up our WPT runs, too.
Comment 4 Kate Cheney 2021-10-14 12:06:47 PDT
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 441240 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=441240&action=review
> 
> r=me

Looks like there are more newly passing tests that I missed. I will update those expectations and wait on EWS before landing.

> 
> > LayoutTests/TestExpectations:-911
> > -imported/w3c/web-platform-tests/content-security-policy/style-src-attr-elem/ [ Skip ]
> 
> Nice!
> 
> > LayoutTests/http/tests/security/contentSecurityPolicy/report-uri-scheme-relative-expected.txt:9
> > +{"csp-report":{"document-uri":"http://127.0.0.1:8000/security/contentSecurityPolicy/report-uri-scheme-relative.py","referrer":"","violated-directive":"script-src-elem","effective-directive":"script-src","original-policy":"script-src 'self'; report-uri //127.0.0.1:8080/security/contentSecurityPolicy/resources/save-report.py","blocked-uri":"inline","status-code":200}}
> 
> I wonder if we could consider removing any of these WebKit-specific tests
> now that we pass the official WPT tests?
> 

I think maybe we can. In another patch I will go through and see if any of them are repeats and remove them.

> > LayoutTests/imported/w3c/web-platform-tests/content-security-policy/script-src-attr-elem/script-src-attr-allowed-src-blocked-expected.txt:3
> > +PASS Should not fire a security policy violation event
> 
> So good to see these timeouts go away. This will help speed up our WPT runs,
> too.
Comment 5 Kate Cheney 2021-10-14 14:06:04 PDT
Created attachment 441279 [details]
Patch
Comment 6 Kate Cheney 2021-10-15 09:16:49 PDT
Comment on attachment 441279 [details]
Patch

iOS failure is unrelated.
Comment 7 Kate Cheney 2021-10-15 09:53:12 PDT
rdar://83332874
Comment 8 EWS 2021-10-15 10:39:47 PDT
Committed r284254 (243063@main): <https://commits.webkit.org/243063@main>

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