system-proxy: Get credentials from NetworkService

This CL implements requesting proxy authentication credentials from the
NetworkService instance of the primary user Profile's default
StoragePartition when System-proxy signals that it requires credentials.

System-proxy is a new system service on CrOS that performs
authentication and connection setup for CrOS services and ARC apps
that are proxy aware, but can't perform the authentication challenge
themselves.

Bug:1042642
Test:unittests, services_unittests, manually on DUT

Change-Id: I1d79da46ee9537c75131ff5e734fb39e525e0d5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2279193
Commit-Queue: Andreea-Elena Costinas <acostinas@google.com>
Reviewed-by: Eric Roman <eroman@chromium.org>
Reviewed-by: Andreea-Elena Costinas <acostinas@google.com>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Omar Morsi <omorsi@google.com>
Cr-Commit-Position: refs/heads/master@{#790345}

chrome/browser/chromeos/policy/system_proxy_manager.cc

@@ -20,7 +20,12 @@
 #include "components/prefs/pref_service.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
+#include "content/public/browser/storage_partition.h"
+#include "net/base/host_port_pair.h"
+#include "net/base/proxy_server.h"
 #include "net/http/http_auth_scheme.h"
+#include "net/http/http_util.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 
 namespace {
 const char kSystemProxyService[] = "system-proxy-service";
@@ -205,11 +210,33 @@ void SystemProxyManager::OnAuthenticationRequired(
     const system_proxy::AuthenticationRequiredDetails& details) {
   system_proxy::ProtectionSpace protection_space =
       details.proxy_protection_space();
+  if (!primary_profile_) {
+    LookupProxyAuthCredentialsCallback(protection_space,
+                                       /* credentials = */ base::nullopt);
+    return;
+  }
 
-  // TODO(acostinas, crbug.com/1098216): Get credentials from the network
-  // service.
-  LookupProxyAuthCredentialsCallback(protection_space,
-                                     /* credentials = */ base::nullopt);
+  // TODO(acostinas,chromium:1104818) |protection_space.origin()| is in a
+  // URI-like format which may be incompatible between Chrome and libcurl, which
+  // is used on the Chrome OS side. We should change |origin()| to be a PAC
+  // string (a more "standard" way of representing proxies) and call
+  // |FromPacString()| to create |proxy_server|.
+  net::ProxyServer proxy_server = net::ProxyServer::FromURI(
+      protection_space.origin(), net::ProxyServer::Scheme::SCHEME_HTTP);
+
+  if (!proxy_server.is_valid()) {
+    LookupProxyAuthCredentialsCallback(protection_space,
+                                       /* credentials = */ base::nullopt);
+    return;
+  }
+  content::BrowserContext::GetDefaultStoragePartition(primary_profile_)
+      ->GetNetworkContext()
+      ->LookupProxyAuthCredentials(
+          proxy_server, protection_space.scheme(),
+          net::HttpUtil::Unquote(protection_space.realm()),
+          base::BindOnce(
+              &SystemProxyManager::LookupProxyAuthCredentialsCallback,
+              weak_factory_.GetWeakPtr(), protection_space));
 }
 
 void SystemProxyManager::LookupProxyAuthCredentialsCallback(

chrome/browser/chromeos/policy/system_proxy_manager.h

@@ -11,6 +11,7 @@
 #include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chromeos/dbus/system_proxy/system_proxy_service.pb.h"
 #include "net/base/auth.h"
@@ -70,8 +71,9 @@ class SystemProxyManager {
   // This function is called when the |WorkerActive| dbus signal is received.
   void OnWorkerActive(const system_proxy::WorkerActiveSignalDetails& details);
 
-  // This function is called when the |AuthenticationRequired| dbus signal is
-  // received.
+  // Requests from the NetworkService the user credentials associated with the
+  // protection space specified in |details|. This function is called when the
+  // |AuthenticationRequired| dbus signal is received.
   void OnAuthenticationRequired(
       const system_proxy::AuthenticationRequiredDetails& details);
 

chrome/browser/chromeos/policy/system_proxy_manager_unittest.cc

@@ -17,7 +17,19 @@
 #include "chromeos/dbus/system_proxy/system_proxy_client.h"
 #include "chromeos/dbus/system_proxy/system_proxy_service.pb.h"
 #include "components/prefs/pref_service.h"
+#include "content/public/browser/network_service_instance.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_utils.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "net/http/http_auth.h"
+#include "net/http/http_auth_cache.h"
+#include "net/http/http_network_session.h"
+#include "net/http/http_transaction_factory.h"
+#include "net/url_request/url_request_context.h"
+#include "services/network/network_context.h"
+#include "services/network/network_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -28,13 +40,40 @@ using testing::WithArg;
 namespace {
 constexpr char kSystemServicesUsername[] = "test_username";
 constexpr char kSystemServicesPassword[] = "test_password";
+constexpr char kBrowserUsername[] = "browser_username";
+constexpr char kBrowserPassword[] = "browser_password";
 constexpr char kKerberosActivePrincipalName[] = "kerberos_princ_name";
 constexpr char kProxyAuthUrl[] = "http://example.com:3128";
+constexpr char kProxyAuthEmptyPath[] = "http://example.com:3128/";
 constexpr char kRealm[] = "My proxy";
-constexpr char kScheme[] = "BaSiC";
+constexpr char kScheme[] = "dIgEsT";
+constexpr char kProxyAuthChallenge[] = "challenge";
+
+std::unique_ptr<network::NetworkContext>
+CreateNetworkContextForDefaultStoragePartition(
+    network::NetworkService* network_service,
+    content::BrowserContext* browser_context) {
+  mojo::PendingRemote<network::mojom::NetworkContext> network_context_remote;
+  auto network_context = std::make_unique<network::NetworkContext>(
+      network_service, network_context_remote.InitWithNewPipeAndPassReceiver(),
+      network::mojom::NetworkContextParams::New());
+  content::BrowserContext::GetDefaultStoragePartition(browser_context)
+      ->SetNetworkContextForTesting(std::move(network_context_remote));
+  return network_context;
+}
+
+network::NetworkService* GetNetworkService() {
+  content::GetNetworkService();
+  // Wait for the Network Service to initialize on the IO thread.
+  content::RunAllPendingInMessageLoop(content::BrowserThread::IO);
+  return network::NetworkService::GetNetworkServiceForTesting();
+}
+
 }  // namespace
 
 namespace policy {
+// TODO(acostinas, https://crbug.com/1102351) Replace RunUntilIdle() in tests
+// with RunLoop::Run() with explicit RunLoop::QuitClosure().
 class SystemProxyManagerTest : public testing::Test {
  public:
   SystemProxyManagerTest() : local_state_(TestingBrowserProcess::GetGlobal()) {}
@@ -158,7 +197,7 @@ TEST_F(SystemProxyManagerTest, KerberosConfig) {
 }
 
 // Tests that when no user is signed in, credential requests are resolved to a
-// d-bus call which sends back to System-proxy empty credentials for the
+// D-Bus call which sends back to System-proxy empty credentials for the
 // specified protection space.
 TEST_F(SystemProxyManagerTest, UserCredentialsRequiredNoUser) {
   SystemProxyManager system_proxy_manager(chromeos::CrosSettings::Get(),
@@ -190,4 +229,56 @@ TEST_F(SystemProxyManagerTest, UserCredentialsRequiredNoUser) {
   EXPECT_EQ("", request.credentials().username());
   EXPECT_EQ("", request.credentials().password());
 }
+
+// Tests that credential requests are resolved to a  D-Bus call which sends back
+// to System-proxy credentials acquired from the NetworkService.
+TEST_F(SystemProxyManagerTest, UserCredentialsRequestedFromNetworkService) {
+  SystemProxyManager system_proxy_manager(chromeos::CrosSettings::Get(),
+                                          local_state_.Get());
+  SetPolicy(true /* system_proxy_enabled */, "" /* system_services_username */,
+            "" /* system_services_password */);
+  system_proxy_manager.StartObservingPrimaryProfilePrefs(profile_.get());
+
+  // Setup the NetworkContext with credentials.
+  std::unique_ptr<network::NetworkContext> network_context =
+      CreateNetworkContextForDefaultStoragePartition(GetNetworkService(),
+                                                     profile_.get());
+  network_context->url_request_context()
+      ->http_transaction_factory()
+      ->GetSession()
+      ->http_auth_cache()
+      ->Add(GURL(kProxyAuthEmptyPath), net::HttpAuth::AUTH_PROXY, kRealm,
+            net::HttpAuth::AUTH_SCHEME_DIGEST, net::NetworkIsolationKey(),
+            kProxyAuthChallenge,
+            net::AuthCredentials(base::ASCIIToUTF16(kBrowserUsername),
+                                 base::ASCIIToUTF16(kBrowserPassword)),
+            std::string() /* path */);
+
+  system_proxy::ProtectionSpace protection_space;
+  protection_space.set_origin(kProxyAuthUrl);
+  protection_space.set_scheme(kScheme);
+  protection_space.set_realm(kRealm);
+
+  system_proxy::AuthenticationRequiredDetails details;
+  *details.mutable_proxy_protection_space() = protection_space;
+
+  EXPECT_EQ(1, client_test_interface()->GetSetAuthenticationDetailsCallCount());
+
+  client_test_interface()->SendAuthenticationRequiredSignal(details);
+  task_environment_.RunUntilIdle();
+
+  EXPECT_EQ(2, client_test_interface()->GetSetAuthenticationDetailsCallCount());
+
+  system_proxy::SetAuthenticationDetailsRequest request =
+      client_test_interface()->GetLastAuthenticationDetailsRequest();
+
+  ASSERT_TRUE(request.has_protection_space());
+  EXPECT_EQ(protection_space.SerializeAsString(),
+            request.protection_space().SerializeAsString());
+
+  ASSERT_TRUE(request.has_credentials());
+  EXPECT_EQ(kBrowserUsername, request.credentials().username());
+  EXPECT_EQ(kBrowserPassword, request.credentials().password());
+  system_proxy_manager.StopObservingPrimaryProfilePrefs();
+}
 }  // namespace policy

services/network/network_context.cc

@@ -5,6 +5,7 @@
 #include "services/network/network_context.h"
 
 #include <memory>
+#include <string>
 #include <utility>
 
 #include "base/barrier_closure.h"
@@ -1723,6 +1724,41 @@ void NetworkContext::LookupServerBasicAuthCredentials(
     std::move(callback).Run(base::nullopt);
 }
 
+#if defined(OS_CHROMEOS)
+void NetworkContext::LookupProxyAuthCredentials(
+    const net::ProxyServer& proxy_server,
+    const std::string& auth_scheme,
+    const std::string& realm,
+    LookupProxyAuthCredentialsCallback callback) {
+  net::HttpAuth::Scheme net_scheme =
+      net::HttpAuth::StringToScheme(base::ToLowerASCII(auth_scheme));
+  if (net_scheme == net::HttpAuth::Scheme::AUTH_SCHEME_MAX) {
+    std::move(callback).Run(base::nullopt);
+    return;
+  }
+  net::HttpAuthCache* http_auth_cache =
+      url_request_context_->http_transaction_factory()
+          ->GetSession()
+          ->http_auth_cache();
+  const char* scheme = proxy_server.is_https() ? "https://" : "http://";
+  GURL proxy_url(scheme + proxy_server.host_port_pair().ToString());
+  if (!proxy_url.is_valid()) {
+    std::move(callback).Run(base::nullopt);
+    return;
+  }
+
+  //  Unlike server credentials, proxy credentials are not keyed on
+  //  NetworkIsolationKey.
+  net::HttpAuthCache::Entry* entry =
+      http_auth_cache->Lookup(proxy_url, net::HttpAuth::AUTH_PROXY, realm,
+                              net_scheme, net::NetworkIsolationKey());
+  if (entry)
+    std::move(callback).Run(entry->credentials());
+  else
+    std::move(callback).Run(base::nullopt);
+}
+#endif
+
 const net::HttpAuthPreferences* NetworkContext::GetHttpAuthPreferences() const {
   return &http_auth_merged_preferences_;
 }

services/network/network_context.h

@@ -407,6 +407,13 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
       const GURL& url,
       const net::NetworkIsolationKey& network_isolation_key,
       LookupServerBasicAuthCredentialsCallback callback) override;
+#if defined(OS_CHROMEOS)
+  void LookupProxyAuthCredentials(
+      const net::ProxyServer& proxy_server,
+      const std::string& auth_scheme,
+      const std::string& realm,
+      LookupProxyAuthCredentialsCallback callback) override;
+#endif
   void GetOriginPolicyManager(
       mojo::PendingReceiver<mojom::OriginPolicyManager> receiver) override;
 

services/network/network_context_unittest.cc

@@ -1905,6 +1905,116 @@ TEST_F(NetworkContextTest, LookupServerBasicAuthCredentials) {
   EXPECT_FALSE(result.has_value());
 }
 
+#if defined(OS_CHROMEOS)
+base::Optional<net::AuthCredentials> GetProxyAuthCredentials(
+    NetworkContext* network_context,
+    const net::ProxyServer& proxy_server,
+    const std::string& scheme,
+    const std::string& realm) {
+  base::RunLoop run_loop;
+  base::Optional<net::AuthCredentials> result;
+  network_context->LookupProxyAuthCredentials(
+      proxy_server, scheme, realm,
+      base::BindLambdaForTesting(
+          [&](const base::Optional<net::AuthCredentials>& credentials) {
+            result = credentials;
+            run_loop.Quit();
+          }));
+  run_loop.Run();
+  return result;
+}
+
+TEST_F(NetworkContextTest, LookupProxyAuthCredentials) {
+  GURL http_proxy("http://bar.test:1080");
+  GURL https_proxy("https://bar.test:443");
+  GURL http_proxy2("http://bar.test:443");
+  GURL foo_proxy("foo://bar.test:1080");
+  GURL server_origin("http://foo.test:3128");
+
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateContextParams());
+  network_context->SetSplitAuthCacheByNetworkIsolationKey(true);
+  net::HttpAuthCache* cache = network_context->url_request_context()
+                                  ->http_transaction_factory()
+                                  ->GetSession()
+                                  ->http_auth_cache();
+
+  base::string16 user = base::ASCIIToUTF16("user");
+  base::string16 password = base::ASCIIToUTF16("pass");
+  cache->Add(http_proxy, net::HttpAuth::AUTH_PROXY, "Realm",
+             net::HttpAuth::AUTH_SCHEME_BASIC, net::NetworkIsolationKey(),
+             "basic realm=Realm", net::AuthCredentials(user, password),
+             /* path = */ "");
+  cache->Add(https_proxy, net::HttpAuth::AUTH_PROXY, "Realm",
+             net::HttpAuth::AUTH_SCHEME_BASIC, net::NetworkIsolationKey(),
+             "basic realm=Realm", net::AuthCredentials(user, password),
+             /* path = */ "");
+  cache->Add(server_origin, net::HttpAuth::AUTH_SERVER, "Realm",
+             net::HttpAuth::AUTH_SCHEME_BASIC, net::NetworkIsolationKey(),
+             "basic realm=Realm", net::AuthCredentials(user, password),
+             /* path = */ "/");
+  base::Optional<net::AuthCredentials> result = GetProxyAuthCredentials(
+      network_context.get(),
+      net::ProxyServer(net::ProxyServer::Scheme::SCHEME_HTTP,
+                       net::HostPortPair::FromURL(http_proxy)),
+      "bAsIc", "Realm");
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(user, result->username());
+  EXPECT_EQ(password, result->password());
+
+  result = GetProxyAuthCredentials(
+      network_context.get(),
+      net::ProxyServer(net::ProxyServer::Scheme::SCHEME_HTTPS,
+                       net::HostPortPair::FromURL(https_proxy)),
+      "bAsIc", "Realm");
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(user, result->username());
+  EXPECT_EQ(password, result->password());
+
+  // Check that the proxy scheme is taken into account when looking for
+  // credentials
+  result = GetProxyAuthCredentials(
+      network_context.get(),
+      net::ProxyServer(net::ProxyServer::Scheme::SCHEME_HTTP,
+                       net::HostPortPair::FromURL(http_proxy2)),
+      "basic", "Realm");
+  EXPECT_FALSE(result.has_value());
+
+  // Check that the proxy authentication method is taken into account when
+  // looking for credentials
+  result = GetProxyAuthCredentials(
+      network_context.get(),
+      net::ProxyServer(net::ProxyServer::Scheme::SCHEME_HTTP,
+                       net::HostPortPair::FromURL(http_proxy)),
+      "digest", "Realm");
+  EXPECT_FALSE(result.has_value());
+
+  // Check that the realm is taken into account when looking for credentials
+  result = GetProxyAuthCredentials(
+      network_context.get(),
+      net::ProxyServer(net::ProxyServer::Scheme::SCHEME_HTTP,
+                       net::HostPortPair::FromURL(http_proxy)),
+      "basic", "Realm 2");
+  EXPECT_FALSE(result.has_value());
+
+  // All non-https proxies are cached as "http://" proxies
+  result = GetProxyAuthCredentials(
+      network_context.get(),
+      net::ProxyServer(net::ProxyServer::Scheme::SCHEME_HTTP,
+                       net::HostPortPair::FromURL(foo_proxy)),
+      "basic", "Realm");
+  EXPECT_FALSE(result.has_value());
+
+  // Server credentials should not be returned
+  result = GetProxyAuthCredentials(
+      network_context.get(),
+      net::ProxyServer(net::ProxyServer::Scheme::SCHEME_HTTP,
+                       net::HostPortPair::FromURL(server_origin)),
+      "basic", "Realm");
+  EXPECT_FALSE(result.has_value());
+}
+#endif
+
 #if BUILDFLAG(ENABLE_REPORTING)
 TEST_F(NetworkContextTest, ClearReportingCacheReports) {
   auto reporting_context = std::make_unique<net::TestReportingContext>(

services/network/public/mojom/network_context.mojom

@@ -1376,6 +1376,16 @@ interface NetworkContext {
       NetworkIsolationKey network_isolation_key)
       => (AuthCredentials? credentials);
 
+  // Looks up the proxy authentication credentials associated with
+  // |proxy_server|, |auth_scheme| and |realm| in the HttpAuthCache.
+  // |auth_scheme| is the authentication scheme of the challenge and it's
+  // specified as a case-insensitive string. Unlike server credentials, proxy
+  // credentials are not keyed on NetworkIsolationKey.
+  [EnableIf=is_chromeos]
+  LookupProxyAuthCredentials(proxy_resolver.mojom.ProxyServer proxy_server,
+      string auth_scheme, string realm)
+      => (AuthCredentials? credentials);
+
   [Sync]
   // Enables the checking of static PKP records.
   EnableStaticKeyPinningForTesting() => ();

services/network/test/test_network_context.h

@@ -264,6 +264,13 @@ class TestNetworkContext : public mojom::NetworkContext {
       const GURL& url,
       const net::NetworkIsolationKey& network_isolation_key,
       LookupServerBasicAuthCredentialsCallback callback) override {}
+#if defined(OS_CHROMEOS)
+  void LookupProxyAuthCredentials(
+      const net::ProxyServer& proxy_server,
+      const std::string& auth_scheme,
+      const std::string& realm,
+      LookupProxyAuthCredentialsCallback callback) override {}
+#endif
   void GetOriginPolicyManager(
       mojo::PendingReceiver<mojom::OriginPolicyManager> receiver) override {}
 };