From 0908daaaf00e6b56ebed0a0fce9c3e3fe183a06b Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Mon, 10 Apr 2017 17:43:08 +0900 Subject: [PATCH 1/2] Captive portal: rotate fallback urls This patch introduces a new settings value to specify more than one url for the fallback http probe in addition to the existing settings value. If more than one url exists, a network will rotate urls for the fallback probe one by one everytime the fallback probe is sent. Test: built, flashed, tested manually with various portal networks. Bug: 36532213 Change-Id: Icaa1f95c5914e8840c83ccdf071047358a5b760f --- core/java/android/provider/Settings.java | 9 ++++ .../server/connectivity/NetworkMonitor.java | 46 +++++++++++++++---- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 8e55f4b804cfc..60acbafe5895f 100755 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -8182,6 +8182,15 @@ public final class Settings { */ public static final String CAPTIVE_PORTAL_FALLBACK_URL = "captive_portal_fallback_url"; + /** + * A "|" separated list of URLs used for captive portal detection in addition to the + * fallback HTTP url associated with the CAPTIVE_PORTAL_FALLBACK_URL settings. + * + * @hide + */ + public static final String CAPTIVE_PORTAL_OTHER_FALLBACK_URLS = + "captive_portal_other_fallback_urls"; + /** * 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 diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java index 3f2ca1440508d..f5b2b77765f71 100644 --- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java @@ -70,6 +70,7 @@ import java.net.InetAddress; import java.net.MalformedURLException; import java.net.URL; import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.List; import java.util.Random; import java.util.concurrent.CountDownLatch; @@ -90,6 +91,8 @@ public class NetworkMonitor extends StateMachine { private static final String DEFAULT_HTTP_URL = "http://connectivitycheck.gstatic.com/generate_204"; private static final String DEFAULT_FALLBACK_URL = "http://www.google.com/gen_204"; + private static final String DEFAULT_OTHER_FALLBACK_URLS = + "http://play.googleapis.com/generate_204"; private static final String DEFAULT_USER_AGENT = "Mozilla/5.0 (X11; Linux x86_64) " + "AppleWebKit/537.36 (KHTML, like Gecko) " + "Chrome/52.0.2743.82 Safari/537.36"; @@ -263,7 +266,8 @@ public class NetworkMonitor extends StateMachine { private final String mCaptivePortalUserAgent; private final URL mCaptivePortalHttpsUrl; private final URL mCaptivePortalHttpUrl; - private final URL mCaptivePortalFallbackUrl; + private final URL[] mCaptivePortalFallbackUrls; + private int mNextFallbackUrlIndex = 0; public NetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo, NetworkRequest defaultRequest) { @@ -302,7 +306,7 @@ public class NetworkMonitor extends StateMachine { mCaptivePortalUserAgent = getCaptivePortalUserAgent(context); mCaptivePortalHttpsUrl = makeURL(getCaptivePortalServerHttpsUrl(context)); mCaptivePortalHttpUrl = makeURL(getCaptivePortalServerHttpUrl(context)); - mCaptivePortalFallbackUrl = makeURL(getCaptivePortalFallbackUrl(context)); + mCaptivePortalFallbackUrls = makeCaptivePortalFallbackUrls(context); start(); } @@ -450,7 +454,7 @@ public class NetworkMonitor extends StateMachine { intent.putExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_URL, mLastPortalProbeResult.detectUrl); intent.putExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_USER_AGENT, - getCaptivePortalUserAgent(mContext)); + mCaptivePortalUserAgent); intent.setFlags( Intent.FLAG_ACTIVITY_BROUGHT_TO_FRONT | Intent.FLAG_ACTIVITY_NEW_TASK); mContext.startActivityAsUser(intent, UserHandle.CURRENT); @@ -666,9 +670,24 @@ public class NetworkMonitor extends StateMachine { return getSetting(context, Settings.Global.CAPTIVE_PORTAL_HTTP_URL, DEFAULT_HTTP_URL); } - private static String getCaptivePortalFallbackUrl(Context context) { - return getSetting(context, + private URL[] makeCaptivePortalFallbackUrls(Context context) { + String separator = "|"; + String firstUrl = getSetting(context, Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL, DEFAULT_FALLBACK_URL); + String joinedUrls = firstUrl + separator + getSetting(context, + Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS, DEFAULT_OTHER_FALLBACK_URLS); + List urls = new ArrayList<>(); + for (String s : joinedUrls.split(separator)) { + URL u = makeURL(s); + if (u == null) { + continue; + } + urls.add(u); + } + if (urls.isEmpty()) { + Log.e(TAG, String.format("could not create any url from %s", joinedUrls)); + } + return urls.toArray(new URL[urls.size()]); } private static String getCaptivePortalUserAgent(Context context) { @@ -680,6 +699,15 @@ public class NetworkMonitor extends StateMachine { return value != null ? value : defaultValue; } + private URL nextFallbackUrl() { + if (mCaptivePortalFallbackUrls.length == 0) { + return null; + } + int idx = Math.abs(mNextFallbackUrlIndex) % mCaptivePortalFallbackUrls.length; + mNextFallbackUrlIndex += new Random().nextInt(); // randomely change url without memory. + return mCaptivePortalFallbackUrls[idx]; + } + @VisibleForTesting protected CaptivePortalProbeResult isCaptivePortal() { if (!mIsCaptivePortalCheckEnabled) { @@ -690,7 +718,6 @@ public class NetworkMonitor extends StateMachine { URL pacUrl = null; URL httpsUrl = mCaptivePortalHttpsUrl; URL httpUrl = mCaptivePortalHttpUrl; - URL fallbackUrl = mCaptivePortalFallbackUrl; // 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: @@ -727,7 +754,7 @@ public class NetworkMonitor extends StateMachine { if (pacUrl != null) { result = sendDnsAndHttpProbes(null, pacUrl, ValidationProbeEvent.PROBE_PAC); } else if (mUseHttps) { - result = sendParallelHttpProbes(proxyInfo, httpsUrl, httpUrl, fallbackUrl); + result = sendParallelHttpProbes(proxyInfo, httpsUrl, httpUrl); } else { result = sendDnsAndHttpProbes(proxyInfo, httpUrl, ValidationProbeEvent.PROBE_HTTP); } @@ -861,7 +888,7 @@ public class NetworkMonitor extends StateMachine { } private CaptivePortalProbeResult sendParallelHttpProbes( - ProxyInfo proxy, URL httpsUrl, URL httpUrl, URL fallbackUrl) { + ProxyInfo proxy, URL httpsUrl, URL httpUrl) { // Number of probes to wait for. If a probe completes with a conclusive answer // it shortcuts the latch immediately by forcing the count to 0. final CountDownLatch latch = new CountDownLatch(2); @@ -920,7 +947,8 @@ public class NetworkMonitor extends StateMachine { if (httpsResult.isPortal() || httpsResult.isSuccessful()) { return httpsResult; } - // If a fallback url is specified, use a fallback probe to try again portal detection. + // If a fallback url exists, use a fallback probe to try again portal detection. + URL fallbackUrl = nextFallbackUrl(); if (fallbackUrl != null) { CaptivePortalProbeResult result = sendHttpProbe(fallbackUrl, ValidationProbeEvent.PROBE_FALLBACK); From a415870221ec732290a0c77698da3c1b87818e8d Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Mon, 10 Apr 2017 17:08:06 +0900 Subject: [PATCH 2/2] Captive portal: regroup hardcoded http response codes. This patch regroups various hardcoded http codes into well defined constants. This reduces risk of errors and makes the captive portal logic clearer. This patch also fixes the logging when a captive portal detection probe fails, to take into account https ssl handshake failures: for well-behaved portals it is expected that the https probe will fail, however the error message was written before the introduction of the https probe and had become ambiguous. Test: built, flashed, tested manually with various portal networks Bug: 36532213 Change-Id: Ia15f77e268cb414816fc52f92835289f9a9ce92b --- .../server/connectivity/NetworkMonitor.java | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java index f5b2b77765f71..19929c7b20de7 100644 --- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java @@ -478,7 +478,11 @@ public class NetworkMonitor extends StateMachine { */ @VisibleForTesting public static final class CaptivePortalProbeResult { - static final CaptivePortalProbeResult FAILED = new CaptivePortalProbeResult(599); + static final int SUCCESS_CODE = 204; + static final int FAILED_CODE = 599; + + static final CaptivePortalProbeResult FAILED = new CaptivePortalProbeResult(FAILED_CODE); + static final CaptivePortalProbeResult SUCCESS = new CaptivePortalProbeResult(SUCCESS_CODE); private final int mHttpResponseCode; // HTTP response code returned from Internet probe. final String redirectUrl; // Redirect destination returned from Internet probe. @@ -496,9 +500,16 @@ public class NetworkMonitor extends StateMachine { this(httpResponseCode, null, null); } - boolean isSuccessful() { return mHttpResponseCode == 204; } + boolean isSuccessful() { + return mHttpResponseCode == SUCCESS_CODE; + } + boolean isPortal() { - return !isSuccessful() && mHttpResponseCode >= 200 && mHttpResponseCode <= 399; + return !isSuccessful() && (mHttpResponseCode >= 200) && (mHttpResponseCode <= 399); + } + + boolean isFailed() { + return !isSuccessful() && !isPortal(); } } @@ -712,7 +723,7 @@ public class NetworkMonitor extends StateMachine { protected CaptivePortalProbeResult isCaptivePortal() { if (!mIsCaptivePortalCheckEnabled) { validationLog("Validation disabled."); - return new CaptivePortalProbeResult(204); + return CaptivePortalProbeResult.SUCCESS; } URL pacUrl = null; @@ -816,7 +827,7 @@ public class NetworkMonitor extends StateMachine { @VisibleForTesting protected CaptivePortalProbeResult sendHttpProbe(URL url, int probeType) { HttpURLConnection urlConnection = null; - int httpResponseCode = 599; + int httpResponseCode = CaptivePortalProbeResult.FAILED_CODE; String redirectUrl = null; final Stopwatch probeTimer = new Stopwatch().start(); try { @@ -854,7 +865,7 @@ public class NetworkMonitor extends StateMachine { if (probeType == ValidationProbeEvent.PROBE_PAC) { validationLog( probeType, url, "PAC fetch 200 response interpreted as 204 response."); - httpResponseCode = 204; + httpResponseCode = CaptivePortalProbeResult.SUCCESS_CODE; } else if (urlConnection.getContentLengthLong() == 0) { // Consider 200 response with "Content-length=0" to not be a captive portal. // There's no point in considering this a captive portal as the user cannot @@ -862,20 +873,20 @@ public class NetworkMonitor extends StateMachine { // See http://b/9972012. validationLog(probeType, url, "200 response with Content-length=0 interpreted as 204 response."); - httpResponseCode = 204; + httpResponseCode = CaptivePortalProbeResult.SUCCESS_CODE; } else if (urlConnection.getContentLengthLong() == -1) { // When no Content-length (default value == -1), attempt to read a byte from the // response. Do not use available() as it is unreliable. See http://b/33498325. if (urlConnection.getInputStream().read() == -1) { validationLog( probeType, url, "Empty 200 response interpreted as 204 response."); - httpResponseCode = 204; + httpResponseCode = CaptivePortalProbeResult.SUCCESS_CODE; } } } } catch (IOException e) { - validationLog(probeType, url, "Probably not a portal: exception " + e); - if (httpResponseCode == 599) { + validationLog(probeType, url, "Probe failed with exception " + e); + if (httpResponseCode == CaptivePortalProbeResult.FAILED_CODE) { // TODO: Ping gateway and DNS server and log results. } } finally {