Make isCaptivePortal perform both HTTP and HTTPS probes.
Also a couple of minor cleanups and logging tweaks.
Bug: 26075613
Change-Id: I67b09e96d72764179339b616072bb2ce06aabf33
diff --git a/api/system-current.txt b/api/system-current.txt
index 41b73eb..ddd9ad6 100644
--- a/api/system-current.txt
+++ b/api/system-current.txt
@@ -26170,8 +26170,12 @@
method public static void logEvent(int, long, int, int);
method public void writeToParcel(android.os.Parcel, int);
field public static final android.os.Parcelable.Creator<android.net.metrics.ValidationProbeEvent> CREATOR;
- field public static final int PROBE_HTTP = 0; // 0x0
- field public static final int PROBE_HTTPS = 1; // 0x1
+ field public static final int DNS_FAILURE = 0; // 0x0
+ field public static final int DNS_SUCCESS = 1; // 0x1
+ field public static final int PROBE_DNS = 0; // 0x0
+ field public static final int PROBE_HTTP = 1; // 0x1
+ field public static final int PROBE_HTTPS = 2; // 0x2
+ field public static final int PROBE_PAC = 3; // 0x3
field public final long durationMs;
field public final int netId;
field public final int probeType;
diff --git a/core/java/android/net/metrics/ValidationProbeEvent.java b/core/java/android/net/metrics/ValidationProbeEvent.java
index 5c8ea65..751c35f 100644
--- a/core/java/android/net/metrics/ValidationProbeEvent.java
+++ b/core/java/android/net/metrics/ValidationProbeEvent.java
@@ -29,8 +29,13 @@
@SystemApi
public final class ValidationProbeEvent extends IpConnectivityEvent implements Parcelable {
- public static final int PROBE_HTTP = 0;
- public static final int PROBE_HTTPS = 1;
+ public static final int PROBE_DNS = 0;
+ public static final int PROBE_HTTP = 1;
+ public static final int PROBE_HTTPS = 2;
+ public static final int PROBE_PAC = 3;
+
+ public static final int DNS_FAILURE = 0;
+ public static final int DNS_SUCCESS = 1;
public final int netId;
public final long durationMs;
@@ -73,6 +78,11 @@
}
};
+ /** @hide */
+ public static String getProbeName(int probeType) {
+ return Decoder.constants.get(probeType, "PROBE_???");
+ }
+
public static void logEvent(int netId, long durationMs, int probeType, int returnCode) {
logEvent(new ValidationProbeEvent(netId, durationMs, probeType, returnCode));
}
@@ -80,7 +90,7 @@
@Override
public String toString() {
return String.format("ValidationProbeEvent(%d, %s:%d, %dms)",
- netId, Decoder.constants.get(probeType), returnCode, durationMs);
+ netId, getProbeName(probeType), returnCode, durationMs);
}
final static class Decoder {
diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java
index 2394531..b1bf355 100755
--- a/core/java/android/provider/Settings.java
+++ b/core/java/android/provider/Settings.java
@@ -7706,6 +7706,15 @@
public static final String CAPTIVE_PORTAL_SERVER = "captive_portal_server";
/**
+ * Whether to use HTTPS for network validation. This is enabled by default and the setting
+ * needs to be set to 0 to disable it. This setting is a misnomer because captive portals
+ * don't actually use HTTPS, but it's consistent with the other settings.
+ *
+ * @hide
+ */
+ public static final String CAPTIVE_PORTAL_USE_HTTPS = "captive_portal_use_https";
+
+ /**
* Whether network service discovery is enabled.
*
* @hide
diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
index f7784ac..f4e1424 100644
--- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java
+++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
@@ -71,7 +71,11 @@
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.InetAddress;
+import java.net.MalformedURLException;
+import java.net.UnknownHostException;
import java.net.URL;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.List;
import java.util.Random;
@@ -228,7 +232,8 @@
private final AlarmManager mAlarmManager;
private final NetworkRequest mDefaultRequest;
- private boolean mIsCaptivePortalCheckEnabled = false;
+ private boolean mIsCaptivePortalCheckEnabled;
+ private boolean mUseHttps;
// Set if the user explicitly selected "Do not use this network" in captive portal sign-in app.
private boolean mUserDoesNotWant = false;
@@ -276,6 +281,8 @@
mIsCaptivePortalCheckEnabled = Settings.Global.getInt(mContext.getContentResolver(),
Settings.Global.CAPTIVE_PORTAL_DETECTION_ENABLED, 1) == 1;
+ mUseHttps = Settings.Global.getInt(mContext.getContentResolver(),
+ Settings.Global.CAPTIVE_PORTAL_USE_HTTPS, 1) == 1;
start();
}
@@ -324,6 +331,21 @@
return HANDLED;
case CMD_CAPTIVE_PORTAL_APP_FINISHED:
log("CaptivePortal App responded with " + message.arg1);
+
+ // If the user has seen and acted on a captive portal notification, and the
+ // captive portal app is now closed, disable HTTPS probes. This avoids the
+ // following pathological situation:
+ //
+ // 1. HTTP probe returns a captive portal, HTTPS probe fails or times out.
+ // 2. User opens the app and logs into the captive portal.
+ // 3. HTTP starts working, but HTTPS still doesn't work for some other reason -
+ // perhaps due to the network blocking HTTPS?
+ //
+ // In this case, we'll fail to validate the network even after the app is
+ // dismissed. There is now no way to use this network, because the app is now
+ // gone, so the user cannot select "Use this network as is".
+ mUseHttps = false;
+
switch (message.arg1) {
case APP_RETURN_DISMISSED:
sendMessage(CMD_FORCE_REEVALUATION, 0 /* no UID */, 0);
@@ -424,6 +446,8 @@
*/
@VisibleForTesting
public static final class CaptivePortalProbeResult {
+ static final CaptivePortalProbeResult FAILED = new CaptivePortalProbeResult(599, null);
+
final int mHttpResponseCode; // HTTP response code returned from Internet probe.
final String mRedirectUrl; // Redirect destination returned from Internet probe.
@@ -431,6 +455,11 @@
mHttpResponseCode = httpResponseCode;
mRedirectUrl = redirectUrl;
}
+
+ boolean isSuccessful() { return mHttpResponseCode == 204; }
+ boolean isPortal() {
+ return !isSuccessful() && mHttpResponseCode >= 200 && mHttpResponseCode <= 399;
+ }
}
// Being in the EvaluatingState State indicates the Network is being evaluated for internet
@@ -481,6 +510,7 @@
// expensive metered network, or unwanted leaking of the User Agent string.
if (!mDefaultRequest.networkCapabilities.satisfiedByNetworkCapabilities(
mNetworkAgentInfo.networkCapabilities)) {
+ validationLog("Network would not satisfy default request, not validating");
transitionTo(mValidatedState);
return HANDLED;
}
@@ -492,10 +522,9 @@
// will be unresponsive. isCaptivePortal() could be executed on another Thread
// if this is found to cause problems.
CaptivePortalProbeResult probeResult = isCaptivePortal();
- if (probeResult.mHttpResponseCode == 204) {
+ if (probeResult.isSuccessful()) {
transitionTo(mValidatedState);
- } else if (probeResult.mHttpResponseCode >= 200 &&
- probeResult.mHttpResponseCode <= 399) {
+ } else if (probeResult.isPortal()) {
mConnectivityServiceHandler.sendMessage(obtainMessage(EVENT_NETWORK_TESTED,
NETWORK_TEST_RESULT_INVALID, mNetId, probeResult.mRedirectUrl));
transitionTo(mCaptivePortalState);
@@ -659,11 +688,112 @@
}
}
- public static String getCaptivePortalServerUrl(Context context) {
+ private static String getCaptivePortalServerUrl(Context context, boolean isHttps) {
String server = Settings.Global.getString(context.getContentResolver(),
Settings.Global.CAPTIVE_PORTAL_SERVER);
if (server == null) server = DEFAULT_SERVER;
- return "http://" + server + "/generate_204";
+ return (isHttps ? "https" : "http") + "://" + server + "/generate_204";
+ }
+
+ public static String getCaptivePortalServerUrl(Context context) {
+ return getCaptivePortalServerUrl(context, false);
+ }
+
+ @VisibleForTesting
+ protected CaptivePortalProbeResult isCaptivePortal() {
+ if (!mIsCaptivePortalCheckEnabled) return new CaptivePortalProbeResult(204, null);
+
+ URL pacUrl = null, httpUrl = null, httpsUrl = null;
+
+ // On networks with a PAC instead of fetching a URL that should result in a 204
+ // response, we instead simply fetch the PAC script. This is done for a few reasons:
+ // 1. At present our PAC code does not yet handle multiple PACs on multiple networks
+ // until something like https://android-review.googlesource.com/#/c/115180/ lands.
+ // Network.openConnection() will ignore network-specific PACs and instead fetch
+ // using NO_PROXY. If a PAC is in place, the only fetch we know will succeed with
+ // NO_PROXY is the fetch of the PAC itself.
+ // 2. To proxy the generate_204 fetch through a PAC would require a number of things
+ // happen before the fetch can commence, namely:
+ // a) the PAC script be fetched
+ // b) a PAC script resolver service be fired up and resolve the captive portal
+ // server.
+ // Network validation could be delayed until these prerequisities are satisifed or
+ // could simply be left to race them. Neither is an optimal solution.
+ // 3. PAC scripts are sometimes used to block or restrict Internet access and may in
+ // fact block fetching of the generate_204 URL which would lead to false negative
+ // results for network validation.
+ final ProxyInfo proxyInfo = mNetworkAgentInfo.linkProperties.getHttpProxy();
+ if (proxyInfo != null && !Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) {
+ try {
+ pacUrl = new URL(proxyInfo.getPacFileUrl().toString());
+ } catch (MalformedURLException e) {
+ validationLog("Invalid PAC URL: " + proxyInfo.getPacFileUrl().toString());
+ return CaptivePortalProbeResult.FAILED;
+ }
+ }
+
+ if (pacUrl == null) {
+ try {
+ httpUrl = new URL(getCaptivePortalServerUrl(mContext, false));
+ httpsUrl = new URL(getCaptivePortalServerUrl(mContext, true));
+ } catch (MalformedURLException e) {
+ validationLog("Bad validation URL: " + getCaptivePortalServerUrl(mContext, false));
+ return CaptivePortalProbeResult.FAILED;
+ }
+ }
+
+ long startTime = SystemClock.elapsedRealtime();
+
+ // Pre-resolve the captive portal server host so we can log it.
+ // Only do this if HttpURLConnection is about to, to avoid any potentially
+ // unnecessary resolution.
+ String hostToResolve = null;
+ if (pacUrl != null) {
+ hostToResolve = pacUrl.getHost();
+ } else if (proxyInfo != null) {
+ hostToResolve = proxyInfo.getHost();
+ } else {
+ hostToResolve = httpUrl.getHost();
+ }
+
+ if (!TextUtils.isEmpty(hostToResolve)) {
+ String probeName = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS);
+ final Stopwatch dnsTimer = new Stopwatch().start();
+ try {
+ InetAddress[] addresses = mNetworkAgentInfo.network.getAllByName(hostToResolve);
+ long dnsLatency = dnsTimer.stop();
+ ValidationProbeEvent.logEvent(mNetId, dnsLatency,
+ ValidationProbeEvent.PROBE_DNS, ValidationProbeEvent.DNS_SUCCESS);
+ final StringBuffer connectInfo = new StringBuffer(", " + hostToResolve + "=");
+ for (InetAddress address : addresses) {
+ connectInfo.append(address.getHostAddress());
+ if (address != addresses[addresses.length-1]) connectInfo.append(",");
+ }
+ validationLog(probeName + " OK " + dnsLatency + "ms" + connectInfo);
+ } catch (UnknownHostException e) {
+ long dnsLatency = dnsTimer.stop();
+ ValidationProbeEvent.logEvent(mNetId, dnsLatency,
+ ValidationProbeEvent.PROBE_DNS, ValidationProbeEvent.DNS_FAILURE);
+ validationLog(probeName + " FAIL " + dnsLatency + "ms, " + hostToResolve);
+ }
+ }
+
+ CaptivePortalProbeResult result;
+ if (pacUrl != null) {
+ result = sendHttpProbe(pacUrl, ValidationProbeEvent.PROBE_PAC);
+ } else if (mUseHttps) {
+ result = sendParallelHttpProbes(httpsUrl, httpUrl);
+ } else {
+ result = sendHttpProbe(httpUrl, ValidationProbeEvent.PROBE_HTTP);
+ }
+
+ long endTime = SystemClock.elapsedRealtime();
+
+ sendNetworkConditionsBroadcast(true /* response received */,
+ result.isPortal() /* isCaptivePortal */,
+ startTime, endTime);
+
+ return result;
}
/**
@@ -671,60 +801,14 @@
* Returns HTTP response code.
*/
@VisibleForTesting
- protected CaptivePortalProbeResult isCaptivePortal() {
- if (!mIsCaptivePortalCheckEnabled) return new CaptivePortalProbeResult(204, null);
-
+ protected CaptivePortalProbeResult sendHttpProbe(URL url, int probeType) {
HttpURLConnection urlConnection = null;
int httpResponseCode = 599;
String redirectUrl = null;
final Stopwatch probeTimer = new Stopwatch().start();
try {
- URL url = new URL(getCaptivePortalServerUrl(mContext));
- // On networks with a PAC instead of fetching a URL that should result in a 204
- // response, we instead simply fetch the PAC script. This is done for a few reasons:
- // 1. At present our PAC code does not yet handle multiple PACs on multiple networks
- // until something like https://android-review.googlesource.com/#/c/115180/ lands.
- // Network.openConnection() will ignore network-specific PACs and instead fetch
- // using NO_PROXY. If a PAC is in place, the only fetch we know will succeed with
- // NO_PROXY is the fetch of the PAC itself.
- // 2. To proxy the generate_204 fetch through a PAC would require a number of things
- // happen before the fetch can commence, namely:
- // a) the PAC script be fetched
- // b) a PAC script resolver service be fired up and resolve the captive portal
- // server.
- // Network validation could be delayed until these prerequisities are satisifed or
- // could simply be left to race them. Neither is an optimal solution.
- // 3. PAC scripts are sometimes used to block or restrict Internet access and may in
- // fact block fetching of the generate_204 URL which would lead to false negative
- // results for network validation.
- boolean fetchPac = false;
- final ProxyInfo proxyInfo = mNetworkAgentInfo.linkProperties.getHttpProxy();
- if (proxyInfo != null && !Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) {
- url = new URL(proxyInfo.getPacFileUrl().toString());
- fetchPac = true;
- }
- final StringBuffer connectInfo = new StringBuffer();
- String hostToResolve = null;
- // Only resolve a host if HttpURLConnection is about to, to avoid any potentially
- // unnecessary resolution.
- if (proxyInfo == null || fetchPac) {
- hostToResolve = url.getHost();
- } else if (proxyInfo != null) {
- hostToResolve = proxyInfo.getHost();
- }
- if (!TextUtils.isEmpty(hostToResolve)) {
- connectInfo.append(", " + hostToResolve + "=");
- final InetAddress[] addresses =
- mNetworkAgentInfo.network.getAllByName(hostToResolve);
- for (InetAddress address : addresses) {
- connectInfo.append(address.getHostAddress());
- if (address != addresses[addresses.length-1]) connectInfo.append(",");
- }
- }
- validationLog("Checking " + url.toString() + " on " +
- mNetworkAgentInfo.networkInfo.getExtraInfo() + connectInfo);
urlConnection = (HttpURLConnection) mNetworkAgentInfo.network.openConnection(url);
- urlConnection.setInstanceFollowRedirects(fetchPac);
+ urlConnection.setInstanceFollowRedirects(probeType == ValidationProbeEvent.PROBE_PAC);
urlConnection.setConnectTimeout(SOCKET_TIMEOUT_MS);
urlConnection.setReadTimeout(SOCKET_TIMEOUT_MS);
urlConnection.setUseCaches(false);
@@ -738,7 +822,9 @@
// Time how long it takes to get a response to our request
long responseTimestamp = SystemClock.elapsedRealtime();
- validationLog("isCaptivePortal: ret=" + httpResponseCode +
+ validationLog(ValidationProbeEvent.getProbeName(probeType) + " " + url +
+ " time=" + (responseTimestamp - requestTimestamp) + "ms" +
+ " ret=" + httpResponseCode +
" headers=" + urlConnection.getHeaderFields());
// NOTE: We may want to consider an "HTTP/1.0 204" response to be a captive
// portal. The only example of this seen so far was a captive portal. For
@@ -756,14 +842,10 @@
httpResponseCode = 204;
}
- if (httpResponseCode == 200 && fetchPac) {
+ if (httpResponseCode == 200 && probeType == ValidationProbeEvent.PROBE_PAC) {
validationLog("PAC fetch 200 response interpreted as 204 response.");
httpResponseCode = 204;
}
-
- sendNetworkConditionsBroadcast(true /* response received */,
- httpResponseCode != 204 /* isCaptivePortal */,
- requestTimestamp, responseTimestamp);
} catch (IOException e) {
validationLog("Probably not a portal: exception " + e);
if (httpResponseCode == 599) {
@@ -774,11 +856,68 @@
urlConnection.disconnect();
}
}
- final int probeType = ValidationProbeEvent.PROBE_HTTP;
ValidationProbeEvent.logEvent(mNetId, probeTimer.stop(), probeType, httpResponseCode);
return new CaptivePortalProbeResult(httpResponseCode, redirectUrl);
}
+ private CaptivePortalProbeResult sendParallelHttpProbes(URL httpsUrl, URL httpUrl) {
+ // Number of probes to wait for. We might wait for all of them, but we might also return if
+ // only one of them has replied. For example, we immediately return if the HTTP probe finds
+ // a captive portal, even if the HTTPS probe is timing out.
+ final CountDownLatch latch = new CountDownLatch(2);
+
+ // Which probe result we're going to use. This doesn't need to be atomic, but it does need
+ // to be final because otherwise we can't set it from the ProbeThreads.
+ final AtomicReference<CaptivePortalProbeResult> finalResult = new AtomicReference<>();
+
+ final class ProbeThread extends Thread {
+ private final boolean mIsHttps;
+ private volatile CaptivePortalProbeResult mResult;
+
+ public ProbeThread(boolean isHttps) {
+ mIsHttps = isHttps;
+ }
+
+ public CaptivePortalProbeResult getResult() {
+ return mResult;
+ }
+
+ @Override
+ public void run() {
+ if (mIsHttps) {
+ mResult = sendHttpProbe(httpsUrl, ValidationProbeEvent.PROBE_HTTPS);
+ } else {
+ mResult = sendHttpProbe(httpUrl, ValidationProbeEvent.PROBE_HTTP);
+ }
+ if ((mIsHttps && mResult.isSuccessful()) || (!mIsHttps && mResult.isPortal())) {
+ // HTTPS succeeded, or HTTP found a portal. Don't wait for the other probe.
+ finalResult.compareAndSet(null, mResult);
+ latch.countDown();
+ }
+ // Signal that one probe has completed. If we've already made a decision, or if this
+ // is the second probe, the latch will be at zero and we'll return a result.
+ latch.countDown();
+ }
+ }
+
+ ProbeThread httpsProbe = new ProbeThread(true);
+ ProbeThread httpProbe = new ProbeThread(false);
+ httpsProbe.start();
+ httpProbe.start();
+
+ try {
+ latch.await();
+ } catch (InterruptedException e) {
+ validationLog("Error: probe wait interrupted!");
+ return CaptivePortalProbeResult.FAILED;
+ }
+
+ // If there was no deciding probe, that means that both probes completed. Return HTTPS.
+ finalResult.compareAndSet(null, httpsProbe.getResult());
+
+ return finalResult.get();
+ }
+
/**
* @param responseReceived - whether or not we received a valid HTTP response to our request.
* If false, isCaptivePortal and responseTimestampMs are ignored