PlzNavigate: Do not disclose urls between cross-origin renderers.

content/common/content_security_policy/content_security_policy.cc

Index: content/common/content_security_policy/content_security_policy.cc
diff --git a/content/common/content_security_policy/content_security_policy.cc b/content/common/content_security_policy/content_security_policy.cc
index 87b035eb1c2ef44916452f1ea3e186ae29f75d64..65d68b67c9e3fee5819201b10b234363bfda9776 100644
--- a/content/common/content_security_policy/content_security_policy.cc
+++ b/content/common/content_security_policy/content_security_policy.cc
@@ -44,6 +44,20 @@ void ReportViolation(CSPContext* context,
                      const GURL& url,
                      bool is_redirect,
                      const SourceLocation& source_location) {
+  // For security reasons, some urls must not be disclosed. It includes the
+  // blocked url and the source location of the error. Care must be taken to
+  // ensure that these information are not transmitted between different

[alexmos, 2017/05/12 01:37:20]

nit: "these information are" -> "these are" (or "this information is"

[arthursonzogni, 2017/05/15 12:20:46]

Done.

+  // cross-origin renderers.

[alexmos, 2017/05/12 01:37:20]

Perhaps also include a reference to https://crbug.com/633306?

[arthursonzogni, 2017/05/15 12:20:46]

Done (In RenderFrameHostImpl::SanitizeDataForUseInCspViolation)

+  GURL safe_url = context->ShouldProtectDataInCspViolation(url::Origin(url))
+                      ? url.GetOrigin()
+                      : url;

[Mike West, 2017/05/12 14:16:44]

I think we can skip this if |is_redirect| is false (because if it's not a
redirect, the initiating renderer already knows the URL, since it created the
request. I suspect that will solve some of the changed tests.

[arthursonzogni, 2017/05/15 12:20:46]

For form-action: I think you are right.
For frame-src: When a cross-origin subframe navigates by itself, I think the
main frame doesn't know (and mustn't know) the url.

I think we can skip if |is_redirect| is false and |directive| is form-action.

[alexmos, 2017/05/16 05:56:48]

Ack - I think that makes sense to me.  Mike, do you agree?

+
+  url::Origin source_location_origin(GURL(source_location.url));
+  SourceLocation safe_source_location =
+      context->ShouldProtectDataInCspViolation(source_location_origin)
+          ? SourceLocation(source_location_origin.Serialize(), 0u, 0u)
+          : source_location;
+
   // We should never have a violation against `child-src` or `default-src`
   // directly; the effective directive should always be one of the explicit
   // fetch directives.
@@ -60,7 +74,7 @@ void ReportViolation(CSPContext* context,
   else if (directive_name == CSPDirective::FrameSrc)
     message << "Refused to frame '";
 
-  message << ElideURLForReportViolation(url)
+  message << ElideURLForReportViolation(safe_url)
           << "' because it violates the following Content Security Policy "
              "directive: \""
           << directive.ToString() << "\".";
@@ -75,9 +89,9 @@ void ReportViolation(CSPContext* context,
 
   context->ReportContentSecurityPolicyViolation(CSPViolationParams(
       CSPDirective::NameToString(directive.name),
-      CSPDirective::NameToString(directive_name), message.str(), url,
+      CSPDirective::NameToString(directive_name), message.str(), safe_url,
       policy.report_endpoints, policy.header.header_value, policy.header.type,
-      is_redirect, source_location));
+      is_redirect, safe_source_location));
 }
 
 bool AllowDirective(CSPContext* context,