Merge "Do not detect portals when DNS returns private IPs"
diff --git a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
old mode 100644
new mode 100755
index 48cd48b..deee91a
--- a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
+++ b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
@@ -36,6 +36,12 @@
*/
public static final int PARTIAL_CODE = -1;
+ // DNS response with private IP on the probe URL means the network, especially Wi-Fi network is
+ // not connected to the Internet. This code represents the result of a single probe, for correct
+ // logging of the probe results. The result of the whole evaluation would typically be FAILED if
+ // one of the probes returns this status.
+ public static final int DNS_PRIVATE_IP_RESPONSE_CODE = -2;
+
@NonNull
public static final CaptivePortalProbeResult FAILED = new CaptivePortalProbeResult(FAILED_CODE);
@NonNull
@@ -43,6 +49,8 @@
new CaptivePortalProbeResult(SUCCESS_CODE);
public static final CaptivePortalProbeResult PARTIAL =
new CaptivePortalProbeResult(PARTIAL_CODE);
+ public static final CaptivePortalProbeResult PRIVATE_IP =
+ new CaptivePortalProbeResult(DNS_PRIVATE_IP_RESPONSE_CODE);
private final int mHttpResponseCode; // HTTP response code returned from Internet probe.
@Nullable
@@ -85,4 +93,8 @@
public boolean isPartialConnectivity() {
return mHttpResponseCode == PARTIAL_CODE;
}
+
+ public boolean isDnsPrivateIpResponse() {
+ return mHttpResponseCode == DNS_PRIVATE_IP_RESPONSE_CODE;
+ }
}
diff --git a/res/values/config.xml b/res/values/config.xml
index a2f3959..8bd8eff 100644
--- a/res/values/config.xml
+++ b/res/values/config.xml
@@ -59,4 +59,12 @@
<integer name="config_nud_steadystate_solicit_interval">750</integer>
<integer name="config_nud_postroaming_solicit_num">5</integer>
<integer name="config_nud_postroaming_solicit_interval">750</integer>
+
+ <!-- Whether to force considering DNS probes returning private IP addresses as failed
+ when attempting to detect captive portals.
+ The impact of this feature will be evaluated separately through experiments.
+ Force-enabling the feature regardless of the experiment results is not advised, as it
+ could result in valid captive portals being incorrectly classified as having no
+ connectivity.-->
+ <bool name="config_force_dns_probe_private_ip_no_internet">false</bool>
</resources>
diff --git a/res/values/overlayable.xml b/res/values/overlayable.xml
index 052266d..ed86814 100644
--- a/res/values/overlayable.xml
+++ b/res/values/overlayable.xml
@@ -44,6 +44,14 @@
<item type="integer" name="config_nud_steadystate_solicit_interval"/>
<item type="integer" name="config_nud_postroaming_solicit_num"/>
<item type="integer" name="config_nud_postroaming_solicit_interval"/>
+
+ <!-- Whether to force considering DNS probes returning private IP addresses as failed
+ when attempting to detect captive portals.
+ The impact of this feature will be evaluated separately through experiments.
+ Force-enabling the feature regardless of the experiment results is not advised, as it
+ could result in valid captive portals being incorrectly classified as having no
+ connectivity.-->
+ <item type="bool" name="config_force_dns_probe_private_ip_no_internet"/>
</policy>
</overlayable>
</resources>
diff --git a/src/android/net/util/NetworkStackUtils.java b/src/android/net/util/NetworkStackUtils.java
old mode 100644
new mode 100755
index bc41549..2de18de
--- a/src/android/net/util/NetworkStackUtils.java
+++ b/src/android/net/util/NetworkStackUtils.java
@@ -172,6 +172,15 @@
public static final String DISMISS_PORTAL_IN_VALIDATED_NETWORK =
"dismiss_portal_in_validated_network";
+ /**
+ * Experiment flag to enable considering DNS probes returning private IP addresses as failed
+ * when attempting to detect captive portals.
+ *
+ * This flag is enabled if !=0 and less than the module APK version.
+ */
+ public static final String DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION =
+ "dns_probe_private_ip_no_internet";
+
static {
System.loadLibrary("networkstackutilsjni");
}
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
old mode 100644
new mode 100755
index 7ea61fa..0540258
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -70,6 +70,7 @@
import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTPS_URLS;
import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTP_URLS;
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.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;
@@ -423,6 +424,8 @@
private boolean mAcceptPartialConnectivity = false;
private final EvaluationState mEvaluationState = new EvaluationState();
+ private final boolean mPrivateIpNotPortalEnabled;
+
private int getCallbackVersion(INetworkMonitorCallbacks cb) {
int version;
try {
@@ -484,6 +487,7 @@
// CHECKSTYLE:ON IndentationCheck
mIsCaptivePortalCheckEnabled = getIsCaptivePortalCheckEnabled();
+ mPrivateIpNotPortalEnabled = getIsPrivateIpNotPortalEnabled();
mUseHttps = getUseHttpsValidation();
mCaptivePortalUserAgent = getCaptivePortalUserAgent();
mCaptivePortalHttpsUrls = makeCaptivePortalHttpsUrls();
@@ -1434,6 +1438,12 @@
return mode != CAPTIVE_PORTAL_MODE_IGNORE;
}
+ private boolean getIsPrivateIpNotPortalEnabled() {
+ return mDependencies.isFeatureEnabled(mContext, DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION)
+ || mContext.getResources().getBoolean(
+ R.bool.config_force_dns_probe_private_ip_no_internet);
+ }
+
private boolean getUseHttpsValidation() {
return mDependencies.getDeviceConfigPropertyInt(NAMESPACE_CONNECTIVITY,
CAPTIVE_PORTAL_USE_HTTPS, 1) == 1;
@@ -1877,7 +1887,13 @@
// state machine thread. Reporting results here would cause races and potentially send
// information to callers that does not make sense because the state machine has already
// changed state.
- sendDnsProbe(host);
+ 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
+ && (proxy == null) && hasPrivateIpAddress(resolvedAddr)) {
+ return CaptivePortalProbeResult.PRIVATE_IP;
+ }
return sendHttpProbe(url, probeType, null);
}
@@ -1891,23 +1907,41 @@
}
/** Do a DNS resolution of the given server. */
- private void sendDnsProbe(String host) {
+ private InetAddress[] sendDnsProbe(String host) {
if (TextUtils.isEmpty(host)) {
- return;
+ return null;
}
- final String name = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS);
final Stopwatch watch = new Stopwatch().start();
int result;
- String connectInfo;
+ InetAddress[] addresses;
try {
- InetAddress[] addresses = sendDnsProbeWithTimeout(host, getDnsProbeTimeout());
+ addresses = sendDnsProbeWithTimeout(host, getDnsProbeTimeout());
result = ValidationProbeEvent.DNS_SUCCESS;
} catch (UnknownHostException e) {
+ addresses = null;
result = ValidationProbeEvent.DNS_FAILURE;
}
final long latency = watch.stop();
logValidationProbe(latency, ValidationProbeEvent.PROBE_DNS, result);
+ return addresses;
+ }
+
+ /**
+ * Check if any of the provided IP addresses include a private IP, as defined by
+ * {@link com.android.server.util.NetworkStackConstants#PRIVATE_IPV4_RANGES}.
+ * @return true if an IP address is private.
+ */
+ private static boolean hasPrivateIpAddress(@Nullable InetAddress[] addresses) {
+ if (addresses == null) {
+ return false;
+ }
+ for (InetAddress address : addresses) {
+ if (address.isLinkLocalAddress() || address.isSiteLocalAddress()) {
+ return true;
+ }
+ }
+ return false;
}
/**
@@ -2230,6 +2264,15 @@
reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTPS, httpsResult);
return httpsResult;
}
+ // 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.
+ // 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())) {
+ validationLog("DNS response to the URL is private IP");
+ return CaptivePortalProbeResult.FAILED;
+ }
// If a fallback method exists, use it to retry portal detection.
// If we have new-style probe specs, use those. Otherwise, use the fallback URLs.
final CaptivePortalProbeSpec probeSpec = nextFallbackSpec();
@@ -2449,6 +2492,16 @@
}
/**
+ * Check whether or not one experimental feature in the connectivity namespace is
+ * enabled.
+ * @param name Flag name of the experiment in the connectivity namespace.
+ * @see NetworkStackUtils#isFeatureEnabled(Context, String, String)
+ */
+ public boolean isFeatureEnabled(@NonNull Context context, @NonNull String name) {
+ return NetworkStackUtils.isFeatureEnabled(context, NAMESPACE_CONNECTIVITY, name);
+ }
+
+ /**
* Send a broadcast indicating network conditions.
*/
public void sendNetworkConditionsBroadcast(@NonNull Context context,
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index ab1ebd6..b0efa33 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -38,6 +38,7 @@
import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS;
import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS;
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 com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;
import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_TCP_METRICS;
@@ -93,6 +94,7 @@
import android.net.DnsResolver;
import android.net.INetd;
import android.net.INetworkMonitorCallbacks;
+import android.net.InetAddresses;
import android.net.LinkProperties;
import android.net.Network;
import android.net.NetworkCapabilities;
@@ -152,6 +154,7 @@
import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.net.HttpURLConnection;
+import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.URL;
import java.net.UnknownHostException;
@@ -747,6 +750,38 @@
runPortalNetworkTest(VALIDATION_RESULT_PORTAL);
}
+ private void setupPrivateIpResponse(String privateAddr) throws Exception {
+ setSslException(mHttpsConnection);
+ setPortal302(mHttpConnection);
+ final String httpHost = new URL(TEST_HTTP_URL).getHost();
+ mFakeDns.setAnswer(httpHost, new String[] { "2001:db8::123" }, TYPE_AAAA);
+ final InetAddress parsedPrivateAddr = InetAddresses.parseNumericAddress(privateAddr);
+ mFakeDns.setAnswer(httpHost, new String[] { privateAddr },
+ (parsedPrivateAddr instanceof Inet6Address) ? TYPE_AAAA : TYPE_A);
+ }
+
+ @Test
+ public void testIsCaptivePortal_PrivateIpNotPortal_Enabled_IPv4() throws Exception {
+ when(mDependencies.isFeatureEnabled(any(), eq(DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION)))
+ .thenReturn(true);
+ setupPrivateIpResponse("192.168.1.1");
+ runFailedNetworkTest();
+ }
+
+ @Test
+ public void testIsCaptivePortal_PrivateIpNotPortal_Enabled_IPv6() throws Exception {
+ when(mDependencies.isFeatureEnabled(any(), eq(DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION)))
+ .thenReturn(true);
+ setupPrivateIpResponse("fec0:1234::1");
+ runFailedNetworkTest();
+ }
+
+ @Test
+ public void testIsCaptivePortal_PrivateIpNotPortal_Disabled() throws Exception {
+ setupPrivateIpResponse("192.168.1.1");
+ runPortalNetworkTest(VALIDATION_RESULT_PORTAL);
+ }
+
@Test
public void testIsCaptivePortal_HttpsProbeIsNotPortal() throws IOException {
setStatus(mHttpsConnection, 204);