Part 3.3: Is policy list subsumed under subsuming policy?

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/frame/csp/SourceListDirective.h"
 
 #include "core/frame/csp/CSPSource.h"
 #include "core/frame/csp/ContentSecurityPolicy.h"
 #include "platform/network/ContentSecurityPolicyParsers.h"
 #include "platform/weborigin/KURL.h"
@@ skipping 561 matching lines @@
     const KURL& url,
     ResourceRequest::RedirectStatus redirectStatus) const {
   for (size_t i = 0; i < m_list.size(); ++i) {
     if (m_list[i]->matches(url, redirectStatus))
       return true;
   }
 
   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.
   if (!m_list.size() || !other.size())
     return !m_list.size();
 
   HeapVector<Member<CSPSource>> normalizedA = m_list;
   if (m_allowSelf)
     normalizedA.append(m_policy->getSelfSource());
 
   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);
 }
 
 HashMap<String, CSPSource*> SourceListDirective::getIntersectSchemesOnly(
     HeapVector<Member<CSPSource>> other) {
   HashMap<String, CSPSource*> schemesA;
   for (const auto& sourceA : m_list) {
     if (sourceA->isSchemeOnly())
       addSourceToMap(schemesA, sourceA);
@@ skipping 61 matching lines @@
   return normalized;
 }
 
 DEFINE_TRACE(SourceListDirective) {
   visitor->trace(m_policy);
   visitor->trace(m_list);
   CSPDirective::trace(visitor);
 }
 
 }  // namespace blink