Merge changes Ib24809ec,Id9d26435 into main

* changes:
  Remove the fail case on IPv6 in testLockdownVpn
  Remove LockdownVpnTracker from testLegacyLockdownVpn

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -418,7 +418,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;
@@ -1500,14 +1499,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,
@@ -1676,12 +1674,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
@@ -1692,12 +1684,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
@@ -10238,74 +10224,28 @@ public class ConnectivityServiceTest {
         // Pretend lockdown VPN was configured.
         final VpnProfile profile = setupLegacyLockdownVpn();
 
-        // LockdownVpnTracker disables the Vpn teardown code and enables lockdown.
+        // Init lockdown state to simulate LockdownVpnTracker behavior.
-        // Check the VPN's state before it does so.
+        mCm.setLegacyLockdownVpnEnabled(true);
-        assertTrue(mMockVpn.getEnableTeardown());
+        mMockVpn.setEnableTeardown(false);
-        assertFalse(mMockVpn.getLockdown());
+        mMockVpn.setLockdown(true);
-
-        // 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());
 
         // Bring up a network.
-        // Expect nothing to happen because the network does not have an IPv4 default route: legacy
-        // VPN only supports IPv4.
         final LinkProperties cellLp = new LinkProperties();
         cellLp.setInterfaceName("rmnet0");
-        cellLp.addLinkAddress(new LinkAddress("2001:db8::1/64"));
-        cellLp.addRoute(new RouteInfo(new IpPrefix("::/0"), null, "rmnet0"));
-        mCellAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp);
-        mCellAgent.connect(false /* validated */);
-        callback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
-        defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
-        systemDefaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
-        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.
         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);
-        callback.expect(LINK_PROPERTIES_CHANGED, mCellAgent);
-        defaultCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent);
-        systemDefaultCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent);
-        waitForIdle();
-        assertNull(mMockVpn.getAgent());
-
-        // Disconnect, then try again with a network that supports IPv4 at connection time.
-        // Expect lockdown VPN to come up.
-        ExpectedBroadcast b1 = expectConnectivityAction(TYPE_MOBILE, DetailedState.DISCONNECTED);
-        mCellAgent.disconnect();
-        callback.expect(LOST, mCellAgent);
-        defaultCallback.expect(LOST, mCellAgent);
-        systemDefaultCallback.expect(LOST, mCellAgent);
-        b1.expectBroadcast();
-
         // When lockdown VPN is active, the NetworkInfo state in CONNECTIVITY_ACTION is overwritten
         // with the state of the VPN network. So expect a CONNECTING broadcast.
-        b1 = expectConnectivityAction(TYPE_MOBILE, DetailedState.CONNECTING);
+        ExpectedBroadcast b1 = expectConnectivityAction(TYPE_MOBILE, DetailedState.CONNECTING);
         mCellAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp);
         mCellAgent.connect(false /* validated */);
         callback.expectAvailableCallbacksUnvalidatedAndBlocked(mCellAgent);
         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);
@@ -10359,23 +10299,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
+        // Simulate LockdownVpnTracker restarting the VPN since it received the systemDefault
-        // connected, so the network is not considered blocked by the lockdown UID ranges? But the
+        // callback with different network.
-        // fact that a VPN is connected should only result in the VPN itself being unblocked, not
+        final ExpectedBroadcast b3 = expectConnectivityAction(TYPE_VPN, DetailedState.DISCONNECTED);
-        // any other network. Bug in isUidBlockedByVpn?
+        mMockVpn.stopVpnRunnerPrivileged();
-        callback.expectAvailableCallbacksUnvalidated(mWiFiAgent);
+        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);
@@ -10420,15 +10362,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.stopVpnRunnerPrivileged();
-        mMockVpn.expectStopVpnRunnerPrivileged();
         callback.expect(LOST, mMockVpn);
+        defaultCallback.expect(LOST, mMockVpn);
         b2.expectBroadcast();
 
-        VMSHandlerThread.quitSafely();
+        assertNoCallbacks(callback, defaultCallback, systemDefaultCallback);
-        VMSHandlerThread.join();
     }
 
     @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2)

tests/unit/java/com/android/server/connectivity/VpnTest.java

@@ -1839,6 +1839,22 @@ public class VpnTest extends VpnTestBase {
         // a subsequent CL.
     }
 
+    @Test
+    public void testStartLegacyVpnIpv6() throws Exception {
+        setMockedUsers(PRIMARY_USER);
+        final Vpn vpn = createVpn(PRIMARY_USER.id);
+        final LinkProperties lp = new LinkProperties();
+        lp.setInterfaceName(EGRESS_IFACE);
+        lp.addLinkAddress(new LinkAddress("2001:db8::1/64"));
+        final RouteInfo defaultRoute = new RouteInfo(
+                new IpPrefix(Inet6Address.ANY, 0), null, EGRESS_IFACE);
+        lp.addRoute(defaultRoute);
+
+        // IllegalStateException thrown since legacy VPN only supports IPv4.
+        assertThrows(IllegalStateException.class,
+                () -> vpn.startLegacyVpn(mVpnProfile, EGRESS_NETWORK, lp));
+    }
+
     private Vpn startLegacyVpn(final Vpn vpn, final VpnProfile vpnProfile) throws Exception {
         setMockedUsers(PRIMARY_USER);