Fix report-to CSP directive to only allow one endpoint

According to https://w3c.github.io/webappsec-csp/#directive-report-to,
the Content-Security-Policy directive "report-to" should only accept one
token (endpint). However, our previous implementation allowed several
endpoints to be specified.

Bug: 916265
Change-Id: Ie11ee736f577d015921a5291824dcedcbc790177
Fixed: 916265
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162826Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762813}

services/network/public/cpp/content_security_policy/content_security_policy.cc

@@ -400,6 +400,10 @@ void ParseReportDirective(const GURL& request_url,
     //   the "Report-To" header. See https://w3c.github.io/reporting
     if (using_reporting_api) {
       report_endpoints->push_back(uri.as_string());
+
+      // 'report-to' only allows for a single token. The following ones are
+      // ignored. A console error warning is displayed from blink's CSP parser.
+      break;
     } else {
       GURL url = request_url.Resolve(uri);
 

third_party/blink/renderer/core/frame/csp/content_security_policy.cc

@@ -1399,6 +1399,13 @@ void ContentSecurityPolicy::ReportInvalidSourceExpression(
   LogToConsole(message);
 }
 
+void ContentSecurityPolicy::ReportMultipleReportToEndpoints() {
+  LogToConsole(
+      "The Content Security Policy directive 'report-to' contains more than "
+      "one endpoint. Only the first one will be used, the other ones will be "
+      "ignored.");
+}
+
 void ContentSecurityPolicy::LogToConsole(const String& message,
                                          mojom::ConsoleMessageLevel level) {
   LogToConsole(MakeGarbageCollected<ConsoleMessage>(

third_party/blink/renderer/core/frame/csp/content_security_policy.h

@@ -382,6 +382,7 @@ class CORE_EXPORT ContentSecurityPolicy final
   void ReportInvalidSandboxFlags(const String&);
   void ReportInvalidSourceExpression(const String& directive_name,
                                      const String& source);
+  void ReportMultipleReportToEndpoints();
   void ReportUnsupportedDirective(const String&);
   void ReportInvalidInReportOnly(const String&);
   void ReportInvalidDirectiveInMeta(const String& directive_name);

third_party/blink/renderer/core/frame/csp/csp_directive_list.cc

@@ -1114,6 +1114,12 @@ void CSPDirectiveList::ParseReportTo(const String& name, const String& value) {
   }
 
   ParseAndAppendReportEndpoints(value);
+
+  if (report_endpoints_.size() > 1) {
+    // The directive "report-to" only accepts one endpoint.
+    report_endpoints_.Shrink(1);
+    policy_->ReportMultipleReportToEndpoints();
+  }
 }
 
 void CSPDirectiveList::ParseReportURI(const String& name, const String& value) {
@@ -1143,10 +1149,9 @@ void CSPDirectiveList::ParseReportURI(const String& name, const String& value) {
 
 // For "report-to" directive, the spec says |value| is a single token
 // but we use the same logic as "report-uri" and thus we split |value| by
-// ASCII whitespaces.
+// ASCII whitespaces. The tokens after the first one are discarded in
+// CSPDirectiveList::ParseReportTo.
 // https://w3c.github.io/webappsec-csp/#directive-report-to
-//
-// TODO(https://crbug.com/916265): Fix this inconsistency.
 void CSPDirectiveList::ParseAndAppendReportEndpoints(const String& value) {
   Vector<UChar> characters;
   value.AppendTo(characters);

third_party/blink/renderer/core/frame/csp/csp_directive_list_test.cc

@@ -1215,6 +1215,11 @@ TEST_F(CSPDirectiveListTest, ReportEndpointsProperlyParsed) {
        ContentSecurityPolicySource::kMeta,
        {"group"},
        true},
+      {"script-src 'self'; report-to group group2",
+       ContentSecurityPolicySource::kHTTP,
+       // Only the first report-to endpoint is used. The other ones are ignored.
+       {"group"},
+       true},
       {"script-src 'self'; report-to group; report-to group2;",
        ContentSecurityPolicySource::kHTTP,
        {"group"},

third_party/blink/renderer/platform/network/http_parsers_test.cc

@@ -711,9 +711,10 @@ TEST(HTTPParsersTest, ParseContentSecurityPolicyReportTo) {
   auto policies =
       ParseContentSecurityPolicy("Content-Security-Policy: report-to a b\r\n");
   EXPECT_TRUE(policies[0]->use_reporting_api);
-  ASSERT_EQ(2u, policies[0]->report_endpoints.size());
+  // The specification https://w3c.github.io/webappsec-csp/#directive-report-to
+  // only allows for one endpoints to be defined. The other ones are ignored.
+  ASSERT_EQ(1u, policies[0]->report_endpoints.size());
   EXPECT_EQ("a", policies[0]->report_endpoints[0]);
-  EXPECT_EQ("b", policies[0]->report_endpoints[1]);
 }
 
 TEST(HTTPParsersTest, ParseContentSecurityPolicyReportUri) {

third_party/blink/web_tests/external/wpt/content-security-policy/reporting-api/reporting-api-report-to-only-sends-reports-to-first-endpoint.https.sub.html

+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test that report-to ignores tokens after the first one</title>
+  <script src='/resources/testharness.js'></script>
+  <script src='/resources/testharnessreport.js'></script>
+</head>
+<body>
+  <script>
+    var t1 = async_test("Test that image does not load");
+    async_test(function(t2) {
+    window.addEventListener("securitypolicyviolation", t2.step_func(function(e) {
+        assert_equals(e.blockedURI, "{{location[scheme]}}://{{location[host]}}/content-security-policy/support/fail.png");
+        assert_equals(e.violatedDirective, "img-src");
+        t2.done();
+      }));
+    }, "Event is fired");
+  </script>
+  <img src='/content-security-policy/support/fail.png'
+       onload='t1.unreached_func("The image should not have loaded");'
+       onerror='t1.done();'>
+  <!-- The second token of the report-to directive should be ignored, since the directive only supports one token. So we should not have any reports sent to this endpoint. -->
+  <script async defer src='../support/checkReport.sub.js?reportExists=false'></script>
+</body>
+</html>

third_party/blink/web_tests/external/wpt/content-security-policy/reporting-api/reporting-api-report-to-only-sends-reports-to-first-endpoint.https.sub.html.sub.headers

+Expires: Mon, 26 Jul 1997 05:00:00 GMT
+Cache-Control: no-store, no-cache, must-revalidate
+Cache-Control: post-check=0, pre-check=0, false
+Pragma: no-cache
+Set-Cookie: reporting-api-report-to-only-sends-reports-to-first-endpoint={{$id:uuid()}}; Path=/content-security-policy/reporting-api
+Content-Security-Policy: script-src 'self' 'unsafe-inline'; img-src 'none'; report-to csp-group csp-group-2
+Report-To: { "group": "csp-group", "max_age": 10886400, "endpoints": [{ "url": "https://{{host}}:{{ports[https][0]}}/content-security-policy/support/report.py?op=put&reportID={{uuid()}}" }] }, { "group": "csp-group-2", "max_age": 10886400, "endpoints": [{ "url": "https://{{host}}:{{ports[https][0]}}/content-security-policy/support/report.py?op=put&reportID={{$id}}" }] }

third_party/blink/web_tests/external/wpt/content-security-policy/reporting-api/reporting-api-report-to-overrides-report-uri-1.https.sub.html

@@ -20,6 +20,6 @@
        onload='t1.unreached_func("The image should not have loaded");'
        onerror='t1.done();'>
   <!-- report-to overrides the report-uri so the report goes to a different endpoint and we should not have any reports sent to this endpoint -->
-  <script async defer src='../support/checkReport.sub.js?reportExists=false></script>
+  <script async defer src='../support/checkReport.sub.js?reportExists=false'></script>
 </body>
 </html>

third_party/blink/web_tests/external/wpt/content-security-policy/reporting-api/reporting-api-report-to-overrides-report-uri-2.https.sub.html

@@ -20,6 +20,6 @@
        onload='t1.unreached_func("The image should not have loaded");'
        onerror='t1.done();'>
   <!-- report-to overrides the report-uri so the report goes to a different endpoint and we should not have any reports sent to this endpoint -->
-  <script async defer src='../support/checkReport.sub.js?reportExists=false></script>
+  <script async defer src='../support/checkReport.sub.js?reportExists=false'></script>
 </body>
 </html>