Reporting: Verify NetworkService ignores headers

The NetworkService currently ignores all Reporting headers, since we
haven't implemented the BACKGROUND_SYNC permissions check in a way that
works across service/process boundaries.  This adds a browser test that
verifies this (and while we're at it, verifies that we _don't_ ignore
Reporting headers when we're using the legacy non-service network
stack).

Bug: 845559
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;luci.chromium.try:linux_mojo;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I3e7c1ab446e067e7f8af6fd80d3abc9b2d5187ad
Reviewed-on: https://chromium-review.googlesource.com/1067465
Commit-Queue: Douglas Creager <dcreager@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568488}

chrome/browser/net/reporting_browsertest.cc

+// Copyright 2018 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 <memory>
+#include <string>
+
+#include "base/strings/stringprintf.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/values_test_util.h"
+#include "base/time/time.h"
+#include "base/values.h"
+#include "chrome/browser/ssl/cert_verifier_browser_test.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/browser_navigator_params.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/reporting/reporting_policy.h"
+#include "net/test/embedded_test_server/controllable_http_response.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "services/network/public/cpp/features.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+class ReportingBrowserTest : public CertVerifierBrowserTest {
+ public:
+  ReportingBrowserTest()
+      : https_server_(net::test_server::EmbeddedTestServer::TYPE_HTTPS) {}
+  ~ReportingBrowserTest() override = default;
+
+  void SetUp() override;
+  void SetUpOnMainThread() override;
+
+  net::EmbeddedTestServer* server() { return &https_server_; }
+  int port() const { return https_server_.port(); }
+
+  net::test_server::ControllableHttpResponse* original_response() {
+    return original_response_.get();
+  }
+
+  net::test_server::ControllableHttpResponse* upload_response() {
+    return upload_response_.get();
+  }
+
+  GURL GetReportingEnabledURL() const {
+    return GURL(base::StringPrintf("https://example.com:%d/original", port()));
+  }
+
+  GURL GetCollectorURL() const {
+    return GURL(base::StringPrintf("https://example.com:%d/upload", port()));
+  }
+
+  std::string GetReportToHeader() const {
+    return "Report-To: {\"endpoints\":[{\"url\":\"" + GetCollectorURL().spec() +
+           "\"}],\"max_age\":86400}\r\n";
+  }
+
+  std::string GetNELHeader() const {
+    return "NEL: "
+           "{\"report_to\":\"default\",\"max_age\":86400,\"success_fraction\":"
+           "1.0}\r\n";
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+  net::EmbeddedTestServer https_server_;
+  std::unique_ptr<net::test_server::ControllableHttpResponse>
+      original_response_;
+  std::unique_ptr<net::test_server::ControllableHttpResponse> upload_response_;
+
+  DISALLOW_COPY_AND_ASSIGN(ReportingBrowserTest);
+};
+
+void ReportingBrowserTest::SetUp() {
+  scoped_feature_list_.InitWithFeatures(
+      {network::features::kReporting, network::features::kNetworkErrorLogging},
+      {});
+  CertVerifierBrowserTest::SetUp();
+
+  // Make report delivery happen instantly.
+  net::ReportingPolicy policy;
+  policy.delivery_interval = base::TimeDelta::FromSeconds(0);
+  net::ReportingPolicy::UsePolicyForTesting(policy);
+}
+
+void ReportingBrowserTest::SetUpOnMainThread() {
+  CertVerifierBrowserTest::SetUpOnMainThread();
+
+  host_resolver()->AddRule("*", "127.0.0.1");
+
+  original_response_ =
+      std::make_unique<net::test_server::ControllableHttpResponse>(server(),
+                                                                   "/original");
+  upload_response_ =
+      std::make_unique<net::test_server::ControllableHttpResponse>(server(),
+                                                                   "/upload");
+
+  // Reporting and NEL will ignore configurations headers if the response
+  // doesn't come from an HTTPS origin, or if the origin's certificate is
+  // invalud.  Our test certs are valid, so we need a mock certificate verifier
+  // to trick the Reporting stack into paying attention to our test headers.
+  mock_cert_verifier()->set_default_result(net::OK);
+  ASSERT_TRUE(server()->Start());
+}
+
+std::unique_ptr<base::Value> ParseReportUpload(const std::string& payload) {
+  auto parsed_payload = base::test::ParseJson(payload);
+  // Clear out any non-reproducible fields.
+  for (auto& report : parsed_payload->GetList()) {
+    report.RemoveKey("age");
+    report.RemovePath({"report", "elapsed_time"});
+  }
+  return parsed_payload;
+}
+
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(ReportingBrowserTest, TestReportingHeadersProcessed) {
+  NavigateParams params(browser(), GetReportingEnabledURL(),
+                        ui::PAGE_TRANSITION_LINK);
+  Navigate(&params);
+
+  original_response()->WaitForRequest();
+  original_response()->Send("HTTP/1.1 204 OK\r\n");
+  original_response()->Send(GetReportToHeader());
+  original_response()->Send(GetNELHeader());
+  original_response()->Send("\r\n");
+  original_response()->Done();
+
+  upload_response()->WaitForRequest();
+  auto actual = ParseReportUpload(upload_response()->http_request()->content);
+  upload_response()->Send("HTTP/1.1 204 OK\r\n");
+  upload_response()->Send("\r\n");
+  upload_response()->Done();
+
+  // Verify the contents of the report that we received.
+  EXPECT_TRUE(actual != nullptr);
+  auto expected = base::test::ParseJson(base::StringPrintf(
+      R"json(
+        [
+          {
+            "report": {
+              "protocol": "http/1.1",
+              "referrer": "",
+              "sampling_fraction": 1.0,
+              "server_ip": "127.0.0.1",
+              "status_code": 204,
+              "type": "ok",
+              "uri": "https://example.com:%d/original",
+            },
+            "type": "network-error",
+            "url": "https://example.com:%d/original",
+          },
+        ]
+      )json",
+      port(), port()));
+  EXPECT_EQ(*expected, *actual);
+}

chrome/test/BUILD.gn

@@ -629,6 +629,7 @@ test("browser_tests") {
       "../browser/net/predictor_browsertest.cc",
       "../browser/net/profile_network_context_service_browsertest.cc",
       "../browser/net/proxy_browsertest.cc",
+      "../browser/net/reporting_browsertest.cc",
       "../browser/net/variations_http_headers_browsertest.cc",
       "../browser/net/websocket_browsertest.cc",
       "../browser/no_background_tasks_browsertest.cc",

components/cronet/url_request_context_config.cc

@@ -520,7 +520,7 @@ void URLRequestContextConfig::ParseAndSetExperimentalOptions(
 
 #if BUILDFLAG(ENABLE_REPORTING)
   if (nel_enable) {
-    auto policy = std::make_unique<net::ReportingPolicy>();
+    auto policy = net::ReportingPolicy::Create();
 
     // Apps (like Cronet embedders) are generally allowed to run in the
     // background, even across network changes, so use more relaxed privacy

content/shell/browser/shell_url_request_context_getter.cc

@@ -187,7 +187,7 @@ net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
 #if BUILDFLAG(ENABLE_REPORTING)
     if (base::FeatureList::IsEnabled(network::features::kReporting)) {
       std::unique_ptr<net::ReportingPolicy> reporting_policy =
-          std::make_unique<net::ReportingPolicy>();
+          net::ReportingPolicy::Create();
       if (command_line.HasSwitch(switches::kRunWebTests))
         reporting_policy->delivery_interval =
             base::TimeDelta::FromMilliseconds(100);

net/network_error_logging/network_error_logging_end_to_end_test.cc

@@ -60,7 +60,7 @@ class NetworkErrorLoggingEndToEndTest : public TestWithScopedTaskEnvironment {
         upload_should_hang_(false),
         upload_received_(false) {
     // Make report delivery happen instantly.
-    auto policy = std::make_unique<ReportingPolicy>();
+    auto policy = ReportingPolicy::Create();
     policy->delivery_interval = base::TimeDelta::FromSeconds(0);
 
     URLRequestContextBuilder builder;

net/reporting/reporting_policy.cc

@@ -4,10 +4,32 @@
 
 #include "net/reporting/reporting_policy.h"
 
+#include "base/no_destructor.h"
 #include "base/time/time.h"
 
 namespace net {
 
+namespace {
+
+ReportingPolicy* policy_for_testing = nullptr;
+
+}  // namespace
+
+// static
+std::unique_ptr<ReportingPolicy> ReportingPolicy::Create() {
+  if (policy_for_testing != nullptr) {
+    return std::make_unique<ReportingPolicy>(*policy_for_testing);
+  }
+  return std::make_unique<ReportingPolicy>();
+}
+
+// static
+void ReportingPolicy::UsePolicyForTesting(const ReportingPolicy& policy) {
+  static base::NoDestructor<ReportingPolicy> owned_policy;
+  policy_for_testing = owned_policy.get();
+  *owned_policy = policy;
+}
+
 ReportingPolicy::ReportingPolicy()
     : max_report_count(100u),
       max_client_count(1000u),

net/reporting/reporting_policy.h

@@ -5,6 +5,8 @@
 #ifndef NET_REPORTING_REPORTING_POLICY_H_
 #define NET_REPORTING_REPORTING_POLICY_H_
 
+#include <memory>
+
 #include "base/time/time.h"
 #include "net/base/backoff_entry.h"
 #include "net/base/net_export.h"
@@ -14,6 +16,13 @@ namespace net {
 // Various policy knobs for the Reporting system.
 struct NET_EXPORT ReportingPolicy {
   // Provides a reasonable default for use in a browser embedder.
+  static std::unique_ptr<ReportingPolicy> Create();
+
+  // Lets you override the default policy returned by |Create|.  Use this in
+  // browser tests, where there isn't any other way to pass in a specific test
+  // policy to use.
+  static void UsePolicyForTesting(const ReportingPolicy& policy);
+
   ReportingPolicy();
   ReportingPolicy(const ReportingPolicy& other);
   ~ReportingPolicy();

services/network/network_context.cc

@@ -808,7 +808,7 @@ URLRequestContextOwner NetworkContext::ApplyContextParamsToBuilder(
 
 #if BUILDFLAG(ENABLE_REPORTING)
   if (base::FeatureList::IsEnabled(features::kReporting))
-    builder->set_reporting_policy(std::make_unique<net::ReportingPolicy>());
+    builder->set_reporting_policy(net::ReportingPolicy::Create());
   else
     builder->set_reporting_policy(nullptr);
 

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -429,6 +429,11 @@
 # https://crbug.com/846339: CORB: Don't block requests from plugins.
 -OutOfProcessPPAPITest.URLLoaderTrusted
 
+# https://crbug.com/845559
+# Reporting needs to check BACKGROUND_SYNC permission before uploading reports
+# about an origin.
+-ReportingBrowserTest.TestReportingHeadersProcessed
+
 # NOTE: if adding an exclusion for an existing failure (e.g. additional test for
 # feature X that is already not working), please add it beside the existing
 # failures. Otherwise please reach out to network-service-dev@.