Part 3.3: Is policy list subsumed under subsuming policy?

third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp

Index: third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp b/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
index 06f2e17e0903170f6234295edb44f5de445e0172..79cbfa20062664e1c59f98a8965f8009ef47324e 100644
--- a/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
@@ -579,6 +579,19 @@ bool SourceListDirective::hasSourceMatchInList(
   return false;
 }
 
+bool SourceListDirective::allowAllInline() {
+  const ContentSecurityPolicy::DirectiveType& type =
+      ContentSecurityPolicy::getDirectiveType(m_directiveName);
+  if (type != ContentSecurityPolicy::DirectiveType::DefaultSrc &&

[amalika, 2016/11/29 09:42:29]

Added default-src since otherwise unspecified script-src will be failing.

+      type != ContentSecurityPolicy::DirectiveType::StyleSrc &&
+      type != ContentSecurityPolicy::DirectiveType::ScriptSrc) {
+    return false;
+  }
+  return m_allowInline && !isHashOrNoncePresent() &&
+         (type != ContentSecurityPolicy::DirectiveType::ScriptSrc ||
+          !m_allowDynamic);
+}
+
 bool SourceListDirective::subsumes(
     HeapVector<Member<SourceListDirective>> other) {
   // TODO(amalika): Handle here special keywords.
@@ -592,8 +605,27 @@ bool SourceListDirective::subsumes(
   HeapVector<Member<CSPSource>> normalizedB = other[0]->m_list;
   if (other[0]->m_allowSelf)
     normalizedB.append(other[0]->m_policy->getSelfSource());
-  for (size_t i = 1; i < other.size(); i++)
+
+  bool allowInlineOther = other[0]->m_allowInline;

[Mike West, 2016/11/29 12:00:50]

Why do you need these three variables? Don't they boil down to calling
`allowAllInline()` on all the items?

[amalika, 2016/11/29 12:27:33]

To call `allowAllInline()` is a method on SourceListDirective, so we can only
call it on Embedding-CSP.
For returned CSP, we create these variables to keep track of `normalized`
values, i.e. normalized policy allows `unsafe-eval` only if all returned
policies allow it. Then we check `allowAllInlineOther` for returned CSP on lin2
623 here.

[Mike West, 2016/11/30 09:57:19]

Hrm. Ok. But you only need them to check `allowAllInlineOther`, right?

It seems like you could simplify this (maybe simpler?) with something like the
following:

```
for (const auto& directive : other) {
  if (!allowAllInline() && (directive->m_allowInline && !((type ==
ContentSecurityPolicy::DirectiveType::ScriptSrc &&directive->m_allowDynamic) ||
directive->isHashOrNoncePresent()))
    return false;

  ...
}
```

WDYT?

[amalika, 2016/11/30 10:06:06]

I dont think this would give an expected behavior because we need to find
`effective` allow*() values?
For example, if there are two policies in returned CSP:
Content-Security-Policy: script-src 'unsafe-inline' http://example.com
Content-Security-Policy: script-src 'strict-dynamic' http://example.com

Then the `effective` policy should be Content-Security-Policy: script-src
http://example.com
(is that correct?)

Then we can't judge allowAllInline() based on individual policies? Otherwise, we
would return `false` after the first policy.
So we have to iterate over all of them, saving if all of them allow*() that
attribute and then compare with the Embedding-CSP? 

+  bool allowDynamicOther = other[0]->m_allowDynamic;
+  bool isHashOrNoncePresentOther = other[0]->isHashOrNoncePresent();
+
+  for (size_t i = 1; i < other.size(); i++) {
+    allowInlineOther = allowInlineOther && other[i]->m_allowInline;
+    allowDynamicOther = allowDynamicOther && other[i]->m_allowDynamic;
+    isHashOrNoncePresentOther =
+        isHashOrNoncePresentOther && other[i]->isHashOrNoncePresent();
     normalizedB = other[i]->getIntersectCSPSources(normalizedB);
+  }
+
+  const ContentSecurityPolicy::DirectiveType type =
+      ContentSecurityPolicy::getDirectiveType(m_directiveName);
+  bool allowAllInlineOther =
+      allowInlineOther && !isHashOrNoncePresentOther &&
+      (type != ContentSecurityPolicy::DirectiveType::ScriptSrc ||
+       !allowDynamicOther);
+  if (!allowAllInline() && allowAllInlineOther)
+    return false;
 
   return CSPSource::firstSubsumesSecond(normalizedA, normalizedB);
 }