CSP: Permit exempting schemes only for certain policy areas.

CSP: Permit exempting schemes only for certain policy areas.

Only the image and style policy areas are included in this CL,
but the approach can be easily extended to other policy areas
if desired.

BUG=414849
TEST=http/tests/security/contentSecurityPolicy, blink_platform_unittests:SchemeRegistryTest.*

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185554

[jbroman, 2014-11-18 19:11:32]

jbroman@chromium.org changed reviewers:
+ mkwst@chromium.org, tsepez@chromium.org

[jbroman, 2014-11-18 19:11:32]

jbroman@chromium.org changed required reviewers:
+ mkwst@chromium.org

[jbroman, 2014-11-18 19:11:32]

As discussed on https://codereview.chromium.org/647853002/.

If this LGTY, I can modify that CL to only whitelist images and CSS (or whatever
set of policy areas we agree is reasonable) on chrome-resource://.

[Tom Sepez, 2014-11-18 19:35:54]

https://codereview.chromium.org/730203007/diff/40001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme-partial.html
(right):

https://codereview.chromium.org/730203007/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme-partial.html:40:
setTimeout(function() {
We may have to leave <link> uncovered rather than use setTimeout :(.

https://codereview.chromium.org/730203007/diff/40001/Source/platform/weborigi...
File Source/platform/weborigin/SchemeRegistry.cpp (right):

https://codereview.chromium.org/730203007/diff/40001/Source/platform/weborigi...
Source/platform/weborigin/SchemeRegistry.cpp:316: return
(~ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) == 0;
nit: I thought this would be:

(ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) !=0)

[Tom Sepez, 2014-11-18 19:46:35]

> (~ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) == 0;
> nit: I thought this would be:
> 
> (ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) !=0)
Nevermind.

[Tom Sepez, 2014-11-18 19:49:35]

https://codereview.chromium.org/730203007/diff/40001/Source/platform/weborigi...
File Source/platform/weborigin/SchemeRegistry.cpp (right):

https://codereview.chromium.org/730203007/diff/40001/Source/platform/weborigi...
Source/platform/weborigin/SchemeRegistry.cpp:311: if (scheme.isEmpty())
Should we assert policyAreas != 0?

[Tom Sepez, 2014-11-18 19:49:45]

On 2014/11/18 19:46:35, Tom Sepez wrote:
> > (~ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) == 0;
> > nit: I thought this would be:
> > 
> > (ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) !=0)
> Nevermind.

it would have to be (ContentSecurityPolicyBypassingSchemes().get(scheme) &
policyAreas) == policyAreas) which I think reads clearer.

[Tom Sepez, 2014-11-18 19:49:45]

On 2014/11/18 19:46:35, Tom Sepez wrote:
> > (~ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) == 0;
> > nit: I thought this would be:
> > 
> > (ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) !=0)
> Nevermind.

it would have to be (ContentSecurityPolicyBypassingSchemes().get(scheme) &
policyAreas) == policyAreas) which I think reads clearer.

[jbroman, 2014-11-18 20:02:04]

On 2014/11/18 19:49:45, Tom Sepez wrote:
> On 2014/11/18 19:46:35, Tom Sepez wrote:
> > > (~ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) == 0;
> > > nit: I thought this would be:
> > > 
> > > (ContentSecurityPolicyBypassingSchemes().get(scheme) & policyAreas) !=0)
> > Nevermind.
> 
> it would have to be (ContentSecurityPolicyBypassingSchemes().get(scheme) &
> policyAreas) == policyAreas) which I think reads clearer.

Done. I'd had that originally before I changed it to what you see here, and it
passes the unit tests. :)

https://codereview.chromium.org/730203007/diff/40001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme-partial.html
(right):

https://codereview.chromium.org/730203007/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme-partial.html:40:
setTimeout(function() {
On 2014/11/18 19:35:54, Tom Sepez wrote:
> We may have to leave <link> uncovered rather than use setTimeout :(.

A way to deflake this just occurred to me. It involves iframe[srcdoc], but works
and has no setTimeout. :)

https://codereview.chromium.org/730203007/diff/40001/Source/platform/weborigi...
File Source/platform/weborigin/SchemeRegistry.cpp (right):

https://codereview.chromium.org/730203007/diff/40001/Source/platform/weborigi...
Source/platform/weborigin/SchemeRegistry.cpp:311: if (scheme.isEmpty())
On 2014/11/18 19:49:35, Tom Sepez wrote:
> Should we assert policyAreas != 0?

Done.

[Tom Sepez, 2014-11-18 20:21:24]

LGTM.  Nice.

[Mike West, 2014-11-18 20:35:46]

I like it. LGTM % comment.

https://codereview.chromium.org/730203007/diff/80001/public/web/WebSecurityPo...
File public/web/WebSecurityPolicy.h (right):

https://codereview.chromium.org/730203007/diff/80001/public/web/WebSecurityPo...
public/web/WebSecurityPolicy.h:71: // This enum should be kept in sync with
Source/platform/weborigin/SchemeRegistry.h.
Can you add something to Source/web/AssertMatchingEnums.cpp to verify that this
is kept up to date?

[jbroman, 2014-11-18 20:53:35]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2014-11-18 20:53:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/730203007/100001

[commit-bot: I haz the power, 2014-11-18 21:11:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-18 21:11:49]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/3...)

[jbroman, 2014-11-18 22:08:16]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2014-11-18 22:09:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/730203007/100001

[commit-bot: I haz the power, 2014-11-18 23:29:48]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185554