CSP: Fix 'strict-dynamic' with multiple policies.

CSP: Fix 'strict-dynamic' with multiple policies.

The checks we wrote for 'strict-dynamic' fail to allow dynamically-
injected scripts if more than one policy is present. This patch
addresses that by delegating the dynamic check to 'ContentSecurityPolicy'
(rather than bypassing CSP entirely from 'ScriptLoader'). Most of the
patch is just piping the "Was this parser-inserted?" bit from
'ScriptLoader::fetchScript' to 'CSPDirectiveList::allowScriptFromSource'.
to another.

BUG=653511
Committed: https://crrev.com/cbb622f22b8e8c42e509fa7e1229382ceb0b0b14
Cr-Commit-Position: refs/heads/master@{#423557}

[Mike West, 2016-10-06 13:03:49]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 13:04:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-10-06 13:07:33]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org

[Mike West, 2016-10-06 13:07:34]

This needs more tests, but WDYT about the direction?

[commit-bot: I haz the power, 2016-10-06 13:19:57]

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

[commit-bot: I haz the power, 2016-10-06 13:19:58]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mikispag, 2016-10-06 13:45:37]

On 2016/10/06 13:07:34, Mike West wrote:
> This needs more tests, but WDYT about the direction?

Please test it on https://poc.miki.it/strict-dynamic-tests/ .

[jochen (gone - plz use gerrit), 2016-10-06 14:00:08]

yeah, looks reasonable

https://codereview.chromium.org/2401573003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ResourceLoaderOptions.h (right):

https://codereview.chromium.org/2401573003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ResourceLoaderOptions.h:72: enum
ParserDisposition { ParserInserted, NotParserInserted };
why not an enum class?

[Mike West, 2016-10-06 14:37:24]

> On 2016/10/06 13:07:34, Mike West wrote:
> Please test it on https://poc.miki.it/strict-dynamic-tests/ .

Please land those as layout tests so we test them automatically. :)

[Mike West, 2016-10-06 14:45:36]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 14:45:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-10-06 14:47:15]

Ok, looks better now.

This passes the tests at https://poc.miki.it/strict-dynamic-tests/, with the
exception of
https://poc.miki.it/strict-dynamic-tests/strict-dynamic-double-enforcing-with...,
which I think is broken. I can either try to bring those into this patch
directly, or Miki can land them separately. I don't have a strong opinion either
way.

WDYT, Jochen?

https://codereview.chromium.org/2401573003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ResourceLoaderOptions.h (right):

https://codereview.chromium.org/2401573003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ResourceLoaderOptions.h:72: enum
ParserDisposition { ParserInserted, NotParserInserted };
On 2016/10/06 at 14:00:08, jochen wrote:
> why not an enum class?

Because nothing else in this file is an enum class?

I'll make it an enum class if you like, no big deal.

[jochen (gone - plz use gerrit), 2016-10-06 14:50:35]

lgtm

[Mike West, 2016-10-06 16:06:55]

The CQ bit was unchecked by mkwst@chromium.org

[Mike West, 2016-10-06 16:07:00]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-10-06 16:07:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 16:23:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-06 16:25:23]

Description was changed from

==========
CSP: Fix 'strict-dynamic' with multiple policies.

The checks we wrote for 'strict-dynamic' fail to allow dynamically-
injected scripts if more than one policy is present. This patch
addresses that by delegating the dynamic check to 'ContentSecurityPolicy'
(rather than bypassing CSP entirely from 'ScriptLoader'). Most of the
patch is just piping the "Was this parser-inserted?" bit from
'ScriptLoader::fetchScript' to 'CSPDirectiveList::allowScriptFromSource'.
to another.

BUG=653511
==========

to

==========
CSP: Fix 'strict-dynamic' with multiple policies.

The checks we wrote for 'strict-dynamic' fail to allow dynamically-
injected scripts if more than one policy is present. This patch
addresses that by delegating the dynamic check to 'ContentSecurityPolicy'
(rather than bypassing CSP entirely from 'ScriptLoader'). Most of the
patch is just piping the "Was this parser-inserted?" bit from
'ScriptLoader::fetchScript' to 'CSPDirectiveList::allowScriptFromSource'.
to another.

BUG=653511

Committed: https://crrev.com/cbb622f22b8e8c42e509fa7e1229382ceb0b0b14
Cr-Commit-Position: refs/heads/master@{#423557}
==========

[commit-bot: I haz the power, 2016-10-06 16:25:24]

Patchset 2 (id:??) landed as
https://crrev.com/cbb622f22b8e8c42e509fa7e1229382ceb0b0b14
Cr-Commit-Position: refs/heads/master@{#423557}