Replicate Content-Security-Policy into remote frame proxies.

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

 /*
  * Copyright (C) 2011 Google, Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 13 matching lines @@
  */
 
 #include "core/frame/csp/ContentSecurityPolicy.h"
 
 #include "bindings/core/v8/ScriptCallStack.h"
 #include "bindings/core/v8/ScriptController.h"
 #include "core/dom/DOMStringList.h"
 #include "core/dom/Document.h"
 #include "core/dom/SandboxFlags.h"
 #include "core/events/SecurityPolicyViolationEvent.h"
+#include "core/frame/FrameClient.h"
 #include "core/frame/LocalDOMWindow.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/UseCounter.h"
 #include "core/frame/csp/CSPDirectiveList.h"
 #include "core/frame/csp/CSPSource.h"
 #include "core/frame/csp/CSPSourceList.h"
 #include "core/frame/csp/MediaListDirective.h"
 #include "core/frame/csp/SourceListDirective.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "core/inspector/InspectorInstrumentation.h"
 #include "core/loader/DocumentLoader.h"
+#include "core/loader/FrameLoaderClient.h"
 #include "core/loader/PingLoader.h"
 #include "platform/JSONValues.h"
 #include "platform/ParsingUtilities.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/network/ContentSecurityPolicyParsers.h"
 #include "platform/network/ContentSecurityPolicyResponseHeaders.h"
 #include "platform/network/EncodedFormData.h"
 #include "platform/network/ResourceRequest.h"
 #include "platform/network/ResourceResponse.h"
 #include "platform/weborigin/KURL.h"
@@ skipping 95 matching lines @@
     , m_insecureRequestsPolicy(SecurityContext::InsecureRequestsDoNotUpgrade)
 {
 }
 
 void ContentSecurityPolicy::bindToExecutionContext(ExecutionContext* executionContext)
 {
     m_executionContext = executionContext;
     applyPolicySideEffectsToExecutionContext();
 }
 
+void ContentSecurityPolicy::bindToSecurityOrigin(const SecurityOrigin& securityOrigin)

[Mike West, 2016/05/11 06:21:04]

Nit: Now about naming this something like `setupSelf`? "Bind to security origin"
sounds like something different than what this method is actually doing.

[Łukasz Anforowicz, 2016/05/11 23:14:48]

Good point - this name wasn't quite right.  Done.

+{
+    // Ensure that 'self' processes correctly.
+    m_selfProtocol = securityOrigin.protocol();
+    m_selfSource = new CSPSource(this, m_selfProtocol, securityOrigin.host(), securityOrigin.port(), String(), CSPSource::NoWildcard, CSPSource::NoWildcard);
+}
+
 void ContentSecurityPolicy::applyPolicySideEffectsToExecutionContext()
 {
     ASSERT(m_executionContext);
-    ASSERT(getSecurityOrigin());
-    // Ensure that 'self' processes correctly.
-    m_selfProtocol = getSecurityOrigin()->protocol();
-    m_selfSource = new CSPSource(this, m_selfProtocol, getSecurityOrigin()->host(), getSecurityOrigin()->port(), String(), CSPSource::NoWildcard, CSPSource::NoWildcard);
+
+    SecurityOrigin* securityOrigin = m_executionContext->securityContext().getSecurityOrigin();
+    ASSERT(securityOrigin);
+    bindToSecurityOrigin(*securityOrigin);
 
     if (didSetReferrerPolicy())
         m_executionContext->setReferrerPolicy(m_referrerPolicy);
 
     // If we're in a Document, set mixed content checking and sandbox
     // flags, then dump all the parsing error messages, then poke at histograms.
     if (Document* document = this->document()) {
         if (m_sandboxMask != SandboxNone) {
             UseCounter::count(document, UseCounter::SandboxViaCSP);
             document->enforceSandboxFlags(m_sandboxMask);
         }
         if (m_enforceStrictMixedContentChecking)
             document->enforceStrictMixedContentChecking();
         if (m_treatAsPublicAddress)
             document->setAddressSpace(WebAddressSpacePublic);
         if (m_insecureRequestsPolicy == SecurityContext::InsecureRequestsUpgrade) {
             UseCounter::count(document, UseCounter::UpgradeInsecureRequestsEnabled);
             document->setInsecureRequestsPolicy(m_insecureRequestsPolicy);
-            if (!getSecurityOrigin()->host().isNull())
-                document->addInsecureNavigationUpgrade(getSecurityOrigin()->host().impl()->hash());
+            if (!securityOrigin->host().isNull())
+                document->addInsecureNavigationUpgrade(securityOrigin->host().impl()->hash());
         }
 
         for (const auto& consoleMessage : m_consoleMessages)
             m_executionContext->addConsoleMessage(consoleMessage);
         m_consoleMessages.clear();
 
         for (const auto& policy : m_policies)
             UseCounter::count(*document, getUseCounterType(policy->headerType()));
 
         if (allowDynamic())
@@ skipping 51 matching lines @@
 void ContentSecurityPolicy::didReceiveHeader(const String& header, ContentSecurityPolicyHeaderType type, ContentSecurityPolicyHeaderSource source)
 {
     addPolicyFromHeaderValue(header, type, source);
 
     // This might be called after we've been bound to an execution context. For example, a <meta>
     // element might be injected after page load.
     if (m_executionContext)
         applyPolicySideEffectsToExecutionContext();
 }
 
-void ContentSecurityPolicy::addPolicyFromHeaderValue(const String& header, ContentSecurityPolicyHeaderType type, ContentSecurityPolicyHeaderSource source)
+void ContentSecurityPolicy::replicateHeader(const String& header, ContentSecurityPolicyHeaderType type, ContentSecurityPolicyHeaderSource source)
+{
+    // Replication should only happen for remote frames.
+    ASSERT(!m_executionContext);

[Mike West, 2016/05/11 06:21:04]

We create CSP without an execution context for local frames as well (in
`DocumentLoader::responseReceived`). Eventually we'll be able to get rid of that
mechanism by moving everything to the browser, but until then it's not clear to
me what the assertion needs to be.

[Łukasz Anforowicz, 2016/05/11 23:14:48]

I've just removed the assert.

+
+    addPolicyFromHeaderValue(header, type, source, false);
+}
+
+void ContentSecurityPolicy::addPolicyFromHeaderValue(const String& header, ContentSecurityPolicyHeaderType type, ContentSecurityPolicyHeaderSource source, bool notifyFrameLoaderClient)
 {
     // If this is a report-only header inside a <meta> element, bail out.
     if (source == ContentSecurityPolicyHeaderSourceMeta && type == ContentSecurityPolicyHeaderTypeReport) {
         reportReportOnlyInMeta(header);
         return;
     }
 
+    // Notify about the new header, so that it can be reported back to the
+    // browser process.  This is needed in order to:
+    // 1) now / short-term: replicate CSP directives (i.e. frame-src) to OOPIFs.
+    // 2) not-yet / long-term: CSP can be enforced by the browser.

[alexmos, 2016/05/11 19:46:41]

Might be good to add a link to the bug for moving CSP to the browser (I think
it's 376522?).  For 2), do you know how this would reconcile with parsing CSP
HTTP headers in the browser?

[Łukasz Anforowicz, 2016/05/11 23:14:48]

Done.

I am having second thoughts about this comment... maybe I should just remove
stuff from "This is needed in order to..." forward?
This is an interesting question.  nasko@ or mkwst@ might probably have a better
idea on how to accomplish this, but I thought about:
1. moving parsing, storing, checking code into //content/common layer.
2. rewriting blink::ContentSecurityPolicy to use some kind of a delegate to do
parsing, storing, checking via //content layer (yucky?)
3. reuse //content/common code to do parsing, storing, checking in the browser
as well.

+    if (notifyFrameLoaderClient && document() && document()->frame()) {
+        static_cast<FrameLoaderClient*>(document()->frame()->client())
+            ->didAddContentSecurityPolicy(header, type, source);

[alexmos, 2016/05/11 19:46:41]

Does this also work with pending navigations that are canceled?  E.g., would it
be at all possible to have a pending navigation that sends a CSP http header
before commit but then never commits due to an error?  Can 
DocumentLoader::responseReceived lead to a DidFailProvisionalLoad after applying
the CSP header?  I'm basically curious whether we need to delay replicating CSP
headers until a successful commit.

[Łukasz Anforowicz, 2016/05/13 17:29:15]

This is a great question - thanks for raising this.

In patchset 7, we are resetting the old CSP headers (and processing new HTTP
headers in NavigationHandle::WillProcessResponse) right when we know it is okay
to say ReadyToCommitNavigation().  Before that point we should have processed
any CSP from the not-yet-commited page (because HTTP headers are dropped on the
floor on Blink side + because Blink couldn't yet process CSP from <meta>
elements because we haven't yet sent to Blink the body of the HTTP response). 
After that point we are commited to the navigation, so it is okay to forget the
old CSP headers.

OTOH, there are still some questions left:

1. Can a non-http navigation clobber old headers too early - i.e. via Blink
notifications that happen too early / before commit.  In particular about:blank
navigation is a bit special because Blink copies CSP headers from the parent in
this case [1].

2. In patchset 7 I've removed the call to
frame_tree_node->ResetContentSecurityPolicy() that in earlier patchsets was done
from NavigatorImpl::DidNavigate.  This means that this call is now only
happening for HTTP responses, because otherwise
NavigationHandle::WillProcessResponse won't get involved (right?).  So - there
are 2 subquestions here:

2a. Should navigating from http to non-http location reset CSP for a frame?

2b. What would be a good place to call
frame_tree_node->ResetContentSecurityPolicy() in this case (place that would
happen earlier than NavigationHandle::WillProcessResponse because otherwise we
would clobber CSP the browser found in the headers).

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[Łukasz Anforowicz, 2016/05/13 23:31:43]

Nick / Charlie - could you please also look at Alex's question?

+    }
+
     Vector<UChar> characters;
     header.appendTo(characters);
 
     const UChar* begin = characters.data();
     const UChar* end = begin + characters.size();
 
     // RFC2616, section 4.2 specifies that headers appearing multiple times can
     // be combined with a comma. Walk the header string, and parse each comma
     // separated chunk as a separate header.
     const UChar* position = begin;
@@ skipping 427 matching lines @@
 
 bool ContentSecurityPolicy::didSetReferrerPolicy() const
 {
     for (const auto& policy : m_policies) {
         if (policy->didSetReferrerPolicy())
             return true;
     }
     return false;
 }
 
-SecurityOrigin* ContentSecurityPolicy::getSecurityOrigin() const
-{
-    return m_executionContext->securityContext().getSecurityOrigin();
-}
-
 const KURL ContentSecurityPolicy::url() const
 {
     return m_executionContext->contextURL();
 }
 
 KURL ContentSecurityPolicy::completeURL(const String& url) const
 {
     return m_executionContext->contextCompleteURL(url);
 }
 
@@ skipping 61 matching lines @@
         KURL source = KURL(ParsedURLString, stack->topSourceURL());
         init.setSourceFile(stripURLForUseInReport(document, source));
         init.setLineNumber(stack->topLineNumber());
         init.setColumnNumber(stack->topColumnNumber());
     }
 }
 
 void ContentSecurityPolicy::reportViolation(const String& directiveText, const String& effectiveDirective, const String& consoleMessage, const KURL& blockedURL, const Vector<String>& reportEndpoints, const String& header, ViolationType violationType, LocalFrame* contextFrame)
 {
     ASSERT(violationType == URLViolation || blockedURL.isEmpty());
+
+    // FIXME: Support sending reports from OOPIFs (or move CSP child-src and
+    // frame-src checks to the browser).

[Mike West, 2016/05/11 06:21:04]

Nit: Prefer `TODO(...)` to FIXME.

Nit: Can you point to a bug here so that we don't lose track of the work?

[Łukasz Anforowicz, 2016/05/11 23:14:48]

Done.

+    if (!m_executionContext && !contextFrame) {
+        ASSERT(equalIgnoringCase(effectiveDirective, ContentSecurityPolicy::ChildSrc)
+            || equalIgnoringCase(effectiveDirective, ContentSecurityPolicy::FrameSrc));
+        return;
+    }
+
     ASSERT((m_executionContext && !contextFrame) || (equalIgnoringCase(effectiveDirective, ContentSecurityPolicy::FrameAncestors) && contextFrame));
 
     // FIXME: Support sending reports from worker.
     Document* document = contextFrame ? contextFrame->document() : this->document();
     if (!document)
         return;
 
     LocalFrame* frame = document->frame();
     if (!frame)
         return;
@@ skipping 258 matching lines @@
     // Collisions have no security impact, so we can save space by storing only the string's hash rather than the whole report.
     return !m_violationReportsSent.contains(report.impl()->hash());
 }
 
 void ContentSecurityPolicy::didSendViolationReport(const String& report)
 {
     m_violationReportsSent.add(report.impl()->hash());
 }
 
 } // namespace blink