Add a scheme registry for bypassing Content Security Policy.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mike West]

In certain cases, we'd like to ensure that resources load regardless of a page's Content Security Policy. Extensions, for instance, should be able to inject their own resources into a page even if that page would otherwise block them.

The spec agrees, noting that "Enforcing a CSP policy should not interfere with the operation of user-supplied scripts such as third-party user-agent add-ons and JavaScript bookmarklets." (https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#processing-model)

See http://crbug.com/133223 for additional context.

[Mike West]

I've started on a patch which I'll upload in a moment. I'm not sure how to write a test for this functionality, however. I took a quick look at some other registries, but haven't found anything useful. Is this just a no-test zone, or can you point me in the right direction, Adam? :)

[Adam Barth]

The easiest thing is probably to add an API to Internals.idl for registering the scheme.  We haven't been that good about testing these things in the past because we didn't have Internals.idl.

[Mike West]

Got it(In reply to comment #2)
> The easiest thing is probably to add an API to Internals.idl for registering the scheme.  We haven't been that good about testing these things in the past because we didn't have Internals.idl.

Got it. That looks straightforward.

[Mike West]

Created attachment 148167 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adam Barth]

Comment on attachment 148167 [details]
Patch

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

> Source/WebCore/platform/SchemeRegistry.h:89
> +    static bool shouldTreatURLSchemeAsBypassingCSP(const String& scheme);

shouldTreatURLSchemeAsBypassingCSP <-- schemeShouldBypassContentSecurityPolicy ?

Generally, we don't like to use abbreviations because they can be hard for folks to decode when they're not experts, especially in a file like this that's not really related to CSP.

> Source/WebCore/testing/Internals.cpp:1151
> +    SchemeRegistry::registerURLSchemeAsBypassingCSP(scheme);

We need to unregister the scheme when the test is over so we don't affect later tests.

[Gustavo Noronha (kov)]

Comment on attachment 148167 [details]
Patch

Attachment 148167 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12972566

[Build Bot]

Comment on attachment 148167 [details]
Patch

Attachment 148167 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12979265

[Build Bot]

Comment on attachment 148167 [details]
Patch

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

[Mike West]

Hrm. Based on a bit of digging, it looks like I need to expose these new symbols so that the testing framework can use them. Is there an automated mechanism for generating the symbols into files like `Source/WebCore/WebCore.exp.in` and so on?

[Mike West]

(In reply to comment #10)
> Hrm. Based on a bit of digging, it looks like I need to expose these new symbols so that the testing framework can use them. Is there an automated mechanism for generating the symbols into files like `Source/WebCore/WebCore.exp.in` and so on?

I'm looking at https://bugs.webkit.org/attachment.cgi?id=147713&action=prettypatch btw.

[Adam Barth]

> Is there an automated mechanism for generating the symbols into files like `Source/WebCore/WebCore.exp.in` and so on?

Sadly, no.  I tend to look at the build errors and grab the symbols from there.

[Mike West]

Created attachment 148177 [details]
Crossing my fingers, but expecting to have to redo the symbols.

[Build Bot]

Comment on attachment 148177 [details]
Crossing my fingers, but expecting to have to redo the symbols.

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

[Gustavo Noronha (kov)]

Comment on attachment 148177 [details]
Crossing my fingers, but expecting to have to redo the symbols.

Attachment 148177 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12972582

[Mike West]

Created attachment 148183 [details]
Sigh.

[Gustavo Noronha (kov)]

Comment on attachment 148183 [details]
Sigh.

Attachment 148183 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12971759

[Build Bot]

Comment on attachment 148183 [details]
Sigh.

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

[Mike West]

Created attachment 148189 [details]
Crossing different fingers.

[Mike West]

Comment on attachment 148189 [details]
Crossing different fingers.

Alright. Procedural symbol stuff out of the way. Assuming that the rest of the bots are happy (and why wouldn't they be?! :) ), what do you think?

[Adam Barth]

Comment on attachment 148189 [details]
Crossing different fingers.

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

> LayoutTests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme.html:19
> +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'FAIL' : 'PASS')">
> +    <script>
> +        internals.registerURLSchemeAsBypassingContentSecurityPolicy('http');
> +    </script>
> +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'PASS' : 'FAIL')">
> +    <script>
> +        internals.removeURLSchemeRegisteredAsBypassingContentSecurityPolicy('http');
> +    </script>
> +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'FAIL' : 'PASS')">

The code looks good, but isn't this test racy?  I guess we only check CSP when kicking off the load and we kick off the load synchronously....  I guess the ordering between the onload events doesn't matter because only one of them actually happens.  So, it's not racy, but for subtle reasons.  Maybe that's ok.

[Mike West]

(In reply to comment #21)
> (From update of attachment 148189 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148189&action=review
> 
> > LayoutTests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme.html:19
> > +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'FAIL' : 'PASS')">
> > +    <script>
> > +        internals.registerURLSchemeAsBypassingContentSecurityPolicy('http');
> > +    </script>
> > +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'PASS' : 'FAIL')">
> > +    <script>
> > +        internals.removeURLSchemeRegisteredAsBypassingContentSecurityPolicy('http');
> > +    </script>
> > +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'FAIL' : 'PASS')">
> 
> The code looks good, but isn't this test racy?  I guess we only check CSP when kicking off the load and we kick off the load synchronously....  I guess the ordering between the onload events doesn't matter because only one of them actually happens.  So, it's not racy, but for subtle reasons.  Maybe that's ok.

I don't think it's racy, for the reasons you mentioned.

That said, I tried to rewrite the test to be explicitly ordered, but ran into an issue. Neither `onload

[Mike West]

(In reply to comment #21)
> (From update of attachment 148189 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148189&action=review
> 
> > LayoutTests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme.html:19
> > +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'FAIL' : 'PASS')">
> > +    <script>
> > +        internals.registerURLSchemeAsBypassingContentSecurityPolicy('http');
> > +    </script>
> > +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'PASS' : 'FAIL')">
> > +    <script>
> > +        internals.removeURLSchemeRegisteredAsBypassingContentSecurityPolicy('http');
> > +    </script>
> > +    <img src="../resources/abe.png" onload="alert(this.width == 76 ? 'FAIL' : 'PASS')">
> 
> The code looks good, but isn't this test racy?  I guess we only check CSP when kicking off the load and we kick off the load synchronously....  I guess the ordering between the onload events doesn't matter because only one of them actually happens.  So, it's not racy, but for subtle reasons.  Maybe that's ok.

I don't think it's racy, for the reasons you mentioned.

That said, I tried to rewrite the test to be explicitly ordered, but ran into an issue. Neither `onload` nor `onerror` are fired for blocked images. A console message is generated, but that seems to be it. Is there another event I should be listening for?

[Mike West]

Created attachment 148249 [details]
Trying to order the test.

[Mike West]

(In reply to comment #24)
> Created an attachment (id=148249) [details]
> Trying to order the test.

The attached patch is the closest I could get. Neither the try/catch nor the `onerror` events are firing. :/

[Adam Barth]

That's another bug, by the way.  We should be firing the onerror event for things that CSP blocks.  :)

[Mike West]

(In reply to comment #26)
> That's another bug, by the way.  We should be firing the onerror event for things that CSP blocks.  :)

Filed as https://bugs.webkit.org/show_bug.cgi?id=89440. Thanks!

[WebKit Review Bot]

Comment on attachment 148249 [details]
Trying to order the test.

Clearing flags on attachment: 148249

Committed r120684: <http://trac.webkit.org/changeset/120684>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.