Replicate Content-Security-Policy into remote frame proxies.

content/browser/frame_host/frame_tree_node.cc

Index: content/browser/frame_host/frame_tree_node.cc
diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc
index 73ef0c3d9d084f6d63db8fff506099ec1fc61bac..109ab457edb42cac68ae0fc60dda84060788e788 100644
--- a/content/browser/frame_host/frame_tree_node.cc
+++ b/content/browser/frame_host/frame_tree_node.cc
@@ -245,6 +245,26 @@ void FrameTreeNode::SetFrameName(const std::string& name,
   replication_state_.unique_name = unique_name;
 }
 
+void FrameTreeNode::AddContentSecurityPolicy(
+    const ContentSecurityPolicyHeader& header) {
+  // Deduplicate the headers (Blink can send a notification about the same
+  // header multiple times, as ContentSecurityPolicy object is copied around).

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

I tried to look into why Blink processes the same header multiple times - it
seems that there are multiple copies of ContentSecurityPolicy object floating
around (multiple copies per html document).  I am not sure exactly why there are
multiple copies.

For example - when running
SitePerProcessBrowserTest.CrossSiteIframeBlockedByParentCSPFromHeaders (with the
second iframe commented out, so there is only one frame with CSP), I see that
CSP::bindToExecutionContext is called 3 times (each time on a different instance
of ContentSecurityPolicy).  Having 3 copies of ContentSecurityPolicy seems a bit
wasteful - each time ContentSecurityPolicy::addPolicyFromHeaderValue gets called
it copies a String (twice: once into Vector<UChar> and once into |policy|) and
parses the CSP.

Callstacks:

#2 0x7f8b1e4ea5ab blink::ContentSecurityPolicy::bindToExecutionContext()
#3 0x7f8b1e79a927 blink::FrameLoader::didInstallNewDocument()
#4 0x7f8b1e75857d blink::DocumentLoader::createWriterFor()
#5 0x7f8b1e7581c1 blink::DocumentLoader::ensureWriter()
#6 0x7f8b1e755331 blink::DocumentLoader::commitData()
#7 0x7f8b1e758cfc blink::DocumentLoader::processData()
#8 0x7f8b1e758860 blink::DocumentLoader::dataReceived()
#9 0x7f8b1e2f30ef blink::RawResource::appendData()
#10 0x7f8b2b179e8a content::WebURLLoaderImpl::Context::OnReceivedData()
#11 0x7f8b2b17bfc2 content::WebURLLoaderImpl::RequestPeerImpl::OnReceivedData()
#12 0x7f8b2b10d56d content::ResourceDispatcher::OnReceivedData()

#2 0x7f8b1e4ea5ab blink::ContentSecurityPolicy::bindToExecutionContext()
#3 0x7f8b1d1abc8d blink::Document::initSecurityContext()
#4 0x7f8b1d1a8e6e blink::Document::Document()
#5 0x7f8b1d616aac blink::HTMLDocument::HTMLDocument()
#6 0x7f8b1d16910a blink::DOMImplementation::createDocument()
#7 0x7f8b1e41d7dd blink::LocalDOMWindow::createDocument()
#8 0x7f8b1e41db16 blink::LocalDOMWindow::installNewDocument()
#9 0x7f8b1e758544 blink::DocumentLoader::createWriterFor()
#10 0x7f8b1e7581c1 blink::DocumentLoader::ensureWriter()
#11 0x7f8b1e755331 blink::DocumentLoader::commitData()
#12 0x7f8b1e754dbd blink::DocumentLoader::finishedLoading()
#13 0x7f8b1e7593ae blink::DocumentLoader::maybeLoadEmpty()
#14 0x7f8b1e7596c4 blink::DocumentLoader::startLoadingMainResource()
#15 0x7f8b1e794dba blink::FrameLoader::init()
#16 0x7f8b227ae5b3 blink::WebLocalFrameImpl::initializeCoreFrame()
#17 0x7f8b227aef1e blink::WebLocalFrameImpl::createChildFrame()
#18 0x7f8b1d6611c3 blink::HTMLFrameOwnerElement::loadOrRedirectSubframe()
#19 0x7f8b1d65ac6e blink::HTMLFrameElementBase::openURL()
#20 0x7f8b1d65c0fb blink::HTMLFrameElementBase::setNameAndOpenURL()
#21 0x7f8b1d141953 blink::ContainerNode::notifyNodeInserted()
#22 0x7f8b1d13b690 blink::ContainerNode::parserAppendChild()
#23 0x7f8b1d8a7e6a blink::HTMLConstructionSite::executeTask()
#24 0x7f8b1d8aad10 blink::HTMLConstructionSite::executeQueuedTasks()
#25 0x7f8b1d8c76b9
blink::HTMLDocumentParser::constructTreeFromCompactHTMLToken()
#26 0x7f8b1d8c5c7c
blink::HTMLDocumentParser::processParsedChunkFromBackgroundParser()
#27 0x7f8b1d8be08d blink::HTMLDocumentParser::pumpPendingSpeculations()

#2 0x7f8b1e4ea5ab blink::ContentSecurityPolicy::bindToExecutionContext()
#3 0x7f8b1e79a927 blink::FrameLoader::didInstallNewDocument()
#4 0x7f8b1e75857d blink::DocumentLoader::createWriterFor()
#5 0x7f8b1e7581c1 blink::DocumentLoader::ensureWriter()
#6 0x7f8b1e755331 blink::DocumentLoader::commitData()
#7 0x7f8b1e754dbd blink::DocumentLoader::finishedLoading()
#8 0x7f8b1e7593ae blink::DocumentLoader::maybeLoadEmpty()
#9 0x7f8b1e7596c4 blink::DocumentLoader::startLoadingMainResource()
#10 0x7f8b1e794dba blink::FrameLoader::init()
#11 0x7f8b227ae5b3 blink::WebLocalFrameImpl::initializeCoreFrame()
#12 0x7f8b227aef1e blink::WebLocalFrameImpl::createChildFrame()
#13 0x7f8b1d6611c3 blink::HTMLFrameOwnerElement::loadOrRedirectSubframe()
#14 0x7f8b1d65ac6e blink::HTMLFrameElementBase::openURL()
#15 0x7f8b1d65c0fb blink::HTMLFrameElementBase::setNameAndOpenURL()
#16 0x7f8b1d141953 blink::ContainerNode::notifyNodeInserted()
#17 0x7f8b1d13b690 blink::ContainerNode::parserAppendChild()
#18 0x7f8b1d8a7e6a blink::HTMLConstructionSite::executeTask()
#19 0x7f8b1d8aad10 blink::HTMLConstructionSite::executeQueuedTasks()
#20 0x7f8b1d8c76b9
blink::HTMLDocumentParser::constructTreeFromCompactHTMLToken()
#21 0x7f8b1d8c5c7c
blink::HTMLDocumentParser::processParsedChunkFromBackgroundParser()
#22 0x7f8b1d8be08d blink::HTMLDocumentParser::pumpPendingSpeculations()

[alexmos, 2016/05/16 16:17:03]

Seems like at least two of the copies are plausible -- one is for the initial
about:blank document in the child frame, another is for the actual document
that's loaded afterward.  But it looks like the initial blank Document calls
bindToExecutionContext twice (the stacks with createChildFrame and
maybeLoadEmpty), and that seems redundant.

Thinking some more about your question about when to clear CSP, I'm actually
curious if sending the http CSP headers at commit time (i.e., DidNavigate) and
letting that clear the old headers is really too late?  This is because commit
time is really when these CSP headers can start to have effect -- a doc can't
create a child frame before commit.  That would also take care of this
duplication problem, as you won't see commits from the initial blank document. 
It should work for srcdoc too.  And resetting old headers at commit time sounds
like the right thing also, as that's when they really stop taking effect, and
you won't need to deal with potentially temporary CSP headers from pending RFHs.
 Maybe I'm forgetting something that makes this not work -- thoughts?

I'm not sure if you could apply more than one http CSP header before commit -
but if so, seems like you could buffer them up and send them all as part of
DidCommitProvisionalLoad, similar to the origin.  <meta> CSP would still be sent
at the time the header is added, for but <meta> that would be guaranteed to
happen after commit.

[Łukasz Anforowicz, 2016/05/16 19:44:45]

Thanks for the suggestion - it worked out really well in patchset 11, where I:

- Went back to resetting CSP in NavigatorImpl::DidNavigate
- Removed the code to notify FrameLoaderClient upon
ContentSecurityPolicy::bindToExecutionContext.  This goes back to the behavior
of early patchsets where CSP from http headers (or inherited from parent frame)
won't be reported (mulitple times) via FrameLoaderClient - this allows to also
remove deduplication code in frame_tree_node.cc.
- Added ContentSecurityPolicy::reportAccumulatedHeaders(FrameLoaderClient*) that
gets called right after FrameLoader reports that the navigation has committed.

I've thought about reporting the CSP headers within DidCommitProvisionalLoad
message, but went with the approach above because in the long-term CSP from http
headers (and inherited from parent frame) should be handled within the browser
process - I wanted to avoid adding more temporary code to handle this case
(handling of new DidCommitProvisionalLoad payload + exposing
ContentSecurityPolicy through web layer to enable populating the message
payload).  I think sending CSP from http headers via the same path as used for
CSP from <meta> is working out okay.

[alexmos, 2016/05/16 22:31:55]

Thanks -- yes, sending them right after dispatching the commit message looks
fine to me.

+  for (const auto& other : replication_state_.accumulated_csp_headers) {
+    if (header.header_value == other.header_value &&
+        header.source == other.source && header.type == other.type)
+      return;
+  }
+
+  // Append the newly discovered CSP header and notify render frame proxies.
+  replication_state_.accumulated_csp_headers.push_back(header);
+  render_manager_.OnDidAddContentSecurityPolicy(header);
+}
+
+void FrameTreeNode::ResetContentSecurityPolicy() {
+  replication_state_.accumulated_csp_headers.clear();
+  render_manager_.OnDidResetContentSecurityPolicy();
+}
+
 void FrameTreeNode::SetEnforceStrictMixedContentChecking(bool should_enforce) {
   if (should_enforce ==
       replication_state_.should_enforce_strict_mixed_content_checking) {