Add additional cases to Windows DNS configuration checking code.

Improve DNS configuration checking to handle additional cases where
we must defer to platform DNS, such as checking for DNS proxies and VPN
configurations, and an additional policy that could be used to
configure NRPT rules.

Bug: 1052139
Change-Id: I4884f76b576964e976114169113259e28fc78801
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2055761Reviewed-by: Eric Orth <ericorth@chromium.org>
Commit-Queue: Kaustubha Govind <kaustubhag@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746643}

net/dns/dns_config_service_win.cc

@@ -56,8 +56,19 @@ const base::char16 kPolicyPath[] =
     STRING16_LITERAL("SOFTWARE\\Policies\\Microsoft\\Windows NT\\DNSClient");
 const base::char16 kPrimaryDnsSuffixPath[] =
     STRING16_LITERAL("SOFTWARE\\Policies\\Microsoft\\System\\DNSClient");
-const base::char16 kNRPTPath[] = STRING16_LITERAL(
+const base::char16 kNrptPath[] = STRING16_LITERAL(
     "SOFTWARE\\Policies\\Microsoft\\Windows NT\\DNSClient\\DnsPolicyConfig");
+const base::char16 kControlSetNrptPath[] = STRING16_LITERAL(
+    "SYSTEM\\CurrentControlSet\\Services\\Dnscache\\Parameters"
+    "\\DnsPolicyConfig");
+const base::char16 kDnsConnectionsPath[] = STRING16_LITERAL(
+    "SYSTEM\\CurrentControlSet\\Services\\Dnscache\\Parameters"
+    "\\DnsConnections");
+const base::char16 kDnsActiveIfs[] = STRING16_LITERAL(
+    "SYSTEM\\CurrentControlSet\\Services\\Dnscache\\Parameters\\DnsActiveIfs");
+const base::char16 kDnsConnectionsProxies[] = STRING16_LITERAL(
+    "SYSTEM\\CurrentControlSet\\Services\\Dnscache\\Parameters"
+    "\\DnsConnectionsProxies");
 
 enum HostsParseWinResult {
   HOSTS_PARSE_WIN_OK = 0,
@@ -197,8 +208,21 @@ ConfigParseWinResult ReadSystemSettings(DnsSystemSettings* settings) {
     return CONFIG_PARSE_WIN_READ_PRIMARY_SUFFIX;
   }
 
-  base::win::RegistryKeyIterator nrpt_rules(HKEY_LOCAL_MACHINE, kNRPTPath);
-  settings->have_name_resolution_policy = (nrpt_rules.SubkeyCount() > 0);
+  base::win::RegistryKeyIterator nrpt_rules(HKEY_LOCAL_MACHINE, kNrptPath);
+  base::win::RegistryKeyIterator cs_nrpt_rules(HKEY_LOCAL_MACHINE,
+                                               kControlSetNrptPath);
+  settings->have_name_resolution_policy =
+      (nrpt_rules.SubkeyCount() > 0 || cs_nrpt_rules.SubkeyCount() > 0);
+
+  base::win::RegistryKeyIterator dns_connections(HKEY_LOCAL_MACHINE,
+                                                 kDnsConnectionsPath);
+  base::win::RegistryKeyIterator dns_active_ifs(HKEY_LOCAL_MACHINE,
+                                                kDnsActiveIfs);
+  base::win::RegistryKeyIterator dns_connections_proxies(
+      HKEY_LOCAL_MACHINE, kDnsConnectionsProxies);
+  settings->have_proxy =
+      (dns_connections.SubkeyCount() > 0 || dns_active_ifs.SubkeyCount() > 0 ||
+       dns_connections_proxies.SubkeyCount() > 0);
 
   return CONFIG_PARSE_WIN_OK;
 }
@@ -424,8 +448,7 @@ DnsSystemSettings::DnsSystemSettings()
       policy_devolution(),
       dnscache_devolution(),
       tcpip_devolution(),
-      append_to_multi_label_name(),
-      have_name_resolution_policy(false) {
+      append_to_multi_label_name() {
   policy_search_list.set = false;
   tcpip_search_list.set = false;
   tcpip_domain.set = false;
@@ -498,6 +521,7 @@ bool ParseSearchList(const base::string16& value,
 ConfigParseWinResult ConvertSettingsToDnsConfig(
     const DnsSystemSettings& settings,
     DnsConfig* config) {
+  bool uses_vpn = false;
   *config = DnsConfig();
 
   // Use GetAdapterAddresses to get effective DNS server order and
@@ -505,11 +529,18 @@ ConfigParseWinResult ConvertSettingsToDnsConfig(
   // The order of adapters is the network binding order, so stick to the
   // first good adapter.
   for (const IP_ADAPTER_ADDRESSES* adapter = settings.addresses.get();
-       adapter != nullptr && config->nameservers.empty();
-       adapter = adapter->Next) {
-    if (adapter->OperStatus != IfOperStatusUp)
-      continue;
-    if (adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK)
+       adapter != nullptr; adapter = adapter->Next) {
+    // Check each adapter for a VPN interface. Even if a single such interface
+    // is present, treat this as an unhandled configuration.
+    if (adapter->IfType == IF_TYPE_PPP) {
+      uses_vpn = true;
+    }
+
+    // Skip disconnected and loopback adapters. If a good configuration was
+    // previously found, skip processing another adapter.
+    if (adapter->OperStatus != IfOperStatusUp ||
+        adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK ||
+        !config->nameservers.empty())
       continue;
 
     for (const IP_ADAPTER_DNS_SERVER_ADDRESS* address =
@@ -552,11 +583,14 @@ ConfigParseWinResult ConvertSettingsToDnsConfig(
         (settings.append_to_multi_label_name.value != 0);
   }
 
-  ConfigParseWinResult result = CONFIG_PARSE_WIN_OK;
   if (settings.have_name_resolution_policy) {
-    config->unhandled_options = true;
     // TODO(szym): only set this to true if NRPT has DirectAccess rules.
     config->use_local_ipv6 = true;
+  }
+
+  ConfigParseWinResult result = CONFIG_PARSE_WIN_OK;
+  if (settings.have_name_resolution_policy || settings.have_proxy || uses_vpn) {
+    config->unhandled_options = true;
     result = CONFIG_PARSE_WIN_UNHANDLED_OPTIONS;
   }
 

net/dns/dns_config_service_win.h

@@ -9,6 +9,7 @@
 // include these headers here.
 #include <winsock2.h>
 #include <iphlpapi.h>
+#include <iptypes.h>
 
 #include <memory>
 #include <string>
@@ -99,7 +100,17 @@ struct NET_EXPORT_PRIVATE DnsSystemSettings {
 
   // True when the Name Resolution Policy Table (NRPT) has at least one rule:
   // SOFTWARE\Policies\Microsoft\Windows NT\DNSClient\DnsPolicyConfig\Rule*
-  bool have_name_resolution_policy;
+  // (or)
+  // SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig\Rule*
+  bool have_name_resolution_policy = false;
+
+  // True when a proxy is configured via at least one rule:
+  // SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsConnections
+  // (or)
+  // SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsActiveIfs
+  // (or)
+  // SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsConnectionsProxies
+  bool have_proxy = false;
 };
 
 enum ConfigParseWinResult {

net/dns/dns_config_service_win_unittest.cc

@@ -456,6 +456,48 @@ TEST(DnsConfigServiceWinTest, HaveNRPT) {
   }
 }
 
+// Setting have_proxy should set unhandled_options.
+TEST(DnsConfigServiceWinTest, HaveProxy) {
+  AdapterInfo infos[2] = {
+      {IF_TYPE_USB, IfOperStatusUp, L"connection.suffix", {"1.0.0.1"}},
+      {0},
+  };
+
+  const struct TestCase {
+    bool have_proxy;
+    bool unhandled_options;
+    internal::ConfigParseWinResult result;
+  } cases[] = {
+      {false, false, internal::CONFIG_PARSE_WIN_OK},
+      {true, true, internal::CONFIG_PARSE_WIN_UNHANDLED_OPTIONS},
+  };
+
+  for (const auto& t : cases) {
+    internal::DnsSystemSettings settings;
+    settings.addresses = CreateAdapterAddresses(infos);
+    settings.have_proxy = t.have_proxy;
+    DnsConfig config;
+    EXPECT_EQ(t.result,
+              internal::ConvertSettingsToDnsConfig(settings, &config));
+    EXPECT_EQ(t.unhandled_options, config.unhandled_options);
+  }
+}
+
+// Setting uses_vpn should set unhandled_options.
+TEST(DnsConfigServiceWinTest, UsesVpn) {
+  AdapterInfo infos[3] = {
+      {IF_TYPE_USB, IfOperStatusUp, L"connection.suffix", {"1.0.0.1"}},
+      {IF_TYPE_PPP, IfOperStatusUp, L"connection.suffix", {"1.0.0.1"}},
+      {0},
+  };
+
+  internal::DnsSystemSettings settings;
+  settings.addresses = CreateAdapterAddresses(infos);
+  DnsConfig config;
+  EXPECT_EQ(internal::CONFIG_PARSE_WIN_UNHANDLED_OPTIONS,
+            internal::ConvertSettingsToDnsConfig(settings, &config));
+  EXPECT_TRUE(config.unhandled_options);
+}
 
 }  // namespace