Merge "Only allow HTTP capport URLs on test networks" am: 77d241ba35 (15fe5d9c) · Commits · e / os / android_packages_modules_NetworkStack

apishim/30/com/android/networkstack/apishim/api30/ConstantsShim.java

+5 −0

Original line number	Diff line number	Diff line
		@@ -37,4 +37,9 @@ public class ConstantsShim extends com.android.networkstack.apishim.api29.Consta
		DataStallReport.DETECTION_METHOD_DNS_EVENTS;
		public static final int DETECTION_METHOD_TCP_METRICS =
		DataStallReport.DETECTION_METHOD_TCP_METRICS;

		/**
		* @see android.net.NetworkCapabilities
		*/
		public static final int TRANSPORT_TEST = 7;
		}

apishim/31/com/android/networkstack/apishim/ConstantsShim.java

+3 −0

Original line number	Diff line number	Diff line
		@@ -30,4 +30,7 @@ public class ConstantsShim extends com.android.networkstack.apishim.api30.Consta
		*/
		@VisibleForTesting
		public static final int VERSION = 31;

		// TODO: add TRANSPORT_TEST to system API in API 31 (it is only a test API as of R)
		public static final int TRANSPORT_TEST = 7;
		}

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

+51 −27

Original line number	Diff line number	Diff line
		@@ -83,6 +83,7 @@ import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;

		import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;
		import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_TCP_METRICS;
		import static com.android.networkstack.apishim.ConstantsShim.TRANSPORT_TEST;
		import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX;
		import static com.android.networkstack.util.DnsUtils.TYPE_ADDRCONFIG;

		@@ -387,7 +388,8 @@ public class NetworkMonitor extends StateMachine {
		private static final int CMD_BANDWIDTH_CHECK_TIMEOUT = 24;

		// Start mReevaluateDelayMs at this value and double.
		private static final int INITIAL_REEVALUATE_DELAY_MS = 1000;
		@VisibleForTesting
		static final int INITIAL_REEVALUATE_DELAY_MS = 1000;
		private static final int MAX_REEVALUATE_DELAY_MS = 10 * 60 * 1000;
		// Default timeout of evaluating network bandwidth.
		private static final int DEFAULT_EVALUATING_BANDWIDTH_TIMEOUT_MS = 10_000;
		@@ -1434,8 +1436,9 @@ public class NetworkMonitor extends StateMachine {
		}

		final int token = ++mProbeToken;
		final EvaluationThreadDeps deps = new EvaluationThreadDeps(mNetworkCapabilities);
		mThread = new Thread(() -> sendMessage(obtainMessage(CMD_PROBE_COMPLETE, token, 0,
		isCaptivePortal())));
		isCaptivePortal(deps))));
		mThread.start();
		}

		@@ -2147,8 +2150,25 @@ public class NetworkMonitor extends StateMachine {
		return mCaptivePortalFallbackSpecs[idx];
		}

		@VisibleForTesting
		protected CaptivePortalProbeResult isCaptivePortal() {
		/**
		* Parameters that can be accessed by the evaluation thread in a thread-safe way.
		*
		* Parameters such as LinkProperties and NetworkCapabilities cannot be accessed by the
		* evaluation thread directly, as they are managed in the state machine thread and not
		* synchronized. This class provides a copy of the required data that is not modified and can be
		* used safely by the evaluation thread.
		*/
		private static class EvaluationThreadDeps {
		// TODO: add parameters that are accessed in a non-thread-safe way from the evaluation
		// thread (read from LinkProperties, NetworkCapabilities, useHttps, validationStage)
		private final boolean mIsTestNetwork;

		EvaluationThreadDeps(NetworkCapabilities nc) {
		this.mIsTestNetwork = nc.hasTransport(TRANSPORT_TEST);
		}
		}

		private CaptivePortalProbeResult isCaptivePortal(EvaluationThreadDeps deps) {
		if (!mIsCaptivePortalCheckEnabled) {
		validationLog("Validation disabled.");
		return CaptivePortalProbeResult.success(CaptivePortalProbeResult.PROBE_UNKNOWN);
		@@ -2196,11 +2216,11 @@ public class NetworkMonitor extends StateMachine {
		reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, result);
		} else if (mUseHttps && httpsUrls.length == 1 && httpUrls.length == 1) {
		// Probe results are reported inside sendHttpAndHttpsParallelWithFallbackProbes.
		result = sendHttpAndHttpsParallelWithFallbackProbes(
		proxyInfo, httpsUrls[0], httpUrls[0]);
		result = sendHttpAndHttpsParallelWithFallbackProbes(deps, proxyInfo,
		httpsUrls[0], httpUrls[0]);
		} else if (mUseHttps) {
		// Support result aggregation from multiple Urls.
		result = sendMultiParallelHttpAndHttpsProbes(proxyInfo, httpsUrls, httpUrls);
		result = sendMultiParallelHttpAndHttpsProbes(deps, proxyInfo, httpsUrls, httpUrls);
		} else {
		result = sendDnsAndHttpProbes(proxyInfo, httpUrls[0], ValidationProbeEvent.PROBE_HTTP);
		reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, result);
		@@ -2482,12 +2502,12 @@ public class NetworkMonitor extends StateMachine {
		private final CountDownLatch mLatch;
		private final Probe mProbe;

		ProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, int probeType,
		Uri captivePortalApiUrl) {
		ProbeThread(CountDownLatch latch, EvaluationThreadDeps deps, ProxyInfo proxy, URL url,
		int probeType, Uri captivePortalApiUrl) {
		mLatch = latch;
		mProbe = (probeType == ValidationProbeEvent.PROBE_HTTPS)
		? new HttpsProbe(proxy, url, captivePortalApiUrl)
		: new HttpProbe(proxy, url, captivePortalApiUrl);
		? new HttpsProbe(deps, proxy, url, captivePortalApiUrl)
		: new HttpProbe(deps, proxy, url, captivePortalApiUrl);
		mResult = CaptivePortalProbeResult.failed(probeType);
		}

		@@ -2512,11 +2532,14 @@ public class NetworkMonitor extends StateMachine {
		}

		private abstract static class Probe {
		protected final EvaluationThreadDeps mDeps;
		protected final ProxyInfo mProxy;
		protected final URL mUrl;
		protected final Uri mCaptivePortalApiUrl;

		protected Probe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
		protected Probe(EvaluationThreadDeps deps, ProxyInfo proxy, URL url,
		Uri captivePortalApiUrl) {
		mDeps = deps;
		mProxy = proxy;
		mUrl = url;
		mCaptivePortalApiUrl = captivePortalApiUrl;
		@@ -2526,8 +2549,8 @@ public class NetworkMonitor extends StateMachine {
		}

		final class HttpsProbe extends Probe {
		HttpsProbe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
		super(proxy, url, captivePortalApiUrl);
		HttpsProbe(EvaluationThreadDeps deps, ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
		super(deps, proxy, url, captivePortalApiUrl);
		}

		@Override
		@@ -2537,8 +2560,8 @@ public class NetworkMonitor extends StateMachine {
		}

		final class HttpProbe extends Probe {
		HttpProbe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
		super(proxy, url, captivePortalApiUrl);
		HttpProbe(EvaluationThreadDeps deps, ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
		super(deps, proxy, url, captivePortalApiUrl);
		}

		private CaptivePortalDataShim tryCapportApiProbe() {
		@@ -2551,9 +2574,9 @@ public class NetworkMonitor extends StateMachine {
		// Protocol must be HTTPS
		// (as per https://www.ietf.org/id/draft-ietf-capport-api-07.txt, #4).
		// Only allow HTTP on localhost, for testing.
		final boolean isLocalhostHttp =
		"localhost".equals(url.getHost()) && "http".equals(url.getProtocol());
		if (!"https".equals(url.getProtocol()) && !isLocalhostHttp) {
		final boolean isTestLocalhostHttp = mDeps.mIsTestNetwork
		&& "localhost".equals(url.getHost()) && "http".equals(url.getProtocol());
		if (!"https".equals(url.getProtocol()) && !isTestLocalhostHttp) {
		validationLog("Invalid captive portal API protocol: " + url.getProtocol());
		return null;
		}
		@@ -2636,8 +2659,9 @@ public class NetworkMonitor extends StateMachine {
		&& captivePortalApiUrl == null);
		}

		private CaptivePortalProbeResult sendMultiParallelHttpAndHttpsProbes(@NonNull ProxyInfo proxy,
		@NonNull URL[] httpsUrls, @NonNull URL[] httpUrls) {
		private CaptivePortalProbeResult sendMultiParallelHttpAndHttpsProbes(
		@NonNull EvaluationThreadDeps deps, @Nullable ProxyInfo proxy, @NonNull URL[] httpsUrls,
		@NonNull URL[] httpUrls) {
		// If multiple URLs are required to ensure the correctness of validation, send parallel
		// probes to explore the result in separate probe threads and aggregate those results into
		// one as the final result for either HTTP or HTTPS.
		@@ -2662,13 +2686,13 @@ public class NetworkMonitor extends StateMachine {
		// TODO: Have the capport probe as a different probe for cleanliness.
		final URL urlMaybeWithCapport = httpUrls[0];
		for (final URL url : httpUrls) {
		futures.add(ecs.submit(() -> new HttpProbe(proxy, url,
		futures.add(ecs.submit(() -> new HttpProbe(deps, proxy, url,
		url.equals(urlMaybeWithCapport) ? capportApiUrl : null).sendProbe()));
		}

		for (final URL url : httpsUrls) {
		futures.add(
		ecs.submit(() -> new HttpsProbe(proxy, url, capportApiUrl).sendProbe()));
		futures.add(ecs.submit(() -> new HttpsProbe(deps, proxy, url, capportApiUrl)
		.sendProbe()));
		}

		final ArrayList<CaptivePortalProbeResult> completedProbes = new ArrayList<>();
		@@ -2764,15 +2788,15 @@ public class NetworkMonitor extends StateMachine {
		}

		private CaptivePortalProbeResult sendHttpAndHttpsParallelWithFallbackProbes(
		ProxyInfo proxy, URL httpsUrl, URL httpUrl) {
		EvaluationThreadDeps deps, 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);

		final Uri capportApiUrl = getCaptivePortalApiUrl(mLinkProperties);
		final ProbeThread httpsProbe = new ProbeThread(latch, proxy, httpsUrl,
		final ProbeThread httpsProbe = new ProbeThread(latch, deps, proxy, httpsUrl,
		ValidationProbeEvent.PROBE_HTTPS, capportApiUrl);
		final ProbeThread httpProbe = new ProbeThread(latch, proxy, httpUrl,
		final ProbeThread httpProbe = new ProbeThread(latch, deps, proxy, httpUrl,
		ValidationProbeEvent.PROBE_HTTP, capportApiUrl);

		try {

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

+8 −4

Original line number	Diff line number	Diff line
		@@ -52,6 +52,7 @@ import static android.net.util.NetworkStackUtils.TEST_URL_EXPIRATION_TIME;
		import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;

		import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX;
		import static com.android.server.connectivity.NetworkMonitor.INITIAL_REEVALUATE_DELAY_MS;
		import static com.android.server.connectivity.NetworkMonitor.extractCharset;

		import static junit.framework.Assert.assertEquals;
		@@ -1113,10 +1114,13 @@ public class NetworkMonitorTest {
		verify(mFallbackConnection, times(1)).getResponseCode();
		verify(mOtherFallbackConnection, never()).getResponseCode();

		// Second check uses the URL chosen by Random
		final CaptivePortalProbeResult result = monitor.isCaptivePortal();
		assertTrue(result.isPortal());
		verify(mOtherFallbackConnection, times(1)).getResponseCode();
		// Second check should be triggered automatically after the reevaluate delay, and uses the
		// URL chosen by mRandom
		// This test is appropriate to cover reevaluate behavior as long as the timeout is short
		assertTrue(INITIAL_REEVALUATE_DELAY_MS < 2000);
		verify(mOtherFallbackConnection, timeout(INITIAL_REEVALUATE_DELAY_MS + HANDLER_TIMEOUT_MS))
		.getResponseCode();
		verifyNetworkTested(VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, TEST_LOGIN_URL);
		}

		@Test