Merge "Only allow HTTP capport URLs on test networks" (77d241ba) · Commits · e / os / android_packages_modules_NetworkStack

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

+5 −0

Original line number	Original line	Diff line number	Diff line
	@@ -37,4 +37,9 @@ public class ConstantsShim extends com.android.networkstack.apishim.api29.Consta
	DataStallReport.DETECTION_METHOD_DNS_EVENTS;		DataStallReport.DETECTION_METHOD_DNS_EVENTS;
	public static final int DETECTION_METHOD_TCP_METRICS =		public static final int DETECTION_METHOD_TCP_METRICS =
	DataStallReport.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	Original line	Diff line number	Diff line
	@@ -30,4 +30,7 @@ public class ConstantsShim extends com.android.networkstack.apishim.api30.Consta
	*/		*/
	@VisibleForTesting		@VisibleForTesting
	public static final int VERSION = 31;		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	Original line	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_DNS_EVENTS;
	import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_TCP_METRICS;		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.PRIVATE_DNS_PROBE_HOST_SUFFIX;
	import static com.android.networkstack.util.DnsUtils.TYPE_ADDRCONFIG;		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;		private static final int CMD_BANDWIDTH_CHECK_TIMEOUT = 24;

	// Start mReevaluateDelayMs at this value and double.		// 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;		private static final int MAX_REEVALUATE_DELAY_MS = 10 * 60 * 1000;
	// Default timeout of evaluating network bandwidth.		// Default timeout of evaluating network bandwidth.
	private static final int DEFAULT_EVALUATING_BANDWIDTH_TIMEOUT_MS = 10_000;		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 int token = ++mProbeToken;
			final EvaluationThreadDeps deps = new EvaluationThreadDeps(mNetworkCapabilities);
	mThread = new Thread(() -> sendMessage(obtainMessage(CMD_PROBE_COMPLETE, token, 0,		mThread = new Thread(() -> sendMessage(obtainMessage(CMD_PROBE_COMPLETE, token, 0,
	isCaptivePortal())));		isCaptivePortal(deps))));
	mThread.start();		mThread.start();
	}		}

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

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

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

	private abstract static class Probe {		private abstract static class Probe {
			protected final EvaluationThreadDeps mDeps;
	protected final ProxyInfo mProxy;		protected final ProxyInfo mProxy;
	protected final URL mUrl;		protected final URL mUrl;
	protected final Uri mCaptivePortalApiUrl;		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;		mProxy = proxy;
	mUrl = url;		mUrl = url;
	mCaptivePortalApiUrl = captivePortalApiUrl;		mCaptivePortalApiUrl = captivePortalApiUrl;
	@@ -2526,8 +2549,8 @@ public class NetworkMonitor extends StateMachine {
	}		}

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

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

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

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

	private CaptivePortalProbeResult sendMultiParallelHttpAndHttpsProbes(@NonNull ProxyInfo proxy,		private CaptivePortalProbeResult sendMultiParallelHttpAndHttpsProbes(
	@NonNull URL[] httpsUrls, @NonNull URL[] httpUrls) {		@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		// 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		// probes to explore the result in separate probe threads and aggregate those results into
	// one as the final result for either HTTP or HTTPS.		// 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.		// TODO: Have the capport probe as a different probe for cleanliness.
	final URL urlMaybeWithCapport = httpUrls[0];		final URL urlMaybeWithCapport = httpUrls[0];
	for (final URL url : httpUrls) {		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()));		url.equals(urlMaybeWithCapport) ? capportApiUrl : null).sendProbe()));
	}		}

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

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

	private CaptivePortalProbeResult sendHttpAndHttpsParallelWithFallbackProbes(		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		// 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.		// it shortcuts the latch immediately by forcing the count to 0.
	final CountDownLatch latch = new CountDownLatch(2);		final CountDownLatch latch = new CountDownLatch(2);

	final Uri capportApiUrl = getCaptivePortalApiUrl(mLinkProperties);		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);		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);		ValidationProbeEvent.PROBE_HTTP, capportApiUrl);

	try {		try {

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

+8 −4

Original line number	Original line	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 android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;

	import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX;		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 com.android.server.connectivity.NetworkMonitor.extractCharset;

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

	// Second check uses the URL chosen by Random		// Second check should be triggered automatically after the reevaluate delay, and uses the
	final CaptivePortalProbeResult result = monitor.isCaptivePortal();		// URL chosen by mRandom
	assertTrue(result.isPortal());		// This test is appropriate to cover reevaluate behavior as long as the timeout is short
	verify(mOtherFallbackConnection, times(1)).getResponseCode();		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		@Test