Skip to content
Snippets Groups Projects
Commit 35f6fef3 authored by Hansen Kurli's avatar Hansen Kurli
Browse files

Remove LockdownVpnTracker from testLegacyLockdownVpn 

The test coverage of LockdownVpnTracker has been moved to a
separate unit test file. testLegacyLockdownVpn now calls the Vpn
methods directly instead of creating a new LockdownVpnTracker.
Note this removes calls to expectStopVpnRunnerPrivileged since
stopVpnRunnerPrivileged is now directly called in the test so it
is already guaranteed to be called.

The expected calls/behavior of LockdownVpnTracker can be seen in
LockdownVpnTrackerTest.

Bug: 230548427
Test: atest FrameworksNetTests
Change-Id: Id9d26435bf62ffef954f6c7fa0558ce99540de1f
parent 046b0c1c
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
tests/unit/java/com/android/server/ConnectivityServiceTest.java
+ 37
− 54
View file @ 35f6fef3
......@@ -414,7 +414,6 @@ import com.android.server.connectivity.TcpKeepaliveController;
import com.android.server.connectivity.UidRangeUtils;
import com.android.server.connectivity.Vpn;
import com.android.server.connectivity.VpnProfileStore;
import com.android.server.net.LockdownVpnTracker;
import com.android.server.net.NetworkPinner;
import com.android.testutils.DevSdkIgnoreRule;
import com.android.testutils.DevSdkIgnoreRunner;
......@@ -1496,14 +1495,13 @@ public class ConnectivityServiceTest {
private int mVpnType = VpnManager.TYPE_VPN_SERVICE;
private UnderlyingNetworkInfo mUnderlyingNetworkInfo;
 
// These ConditionVariables allow tests to wait for LegacyVpnRunner to be stopped/started.
// This ConditionVariable allow tests to wait for LegacyVpnRunner to be started.
// TODO: this scheme is ad-hoc and error-prone because it does not fail if, for example, the
// test expects two starts in a row, or even if the production code calls start twice in a
// row. find a better solution. Simply putting a method to create a LegacyVpnRunner into
// Vpn.Dependencies doesn't work because LegacyVpnRunner is not a static class and has
// extensive access into the internals of Vpn.
private ConditionVariable mStartLegacyVpnCv = new ConditionVariable();
private ConditionVariable mStopVpnRunnerCv = new ConditionVariable();
 
public MockVpn(int userId) {
super(startHandlerThreadAndReturnLooper(), mServiceContext,
......@@ -1672,12 +1670,6 @@ public class ConnectivityServiceTest {
public void expectStartLegacyVpnRunner() {
assertTrue("startLegacyVpnRunner not called after " + TIMEOUT_MS + " ms",
mStartLegacyVpnCv.block(TIMEOUT_MS));
// startLegacyVpn calls stopVpnRunnerPrivileged, which will open mStopVpnRunnerCv, just
// before calling startLegacyVpnRunner. Restore mStopVpnRunnerCv, so the test can expect
// that the VpnRunner is stopped and immediately restarted by calling
// expectStartLegacyVpnRunner() and expectStopVpnRunnerPrivileged() back-to-back.
mStopVpnRunnerCv = new ConditionVariable();
}
 
@Override
......@@ -1688,12 +1680,6 @@ public class ConnectivityServiceTest {
mStartLegacyVpnCv = new ConditionVariable();
}
mVpnRunner = null;
mStopVpnRunnerCv.open();
}
public void expectStopVpnRunnerPrivileged() {
assertTrue("stopVpnRunnerPrivileged not called after " + TIMEOUT_MS + " ms",
mStopVpnRunnerCv.block(TIMEOUT_MS));
}
 
@Override
......@@ -10195,28 +10181,10 @@ public class ConnectivityServiceTest {
// Pretend lockdown VPN was configured.
final VpnProfile profile = setupLegacyLockdownVpn();
 
// LockdownVpnTracker disables the Vpn teardown code and enables lockdown.
// Check the VPN's state before it does so.
assertTrue(mMockVpn.getEnableTeardown());
assertFalse(mMockVpn.getLockdown());
// VMSHandlerThread was used inside VpnManagerService and taken into LockDownVpnTracker.
// VpnManagerService was decoupled from this test but this handlerThread is still required
// in LockDownVpnTracker. Keep it until LockDownVpnTracker related verification is moved to
// its own test.
final HandlerThread VMSHandlerThread = new HandlerThread("TestVpnManagerService");
VMSHandlerThread.start();
// LockdownVpnTracker is created from VpnManagerService but VpnManagerService is decoupled
// from ConnectivityServiceTest. Create it directly to simulate LockdownVpnTracker is
// created.
// TODO: move LockdownVpnTracker related tests to its own test.
// Lockdown VPN disables teardown and enables lockdown.
final LockdownVpnTracker lockdownVpnTracker = new LockdownVpnTracker(mServiceContext,
VMSHandlerThread.getThreadHandler(), mMockVpn, profile);
lockdownVpnTracker.init();
assertFalse(mMockVpn.getEnableTeardown());
assertTrue(mMockVpn.getLockdown());
// Init lockdown state to simulate LockdownVpnTracker behavior.
mCm.setLegacyLockdownVpnEnabled(true);
mMockVpn.setEnableTeardown(false);
mMockVpn.setLockdown(true);
 
// Bring up a network.
// Expect nothing to happen because the network does not have an IPv4 default route: legacy
......@@ -10230,12 +10198,15 @@ public class ConnectivityServiceTest {
callback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
systemDefaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
// Simulate LockdownVpnTracker attempting to start the VPN since it received the
// systemDefault callback. IllegalStateException is expected since legacy VPN only supports
// IPv4 and LockdownVpnTracker will catch it to show a notification.
assertThrows(IllegalStateException.class,
() -> mMockVpn.startLegacyVpnPrivileged(profile, mCellAgent.getNetwork(), cellLp));
waitForIdle();
assertNull(mMockVpn.getAgent());
 
// Add an IPv4 address. Ideally the VPN should start, but it doesn't because nothing calls
// LockdownVpnTracker#handleStateChangedLocked. This is a bug.
// TODO: consider fixing this.
// Add an IPv4 address.
cellLp.addLinkAddress(new LinkAddress("192.0.2.2/25"));
cellLp.addRoute(new RouteInfo(new IpPrefix("0.0.0.0/0"), null, "rmnet0"));
mCellAgent.sendLinkProperties(cellLp);
......@@ -10263,6 +10234,9 @@ public class ConnectivityServiceTest {
defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
systemDefaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
b1.expectBroadcast();
// Simulate LockdownVpnTracker attempting to start the VPN since it received the
// systemDefault callback.
mMockVpn.startLegacyVpnPrivileged(profile, mCellAgent.getNetwork(), cellLp);
assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED);
......@@ -10316,23 +10290,25 @@ public class ConnectivityServiceTest {
b1 = expectConnectivityAction(TYPE_MOBILE, DetailedState.DISCONNECTED);
// Wifi is CONNECTING because the VPN isn't up yet.
b2 = expectConnectivityAction(TYPE_WIFI, DetailedState.CONNECTING);
ExpectedBroadcast b3 = expectConnectivityAction(TYPE_VPN, DetailedState.DISCONNECTED);
mWiFiAgent.connect(false /* validated */);
// Wifi is not blocked since VPN network is still connected.
callback.expectAvailableCallbacksUnvalidated(mWiFiAgent);
defaultCallback.assertNoCallback();
systemDefaultCallback.expectAvailableCallbacksUnvalidated(mWiFiAgent);
b1.expectBroadcast();
b2.expectBroadcast();
b3.expectBroadcast();
mMockVpn.expectStopVpnRunnerPrivileged();
mMockVpn.expectStartLegacyVpnRunner();
 
// TODO: why is wifi not blocked? Is it because when this callback is sent, the VPN is still
// connected, so the network is not considered blocked by the lockdown UID ranges? But the
// fact that a VPN is connected should only result in the VPN itself being unblocked, not
// any other network. Bug in isUidBlockedByVpn?
callback.expectAvailableCallbacksUnvalidated(mWiFiAgent);
// Simulate LockdownVpnTracker restarting the VPN since it received the systemDefault
// callback with different network.
final ExpectedBroadcast b3 = expectConnectivityAction(TYPE_VPN, DetailedState.DISCONNECTED);
mMockVpn.stopVpnRunnerPrivileged();
mMockVpn.startLegacyVpnPrivileged(profile, mWiFiAgent.getNetwork(), wifiLp);
mMockVpn.expectStartLegacyVpnRunner();
callback.expect(LOST, mMockVpn);
defaultCallback.expect(LOST, mMockVpn);
defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiAgent);
systemDefaultCallback.expectAvailableCallbacksUnvalidated(mWiFiAgent);
systemDefaultCallback.assertNoCallback();
b3.expectBroadcast();
 
// While the VPN is reconnecting on the new network, everything is blocked.
assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED);
......@@ -10377,15 +10353,22 @@ public class ConnectivityServiceTest {
b2 = expectConnectivityAction(TYPE_VPN, DetailedState.DISCONNECTED);
mWiFiAgent.disconnect();
callback.expect(LOST, mWiFiAgent);
callback.expectCaps(mMockVpn, c -> !c.hasTransport(TRANSPORT_WIFI));
defaultCallback.expectCaps(mMockVpn, c -> !c.hasTransport(TRANSPORT_WIFI));
systemDefaultCallback.expect(LOST, mWiFiAgent);
// TODO: There should only be one LOST callback. Since the WIFI network is underlying a VPN
// network, ConnectivityService#propagateUnderlyingNetworkCapabilities() causes a rematch to
// occur. Notably, this happens before setting the satisfiers of its network requests to
// null. Since the satisfiers are set to null in the rematch, an extra LOST callback is
// called.
systemDefaultCallback.expect(LOST, mWiFiAgent);
b1.expectBroadcast();
callback.expectCaps(mMockVpn, c -> !c.hasTransport(TRANSPORT_WIFI));
mMockVpn.expectStopVpnRunnerPrivileged();
mMockVpn.stopVpnRunnerPrivileged();
callback.expect(LOST, mMockVpn);
defaultCallback.expect(LOST, mMockVpn);
b2.expectBroadcast();
 
VMSHandlerThread.quitSafely();
VMSHandlerThread.join();
assertNoCallbacks(callback, defaultCallback, systemDefaultCallback);
}
 
@Test @IgnoreUpTo(Build.VERSION_CODES.S_V2)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment