Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2474903002:
Part 3.1: Is policy list subsumed under subsuming policy? (Closed)
DescriptionPart 3.1: Is policy list subsumed under subsuming policy?
This is part of an experimental feature Embedding-CSP.
This patch introduces CSPDirectiveList subsumption.
We aggregate SourceListDirectives based on their directive
names (or default directives otherwise) to check if it subsumes
under the embedding csp.
Note: There is only one SourceListDirective for each directive
name type specified by Embedding CSP since it specifies only one
policy.
+ We only consider scheme source /host source expressions to this
point.
Separate patches will be devoted to consideration of
'unsafe-inline' etc.
BUG=647588
Committed: https://crrev.com/0dc68f9f6acad85dd6ffcd2de5332574f886d061
Cr-Commit-Position: refs/heads/master@{#434667}
Patch Set 1 : Comments #
Total comments: 8
Patch Set 2 : More tests + simplyfying getSourceList #
Total comments: 4
Patch Set 3 : Simplyfying getSourceVector #
Total comments: 2
Patch Set 4 : Rebasing on enums changes #
Total comments: 13
Patch Set 5 : OperativeDirective test + other changes based on reviews #
Total comments: 6
Patch Set 6 : Adding one more check on defaulting in tests #
Total comments: 1
Patch Set 7 : Rebasing #
Messages
Total messages: 40 (22 generated)
Description was changed from ========== This is part of an experimental feature Embedding-CSP. In particular this is the second CL devoted to implementing *3.3. Is policy list subsumed under subsuming policy? This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. BUG=647588 ========== to ========== This is part of an experimental feature Embedding-CSP. In particular this is the second CL devoted to implementing *3.3. Is policy list subsumed under subsuming policy? This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. + We only consider scheme source /host source expressions to this point. Separate patches will be devoted to consideration of 'unsafe-inline' etc. BUG=647588 ==========
Description was changed from ========== This is part of an experimental feature Embedding-CSP. In particular this is the second CL devoted to implementing *3.3. Is policy list subsumed under subsuming policy? This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. + We only consider scheme source /host source expressions to this point. Separate patches will be devoted to consideration of 'unsafe-inline' etc. BUG=647588 ========== to ========== Part 3.1: Is policy list subsumed under subsuming policy? This is part of an experimental feature Embedding-CSP. In particular this is the second CL devoted to implementing *3.3. Is policy list subsumed under subsuming policy? This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. + We only consider scheme source /host source expressions to this point. Separate patches will be devoted to consideration of 'unsafe-inline' etc. BUG=647588 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Patchset #2 (id:100001) has been deleted
Patchset #1 (id:80001) has been deleted
Description was changed from ========== Part 3.1: Is policy list subsumed under subsuming policy? This is part of an experimental feature Embedding-CSP. In particular this is the second CL devoted to implementing *3.3. Is policy list subsumed under subsuming policy? This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. + We only consider scheme source /host source expressions to this point. Separate patches will be devoted to consideration of 'unsafe-inline' etc. BUG=647588 ========== to ========== Part 3.1: Is policy list subsumed under subsuming policy? This is part of an experimental feature Embedding-CSP. This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. + We only consider scheme source /host source expressions to this point. Separate patches will be devoted to consideration of 'unsafe-inline' etc. BUG=647588 ==========
amalika@google.com changed reviewers: + jochen@chromium.org, mkwst@chromium.org
Next level: CSPDirectiveList. Point of consideration: how to decrease the `getSourceList` method?
A few quick comments, mostly about the tests. Sorry. Buried today. https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1149: if (name == ContentSecurityPolicy::ScriptSrc) { Perhaps you could put the loop on the outside of all these `if`s? That is: ``` for (const auto& policy : policies) { if (name == ContentSecurityPolicy::FrameSrc) { } else if (...) { } } ``` https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1155: } This could be compressed to something like: ``` if (SourceListDirective* directive = operativeDirective(m_scriptSrc.get())) sourceListDirectives.append(*directive); ``` https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right): https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:443: // The lists, which are at least as restrictive as A, are subsumed. This test says that they're not subsumed (`false` all the way through)? Is it possible that you implemented `isSubsumedBy`? https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:487: ContentSecurityPolicyHeaderTypeReport); I think we'll have more confidence in the final result if we test against a wider variety of policies. For example, does an empty |A| subsume things? What about an |A| that only defines `script-src`? What about a directive that doesn't appear in the policy like `font-src`? What about keyword sources? What about '*'? What about 'none'? etc. https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:497: } I'd like to see more test coverage. You don't explicitly test `getSources` for example, and you don't test any all of the cascading behaviors (e.g. `frame-src` -> `child-src` -> `default-src` vs. `script-src` -> `default-src` vs. `frame-ancestors`).
Updated! Also, should csp3 directives be added such as `manifest-src` and `worker-src`? The former is currently considered in this patch already. https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1149: if (name == ContentSecurityPolicy::ScriptSrc) { On 2016/11/14 at 14:21:23, Mike West wrote: > Perhaps you could put the loop on the outside of all these `if`s? That is: > > ``` > for (const auto& policy : policies) { > if (name == ContentSecurityPolicy::FrameSrc) { > > } else if (...) { > > } > } > ``` I thought it would be better to first get into the right branch based on the name and then knowing that iterate over policies because otherwise, it would be less efficient to check name on each iteration. But let me know if it would be better to put the loop outside for readability purposes? https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right): https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:487: ContentSecurityPolicyHeaderTypeReport); On 2016/11/14 at 14:21:23, Mike West wrote: > I think we'll have more confidence in the final result if we test against a wider variety of policies. For example, does an empty |A| subsume things? What about an |A| that only defines `script-src`? What about a directive that doesn't appear in the policy like `font-src`? What about keyword sources? What about '*'? What about 'none'? etc. Since we did not implement keywords yet, I did not add any tests but made sure to name this one `SubsumesBasedOnCSPSourcesOnly`. It would be then important to check policies with intermix of keywords + CSPSources. But I added reversed testing: to check if the first policy from listB subsumes A and GetSourceList test.
I think this is looking good, but I'd like the tests to be clearer. I'll think a little over lunch about how we might achieve that. Comments inline while you're waiting. https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1149: if (name == ContentSecurityPolicy::ScriptSrc) { On 2016/11/15 at 13:17:18, amalika wrote: > I thought it would be better to first get into the right branch based on the name and then knowing that iterate over policies because otherwise, it would be less efficient to check name on each iteration. We're expecting sites to have some small number of policies; I don't think the additional work would amount to much. > But let me know if it would be better to put the loop outside for readability purposes? It seems to me to be easier to read a single `for` loop wrapping a big `if` than to read a big `if` wrapping many `for` loops. I won't insist on it if you prefer the current structure, but I think I'd prefer that way of doing things. https://codereview.chromium.org/2474903002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1149: if (name == ContentSecurityPolicy::ScriptSrc) { Another way of approaching this would be to refactor all these strings into an enum, and create mapping methods from the string "connect-src" to the enum `ContentSecurityPolicy::DirectiveType::ConnectSrc` and back. That would let you turn this into a big `switch` (and would ensure that we keep it updated when new directives are added). https://codereview.chromium.org/2474903002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1250: ContentSecurityPolicy::ScriptSrc, ContentSecurityPolicy::StyleSrc, You'll want to add 'WorkerSrc', as you noted. https://codereview.chromium.org/2474903002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right): https://codereview.chromium.org/2474903002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:440: const std::vector<const char*> policies; If you move the |listA| definition above this struct, and rename |policies| to |listB|, it might be a little clearer what you're up to. https://codereview.chromium.org/2474903002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:442: bool expectedFirstPolicyOpposite; This is a strange name. :)
Patchset #3 (id:160001) has been deleted
This patch is still independent of changes in "Embedding-CSP: Refactoring directive strings into enum" https://codereview.chromium.org/2516383002, in case, that refactoring is not worth committing. I simplified `getSourceVector` with a number of if/else statements by defining `getSourceListDirective` which given a name just returns an appropriate SourceListDirective in that policy. For example, given "script-src", it would return m_scriptSrc of that policy. If however, we decide to go with enums instead of const char* for directives, we won't need to white list directives (right now it is line 1203 in the `subsumes` method) because `getSourceListDirective` would return nullptr if the directive is not present. So instead, we could just iterate over all values in enum. https://codereview.chromium.org/2474903002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1190: } else if (String(name).endsWith("src")) { Another approach would be to define an enum (such as `DefaultBehavior` enum in tests), a method that would map directives to their default behaviour and then have a switch here instead.
I think an enum is the right way to go. One comment other than that. https://codereview.chromium.org/2474903002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1190: } else if (String(name).endsWith("src")) { On 2016/11/21 at 15:57:06, amalika wrote: > Another approach would be to define an enum (such as `DefaultBehavior` enum in tests), a method that would map directives to their default behaviour and then have a switch here instead. Or do the work in `getSourceListDirective` instead. That is: ``` if (name == whatever) return operativeDirective(m_whatever); if (name == whatever2) return operativeDirective(m_whatever2, operativeDirective(m_whatever3)); ```
Rebased on DirectiveType enum changes https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1167: SourceListDirective* CSPDirectiveList::operativeDirective( Not sure if we should add a case for `DefaultSrc` here or just return nullptr since we don't use `default-src` for subsumption? I probably should have added it since it might be expected based on the name that actual `default-src` is returned and we whitelist directives in `subsumes` method anyways?
Thanks! https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1167: SourceListDirective* CSPDirectiveList::operativeDirective( On 2016/11/23 at 13:57:11, amalika wrote: > Not sure if we should add a case for `DefaultSrc` here or just return nullptr since we don't use `default-src` for subsumption? > > I probably should have added it since it might be expected based on the name that actual `default-src` is returned and we whitelist directives in `subsumes` method anyways? `default-src`'s operative directive is `default-src`. I agree that you're not going to use it for subsumption, but someone's going to use it for something someday. :) https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1198: case ContentSecurityPolicy::DirectiveType::WorkerSrc: 1. `worker-src` is currently defined as sitting on top of `child-src`, just like `frame-src`. 2. We're probably going to change it to sit on top of `script-src`. Would you mind adding a `TODO(mkwst): Reevaluate this` comment? https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1211: CSPDirectiveListVector policies) { `const CSPDirectiveListVector&`? https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1221: bool CSPDirectiveList::subsumes(CSPDirectiveListVector other) { `const CSPDirectiveListVector&`? https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1237: // Navigation Directives I don't think these comments add much. You're not doing anything differently for the different groups, so you might as well lump them together with a comment about "These are the source-list directives." or something, linking to https://w3c.github.io/webappsec-csp/#framework-directive-source-list. https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h (right): https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:276: SourceListDirective* operativeDirective( Please add unit tests, especially for the cascade behavior. https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:278: static SourceListDirectiveVector getSourceVector( Can you add a comment explaining what these functions do?
Patchset #5 (id:220001) has been deleted
https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right): https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1167: SourceListDirective* CSPDirectiveList::operativeDirective( On 2016/11/24 at 13:07:46, Mike West (slow) wrote: > On 2016/11/23 at 13:57:11, amalika wrote: > > Not sure if we should add a case for `DefaultSrc` here or just return nullptr since we don't use `default-src` for subsumption? > > > > I probably should have added it since it might be expected based on the name that actual `default-src` is returned and we whitelist directives in `subsumes` method anyways? > > `default-src`'s operative directive is `default-src`. I agree that you're not going to use it for subsumption, but someone's going to use it for something someday. :) Updated! https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1198: case ContentSecurityPolicy::DirectiveType::WorkerSrc: On 2016/11/24 at 13:07:46, Mike West (slow) wrote: > 1. `worker-src` is currently defined as sitting on top of `child-src`, just like `frame-src`. > > 2. We're probably going to change it to sit on top of `script-src`. Would you mind adding a `TODO(mkwst): Reevaluate this` comment? Addressed. https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:1237: // Navigation Directives On 2016/11/24 at 13:07:46, Mike West (slow) wrote: > I don't think these comments add much. You're not doing anything differently for the different groups, so you might as well lump them together with a comment about "These are the source-list directives." or something, linking to https://w3c.github.io/webappsec-csp/#framework-directive-source-list. Changed! https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h (right): https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:276: SourceListDirective* operativeDirective( On 2016/11/24 at 13:07:46, Mike West (slow) wrote: > Please add unit tests, especially for the cascade behavior. Added! https://codereview.chromium.org/2474903002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:278: static SourceListDirectiveVector getSourceVector( On 2016/11/24 at 13:07:46, Mike West (slow) wrote: > Can you add a comment explaining what these functions do? Added!
LGTM % nits and small comments. Thanks! https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h (right): https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:277: // Tthis function returns a SourceListDirective of that type Nit: Tt. Nit: "of a given type". https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:281: // This function aggregates from a vector of policies all operative Nit: Newline. https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right): https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:594: EXPECT_EQ(sources[0]->m_host, "child-src.com"); Can you verify that these fall back to `default-src` if `child-src` is not present?
https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h (right): https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:277: // Tthis function returns a SourceListDirective of that type On 2016/11/24 at 14:37:37, Mike West (slow) wrote: > Nit: Tt. > > Nit: "of a given type". Updated https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.h:281: // This function aggregates from a vector of policies all operative On 2016/11/24 at 14:37:37, Mike West (slow) wrote: > Nit: Newline. Added https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right): https://codereview.chromium.org/2474903002/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:594: EXPECT_EQ(sources[0]->m_host, "child-src.com"); On 2016/11/24 at 14:37:37, Mike West (slow) wrote: > Can you verify that these fall back to `default-src` if `child-src` is not present? Added https://codereview.chromium.org/2474903002/diff/260001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right): https://codereview.chromium.org/2474903002/diff/260001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:585: CSPDirectiveList* allExceptChildSrcAndThisList = to check that frame-src and worker-src default to defaut-src if there is no child-src
Land it. :)
The CQ bit was checked by amalika@google.com
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/2474903002/#ps260001 (title: "Adding one more check on defaulting in tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/blimp_linux_dbg...)
Looks like this needs a rebase.
The CQ bit was checked by amalika@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by amalika@google.com
The CQ bit was checked by amalika@google.com
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/2474903002/#ps280001 (title: "Rebasing")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 280001, "attempt_start_ts": 1480346944377600,
"parent_rev": "42bb79e3b758613f3b1751b85a1f14599bfc54a0", "commit_rev":
"1dd6a8600bdff18297933f6cc99a8c84d54fcf99"}
Message was sent while issue was closed.
Committed patchset #7 (id:280001)
Message was sent while issue was closed.
Description was changed from ========== Part 3.1: Is policy list subsumed under subsuming policy? This is part of an experimental feature Embedding-CSP. This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. + We only consider scheme source /host source expressions to this point. Separate patches will be devoted to consideration of 'unsafe-inline' etc. BUG=647588 ========== to ========== Part 3.1: Is policy list subsumed under subsuming policy? This is part of an experimental feature Embedding-CSP. This patch introduces CSPDirectiveList subsumption. We aggregate SourceListDirectives based on their directive names (or default directives otherwise) to check if it subsumes under the embedding csp. Note: There is only one SourceListDirective for each directive name type specified by Embedding CSP since it specifies only one policy. + We only consider scheme source /host source expressions to this point. Separate patches will be devoted to consideration of 'unsafe-inline' etc. BUG=647588 Committed: https://crrev.com/0dc68f9f6acad85dd6ffcd2de5332574f886d061 Cr-Commit-Position: refs/heads/master@{#434667} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/0dc68f9f6acad85dd6ffcd2de5332574f886d061 Cr-Commit-Position: refs/heads/master@{#434667} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
