Make IsIPv6Reachable comply with network-bound URLRequestContexts

Currently, IsIPv6Reachable checks if the current default network is a
WiFi network. This is not correct for network-bound URLRequestContexts
where the default network network won't necessarily be used for DNS
lookups.

Allow instead to also create network-bound HostResolverManagers through
CreateNetworkBoundHostResolverManager. This is to allow each
network-bound URLRequestContext to use its own standalone
HostResolverManager (as agreed in crbug.com/1309094).

Note: network-bound HostResolverManagers don't need to listen to network
changes event for the same reason as other network-bound entities.

To keep things working this CL also updates network-bound lookups
unittests and URLRequestContextBuilder::BindToNetwork to make use of
CreateNetworkBoundHostResolverManager and
CreateStandaloneNetworkBoundResolver respectively.

Bug: 1309094
Change-Id: I84e00c95692255c34866b4babd546b14bacaec63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3555673
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Stefano Duo <stefanoduo@google.com>
Cr-Commit-Position: refs/heads/main@{#988896}

net/dns/context_host_resolver_unittest.cc

@@ -19,6 +19,7 @@
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_address.h"
 #include "net/base/ip_endpoint.h"
+#include "net/base/mock_network_change_notifier.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_isolation_key.h"
 #include "net/base/schemeful_site.h"
@@ -841,6 +842,12 @@ class NetworkAwareHostResolverProc : public HostResolverProc {
 };
 
 TEST_F(ContextHostResolverTest, ExistingNetworkBoundLookup) {
+#if BUILDFLAG(IS_ANDROID)
+  auto scoped_mock_network_change_notifier =
+      std::make_unique<test::ScopedMockNetworkChangeNotifier>();
+  scoped_mock_network_change_notifier->mock_network_change_notifier()
+      ->ForceNetworkHandlesSupported();
+
   const url::SchemeHostPort host(url::kHttpsScheme, "example.com",
                                  NetworkAwareHostResolverProc::kPort);
   scoped_refptr<NetworkAwareHostResolverProc> resolver_proc =
@@ -852,11 +859,16 @@ TEST_F(ContextHostResolverTest, ExistingNetworkBoundLookup) {
   // Resolve with `network` == context.GetTargetNetwork(). Confirm that we do
   // indeed receive the IP address associated with that network.
   for (const auto& iter : NetworkAwareHostResolverProc::kResults) {
-    auto context = CreateTestURLRequestContextBuilder()->Build();
+    auto network = iter.first;
+    auto expected_ipv4 = iter.second;
     auto resolve_context = std::make_unique<NetworkBoundResolveContext>(
-        context.get(), false /* enable_caching */, iter.first);
+        nullptr /* url_request_context */, false /* enable_caching */, network);
+    // DNS lookups originated from network-bound ResolveContexts must be
+    // resolved through a HostResolverManager bound to the same network.
+    auto manager = HostResolverManager::CreateNetworkBoundHostResolverManager(
+        HostResolver::ManagerOptions(), network, nullptr /* net_log */);
     auto resolver = std::make_unique<ContextHostResolver>(
-        manager_.get(), std::move(resolve_context));
+        manager.get(), std::move(resolve_context));
     std::unique_ptr<HostResolver::ResolveHostRequest> request =
         resolver->CreateRequest(host, NetworkIsolationKey(), NetLogWithSource(),
                                 absl::nullopt);
@@ -868,8 +880,12 @@ TEST_F(ContextHostResolverTest, ExistingNetworkBoundLookup) {
     ASSERT_EQ(1u, request->GetAddressResults()->endpoints().size());
     EXPECT_THAT(request->GetAddressResults()->endpoints(),
                 testing::ElementsAre(
-                    NetworkAwareHostResolverProc::ToIPEndPoint(iter.second)));
+                    NetworkAwareHostResolverProc::ToIPEndPoint(expected_ipv4)));
   }
+#else   // !BUILDFLAG(IS_ANDROID)
+  GTEST_SKIP()
+      << "Network-bound HostResolverManager are supported only on Android.";
+#endif  // BUILDFLAG(IS_ANDROID)
 }
 
 TEST_F(ContextHostResolverTest, NotExistingNetworkBoundLookup) {
@@ -883,9 +899,8 @@ TEST_F(ContextHostResolverTest, NotExistingNetworkBoundLookup) {
   // Non-bound ResolveContexts should end up with a call to Resolve with
   // `network` == kInvalidNetwork, which NetworkAwareHostResolverProc fails to
   // resolve.
-  auto context = CreateTestURLRequestContextBuilder()->Build();
   auto resolve_context = std::make_unique<ResolveContext>(
-      context.get(), false /* enable_caching */);
+      nullptr /* url_request_context */, false /* enable_caching */);
   auto resolver = std::make_unique<ContextHostResolver>(
       manager_.get(), std::move(resolve_context));
   std::unique_ptr<HostResolver::ResolveHostRequest> request =

net/dns/host_resolver.cc

@@ -30,6 +30,11 @@
 #include "net/dns/resolve_context.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
+#if BUILDFLAG(IS_ANDROID)
+#include "base/android/build_info.h"
+#include "net/android/network_library.h"
+#endif  // BUILDFLAG(IS_ANDROID)
+
 namespace net {
 
 namespace {
@@ -223,6 +228,36 @@ HostResolver::CreateStandaloneContextResolver(
       std::move(resolve_context));
 }
 
+// static
+std::unique_ptr<HostResolver>
+HostResolver::CreateStandaloneNetworkBoundResolver(
+    NetLog* net_log,
+    NetworkChangeNotifier::NetworkHandle target_network,
+    absl::optional<ManagerOptions> options,
+    base::StringPiece host_mapping_rules,
+    bool enable_caching) {
+#if BUILDFLAG(IS_ANDROID)
+  auto resolve_context = std::make_unique<ResolveContext>(
+      nullptr /*url_request_context */, enable_caching);
+
+  // Currently, only the system host resolver can perform lookups for a
+  // specific network.
+  // TODO(crbug.com/1309094): Remove this once the built-in resolver can also do
+  // this.
+  auto manager_options = std::move(options).value_or(ManagerOptions());
+  manager_options.insecure_dns_client_enabled = false;
+  manager_options.additional_types_via_insecure_dns_enabled = false;
+
+  return std::make_unique<ContextHostResolver>(
+      HostResolverManager::CreateNetworkBoundHostResolverManager(
+          manager_options, target_network, net_log),
+      std::move(resolve_context));
+#else   // !BUILDFLAG(IS_ANDROID)
+  NOTIMPLEMENTED();
+  return nullptr;
+#endif  // BUILDFLAG(IS_ANDROID)
+}
+
 // static
 AddressFamily HostResolver::DnsQueryTypeSetToAddressFamily(
     DnsQueryTypeSet dns_query_types) {

net/dns/host_resolver.h

@@ -17,6 +17,7 @@
 #include "net/base/address_family.h"
 #include "net/base/completion_once_callback.h"
 #include "net/base/host_port_pair.h"
+#include "net/base/network_change_notifier.h"
 #include "net/base/network_isolation_key.h"
 #include "net/base/request_priority.h"
 #include "net/dns/host_cache.h"
@@ -419,6 +420,17 @@ class NET_EXPORT HostResolver {
       NetLog* net_log,
       absl::optional<ManagerOptions> options = absl::nullopt,
       bool enable_caching = true);
+  // Same, but bind the resolver to `target_network`: all lookups will be
+  // performed exclusively for `target_network`, lookups will fail if
+  // `target_network` disconnects. This can only be used by network-bound
+  // URLRequestContexts.
+  // Only implemented for Android starting from Marshmallow.
+  static std::unique_ptr<HostResolver> CreateStandaloneNetworkBoundResolver(
+      NetLog* net_log,
+      NetworkChangeNotifier::NetworkHandle network,
+      absl::optional<ManagerOptions> options = absl::nullopt,
+      base::StringPiece host_mapping_rules = "",
+      bool enable_caching = true);
 
   // Helpers for interacting with HostCache and ProcResolver.
   static AddressFamily DnsQueryTypeSetToAddressFamily(

net/dns/host_resolver_manager.cc

@@ -3086,10 +3086,23 @@ HostResolverManager::HostResolverManager(
     const HostResolver::ManagerOptions& options,
     SystemDnsConfigChangeNotifier* system_dns_config_notifier,
     NetLog* net_log)
+    : HostResolverManager(PassKey(),
+                          options,
+                          system_dns_config_notifier,
+                          NetworkChangeNotifier::kInvalidNetworkHandle,
+                          net_log) {}
+
+HostResolverManager::HostResolverManager(
+    base::PassKey<HostResolverManager>,
+    const HostResolver::ManagerOptions& options,
+    SystemDnsConfigChangeNotifier* system_dns_config_notifier,
+    NetworkChangeNotifier::NetworkHandle target_network,
+    NetLog* net_log)
     : max_queued_jobs_(0),
       proc_params_(nullptr, options.max_system_retry_attempts),
       net_log_(net_log),
       system_dns_config_notifier_(system_dns_config_notifier),
+      target_network_(target_network),
       check_ipv6_on_wifi_(options.check_ipv6_on_wifi),
       last_ipv6_probe_result_(true),
       additional_resolver_flags_(0),
@@ -3113,16 +3126,23 @@ HostResolverManager::HostResolverManager(
     BUILDFLAG(IS_FUCHSIA)
   RunLoopbackProbeJob();
 #endif
-  NetworkChangeNotifier::AddIPAddressObserver(this);
-  NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  // Network-bound HostResolverManagers don't need to act on network changes.
+  if (!IsBoundToNetwork()) {
+    NetworkChangeNotifier::AddIPAddressObserver(this);
+    NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  }
   if (system_dns_config_notifier_)
     system_dns_config_notifier_->AddObserver(this);
 #if BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_APPLE) && !BUILDFLAG(IS_OPENBSD) && \
     !BUILDFLAG(IS_ANDROID)
   EnsureDnsReloaderInit();
 #endif
 
-  OnConnectionTypeChanged(NetworkChangeNotifier::GetConnectionType());
+  auto connection_type =
+      IsBoundToNetwork()
+          ? NetworkChangeNotifier::GetNetworkConnectionType(target_network)
+          : NetworkChangeNotifier::GetConnectionType();
+  UpdateConnectionType(connection_type);
 
 #if defined(ENABLE_BUILT_IN_DNS)
   dns_client_ = DnsClient::CreateClient(net_log_);
@@ -3145,12 +3165,31 @@ HostResolverManager::~HostResolverManager() {
   // OnJobComplete will not start any new jobs.
   jobs_.clear();
 
-  NetworkChangeNotifier::RemoveIPAddressObserver(this);
-  NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
+  if (target_network_ == NetworkChangeNotifier::kInvalidNetworkHandle) {
+    NetworkChangeNotifier::RemoveIPAddressObserver(this);
+    NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
+  }
   if (system_dns_config_notifier_)
     system_dns_config_notifier_->RemoveObserver(this);
 }
 
+// static
+std::unique_ptr<HostResolverManager>
+HostResolverManager::CreateNetworkBoundHostResolverManager(
+    const HostResolver::ManagerOptions& options,
+    NetworkChangeNotifier::NetworkHandle target_network,
+    NetLog* net_log) {
+#if BUILDFLAG(IS_ANDROID)
+  DCHECK(NetworkChangeNotifier::AreNetworkHandlesSupported());
+  return std::make_unique<HostResolverManager>(
+      PassKey(), options, nullptr /* system_dns_config_notifier */,
+      target_network, net_log);
+#else   // !BUILDFLAG(IS_ANDROID)
+  NOTIMPLEMENTED();
+  return nullptr;
+#endif  // BUILDFLAG(IS_ANDROID)
+}
+
 std::unique_ptr<HostResolver::ResolveHostRequest>
 HostResolverManager::CreateRequest(
     absl::variant<url::SchemeHostPort, HostPortPair> host,
@@ -3162,6 +3201,8 @@ HostResolverManager::CreateRequest(
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(!invalidation_in_progress_);
 
+  DCHECK_EQ(resolve_context->GetTargetNetwork(), target_network_);
+
   // If required, ResolveContexts must register (via RegisterResolveContext())
   // before use to ensure cached data is invalidated on network and
   // configuration changes.
@@ -4038,13 +4079,25 @@ void HostResolverManager::GetEffectiveParametersForRequest(
   *out_effective_types = effective_types;
 }
 
+namespace {
+
+bool RequestWillUseWiFi(NetworkChangeNotifier::NetworkHandle network) {
+  NetworkChangeNotifier::ConnectionType connection_type;
+  if (network == NetworkChangeNotifier::kInvalidNetworkHandle)
+    connection_type = NetworkChangeNotifier::GetConnectionType();
+  else
+    connection_type = NetworkChangeNotifier::GetNetworkConnectionType(network);
+
+  return connection_type == NetworkChangeNotifier::CONNECTION_WIFI;
+}
+
+}  // namespace
+
 bool HostResolverManager::IsIPv6Reachable(const NetLogWithSource& net_log) {
-  // Don't bother checking if the device is on WiFi and IPv6 is assumed to not
-  // work on WiFi.
-  if (!check_ipv6_on_wifi_ && NetworkChangeNotifier::GetConnectionType() ==
-                                  NetworkChangeNotifier::CONNECTION_WIFI) {
+  // Don't bother checking if the request will use WiFi and IPv6 is assumed to
+  // not work on WiFi.
+  if (!check_ipv6_on_wifi_ && RequestWillUseWiFi(target_network_))
     return false;
-  }
 
   // Cache the result for kIPv6ProbePeriodMs (measured from after
   // IsGloballyReachable() completes).
@@ -4192,6 +4245,7 @@ void HostResolverManager::TryServingAllJobsFromHosts() {
 }
 
 void HostResolverManager::OnIPAddressChanged() {
+  DCHECK(!IsBoundToNetwork());
   last_ipv6_probe_time_ = base::TimeTicks();
   // Abandon all ProbeJobs.
   probe_weak_ptr_factory_.InvalidateWeakPtrs();
@@ -4206,23 +4260,13 @@ void HostResolverManager::OnIPAddressChanged() {
 
 void HostResolverManager::OnConnectionTypeChanged(
     NetworkChangeNotifier::ConnectionType type) {
-  proc_params_.unresponsive_delay =
-      GetTimeDeltaForConnectionTypeFromFieldTrialOrDefault(
-          "DnsUnresponsiveDelayMsByConnectionType",
-          ProcTaskParams::kDnsDefaultUnresponsiveDelay, type);
-
-  // Note that NetworkChangeNotifier always sends a CONNECTION_NONE notification
-  // before non-NONE notifications. This check therefore just ensures each
-  // connection change notification is handled once and has nothing to do with
-  // whether the change is to offline or online.
-  if (type == NetworkChangeNotifier::CONNECTION_NONE && dns_client_) {
-    dns_client_->ReplaceCurrentSession();
-    InvalidateCaches(true /* network_change */);
-  }
+  DCHECK(!IsBoundToNetwork());
+  UpdateConnectionType(type);
 }
 
 void HostResolverManager::OnSystemDnsConfigChanged(
     absl::optional<DnsConfig> config) {
+  DCHECK(!IsBoundToNetwork());
   // If tests have provided a catch-all DNS block and then disabled it, check
   // that we are not at risk of sending queries beyond the local network.
   if (HostResolverProc::GetDefault() && system_resolver_disabled_for_testing_ &&
@@ -4330,6 +4374,23 @@ void HostResolverManager::InvalidateCaches(bool network_change) {
 #endif
 }
 
+void HostResolverManager::UpdateConnectionType(
+    NetworkChangeNotifier::ConnectionType type) {
+  proc_params_.unresponsive_delay =
+      GetTimeDeltaForConnectionTypeFromFieldTrialOrDefault(
+          "DnsUnresponsiveDelayMsByConnectionType",
+          ProcTaskParams::kDnsDefaultUnresponsiveDelay, type);
+
+  // Note that NetworkChangeNotifier always sends a CONNECTION_NONE notification
+  // before non-NONE notifications. This check therefore just ensures each
+  // connection change notification is handled once and has nothing to do with
+  // whether the change is to offline or online.
+  if (type == NetworkChangeNotifier::CONNECTION_NONE && dns_client_) {
+    dns_client_->ReplaceCurrentSession();
+    InvalidateCaches(true /* network_change */);
+  }
+}
+
 std::unique_ptr<DnsProbeRunner> HostResolverManager::CreateDohProbeRunner(
     ResolveContext* resolve_context) {
   DCHECK(resolve_context);

net/dns/host_resolver_manager.h

@@ -101,6 +101,7 @@ class NET_EXPORT HostResolverManager
  public:
   using MdnsListener = HostResolver::MdnsListener;
   using ResolveHostParameters = HostResolver::ResolveHostParameters;
+  using PassKey = base::PassKey<HostResolverManager>;
 
   // Creates a HostResolver as specified by |options|. Blocking tasks are run in
   // ThreadPool.
@@ -126,6 +127,17 @@ class NET_EXPORT HostResolverManager
   // be called.
   ~HostResolverManager() override;
 
+  // Same as constructor above, but binds the HostResolverManager to
+  // `target_network`: all DNS requests will be performed for `target_network`
+  // only, requests will fail if `target_network` disconnects. Only
+  // HostResolvers bound to the same network will be able to use this.
+  // Only implemented for Android.
+  static std::unique_ptr<HostResolverManager>
+  CreateNetworkBoundHostResolverManager(
+      const HostResolver::ManagerOptions& options,
+      NetworkChangeNotifier::NetworkHandle target_network,
+      NetLog* net_log);
+
   // |resolve_context| must have already been added (via
   // RegisterResolveContext()). If |optional_parameters| specifies any cache
   // usage other than LOCAL_ONLY, there must be a 1:1 correspondence between
@@ -231,6 +243,13 @@ class NET_EXPORT HostResolverManager
 
   bool check_ipv6_on_wifi_for_testing() const { return check_ipv6_on_wifi_; }
 
+  // Public to be called from std::make_unique. Not to be called directly.
+  HostResolverManager(base::PassKey<HostResolverManager>,
+                      const HostResolver::ManagerOptions& options,
+                      SystemDnsConfigChangeNotifier* system_dns_config_notifier,
+                      NetworkChangeNotifier::NetworkHandle target_network,
+                      NetLog* net_log);
+
  protected:
   // Callback from HaveOnlyLoopbackAddresses probe.
   void SetHaveOnlyLoopbackAddresses(bool result);
@@ -458,6 +477,12 @@ class NET_EXPORT HostResolverManager
   // by a network connection change.
   void InvalidateCaches(bool network_change = false);
 
+  void UpdateConnectionType(NetworkChangeNotifier::ConnectionType type);
+
+  bool IsBoundToNetwork() const {
+    return target_network_ != NetworkChangeNotifier::kInvalidNetworkHandle;
+  }
+
   // Returns |nullptr| if DoH probes are currently not allowed (due to
   // configuration or current connection state).
   std::unique_ptr<DnsProbeRunner> CreateDohProbeRunner(
@@ -487,6 +512,8 @@ class NET_EXPORT HostResolverManager
 
   raw_ptr<SystemDnsConfigChangeNotifier> system_dns_config_notifier_;
 
+  NetworkChangeNotifier::NetworkHandle target_network_;
+
   // False if IPv6 should not be attempted and assumed unreachable when on a
   // WiFi connection. See https://crbug.com/696569 for further context.
   bool check_ipv6_on_wifi_;