Have NetworkMonitor treat as-is networks as validated.
When the captive portal app reports the network should be used
as-is (typically when there was an SSL error and the user chose
to "continue anyway via browser"), have NetworkMonitor mark the
network as validated and partial connectivity. This slightly
violates the letter of the documentation for VALIDATED, but not
the spirit.
Subsequent attempts to revalidate will also result in a result
of VALIDATED, until the network is disconnected or the captive
portal opens. Nothing triggers a new validation though, so this
will only happen if an app calls reportNetworkConnectivity.
The network is still usable in the mean time.
The validation result is sent immediately as the user taps the
"continue anyway" button. NetworkMonitor then proceeds to perform
private DNS validation, and when the full sequence of validation
is finished it will send another validation result, very likely
with the same values.
This shows a "connected" notification as the user taps on "continue
anyway via browser", which may be slightly unexpected but is
correct, so it's probably fine.
If Private DNS is in strict mode but can't be accessed after the
user decides to use as is, a notification is shown to warn the
user that the Private DNS server can't be accessed. If private
DNS settings are changed, they are reevaluated as normal.
Bug: 168431719
Test: new unit test included
also manually tested
Change-Id: I2e6cf5f840891453cd5587a42d907c90f15f20e7
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 3d2d804..e1705b6 100755
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -388,7 +388,7 @@
private static final int CMD_BANDWIDTH_CHECK_COMPLETE = 23;
/**
- * Message to self to know the bandwidth check is timeouted.
+ * Message to self to know the bandwidth check has timed out.
*/
private static final int CMD_BANDWIDTH_CHECK_TIMEOUT = 24;
@@ -636,6 +636,13 @@
/**
* Request the NetworkMonitor to reevaluate the network.
+ *
+ * TODO : refactor reevaluation to introduce rate limiting. If the system finds a network is
+ * validated but some app can't access their server, or the network is behind a captive portal
+ * that only lets the validation URL through, apps may be calling reportNetworkConnectivity
+ * often, causing many revalidation attempts. Meanwhile, reevaluation attempts that result
+ * from actions that may affect the validation status (e.g. the user just logged in through
+ * the captive portal app) should never be skipped because of the rate limitation.
*/
public void forceReevaluation(int responsibleUid) {
sendMessage(CMD_FORCE_REEVALUATION, responsibleUid, 0);
@@ -916,6 +923,18 @@
// If the user wants to use this network anyway, there is no need to
// perform the bandwidth check even if configured.
mIsBandwidthCheckPassedOrIgnored = true;
+ // If the user wants to use this network anyway, it should always
+ // be reported as validated, but other checks still need to be
+ // done. For example, it should still validate strict private DNS and
+ // show a notification if not available, because the network will
+ // be unusable for this additional reason.
+ mEvaluationState.setCaptivePortalWantedAsIs();
+ // A successful evaluation result should be reported immediately, so
+ // that the network stack may immediately use the validation in ranking
+ // without waiting for a possibly long private DNS or bandwidth eval
+ // step.
+ mEvaluationState.reportEvaluationResult(NETWORK_VALIDATION_RESULT_VALID,
+ null);
// TODO: Distinguish this from a network that actually validates.
// Displaying the "x" on the system UI icon may still be a good idea.
transitionTo(mEvaluatingPrivateDnsState);
@@ -1290,11 +1309,19 @@
// the network so don't bother validating here. Furthermore sending HTTP
// packets over the network may be undesirable, for example an extremely
// expensive metered network, or unwanted leaking of the User Agent string.
+ // Also don't bother validating networks that the user already said they
+ // wanted as-is.
//
// On networks that need to support private DNS in strict mode (e.g., VPNs, but
// not networks that don't provide Internet access), we still need to perform
// private DNS server resolution.
- if (!isValidationRequired()) {
+ if (mEvaluationState.isCaptivePortalWantedAsIs()
+ && isPrivateDnsValidationRequired()) {
+ // Captive portals can only be detected on networks that validate both
+ // validation and private DNS validation.
+ validationLog("Captive portal is used as is, resolving private DNS");
+ transitionTo(mEvaluatingPrivateDnsState);
+ } else if (!isValidationRequired()) {
if (isPrivateDnsValidationRequired()) {
validationLog("Network would not satisfy default request, "
+ "resolving private DNS");
@@ -3390,18 +3417,28 @@
// NETWORK_VALIDATION_RESULT_VALID. But with this scheme, the first two or three validation
// reports are all failures, because they are "HTTP succeeded but validation not yet passed",
// "HTTP and HTTPS succeeded but validation not yet passed", etc.
+ // TODO : rename EvaluationState to not contain "State" in the name, as it makes this class
+ // sound like one of the states of the state machine, which it's not.
@VisibleForTesting
protected class EvaluationState {
// The latest validation result for this network. This is a bitmask of
// INetworkMonitor.NETWORK_VALIDATION_RESULT_* constants.
private int mEvaluationResult = NETWORK_VALIDATION_RESULT_INVALID;
+
+
+ // Set when the captive portal app said this network should be used as is as a result
+ // of user interaction. The valid bit represents the user's decision to override automatic
+ // determination of whether the network has access to Internet, so in this case the
+ // network is always reported as validated.
+ // TODO : Make ConnectivityService aware of this state, so that it can use the network as
+ // the default without setting the VALIDATED bit, as it's a bit of a lie. This can't be
+ // done on Android <= R where CS can't be updated, but it is doable on S+.
+ private boolean mCaptivePortalWantedAsIs = false;
// Indicates which probes have succeeded since clearProbeResults was called.
// This is a bitmask of INetworkMonitor.NETWORK_VALIDATION_PROBE_* constants.
private int mProbeResults = 0;
// A bitmask to record which probes are completed.
private int mProbeCompleted = 0;
- // The latest redirect URL.
- private String mRedirectUrl;
protected void clearProbeResults() {
mProbeResults = 0;
@@ -3435,8 +3472,18 @@
});
}
+ protected void setCaptivePortalWantedAsIs() {
+ mCaptivePortalWantedAsIs = true;
+ }
+
+ protected boolean isCaptivePortalWantedAsIs() {
+ return mCaptivePortalWantedAsIs;
+ }
+
protected void reportEvaluationResult(int result, @Nullable String redirectUrl) {
- if (!isValidationRequired() && mProbeCompleted == 0 && ShimUtils.isAtLeastS()) {
+ if (mCaptivePortalWantedAsIs) {
+ result = NETWORK_VALIDATION_RESULT_VALID;
+ } else if (!isValidationRequired() && mProbeCompleted == 0 && ShimUtils.isAtLeastS()) {
// If validation is not required AND no probes were attempted, the validation was
// skipped. Report this to ConnectivityService for ConnectivityDiagnostics, but only
// if the platform is Android S+, as ConnectivityService must also know how to
@@ -3445,7 +3492,6 @@
}
mEvaluationResult = result;
- mRedirectUrl = redirectUrl;
final NetworkTestResultParcelable p = new NetworkTestResultParcelable();
p.result = result;
p.probesSucceeded = mProbeResults;
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index 606d243..2de9016 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -17,6 +17,7 @@
package com.android.server.connectivity;
import static android.net.CaptivePortal.APP_RETURN_DISMISSED;
+import static android.net.CaptivePortal.APP_RETURN_WANTED_AS_IS;
import static android.net.DnsResolver.TYPE_A;
import static android.net.DnsResolver.TYPE_AAAA;
import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_DNS;
@@ -1878,12 +1879,10 @@
nm.getEvaluationState().getEvaluationResult());
}
- @Test
- public void testLaunchCaptivePortalApp() throws Exception {
+ public void setupAndLaunchCaptivePortalApp(final NetworkMonitor nm) throws Exception {
setSslException(mHttpsConnection);
setPortal302(mHttpConnection);
when(mHttpConnection.getHeaderField(eq("location"))).thenReturn(TEST_LOGIN_URL);
- final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES);
notifyNetworkConnected(nm, CELL_METERED_CAPABILITIES);
verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
@@ -1910,11 +1909,18 @@
final String redirectUrl = bundle.getString(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_URL);
assertEquals(TEST_HTTP_URL, redirectUrl);
+ resetCallbacks();
+ }
+
+ @Test
+ public void testCaptivePortalLogin() throws Exception {
+ final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES);
+ setupAndLaunchCaptivePortalApp(nm);
+
// Have the app report that the captive portal is dismissed, and check that we revalidate.
setStatus(mHttpsConnection, 204);
setStatus(mHttpConnection, 204);
- resetCallbacks();
nm.notifyCaptivePortalAppFinished(APP_RETURN_DISMISSED);
verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce())
.notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable(
@@ -1924,6 +1930,31 @@
}
@Test
+ public void testCaptivePortalUseAsIs() throws Exception {
+ final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES);
+ setupAndLaunchCaptivePortalApp(nm);
+
+ // The user decides this network is wanted as is, either by encountering an SSL error or
+ // encountering an unknown scheme and then deciding to continue through the browser, or by
+ // selecting this option through the options menu.
+ nm.notifyCaptivePortalAppFinished(APP_RETURN_WANTED_AS_IS);
+ // The captive portal is still closed, but the network validates since the user said so.
+ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce())
+ .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable(
+ NETWORK_VALIDATION_RESULT_VALID, 0 /* probesSucceeded */));
+ resetCallbacks();
+
+ // Revalidate.
+ nm.forceReevaluation(0 /* responsibleUid */);
+
+ // The network should still be valid.
+ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce())
+ .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable(
+ NETWORK_VALIDATION_RESULT_VALID, 0 /* probesSucceeded */,
+ TEST_LOGIN_URL));
+ }
+
+ @Test
public void testPrivateDnsSuccess() throws Exception {
setStatus(mHttpsConnection, 204);
setStatus(mHttpConnection, 204);
