system-proxy: Add auth schemes to policy

Currently an attacker can obtain the System-proxy policy set credentials
by tricking the Chromebook to connect to a proxy with a less secure auth
scheme.

To protect against a downgrade attack, this CL adds
policy_credentials_auth_schemes to the SystemProxySettings policy which
will allow admins to specify for which proxy auth schemes the
credentials can be applied to.

If no value is set by policy, then all three username+password auth
schemes are allowed. This is to have backwards compatibility with the
current policy until AdminConsole is updated.

Bug: 1132247
Test: browser test
Change-Id: I67a21c88c411a4373159aaa5f8cadb948e5b9cc9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2484446
Commit-Queue: Andreea-Elena Costinas <acostinas@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Omar Morsi <omorsi@google.com>
Cr-Commit-Position: refs/heads/master@{#824019}

chrome/browser/chromeos/policy/system_proxy_manager.cc

@@ -177,6 +177,15 @@ void SystemProxyManager::OnSystemProxySettingsPolicyChanged() {
   const std::string* password = proxy_settings->FindStringKey(
       chromeos::kSystemProxySettingsKeySystemServicesPassword);
 
+  const base::Value* auth_schemes =
+      proxy_settings->FindListKey(chromeos::kSystemProxySettingsKeyAuthSchemes);
+
+  policy_credentials_auth_schemes_.clear();
+  if (auth_schemes) {
+    for (const auto& auth_scheme : auth_schemes->GetList())
+      policy_credentials_auth_schemes_.push_back(auth_scheme.GetString());
+  }
+
   if (!username || username->empty() || !password || password->empty()) {
     NET_LOG(DEBUG) << "Proxy credentials for system traffic not set: "
                    << kSystemProxyService;
@@ -185,6 +194,8 @@ void SystemProxyManager::OnSystemProxySettingsPolicyChanged() {
     system_services_password_ = *password;
   }
   if (IsManagedProxyConfigured()) {
+    // Force send the configuration in case the credentials hand't changed, but
+    // `policy_credentials_auth_schemes_` has.
     SendPolicyAuthenticationCredentials(system_services_username_,
                                         system_services_password_,
                                         /*force_send=*/true);
@@ -297,6 +308,9 @@ void SystemProxyManager::SendPolicyAuthenticationCredentials(
   system_proxy::Credentials credentials;
   credentials.set_username(username);
   credentials.set_password(password);
+  for (const auto& auth_scheme : policy_credentials_auth_schemes_) {
+    credentials.add_policy_credentials_auth_schemes(auth_scheme);
+  }
   *request.mutable_credentials() = credentials;
 
   request.set_traffic_type(system_proxy::TrafficOrigin::SYSTEM);

chrome/browser/chromeos/policy/system_proxy_manager.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
@@ -180,6 +181,10 @@ class SystemProxyManager : public chromeos::NetworkStateHandlerObserver {
   std::string system_services_address_;
   std::string system_services_username_;
   std::string system_services_password_;
+  // List of proxy authentication schemes for which the policy set credentials
+  // can be used.
+  std::vector<std::string> policy_credentials_auth_schemes_;
+
   // The credentials which were last sent to System-proxy. They can differ from
   // `system_services_username_` and `system_services_username_` if the proxy
   // configuration is not managed; in this case `last_sent_username_` and

chrome/browser/chromeos/policy/system_proxy_manager_browsertest.cc

@@ -134,8 +134,12 @@ constexpr char kSystemProxyPolicyJson[] =
        "system_proxy_enabled": true,
        "system_services_username": "%s",
        "system_services_password": "%s",
+       %s
     })";
 
+constexpr char kAuthSchemesPolicyEntry[] =
+    R"("policy_credentials_auth_schemes": [%s],)";
+
 void RunUntilIdle() {
   DCHECK(base::CurrentThread::Get());
   base::RunLoop loop;
@@ -355,9 +359,15 @@ class SystemProxyManagerPolicyCredentialsBrowserTest
 
  protected:
   void SetPolicyCredentials(const std::string& username,
-                            const std::string& password) {
-    const std::string policy_value = base::StringPrintf(
-        kSystemProxyPolicyJson, username.c_str(), password.c_str());
+                            const std::string& password,
+                            const std::string& auth_schemes = "") {
+    const std::string schemes_json_entry =
+        auth_schemes.empty()
+            ? ""
+            : base::StringPrintf(kAuthSchemesPolicyEntry, auth_schemes.c_str());
+    const std::string policy_value =
+        base::StringPrintf(kSystemProxyPolicyJson, username.c_str(),
+                           password.c_str(), schemes_json_entry.c_str());
     enterprise_management::ChromeDeviceSettingsProto& proto(
         policy_helper_.device_policy()->payload());
     proto.mutable_system_proxy_settings()->set_system_proxy_settings(
@@ -415,12 +425,20 @@ class SystemProxyManagerPolicyCredentialsBrowserTest
                                                      *network);
   }
 
-  void ExpectSystemCredentialsSent(const std::string& username,
-                                   const std::string& password) {
+  void ExpectSystemCredentialsSent(
+      const std::string& username,
+      const std::string& password,
+      const std::vector<std::string>& auth_schemes = {}) {
     system_proxy::SetAuthenticationDetailsRequest request =
         client_test_interface()->GetLastAuthenticationDetailsRequest();
     EXPECT_EQ(username, request.credentials().username());
     EXPECT_EQ(password, request.credentials().password());
+    ASSERT_EQ(auth_schemes.size(),
+              request.credentials().policy_credentials_auth_schemes().size());
+    for (int i = 0; i < auth_schemes.size(); ++i) {
+      EXPECT_EQ(request.credentials().policy_credentials_auth_schemes()[i],
+                auth_schemes[i]);
+    }
     EXPECT_EQ(system_proxy::TrafficOrigin::SYSTEM, request.traffic_type());
   }
 
@@ -573,4 +591,28 @@ IN_PROC_BROWSER_TEST_F(SystemProxyManagerPolicyCredentialsBrowserTest,
   ExpectSystemCredentialsSent("", "");
 }
 
+// Tests that the SystemProxyManager forwards the authentication schemes set by
+// policy to System-proxy via D-Bus.
+IN_PROC_BROWSER_TEST_F(SystemProxyManagerPolicyCredentialsBrowserTest,
+                       PolicySetAuthSchemes) {
+  int set_auth_details_call_count = 0;
+  SetPolicyCredentials(kUsername, kPassword, R"("basic","digest")");
+  EXPECT_EQ(++set_auth_details_call_count,
+            client_test_interface()->GetSetAuthenticationDetailsCallCount());
+  ExpectSystemCredentialsSent("", "", {"basic", "digest"});
+  DisconnectNetworkService(kDefaultServicePath);
+  // Configure a proxy via user ONC policy and expect that credentials were
+  // forwarded to System-proxy.
+  SetOncPolicy(kONCPolicyWifi0Proxy, policy::POLICY_SCOPE_USER);
+  ConnectWifiNetworkService(kWifiServicePath, kWifiGuid, kWifiSsid);
+  EXPECT_EQ(++set_auth_details_call_count,
+            client_test_interface()->GetSetAuthenticationDetailsCallCount());
+  ExpectSystemCredentialsSent(kUsername, kPassword, {"basic", "digest"});
+
+  SetPolicyCredentials(kUsername, kPassword, R"("ntlm")");
+  EXPECT_EQ(++set_auth_details_call_count,
+            client_test_interface()->GetSetAuthenticationDetailsCallCount());
+  ExpectSystemCredentialsSent(kUsername, kPassword, {"ntlm"});
+}
+
 }  // namespace policy

chrome/test/data/policy/policy_test_cases.json

@@ -7884,7 +7884,8 @@
        "SystemProxySettings": {
         "system_proxy_enabled": "true",
         "system_services_username": "test_user",
-        "system_services_password": "0000"
+        "system_services_password": "0000",
+        "policy_credentials_auth_schemes": ["basic","digest"]
        }
       },
       "prefs": {

chromeos/settings/cros_settings_names.cc

@@ -482,6 +482,8 @@ const char kSystemProxySettingsKeySystemServicesUsername[] =
     "system_services_username";
 const char kSystemProxySettingsKeySystemServicesPassword[] =
     "system_services_password";
+const char kSystemProxySettingsKeyAuthSchemes[] =
+    "policy_credentials_auth_schemes";
 
 // An enum pref that indicates whether adb sideloading is allowed on this device
 const char kDeviceCrostiniArcAdbSideloadingAllowed[] =

chromeos/settings/cros_settings_names.h

@@ -273,6 +273,8 @@ COMPONENT_EXPORT(CHROMEOS_SETTINGS)
 extern const char kSystemProxySettingsKeySystemServicesUsername[];
 COMPONENT_EXPORT(CHROMEOS_SETTINGS)
 extern const char kSystemProxySettingsKeySystemServicesPassword[];
+COMPONENT_EXPORT(CHROMEOS_SETTINGS)
+extern const char kSystemProxySettingsKeyAuthSchemes[];
 
 COMPONENT_EXPORT(CHROMEOS_SETTINGS)
 extern const char kDeviceCrostiniArcAdbSideloadingAllowed[];

components/policy/resources/policy_templates.json

@@ -22366,6 +22366,22 @@
             'type': 'string',
             'sensitiveValue': True,
           },
+          'policy_credentials_auth_schemes': {
+            'description': '''The authentication schemes for which the policy credentials can be applied. Can be one of:
+            * basic
+            * digest
+            * ntlm
+            Leaving this option empty will allow all three schemes to be used.''',
+            'type': 'array',
+            'items' : {
+              'type': 'string',
+              'enum': [
+                'basic',
+                'digest',
+                'ntlm',
+              ],
+            },
+          },
         },
       },
       'features': {
@@ -22375,7 +22391,8 @@
       'example_value': {
         'system_proxy_enabled': True,
         'system_services_username': 'test_user',
-        'system_services_password': '0000'
+        'system_services_password': '0000',
+        'policy_credentials_auth_schemes': ['basic','ntlm']
       },
       'caption': '''Configures System-proxy service for <ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph>.''',
       'tags': [],