NetworkIsolationKeys + PAC scripts PS2: ProxyResolverRequestClient

Make proxy_resolver::mojom::ProxyResolverRequestClient::ResolveDns
and proxy_resolver::mojom::ProxyResolverFactoryRequestClient::ResolveDns
take in a NetworkIsolationKey.

Unfortunately, the only production implementation of these classes only
has integration tests, so this CL adds no tests - I'll add them in the
CL where I update net::ProxyResolver to take a NetworkIsolationKey,
which will likely be a huge CL.

Bug: 1021661
Change-Id: Iaefb62fa84cc50bfc90a7f48ada6f77bba3d65d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903760Reviewed-by: Eric Roman <eroman@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714920}

chrome/browser/net/chrome_mojo_proxy_resolver_factory_browsertest.cc

@@ -71,6 +71,7 @@ class DumbProxyResolverFactoryRequestClient
   void ResolveDns(
       const std::string& hostname,
       net::ProxyResolveDnsOperation operation,
+      const net::NetworkIsolationKey& network_isolation_key,
       mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
           client) override {}
 

services/network/proxy_resolver_factory_mojo.cc

@@ -39,6 +39,10 @@
 #include "services/network/mojo_host_resolver_impl.h"
 #include "services/proxy_resolver/public/mojom/proxy_resolver.mojom.h"
 
+namespace net {
+class NetworkIsolationKey;
+}
+
 namespace network {
 
 namespace {
@@ -119,6 +123,7 @@ class ClientMixin : public ClientInterface {
   void ResolveDns(
       const std::string& hostname,
       net::ProxyResolveDnsOperation operation,
+      const net::NetworkIsolationKey& network_isolation_key,
       mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
           client) override {
     bool is_ex = operation == net::ProxyResolveDnsOperation::DNS_RESOLVE_EX ||
@@ -131,8 +136,7 @@ class ClientMixin : public ClientInterface {
           base::BindOnce(&DoMyIpAddressOnWorker, is_ex, std::move(client)));
     } else {
       // Request was for dnsResolve() or dnsResolveEx().
-      // TODO(mmenke): Pass in a NetworkIsolationKey().
-      host_resolver_.Resolve(hostname, net::NetworkIsolationKey(), is_ex,
+      host_resolver_.Resolve(hostname, network_isolation_key, is_ex,
                              std::move(client));
     }
   }

services/network/proxy_resolver_factory_mojo_unittest.cc

@@ -286,7 +286,7 @@ void MockMojoProxyResolver::GetProxyForUrl(
       ignore_result(dns_client.InitWithNewPipeAndPassReceiver());
       client->ResolveDns(url.host(),
                          net::ProxyResolveDnsOperation::DNS_RESOLVE_EX,
-                         std::move(dns_client));
+                         net::NetworkIsolationKey(), std::move(dns_client));
       blocked_clients_.push_back(std::move(client));
       break;
     }
@@ -466,7 +466,7 @@ void MockMojoProxyResolverFactory::CreateResolver(
       ignore_result(dns_client.InitWithNewPipeAndPassReceiver());
       client->ResolveDns(pac_script,
                          net::ProxyResolveDnsOperation::DNS_RESOLVE_EX,
-                         std::move(dns_client));
+                         net::NetworkIsolationKey(), std::move(dns_client));
       blocked_clients_.push_back(std::move(client));
       break;
     }

services/network/public/mojom/BUILD.gn

@@ -23,6 +23,28 @@ mojom("mojom_ip_address") {
   ]
 }
 
+# As with mojom_ip_address, this is a separate target to avoid a circular
+# dependency, which comes from the fact that proxy_resolver service uses
+# this interfaces, and the network service uses the proxy_resolver service.
+# Separate from |mojom_ip_address| because a bunch of other consumers are
+# depending on that and don't need this.
+mojom("mojom_network_isolation_key") {
+  generate_java = true
+  sources = [
+    "network_isolation_key.mojom",
+  ]
+
+  public_deps = [
+    "//url/mojom:url_mojom_origin",
+  ]
+
+  if (!is_ios) {
+    export_class_attribute_blink = "BLINK_PLATFORM_EXPORT"
+    export_define_blink = "BLINK_PLATFORM_IMPLEMENTATION=1"
+    export_header_blink = "third_party/blink/public/platform/web_common.h"
+  }
+}
+
 # These interfaces are put in their own target to avoid a circular dependency,
 # which comes from the fact that the typemap for url_loader.mojom
 # (ResourceRequestBody) uses these interfaces.
@@ -97,7 +119,6 @@ mojom("mojom") {
     "network_change_manager.mojom",
     "network_context.mojom",
     "network_interface.mojom",
-    "network_isolation_key.mojom",
     "network_quality_estimator_manager.mojom",
     "network_service.mojom",
     "network_service_test.mojom",
@@ -124,6 +145,7 @@ mojom("mojom") {
   public_deps = [
     ":data_pipe_interfaces",
     ":mojom_ip_address",
+    ":mojom_network_isolation_key",
     ":mutable_network_traffic_annotation_interface",
     ":websocket_mojom",
     "//components/content_settings/core/common:mojo_bindings",

services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings.h

@@ -14,6 +14,7 @@
 #include "base/threading/thread_checker.h"
 #include "net/base/address_family.h"
 #include "net/base/host_port_pair.h"
+#include "net/base/network_isolation_key.h"
 #include "net/dns/host_resolver.h"
 #include "net/log/net_log_with_source.h"
 #include "net/proxy_resolution/proxy_host_resolver.h"
@@ -67,7 +68,9 @@ class MojoProxyResolverV8TracingBindings
       net::ProxyResolveDnsOperation operation,
       mojo::PendingRemote<mojom::HostResolverRequestClient> client) override {
     DCHECK(thread_checker_.CalledOnValidThread());
-    client_->ResolveDns(hostname, operation, std::move(client));
+    // TODO(mmenke): Pass in a NetworkIsolationKey.
+    client_->ResolveDns(hostname, operation, net::NetworkIsolationKey(),
+                        std::move(client));
   }
 
   base::ThreadChecker thread_checker_;

services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings_unittest.cc

@@ -27,6 +27,7 @@ class MojoProxyResolverV8TracingBindingsTest : public testing::Test {
   void ResolveDns(
       const std::string& hostname,
       net::ProxyResolveDnsOperation operation,
+      const net::NetworkIsolationKey& network_isolation_key,
       mojo::PendingRemote<mojom::HostResolverRequestClient> client) {}
 
  protected:

services/proxy_resolver/proxy_resolver_factory_impl_unittest.cc

@@ -145,6 +145,7 @@ class ProxyResolverFactoryImplTest
   void ResolveDns(
       const std::string& hostname,
       net::ProxyResolveDnsOperation operation,
+      const net::NetworkIsolationKey& network_isolation_key,
       mojo::PendingRemote<mojom::HostResolverRequestClient> client) override {}
 
   void set_idle_callback(base::OnceClosure callback) {

services/proxy_resolver/proxy_resolver_impl_unittest.cc

@@ -56,6 +56,7 @@ class TestRequestClient : public mojom::ProxyResolverRequestClient {
   void ResolveDns(
       const std::string& hostname,
       net::ProxyResolveDnsOperation operation,
+      const net::NetworkIsolationKey& network_isolation_key,
       mojo::PendingRemote<mojom::HostResolverRequestClient> client) override;
 
   void OnDisconnect();
@@ -101,6 +102,7 @@ void TestRequestClient::OnError(int32_t line_number,
 void TestRequestClient::ResolveDns(
     const std::string& hostname,
     net::ProxyResolveDnsOperation operation,
+    const net::NetworkIsolationKey& network_isolation_key,
     mojo::PendingRemote<mojom::HostResolverRequestClient> client) {}
 
 void TestRequestClient::OnDisconnect() {

services/proxy_resolver/public/mojom/BUILD.gn

@@ -11,6 +11,7 @@ mojom("mojom") {
   ]
   deps = [
     "//services/network/public/mojom:mojom_ip_address",
+    "//services/network/public/mojom:mojom_network_isolation_key",
     "//url/mojom:url_mojom_gurl",
   ]
 

services/proxy_resolver/public/mojom/proxy_resolver.mojom

@@ -8,6 +8,7 @@
 module proxy_resolver.mojom;
 
 import "services/network/public/mojom/ip_address.mojom";
+import "services/network/public/mojom/network_isolation_key.mojom";
 import "url/mojom/url.mojom";
 
 // Mirror of net::ProxyResolveDnsOperation.
@@ -62,8 +63,16 @@ interface ProxyResolverRequestClient {
   Alert(string error);
   OnError(int32 line_number, string error);
 
+  // Does a DNS lookup for |host|, and invokes |client|'s ReportResult() method
+  // with the result.
+  //
+  // |network_isolation_key| is the NetworkIsolationKey used by the DNS cache
+  // to separate requests made in different contexts. An IP address received
+  // from a resolution using one NetworkIsolationKey must not be used for a
+  // request made with a different NetworkIsolationKey.
   ResolveDns(string host,
              HostResolveOperation operation,
+             network.mojom.NetworkIsolationKey network_isolation_key,
              pending_remote<HostResolverRequestClient> client);
 };
 
@@ -86,5 +95,6 @@ interface ProxyResolverFactoryRequestClient {
 
   ResolveDns(string host,
              HostResolveOperation operation,
+             network.mojom.NetworkIsolationKey network_isolation_key,
              pending_remote<HostResolverRequestClient> client);
 };