Part 3.7: Is policy list subsumed under subsuming policy?

Part 3.7: Is policy list subsumed under subsuming policy?

This is part of an experimental feature Embedding-CSP.
In this particular CL, we add support for a keyword source expression
`strict-dynamic`. Consideration points:
- We should not consider `self` or `unsafe-inline` when `strict-dynamic`
is present
- We should not consider host/scheme source expressions
- We honor nonce/hash source expressions.
- If B has `strict-dynamic`, then it must be that A has it too. Otherwise,
A does not subsume B.

BUG=647588
Committed: https://crrev.com/6c7269bdd0808219c799e9b60f8470618c6e7986
Cr-Commit-Position: refs/heads/master@{#437236}

[amalika, 2016-12-05 14:15:16]

amalika@google.com changed reviewers:
+ jochen@chromium.org, mkwst@chromium.org

[amalika, 2016-12-05 14:15:17]

https://codereview.chromium.org/2551873003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp (right):

https://codereview.chromium.org/2551873003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp:669: return
allowDynamicOther || !normalizedB.size();
By this point, we should have already checked `unsafe-eval`, `unsafe-inline`,
`unsafe-hash-attributes`, parsed `self`.

The only left is `strict-dynamic` keyword itself and CSPSources. It must also be
that `this` allows `strict-dynamic, so there are two cases:
- `other` also allows `strict-dynamic`. ---> Then CSPSources are ignored in both
cases. Return `true`.
- `other` does not allow `strict-dynamic`. ---> Then effective list of
CSPSources must be empty.

https://codereview.chromium.org/2551873003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp
(right):

https://codereview.chromium.org/2551873003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp:1149:
false},
`other` does not allow `strict-dynamic`. ---> Then effective list of CSPSources
must be empty.
It is not --> return `false`.

[amalika, 2016-12-05 14:23:53]

Patchset #1 (id:1) has been deleted

[Mike West, 2016-12-06 19:03:16]

LGTM!

https://codereview.chromium.org/2551873003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp (right):

https://codereview.chromium.org/2551873003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp:669: return
allowDynamicOther || !normalizedB.size();
On 2016/12/05 at 14:15:17, amalika wrote:
> By this point, we should have already checked `unsafe-eval`, `unsafe-inline`,
`unsafe-hash-attributes`, parsed `self`.
> 
> The only left is `strict-dynamic` keyword itself and CSPSources. It must also
be that `this` allows `strict-dynamic, so there are two cases:
> - `other` also allows `strict-dynamic`. ---> Then CSPSources are ignored in
both cases. Return `true`.
> - `other` does not allow `strict-dynamic`. ---> Then effective list of
CSPSources must be empty.

Can you add a short comment to that effect? I don't think this would have been
clear to me otherwise.

[amalika, 2016-12-07 12:02:08]

I thought that it might actually be easier to remove `allowSelf` to get cleaner
code.
It was originally there because we need to ignore `self` when `strict-dynamic`
is present.
However, since we parse `self` into a CSPSource anyways, and no longer consider
it as a keyword, we don't have to add additional checks (unless for very small
efficiency gains).

+ Added comments

[Mike West, 2016-12-08 08:26:45]

Ok. Still LGTM.

[amalika, 2016-12-08 11:51:02]

The CQ bit was checked by amalika@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-08 11:51:16]

Dry run: 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-12-08 13:50:31]

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

[commit-bot: I haz the power, 2016-12-08 13:50:32]

Dry run: This issue passed the CQ dry run.

[amalika, 2016-12-08 13:53:33]

The CQ bit was checked by amalika@google.com

[amalika, 2016-12-08 13:53:33]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2551873003/#ps80001
(title: "Rebasing")

[commit-bot: I haz the power, 2016-12-08 13:53:49]

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-12-08 13:57:40]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481205213445880,
"parent_rev": "20758e8f01a43139f08e0a8efb6aa0baff50f8fe", "commit_rev":
"ac91a239eb08837771c205e58ba3edd671078797"}

[commit-bot: I haz the power, 2016-12-08 13:58:05]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-12-08 14:00:07]

Description was changed from

==========
Part 3.7: Is policy list subsumed under subsuming policy?

This is part of an experimental feature Embedding-CSP.
In this particular CL, we add support for a keyword source expression
`strict-dynamic`. Consideration points:
- We should not consider `self` or `unsafe-inline` when `strict-dynamic`
is present
- We should not consider host/scheme source expressions
- We honor nonce/hash source expressions.
- If B has `strict-dynamic`, then it must be that A has it too. Otherwise,
A does not subsume B.

BUG=647688
==========

to

==========
Part 3.7: Is policy list subsumed under subsuming policy?

This is part of an experimental feature Embedding-CSP.
In this particular CL, we add support for a keyword source expression
`strict-dynamic`. Consideration points:
- We should not consider `self` or `unsafe-inline` when `strict-dynamic`
is present
- We should not consider host/scheme source expressions
- We honor nonce/hash source expressions.
- If B has `strict-dynamic`, then it must be that A has it too. Otherwise,
A does not subsume B.

BUG=647688

Committed: https://crrev.com/6c7269bdd0808219c799e9b60f8470618c6e7986
Cr-Commit-Position: refs/heads/master@{#437236}
==========

[commit-bot: I haz the power, 2016-12-08 14:00:08]

Patchset 4 (id:??) landed as
https://crrev.com/6c7269bdd0808219c799e9b60f8470618c6e7986
Cr-Commit-Position: refs/heads/master@{#437236}

[amalika, 2016-12-09 11:45:22]

Description was changed from

==========
Part 3.7: Is policy list subsumed under subsuming policy?

This is part of an experimental feature Embedding-CSP.
In this particular CL, we add support for a keyword source expression
`strict-dynamic`. Consideration points:
- We should not consider `self` or `unsafe-inline` when `strict-dynamic`
is present
- We should not consider host/scheme source expressions
- We honor nonce/hash source expressions.
- If B has `strict-dynamic`, then it must be that A has it too. Otherwise,
A does not subsume B.

BUG=647688

Committed: https://crrev.com/6c7269bdd0808219c799e9b60f8470618c6e7986
Cr-Commit-Position: refs/heads/master@{#437236}
==========

to

==========
Part 3.7: Is policy list subsumed under subsuming policy?

This is part of an experimental feature Embedding-CSP.
In this particular CL, we add support for a keyword source expression
`strict-dynamic`. Consideration points:
- We should not consider `self` or `unsafe-inline` when `strict-dynamic`
is present
- We should not consider host/scheme source expressions
- We honor nonce/hash source expressions.
- If B has `strict-dynamic`, then it must be that A has it too. Otherwise,
A does not subsume B.

BUG=647588

Committed: https://crrev.com/6c7269bdd0808219c799e9b60f8470618c6e7986
Cr-Commit-Position: refs/heads/master@{#437236}
==========