Have NetworkMonitor treat as-is networks as validated. am: 520ddc827c (5415c9c6) · Commits · e / os / android_packages_modules_NetworkStack

src/com/android/server/connectivity/NetworkMonitor.java

+52 −6

Original line number	Diff line number	Diff line
		@@ -388,7 +388,7 @@ public class NetworkMonitor extends StateMachine {
		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 @@ public class NetworkMonitor extends StateMachine {

		/**
		* 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 @@ public class NetworkMonitor extends StateMachine {
		// 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 @@ public class NetworkMonitor extends StateMachine {
		// 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 @@ public class NetworkMonitor extends StateMachine {
		// 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 @@ public class NetworkMonitor extends StateMachine {
		});
		}

		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 @@ public class NetworkMonitor extends StateMachine {
		}

		mEvaluationResult = result;
		mRedirectUrl = redirectUrl;
		final NetworkTestResultParcelable p = new NetworkTestResultParcelable();
		p.result = result;
		p.probesSucceeded = mProbeResults;

tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java

+35 −4

Original line number	Diff line number	Diff line
		@@ -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 @@ public class NetworkMonitorTest {
		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 @@ public class NetworkMonitorTest {
		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(
		@@ -1923,6 +1929,31 @@ public class NetworkMonitorTest {
		assertEquals(0, mRegisteredReceivers.size());
		}

		@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);