Merge "Make isCaptivePortal perform both HTTP and HTTPS probes." into nyc-dev

api/system-current.txt

@@ -26170,8 +26170,12 @@ package android.net.metrics {
     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 DNS_FAILURE = 0; // 0x0
-    field public static final int PROBE_HTTPS = 1; // 0x1
+    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;

core/java/android/net/metrics/ValidationProbeEvent.java

@@ -29,8 +29,13 @@ import com.android.internal.util.MessageUtils;
 @SystemApi
 public final class ValidationProbeEvent extends IpConnectivityEvent implements Parcelable {
 
-    public static final int PROBE_HTTP  = 0;
+    public static final int PROBE_DNS   = 0;
-    public static final int PROBE_HTTPS = 1;
+    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 @@ public final class ValidationProbeEvent extends IpConnectivityEvent implements P
         }
     };
 
+    /** @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 @@ public final class ValidationProbeEvent extends IpConnectivityEvent implements P
     @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 {

core/java/android/provider/Settings.java

@@ -7705,6 +7705,15 @@ public final class Settings {
          */
         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.
          *

services/core/java/com/android/server/connectivity/NetworkMonitor.java

@@ -71,7 +71,11 @@ import com.android.server.connectivity.NetworkAgentInfo;
 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 @@ public class NetworkMonitor extends StateMachine {
     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 @@ public class NetworkMonitor extends StateMachine {
 
         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 @@ public class NetworkMonitor extends StateMachine {
                     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 @@ public class NetworkMonitor extends StateMachine {
      */
     @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 @@ public class NetworkMonitor extends StateMachine {
             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 @@ public class NetworkMonitor extends StateMachine {
                     //    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 @@ public class NetworkMonitor extends StateMachine {
                     // 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 &&
+                    } else if (probeResult.isPortal()) {
-                            probeResult.mHttpResponseCode <= 399) {
                         mConnectivityServiceHandler.sendMessage(obtainMessage(EVENT_NETWORK_TESTED,
                                 NETWORK_TEST_RESULT_INVALID, mNetId, probeResult.mRedirectUrl));
                         transitionTo(mCaptivePortalState);
@@ -659,27 +688,23 @@ public class NetworkMonitor extends StateMachine {
         }
     }
 
-    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);
     }
 
-    /**
-     * Do a URL fetch on a known server to see if we get the data we expect.
-     * Returns HTTP response code.
-     */
     @VisibleForTesting
     protected CaptivePortalProbeResult isCaptivePortal() {
         if (!mIsCaptivePortalCheckEnabled) return new CaptivePortalProbeResult(204, null);
 
-        HttpURLConnection urlConnection = null;
+        URL pacUrl = null, httpUrl = null, httpsUrl = 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
@@ -697,34 +722,93 @@ public class NetworkMonitor extends StateMachine {
         // 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());
+            try {
-                fetchPac = true;
+                pacUrl = new URL(proxyInfo.getPacFileUrl().toString());
+            } catch (MalformedURLException e) {
+                validationLog("Invalid PAC URL: " + proxyInfo.getPacFileUrl().toString());
+                return CaptivePortalProbeResult.FAILED;
             }
-            final StringBuffer connectInfo = new StringBuffer();
+        }
-            String hostToResolve = null;
+
-            // Only resolve a host if HttpURLConnection is about to, to avoid any potentially
+        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.
-            if (proxyInfo == null || fetchPac) {
+        String hostToResolve = null;
-                hostToResolve = url.getHost();
+        if (pacUrl != null) {
+            hostToResolve = pacUrl.getHost();
         } else if (proxyInfo != null) {
             hostToResolve = proxyInfo.getHost();
+        } else {
+            hostToResolve = httpUrl.getHost();
         }
+
         if (!TextUtils.isEmpty(hostToResolve)) {
-                connectInfo.append(", " + hostToResolve + "=");
+            String probeName = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS);
-                final InetAddress[] addresses =
+            final Stopwatch dnsTimer = new Stopwatch().start();
-                        mNetworkAgentInfo.network.getAllByName(hostToResolve);
+            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);
             }
-            validationLog("Checking " + url.toString() + " on " +
+        }
-                    mNetworkAgentInfo.networkInfo.getExtraInfo() + connectInfo);
+
+        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;
+    }
+
+    /**
+     * Do a URL fetch on a known server to see if we get the data we expect.
+     * Returns HTTP response code.
+     */
+    @VisibleForTesting
+    protected CaptivePortalProbeResult sendHttpProbe(URL url, int probeType) {
+        HttpURLConnection urlConnection = null;
+        int httpResponseCode = 599;
+        String redirectUrl = null;
+        final Stopwatch probeTimer = new Stopwatch().start();
+        try {
             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 @@ public class NetworkMonitor extends StateMachine {
             // 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 @@ public class NetworkMonitor extends StateMachine {
                 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 @@ public class NetworkMonitor extends StateMachine {
                 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