Refine DNS private IP probes

This addresses comments on the parent change that introduced the
optional feature.
This change also considers IPv6 ULAs as private addresses, corrects some
style and comments.

Bug: 136734947
Test: atest NetworkStackTests:NetworkMonitorTest \
          NetworkStackTests:NetworkStackUtilsTest
Change-Id: I1169320f3a5c09631e754518c60acab337faf49b
Merged-In: If4231804b77143b78f26a32c16db53fc4ac44cd8
(cherry picked from commit 0f62e8c6d21afd28280dbae7c3ea216097def466, aosp/1282802)

2 files changed, 21 insertions, 11 deletions

diff --git a/src/android/net/util/NetworkStackUtils.java b/src/android/net/util/NetworkStackUtils.java
index 2de18de..6fd6043 100755
--- a/src/android/net/util/NetworkStackUtils.java
+++ b/src/android/net/util/NetworkStackUtils.java
@@ -385,4 +385,12 @@ public class NetworkStackUtils {
                 (address instanceof Inet6Address) ? "[%s]:%d" : "%s:%d",
                         address.getHostAddress(), port);
     }
+
+    /**
+     * Return true if the provided address is non-null and an IPv6 Unique Local Address (RFC4193).
+     */
+    public static boolean isIPv6ULA(@Nullable InetAddress addr) {
+        return addr instanceof Inet6Address
+                && ((addr.getAddress()[0] & 0xfe) == 0xfc);
+    }
 }
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 4aba4f9..eed63e6 100755
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -72,6 +72,7 @@ import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTP_URL
 import static android.net.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK;
 import static android.net.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION;
 import static android.net.util.NetworkStackUtils.isEmpty;
+import static android.net.util.NetworkStackUtils.isIPv6ULA;
 import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;
 
 import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;
@@ -427,7 +428,7 @@ public class NetworkMonitor extends StateMachine {
     private boolean mAcceptPartialConnectivity = false;
     private final EvaluationState mEvaluationState = new EvaluationState();
 
-    private final boolean mPrivateIpNotPortalEnabled;
+    private final boolean mPrivateIpNoInternetEnabled;
 
     private int getCallbackVersion(INetworkMonitorCallbacks cb) {
         int version;
@@ -490,7 +491,7 @@ public class NetworkMonitor extends StateMachine {
         // CHECKSTYLE:ON IndentationCheck
 
         mIsCaptivePortalCheckEnabled = getIsCaptivePortalCheckEnabled();
-        mPrivateIpNotPortalEnabled = getIsPrivateIpNotPortalEnabled();
+        mPrivateIpNoInternetEnabled = getIsPrivateIpNoInternetEnabled();
         mUseHttps = getUseHttpsValidation();
         mCaptivePortalUserAgent = getCaptivePortalUserAgent();
         mCaptivePortalHttpsUrls = makeCaptivePortalHttpsUrls();
@@ -1441,7 +1442,7 @@ public class NetworkMonitor extends StateMachine {
         return mode != CAPTIVE_PORTAL_MODE_IGNORE;
     }
 
-    private boolean getIsPrivateIpNotPortalEnabled() {
+    private boolean getIsPrivateIpNoInternetEnabled() {
         return mDependencies.isFeatureEnabled(mContext, DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION)
                 || mContext.getResources().getBoolean(
                         R.bool.config_force_dns_probe_private_ip_no_internet);
@@ -1919,9 +1920,9 @@ public class NetworkMonitor extends StateMachine {
         // information to callers that does not make sense because the state machine has already
         // changed state.
         final InetAddress[] resolvedAddr = sendDnsProbe(host);
-        // The private IP logic only applies to the HTTP probe, not the HTTPS probe (which would
-        // fail anyway) or the PAC probe.
-        if (mPrivateIpNotPortalEnabled && probeType == ValidationProbeEvent.PROBE_HTTP
+        // The private IP logic only applies to captive portal detection (the HTTP probe), not
+        // network validation (the HTTPS probe, which would likely fail anyway) or the PAC probe.
+        if (mPrivateIpNoInternetEnabled && probeType == ValidationProbeEvent.PROBE_HTTP
                 && (proxy == null) && hasPrivateIpAddress(resolvedAddr)) {
             return CaptivePortalProbeResult.PRIVATE_IP;
         }
@@ -1959,8 +1960,7 @@ public class NetworkMonitor extends StateMachine {
     }
 
     /**
-     * Check if any of the provided IP addresses include a private IP, as defined by
-     * {@link com.android.server.util.NetworkStackConstants#PRIVATE_IPV4_RANGES}.
+     * Check if any of the provided IP addresses include a private IP.
      * @return true if an IP address is private.
      */
     private static boolean hasPrivateIpAddress(@Nullable InetAddress[] addresses) {
@@ -1968,7 +1968,8 @@ public class NetworkMonitor extends StateMachine {
             return false;
         }
         for (InetAddress address : addresses) {
-            if (address.isLinkLocalAddress() || address.isSiteLocalAddress()) {
+            if (address.isLinkLocalAddress() || address.isSiteLocalAddress()
+                    || isIPv6ULA(address)) {
                 return true;
             }
         }
@@ -2343,10 +2344,11 @@ public class NetworkMonitor extends StateMachine {
         }
         // Consider a DNS response with a private IP address on the HTTP probe as an indication that
         // the network is not connected to the Internet, and have the whole evaluation fail in that
-        // case.
+        // case, instead of potentially detecting a captive portal. This logic only affects portal
+        // detection, not network validation.
         // This only applies if the DNS probe completed within PROBE_TIMEOUT_MS, as the fallback
         // probe should not be delayed by this check.
-        if (mPrivateIpNotPortalEnabled && (httpResult.isDnsPrivateIpResponse())) {
+        if (mPrivateIpNoInternetEnabled && (httpResult.isDnsPrivateIpResponse())) {
             validationLog("DNS response to the URL is private IP");
             return CaptivePortalProbeResult.FAILED;
         }