Merge changes I14740854,Id1c2dc28,I4168c868 am: 3b0cb56475 am: 06085e6ab3

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1607195

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: I60fd6aea35944751476e2727936e7988c89de442

services/core/java/com/android/server/VcnManagementService.java

@@ -16,6 +16,9 @@
 
 package com.android.server;
 
+import static android.net.vcn.VcnManager.VCN_STATUS_CODE_ACTIVE;
+import static android.net.vcn.VcnManager.VCN_STATUS_CODE_INACTIVE;
+import static android.net.vcn.VcnManager.VCN_STATUS_CODE_NOT_CONFIGURED;
 import static android.net.vcn.VcnManager.VCN_STATUS_CODE_SAFE_MODE;
 
 import static com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot;
@@ -37,6 +40,7 @@ import android.net.vcn.IVcnUnderlyingNetworkPolicyListener;
 import android.net.vcn.VcnConfig;
 import android.net.vcn.VcnManager;
 import android.net.vcn.VcnManager.VcnErrorCode;
+import android.net.vcn.VcnManager.VcnStatusCode;
 import android.net.vcn.VcnUnderlyingNetworkPolicy;
 import android.net.wifi.WifiInfo;
 import android.os.Binder;
@@ -421,6 +425,11 @@ public class VcnManagementService extends IVcnManagementService.Stub {
                                 // Carrier App manually removing/adding a VcnConfig.
                                 if (mVcns.get(uuidToTeardown) == instanceToTeardown) {
                                     stopVcnLocked(uuidToTeardown);
+
+                                    // TODO(b/181789060): invoke asynchronously after Vcn notifies
+                                    // through VcnCallback
+                                    notifyAllPermissionedStatusCallbacksLocked(
+                                            uuidToTeardown, VCN_STATUS_CODE_INACTIVE);
                                 }
                             }
                         }, instanceToTeardown, CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS);
@@ -454,6 +463,17 @@ public class VcnManagementService extends IVcnManagementService.Stub {
         }
     }
 
+    @GuardedBy("mLock")
+    private void notifyAllPermissionedStatusCallbacksLocked(
+            @NonNull ParcelUuid subGroup, @VcnStatusCode int statusCode) {
+        for (final VcnStatusCallbackInfo cbInfo : mRegisteredStatusCallbacks.values()) {
+            if (isCallbackPermissioned(cbInfo, subGroup)) {
+                Binder.withCleanCallingIdentity(
+                        () -> cbInfo.mCallback.onVcnStatusChanged(statusCode));
+            }
+        }
+    }
+
     @GuardedBy("mLock")
     private void startVcnLocked(@NonNull ParcelUuid subscriptionGroup, @NonNull VcnConfig config) {
         Slog.v(TAG, "Starting VCN config for subGrp: " + subscriptionGroup);
@@ -470,6 +490,9 @@ public class VcnManagementService extends IVcnManagementService.Stub {
         // Now that a new VCN has started, notify all registered listeners to refresh their
         // UnderlyingNetworkPolicy.
         notifyAllPolicyListenersLocked();
+
+        // TODO(b/181789060): invoke asynchronously after Vcn notifies through VcnCallback
+        notifyAllPermissionedStatusCallbacksLocked(subscriptionGroup, VCN_STATUS_CODE_ACTIVE);
     }
 
     @GuardedBy("mLock")
@@ -478,7 +501,16 @@ public class VcnManagementService extends IVcnManagementService.Stub {
         Slog.v(TAG, "Starting or updating VCN config for subGrp: " + subscriptionGroup);
 
         if (mVcns.containsKey(subscriptionGroup)) {
-            mVcns.get(subscriptionGroup).updateConfig(config);
+            final Vcn vcn = mVcns.get(subscriptionGroup);
+            final boolean isActive = vcn.isActive();
+            vcn.updateConfig(config);
+
+            // Only notify VcnStatusCallbacks if this VCN was previously in Safe Mode
+            if (!isActive) {
+                // TODO(b/181789060): invoke asynchronously after Vcn notifies through VcnCallback
+                notifyAllPermissionedStatusCallbacksLocked(
+                        subscriptionGroup, VCN_STATUS_CODE_ACTIVE);
+            }
         } else {
             startVcnLocked(subscriptionGroup, config);
         }
@@ -531,9 +563,17 @@ public class VcnManagementService extends IVcnManagementService.Stub {
         Binder.withCleanCallingIdentity(() -> {
             synchronized (mLock) {
                 mConfigs.remove(subscriptionGroup);
+                final boolean vcnExists = mVcns.containsKey(subscriptionGroup);
 
                 stopVcnLocked(subscriptionGroup);
 
+                if (vcnExists) {
+                    // TODO(b/181789060): invoke asynchronously after Vcn notifies through
+                    // VcnCallback
+                    notifyAllPermissionedStatusCallbacksLocked(
+                            subscriptionGroup, VCN_STATUS_CODE_NOT_CONFIGURED);
+                }
+
                 writeConfigsToDiskLocked();
             }
         });
@@ -604,18 +644,20 @@ public class VcnManagementService extends IVcnManagementService.Stub {
                 android.Manifest.permission.NETWORK_FACTORY,
                 "Must have permission NETWORK_FACTORY to register a policy listener");
 
-        PolicyListenerBinderDeath listenerBinderDeath = new PolicyListenerBinderDeath(listener);
+        Binder.withCleanCallingIdentity(() -> {
+            PolicyListenerBinderDeath listenerBinderDeath = new PolicyListenerBinderDeath(listener);
 
-        synchronized (mLock) {
+            synchronized (mLock) {
-            mRegisteredPolicyListeners.put(listener.asBinder(), listenerBinderDeath);
+                mRegisteredPolicyListeners.put(listener.asBinder(), listenerBinderDeath);
 
-            try {
+                try {
-                listener.asBinder().linkToDeath(listenerBinderDeath, 0 /* flags */);
+                    listener.asBinder().linkToDeath(listenerBinderDeath, 0 /* flags */);
-            } catch (RemoteException e) {
+                } catch (RemoteException e) {
-                // Remote binder already died - cleanup registered Listener
+                    // Remote binder already died - cleanup registered Listener
-                listenerBinderDeath.binderDied();
+                    listenerBinderDeath.binderDied();
+                }
             }
-        }
+        });
     }
 
     /** Removes the provided listener from receiving VcnUnderlyingNetworkPolicy updates. */
@@ -625,14 +667,31 @@ public class VcnManagementService extends IVcnManagementService.Stub {
             @NonNull IVcnUnderlyingNetworkPolicyListener listener) {
         requireNonNull(listener, "listener was null");
 
-        synchronized (mLock) {
+        Binder.withCleanCallingIdentity(() -> {
-            PolicyListenerBinderDeath listenerBinderDeath =
+            synchronized (mLock) {
-                    mRegisteredPolicyListeners.remove(listener.asBinder());
+                PolicyListenerBinderDeath listenerBinderDeath =
+                        mRegisteredPolicyListeners.remove(listener.asBinder());
 
-            if (listenerBinderDeath != null) {
+                if (listenerBinderDeath != null) {
-                listener.asBinder().unlinkToDeath(listenerBinderDeath, 0 /* flags */);
+                    listener.asBinder().unlinkToDeath(listenerBinderDeath, 0 /* flags */);
+                }
             }
+        });
+    }
+
+    private int getSubIdForNetworkCapabilities(@NonNull NetworkCapabilities networkCapabilities) {
+        if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)
+                && networkCapabilities.getNetworkSpecifier() instanceof TelephonyNetworkSpecifier) {
+            TelephonyNetworkSpecifier telephonyNetworkSpecifier =
+                    (TelephonyNetworkSpecifier) networkCapabilities.getNetworkSpecifier();
+            return telephonyNetworkSpecifier.getSubscriptionId();
+        } else if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)
+                && networkCapabilities.getTransportInfo() instanceof WifiInfo) {
+            WifiInfo wifiInfo = (WifiInfo) networkCapabilities.getTransportInfo();
+            return mDeps.getSubIdForWifiInfo(wifiInfo);
         }
+
+        return SubscriptionManager.INVALID_SUBSCRIPTION_ID;
     }
 
     /**
@@ -652,51 +711,47 @@ public class VcnManagementService extends IVcnManagementService.Stub {
                 "Must have permission NETWORK_FACTORY or be the SystemServer to get underlying"
                         + " Network policies");
 
-        // Defensive copy in case this call is in-process and the given NetworkCapabilities mutates
+        return Binder.withCleanCallingIdentity(() -> {
-        networkCapabilities = new NetworkCapabilities(networkCapabilities);
+            // Defensive copy in case this call is in-process and the given NetworkCapabilities
+            // mutates
+            final NetworkCapabilities ncCopy = new NetworkCapabilities(networkCapabilities);
 
-        int subId = SubscriptionManager.INVALID_SUBSCRIPTION_ID;
+            final int subId = getSubIdForNetworkCapabilities(ncCopy);
-        if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)
+            boolean isVcnManagedNetwork = false;
-                && networkCapabilities.getNetworkSpecifier() instanceof TelephonyNetworkSpecifier) {
+            boolean isRestrictedCarrierWifi = false;
-            TelephonyNetworkSpecifier telephonyNetworkSpecifier =
+            if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) {
-                    (TelephonyNetworkSpecifier) networkCapabilities.getNetworkSpecifier();
+                synchronized (mLock) {
-            subId = telephonyNetworkSpecifier.getSubscriptionId();
+                    ParcelUuid subGroup = mLastSnapshot.getGroupForSubId(subId);
-        } else if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)
-                && networkCapabilities.getTransportInfo() instanceof WifiInfo) {
-            WifiInfo wifiInfo = (WifiInfo) networkCapabilities.getTransportInfo();
-            subId = mDeps.getSubIdForWifiInfo(wifiInfo);
-        }
 
-        boolean isVcnManagedNetwork = false;
+                    final Vcn vcn = mVcns.get(subGroup);
-        boolean isRestrictedCarrierWifi = false;
+                    if (vcn != null) {
-        if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) {
+                        if (vcn.isActive()) {
-            synchronized (mLock) {
+                            isVcnManagedNetwork = true;
-                ParcelUuid subGroup = mLastSnapshot.getGroupForSubId(subId);
+                        }
 
-                Vcn vcn = mVcns.get(subGroup);
+                        if (ncCopy.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) {
-                if (vcn != null) {
+                            // Carrier WiFi always restricted if VCN exists (even in safe mode).
-                    if (vcn.isActive()) {
+                            isRestrictedCarrierWifi = true;
-                        isVcnManagedNetwork = true;
+                        }
-                    }
-
-                    if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) {
-                        // Carrier WiFi always restricted if VCN exists (even in safe mode).
-                        isRestrictedCarrierWifi = true;
                     }
                 }
             }
-        }
 
-        if (isVcnManagedNetwork) {
+            final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder(ncCopy);
-            networkCapabilities.removeCapability(
-                    NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
-        }
 
-        if (isRestrictedCarrierWifi) {
+            if (isVcnManagedNetwork) {
-            networkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED);
+                ncBuilder.removeCapability(
-        }
+                        NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
+            }
 
-        return new VcnUnderlyingNetworkPolicy(false /* isTearDownRequested */, networkCapabilities);
+            if (isRestrictedCarrierWifi) {
+                ncBuilder.removeCapability(
+                        NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED);
+            }
+
+            return new VcnUnderlyingNetworkPolicy(
+                    false /* isTearDownRequested */, ncBuilder.build());
+        });
     }
 
     /** Binder death recipient used to remove registered VcnStatusCallbacks. */
@@ -857,16 +912,7 @@ public class VcnManagementService extends IVcnManagementService.Stub {
                 }
 
                 notifyAllPolicyListenersLocked();
-
+                notifyAllPermissionedStatusCallbacksLocked(mSubGroup, VCN_STATUS_CODE_SAFE_MODE);
-                // Notify all registered StatusCallbacks for this subGroup
-                for (VcnStatusCallbackInfo cbInfo : mRegisteredStatusCallbacks.values()) {
-                    if (isCallbackPermissioned(cbInfo, mSubGroup)) {
-                        Binder.withCleanCallingIdentity(
-                                () ->
-                                        cbInfo.mCallback.onVcnStatusChanged(
-                                                VCN_STATUS_CODE_SAFE_MODE));
-                    }
-                }
             }
         }
 

services/core/java/com/android/server/vcn/Vcn.java

@@ -128,7 +128,6 @@ public class Vcn extends Handler {
      * from VcnManagementService, and therefore cannot rely on guarantees of running on the VCN
      * Looper.
      */
-    // TODO(b/179429339): update when exiting safemode (when a new VcnConfig is provided)
     private final AtomicBoolean mIsActive = new AtomicBoolean(true);
 
     public Vcn(
@@ -203,7 +202,8 @@ public class Vcn extends Handler {
 
     @Override
     public void handleMessage(@NonNull Message msg) {
-        if (!isActive()) {
+        // Ignore if this Vcn is not active and we're not receiving new configs
+        if (!isActive() && msg.what != MSG_EVENT_CONFIG_UPDATED) {
             return;
         }
 
@@ -237,7 +237,13 @@ public class Vcn extends Handler {
 
         mConfig = config;
 
-        // TODO: Reevaluate active VcnGatewayConnection(s)
+        // TODO(b/181815405): Reevaluate active VcnGatewayConnection(s)
+
+        if (!mIsActive.getAndSet(true)) {
+            // If this VCN was not previously active, it is exiting Safe Mode. Re-register the
+            // request listener to get NetworkRequests again (and all cached requests).
+            mVcnContext.getVcnNetworkProvider().registerListener(mRequestListener);
+        }
     }
 
     private void handleTeardown() {
@@ -253,6 +259,8 @@ public class Vcn extends Handler {
     private void handleEnterSafeMode() {
         handleTeardown();
 
+        mVcnGatewayConnections.clear();
+
         mVcnCallback.onEnteredSafeMode();
     }
 

tests/vcn/java/com/android/server/VcnManagementServiceTest.java

@@ -460,6 +460,34 @@ public class VcnManagementServiceTest {
         verify(mConfigReadWriteHelper).writeToDisk(any(PersistableBundle.class));
     }
 
+    @Test
+    public void testSetVcnConfigNotifiesStatusCallback() throws Exception {
+        mVcnMgmtSvc.systemReady();
+        doReturn(true)
+                .when(mLocationPermissionChecker)
+                .checkLocationPermission(eq(TEST_PACKAGE_NAME), any(), eq(TEST_UID), any());
+        triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(TEST_UUID_2));
+
+        mVcnMgmtSvc.registerVcnStatusCallback(TEST_UUID_2, mMockStatusCallback, TEST_PACKAGE_NAME);
+        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_NOT_CONFIGURED);
+
+        // Use a different UUID to simulate a new VCN config.
+        mVcnMgmtSvc.setVcnConfig(TEST_UUID_2, TEST_VCN_CONFIG, TEST_PACKAGE_NAME);
+
+        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_ACTIVE);
+    }
+
+    @Test
+    public void testSetVcnConfigInSafeModeNotifiesStatusCallback() throws Exception {
+        setupSubscriptionAndStartVcn(TEST_SUBSCRIPTION_ID, TEST_UUID_2, false /* isActive */);
+        mVcnMgmtSvc.registerVcnStatusCallback(TEST_UUID_2, mMockStatusCallback, TEST_PACKAGE_NAME);
+        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE);
+
+        mVcnMgmtSvc.setVcnConfig(TEST_UUID_2, TEST_VCN_CONFIG, TEST_PACKAGE_NAME);
+
+        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_ACTIVE);
+    }
+
     @Test
     public void testClearVcnConfigRequiresNonSystemServer() throws Exception {
         doReturn(Process.SYSTEM_UID).when(mMockDeps).getBinderCallingUid();
@@ -502,6 +530,17 @@ public class VcnManagementServiceTest {
         verify(mConfigReadWriteHelper).writeToDisk(any(PersistableBundle.class));
     }
 
+    @Test
+    public void testClearVcnConfigNotifiesStatusCallback() throws Exception {
+        setupSubscriptionAndStartVcn(TEST_SUBSCRIPTION_ID, TEST_UUID_2, true /* isActive */);
+        mVcnMgmtSvc.registerVcnStatusCallback(TEST_UUID_2, mMockStatusCallback, TEST_PACKAGE_NAME);
+        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_ACTIVE);
+
+        mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2);
+
+        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_NOT_CONFIGURED);
+    }
+
     @Test
     public void testSetVcnConfigClearVcnConfigStartsUpdatesAndTeardsDownVcns() throws Exception {
         // Use a different UUID to simulate a new VCN config.

tests/vcn/java/com/android/server/vcn/VcnTest.java

@@ -22,7 +22,9 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.argThat;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
@@ -48,6 +50,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 
+import java.util.Arrays;
 import java.util.Set;
 import java.util.UUID;
 
@@ -58,7 +61,7 @@ public class VcnTest {
     private static final int PROVIDER_ID = 5;
     private static final int[][] TEST_CAPS =
             new int[][] {
-                new int[] {NET_CAPABILITY_INTERNET, NET_CAPABILITY_MMS},
+                new int[] {NET_CAPABILITY_MMS, NET_CAPABILITY_INTERNET},
                 new int[] {NET_CAPABILITY_DUN}
             };
 
@@ -155,14 +158,6 @@ public class VcnTest {
         }
     }
 
-    @Test
-    public void testGatewayEnteringSafeModeNotifiesVcn() {
-        final NetworkRequestListener requestListener = verifyAndGetRequestListener();
-        for (final int capability : VcnGatewayConnectionConfigTest.EXPOSED_CAPS) {
-            startVcnGatewayWithCapabilities(requestListener, capability);
-        }
-    }
-
     private void triggerVcnRequestListeners(NetworkRequestListener requestListener) {
         for (final int[] caps : TEST_CAPS) {
             startVcnGatewayWithCapabilities(requestListener, caps);
@@ -188,8 +183,20 @@ public class VcnTest {
         return gatewayConnections;
     }
 
+    private void verifySafeMode(
+            NetworkRequestListener requestListener,
+            Set<VcnGatewayConnection> expectedGatewaysTornDown) {
+        assertFalse(mVcn.isActive());
+        assertTrue(mVcn.getVcnGatewayConnections().isEmpty());
+        for (final VcnGatewayConnection gatewayConnection : expectedGatewaysTornDown) {
+            verify(gatewayConnection).teardownAsynchronously();
+        }
+        verify(mVcnNetworkProvider).unregisterListener(requestListener);
+        verify(mVcnCallback).onEnteredSafeMode();
+    }
+
     @Test
-    public void testGatewayEnteringSafemodeNotifiesVcn() {
+    public void testGatewayEnteringSafeModeNotifiesVcn() {
         final NetworkRequestListener requestListener = verifyAndGetRequestListener();
         final Set<VcnGatewayConnection> gatewayConnections =
                 startGatewaysAndGetGatewayConnections(requestListener);
@@ -200,12 +207,7 @@ public class VcnTest {
         statusCallback.onEnteredSafeMode();
         mTestLooper.dispatchAll();
 
-        assertFalse(mVcn.isActive());
+        verifySafeMode(requestListener, gatewayConnections);
-        for (final VcnGatewayConnection gatewayConnection : gatewayConnections) {
-            verify(gatewayConnection).teardownAsynchronously();
-        }
-        verify(mVcnNetworkProvider).unregisterListener(requestListener);
-        verify(mVcnCallback).onEnteredSafeMode();
     }
 
     @Test
@@ -234,4 +236,39 @@ public class VcnTest {
                         any(),
                         mGatewayStatusCallbackCaptor.capture());
     }
+
+    @Test
+    public void testUpdateConfigExitsSafeMode() {
+        final NetworkRequestListener requestListener = verifyAndGetRequestListener();
+        final Set<VcnGatewayConnection> gatewayConnections =
+                new ArraySet<>(startGatewaysAndGetGatewayConnections(requestListener));
+
+        final VcnGatewayStatusCallback statusCallback = mGatewayStatusCallbackCaptor.getValue();
+        statusCallback.onEnteredSafeMode();
+        mTestLooper.dispatchAll();
+        verifySafeMode(requestListener, gatewayConnections);
+
+        doAnswer(invocation -> {
+            final NetworkRequestListener listener = invocation.getArgument(0);
+            triggerVcnRequestListeners(listener);
+            return null;
+        }).when(mVcnNetworkProvider).registerListener(eq(requestListener));
+
+        mVcn.updateConfig(mConfig);
+        mTestLooper.dispatchAll();
+
+        // Registered on start, then re-registered with new configs
+        verify(mVcnNetworkProvider, times(2)).registerListener(eq(requestListener));
+        assertTrue(mVcn.isActive());
+        for (final int[] caps : TEST_CAPS) {
+            // Expect each gateway connection created on initial startup, and again with new configs
+            verify(mDeps, times(2))
+                    .newVcnGatewayConnection(
+                            eq(mVcnContext),
+                            eq(TEST_SUB_GROUP),
+                            eq(mSubscriptionSnapshot),
+                            argThat(config -> Arrays.equals(caps, config.getExposedCapabilities())),
+                            any());
+        }
+    }
 }