[CORS-RFC1918] Fix policy to content setting wiring.

Content settings have integer enum values, whereas the policy is
boolean. We introduce a new handler that performs the translation. We
check the fix through policy_test_cases.json.

Bug: chromium:986744
Change-Id: Id1c7e8368405d64b1a879250c33678281d66c8c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359996
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799784}

chrome/browser/policy/configuration_policy_handler_list_factory.cc

@@ -54,6 +54,7 @@
 #include "components/certificate_transparency/pref_names.h"
 #include "components/component_updater/pref_names.h"
 #include "components/content_settings/core/browser/cookie_settings_policy_handler.h"
+#include "components/content_settings/core/browser/insecure_private_network_policy_handler.h"
 #include "components/content_settings/core/common/pref_names.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h"
 #include "components/embedder_support/pref_names.h"
@@ -252,9 +253,6 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
   { key::kLegacySameSiteCookieBehaviorEnabled,
     prefs::kManagedDefaultLegacyCookieAccessSetting,
     base::Value::Type::INTEGER },
-  { key::kInsecurePrivateNetworkRequestsAllowed,
-    prefs::kManagedDefaultInsecurePrivateNetworkSetting,
-    base::Value::Type::BOOLEAN },
   { key::kDefaultPluginsSetting,
     prefs::kManagedDefaultPluginsSetting,
     base::Value::Type::INTEGER },
@@ -1424,6 +1422,9 @@ std::unique_ptr<ConfigurationPolicyHandlerList> BuildHandlerList(
   handlers->AddHandler(std::make_unique<autofill::AutofillPolicyHandler>());
   handlers->AddHandler(
       std::make_unique<content_settings::CookieSettingsPolicyHandler>());
+  handlers->AddHandler(
+      std::make_unique<
+          content_settings::InsecurePrivateNetworkPolicyHandler>());
   handlers->AddHandler(std::make_unique<DefaultSearchPolicyHandler>());
   handlers->AddHandler(std::make_unique<ForceSafeSearchPolicyHandler>());
   handlers->AddHandler(std::make_unique<ForceYouTubeSafetyModePolicyHandler>());

chrome/browser/policy/content_settings_policy_browsertest.cc

@@ -19,6 +19,7 @@
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/content_settings/core/browser/private_network_settings.h"
 #include "components/content_settings/core/common/features.h"
 #include "components/permissions/permission_manager.h"
 #include "components/permissions/permission_result.h"
@@ -32,6 +33,7 @@
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "device/bluetooth/test/mock_bluetooth_adapter.h"
 #include "services/device/public/cpp/test/fake_usb_device_info.h"
+#include "services/network/public/cpp/features.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -272,6 +274,58 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, WebUsbAllowDevicesForUrls) {
       context->HasDevicePermission(kTestOrigin, kTestOrigin, device_info));
 }
 
+IN_PROC_BROWSER_TEST_F(PolicyTest, PrivateNetworkRequestPolicy) {
+  const auto* settings_map =
+      HostContentSettingsMapFactory::GetForProfile(browser()->profile());
+
+  // By default, we should block requests.
+  EXPECT_EQ(network::mojom::PrivateNetworkRequestPolicy::
+                kBlockFromInsecureToMorePrivate,
+            content_settings::GetPrivateNetworkRequestPolicy(
+                settings_map, GURL("http://bleep.com")));
+
+  PolicyMap policies;
+  SetPolicy(&policies, key::kInsecurePrivateNetworkRequestsAllowed,
+            base::Value(false));
+  UpdateProviderPolicy(policies);
+
+  // Explicitly-disallowing is the same as not setting the policy.
+  EXPECT_EQ(network::mojom::PrivateNetworkRequestPolicy::
+                kBlockFromInsecureToMorePrivate,
+            content_settings::GetPrivateNetworkRequestPolicy(
+                settings_map, GURL("http://bleep.com")));
+
+  base::Value allowlist(base::Value::Type::LIST);
+  allowlist.Append(base::Value("http://bleep.com"));
+  allowlist.Append(base::Value("http://woohoo.com:1234"));
+  SetPolicy(&policies, key::kInsecurePrivateNetworkRequestsAllowedForUrls,
+            std::move(allowlist));
+  UpdateProviderPolicy(policies);
+
+  EXPECT_EQ(network::mojom::PrivateNetworkRequestPolicy::
+                kBlockFromInsecureToMorePrivate,
+            content_settings::GetPrivateNetworkRequestPolicy(
+                settings_map, GURL("http://default.com")));
+
+  EXPECT_EQ(network::mojom::PrivateNetworkRequestPolicy::kAllow,
+            content_settings::GetPrivateNetworkRequestPolicy(
+                settings_map, GURL("http://bleep.com/heyo")));
+
+  EXPECT_EQ(network::mojom::PrivateNetworkRequestPolicy::
+                kBlockFromInsecureToMorePrivate,
+            content_settings::GetPrivateNetworkRequestPolicy(
+                settings_map, GURL("https://bleep.com")));
+
+  EXPECT_EQ(network::mojom::PrivateNetworkRequestPolicy::kAllow,
+            content_settings::GetPrivateNetworkRequestPolicy(
+                settings_map, GURL("http://woohoo.com:1234/index.html")));
+
+  EXPECT_EQ(network::mojom::PrivateNetworkRequestPolicy::
+                kBlockFromInsecureToMorePrivate,
+            content_settings::GetPrivateNetworkRequestPolicy(
+                settings_map, GURL("http://woohoo.com/index.html")));
+}
+
 class DisallowWildcardPolicyTest : public PolicyTest {
  public:
   DisallowWildcardPolicyTest() {

chrome/test/data/policy/policy_test_cases.json

@@ -2048,13 +2048,17 @@
   "InsecurePrivateNetworkRequestsAllowed": {
     "os": ["win", "linux", "mac", "chromeos"],
     "policy_pref_mapping_test": [
+      {
+        "policies": {},
+        "prefs": { "profile.managed_default_content_settings.insecure_private_network": { "expect_default": true } }
+      },
       {
         "policies": { "InsecurePrivateNetworkRequestsAllowed": false },
-        "prefs": { "profile.managed_default_content_settings.insecure_private_network": {} }
+        "prefs": { "profile.managed_default_content_settings.insecure_private_network": { "value": 2 } }
       },
       {
         "policies": { "InsecurePrivateNetworkRequestsAllowed": true },
-        "prefs": { "profile.managed_default_content_settings.insecure_private_network": {} }
+        "prefs": { "profile.managed_default_content_settings.insecure_private_network": { "value": 1 } }
       }
     ]
   },
@@ -2063,8 +2067,12 @@
     "os": ["win", "linux", "mac", "chromeos"],
     "policy_pref_mapping_test": [
       {
-        "policies": { "InsecurePrivateNetworkRequestsAllowedForUrls": ["[*.]google.com"] },
-        "prefs": { "profile.managed_insecure_private_network_allowed_for_urls": {} }
+        "policies": { "InsecurePrivateNetworkRequestsAllowedForUrls": ["[*.]google.com", "http://example.com:1234"] },
+        "prefs": {
+          "profile.managed_insecure_private_network_allowed_for_urls": {
+            "value": ["[*.]google.com", "http://example.com:1234"]
+          }
+        }
       }
     ]
   },

components/content_settings/core/browser/BUILD.gn

@@ -75,6 +75,8 @@ static_library("browser") {
     sources += [
       "cookie_settings_policy_handler.cc",
       "cookie_settings_policy_handler.h",
+      "insecure_private_network_policy_handler.cc",
+      "insecure_private_network_policy_handler.h",
     ]
     deps += [
       "//components/policy:generated",

components/content_settings/core/browser/insecure_private_network_policy_handler.cc

+// Copyright 2020 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 "components/content_settings/core/browser/insecure_private_network_policy_handler.h"
+
+#include "base/values.h"
+#include "components/content_settings/core/common/content_settings.h"
+#include "components/content_settings/core/common/pref_names.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/policy_constants.h"
+#include "components/prefs/pref_value_map.h"
+
+namespace content_settings {
+namespace {
+
+ContentSetting ConvertBooleanToContentSetting(bool is_allowed) {
+  return is_allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
+}
+
+}  // namespace
+
+InsecurePrivateNetworkPolicyHandler::InsecurePrivateNetworkPolicyHandler()
+    : TypeCheckingPolicyHandler(
+          policy::key::kInsecurePrivateNetworkRequestsAllowed,
+          base::Value::Type::BOOLEAN) {}
+
+InsecurePrivateNetworkPolicyHandler::~InsecurePrivateNetworkPolicyHandler() =
+    default;
+
+void InsecurePrivateNetworkPolicyHandler::ApplyPolicySettings(
+    const policy::PolicyMap& policies,
+    PrefValueMap* prefs) {
+  const base::Value* value = policies.GetValue(policy_name());
+  bool is_allowed = false;
+  if (value && value->GetAsBoolean(&is_allowed)) {
+    prefs->SetValue(prefs::kManagedDefaultInsecurePrivateNetworkSetting,
+                    base::Value(ConvertBooleanToContentSetting(is_allowed)));
+  }
+}
+
+}  // namespace content_settings

components/content_settings/core/browser/insecure_private_network_policy_handler.h

+// Copyright 2020 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.
+
+#ifndef COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_INSECURE_PRIVATE_NETWORK_POLICY_HANDLER_H_
+#define COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_INSECURE_PRIVATE_NETWORK_POLICY_HANDLER_H_
+
+#include "components/policy/core/browser/configuration_policy_handler.h"
+
+namespace content_settings {
+
+// Handler for the InsecurePrivateNetworkRequestAllowed policy.
+class InsecurePrivateNetworkPolicyHandler
+    : public policy::TypeCheckingPolicyHandler {
+ public:
+  InsecurePrivateNetworkPolicyHandler();
+  ~InsecurePrivateNetworkPolicyHandler() override;
+
+  InsecurePrivateNetworkPolicyHandler(
+      const InsecurePrivateNetworkPolicyHandler&) = delete;
+  InsecurePrivateNetworkPolicyHandler& operator=(
+      const InsecurePrivateNetworkPolicyHandler&) = delete;
+
+  // TypeCheckingPolicyHandler methods:
+  void ApplyPolicySettings(const policy::PolicyMap& policies,
+                           PrefValueMap* prefs) override;
+};
+
+}  // namespace content_settings
+
+#endif  // COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_INSECURE_PRIVATE_NETWORK_POLICY_HANDLER_H_

components/content_settings/core/browser/private_network_settings.cc

@@ -28,9 +28,6 @@ using Policy = network::mojom::PrivateNetworkRequestPolicy;
 //    - if an origin is listed in this policy, then the content setting is
 //      always ALLOW for URLs of that origin
 //
-// TODO(crbug.com/986744): Currently the above description is slightly
-// incorrect, since the enterprise policies are not wired properly to content
-// settings.
 Policy GetPrivateNetworkRequestPolicy(const HostContentSettingsMap* map,
                                       const GURL& url) {
   const std::string unused_resource_identifier;

components/policy/core/browser/policy_pref_mapping_test.cc

@@ -491,8 +491,10 @@ void VerifyPolicyToPrefMappings(const base::FilePath& test_case_path,
             EXPECT_FALSE(pref->IsUserControlled());
             EXPECT_TRUE(pref->IsManaged());
           }
-          if (pref_case->value())
-            EXPECT_TRUE(pref->GetValue()->Equals(pref_case->value()));
+          if (pref_case->value()) {
+            EXPECT_TRUE(pref->GetValue()->Equals(pref_case->value()))
+                << *pref->GetValue() << " != " << *pref_case->value();
+          }
         }
       }
     }

components/policy/resources/policy_templates.json

@@ -6594,13 +6594,20 @@
       },
       'example_value': False,
       'id': 766,
-      'caption': '''Specifies whether to allow insecure websites to make requests to more-private network endpoints.''',
+      'caption': '''Specifies whether to allow insecure websites to make requests to more-private network endpoints''',
       'tags': ['system-security'],
-      'desc': '''Controls whether insecure websites are allowed to make requests to more-private network endpoints. E.g. controls whether http://evil.com is allowed to make requests to both `localhost` and endpoints on the private network, neither of which are accessible from the public internet. See https://wicg.github.io/cors-rfc1918/#goals for more details.
+      'desc': '''Controls whether insecure websites are allowed to make requests to more-private network endpoints.
 
-          When this policy is not set, the default behavior for requests from insecure contexts to more-private network endpoints will depend on the user's personal configuration for the <ph name="BLOCK_INSECURE_PRIVATE_NETWORK_REQUESTS_FEATURE_NAME">BlockInsecurePrivateNetworkRequests</ph> feature, which may be set by a field trial or on the command line.
+          This policy relates to the CORS-RFC1918 specification. See https://wicg.github.io/cors-rfc1918 for more details.
 
-          When this policy is set to false, insecure websites are forbidden from making requests to more-private network endpoints. That is, insecure websites served from public IP addresses are forbidden from making requests to both localhost and private IP addresses. Insecure websites served from private IP addresses are forbidden from making requests to localhost.
+          A network endpoint is more private than another if:
+          1) Its IP address is localhost and the other is not.
+          2) Its IP address is private and the other is public.
+          In the future, depending on spec evolution, this policy might apply to all cross-origin requests directed at private IPs or localhost.
+
+          A website is deemed secure if it meets the definition of a secure context in https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts. Otherwise, it will be treated as an insecure context.
+
+          When this policy is either not set or set to false, the default behavior for requests from insecure contexts to more-private network endpoints will depend on the user's personal configuration for the <ph name="BLOCK_INSECURE_PRIVATE_NETWORK_REQUESTS_FEATURE_NAME">BlockInsecurePrivateNetworkRequests</ph> feature, which may be set by a field trial or on the command line.
 
           When this policy is set to true, insecure websites are allowed to make requests to any network endpoint, subject to other cross-origin checks.''',
     },