Merge "[RFPM01] Merge package added/removed methods"

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

@@ -5141,14 +5141,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
-    private void onPackageAdded(String packageName, int uid) {
-        if (TextUtils.isEmpty(packageName) || uid < 0) {
-            Slog.wtf(TAG, "Invalid package in onPackageAdded: " + packageName + " | " + uid);
-            return;
-        }
-        mPermissionMonitor.onPackageAdded(packageName, uid);
-    }
-
     private void onPackageReplaced(String packageName, int uid) {
         if (TextUtils.isEmpty(packageName) || uid < 0) {
             Slog.wtf(TAG, "Invalid package in onPackageReplaced: " + packageName + " | " + uid);
@@ -5174,7 +5166,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
             Slog.wtf(TAG, "Invalid package in onPackageRemoved: " + packageName + " | " + uid);
             return;
         }
-        mPermissionMonitor.onPackageRemoved(uid);
 
         final int userId = UserHandle.getUserId(uid);
         synchronized (mVpns) {
@@ -5224,8 +5215,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 onUserRemoved(userId);
             } else if (Intent.ACTION_USER_UNLOCKED.equals(action)) {
                 onUserUnlocked(userId);
-            } else if (Intent.ACTION_PACKAGE_ADDED.equals(action)) {
-                onPackageAdded(packageName, uid);
             } else if (Intent.ACTION_PACKAGE_REPLACED.equals(action)) {
                 onPackageReplaced(packageName, uid);
             } else if (Intent.ACTION_PACKAGE_REMOVED.equals(action)) {

services/core/java/com/android/server/connectivity/PermissionMonitor.java

@@ -72,7 +72,7 @@ import java.util.Set;
  *
  * @hide
  */
-public class PermissionMonitor {
+public class PermissionMonitor implements PackageManagerInternal.PackageListObserver {
     private static final String TAG = "PermissionMonitor";
     private static final boolean DBG = true;
     protected static final Boolean SYSTEM = Boolean.TRUE;
@@ -102,44 +102,6 @@ public class PermissionMonitor {
     @GuardedBy("this")
     private final Set<Integer> mAllApps = new HashSet<>();
 
-    private class PackageListObserver implements PackageManagerInternal.PackageListObserver {
-
-        private int getPermissionForUid(int uid) {
-            int permission = 0;
-            // Check all the packages for this UID. The UID has the permission if any of the
-            // packages in it has the permission.
-            String[] packages = mPackageManager.getPackagesForUid(uid);
-            if (packages != null && packages.length > 0) {
-                for (String name : packages) {
-                    final PackageInfo app = getPackageInfo(name);
-                    if (app != null && app.requestedPermissions != null) {
-                        permission |= getNetdPermissionMask(app.requestedPermissions,
-                              app.requestedPermissionsFlags);
-                    }
-                }
-            } else {
-                // The last package of this uid is removed from device. Clean the package up.
-                permission = INetd.PERMISSION_UNINSTALLED;
-            }
-            return permission;
-        }
-
-        @Override
-        public void onPackageAdded(String packageName, int uid) {
-            sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
-        }
-
-        @Override
-        public void onPackageChanged(@NonNull String packageName, int uid) {
-            sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
-        }
-
-        @Override
-        public void onPackageRemoved(String packageName, int uid) {
-            sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
-        }
-    }
-
     public PermissionMonitor(Context context, INetd netd) {
         mPackageManager = context.getPackageManager();
         mUserManager = (UserManager) context.getSystemService(Context.USER_SERVICE);
@@ -153,7 +115,7 @@ public class PermissionMonitor {
 
         PackageManagerInternal pmi = LocalServices.getService(PackageManagerInternal.class);
         if (pmi != null) {
-            pmi.getPackageList(new PackageListObserver());
+            pmi.getPackageList(this);
         } else {
             loge("failed to get the PackageManagerInternal service");
         }
@@ -363,15 +325,38 @@ public class PermissionMonitor {
         return currentPermission;
     }
 
+    private int getPermissionForUid(final int uid) {
+        int permission = INetd.PERMISSION_NONE;
+        // Check all the packages for this UID. The UID has the permission if any of the
+        // packages in it has the permission.
+        final String[] packages = mPackageManager.getPackagesForUid(uid);
+        if (packages != null && packages.length > 0) {
+            for (String name : packages) {
+                final PackageInfo app = getPackageInfo(name);
+                if (app != null && app.requestedPermissions != null) {
+                    permission |= getNetdPermissionMask(app.requestedPermissions,
+                            app.requestedPermissionsFlags);
+                }
+            }
+        } else {
+            // The last package of this uid is removed from device. Clean the package up.
+            permission = INetd.PERMISSION_UNINSTALLED;
+        }
+        return permission;
+    }
+
     /**
-     * Called when a package is added. See {link #ACTION_PACKAGE_ADDED}.
+     * Called when a package is added.
      *
      * @param packageName The name of the new package.
      * @param uid The uid of the new package.
      *
      * @hide
      */
-    public synchronized void onPackageAdded(String packageName, int uid) {
+    @Override
+    public synchronized void onPackageAdded(@NonNull final String packageName, final int uid) {
+        sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
+
         // If multiple packages share a UID (cf: android:sharedUserId) and ask for different
         // permissions, don't downgrade (i.e., if it's already SYSTEM, leave it as is).
         final Boolean permission = highestPermissionForUid(mApps.get(uid), packageName);
@@ -398,13 +383,17 @@ public class PermissionMonitor {
     }
 
     /**
-     * Called when a package is removed. See {link #ACTION_PACKAGE_REMOVED}.
+     * Called when a package is removed.
      *
+     * @param packageName The name of the removed package or null.
      * @param uid containing the integer uid previously assigned to the package.
      *
      * @hide
      */
-    public synchronized void onPackageRemoved(int uid) {
+    @Override
+    public synchronized void onPackageRemoved(@NonNull final String packageName, final int uid) {
+        sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
+
         // If the newly-removed package falls within some VPN's uid range, update Netd with it.
         // This needs to happen before the mApps update below, since removeBypassingUids() depends
         // on mApps to check if the package can bypass VPN.
@@ -449,6 +438,19 @@ public class PermissionMonitor {
         }
     }
 
+    /**
+     * Called when a package is changed.
+     *
+     * @param packageName The name of the changed package.
+     * @param uid The uid of the changed package.
+     *
+     * @hide
+     */
+    @Override
+    public synchronized void onPackageChanged(@NonNull final String packageName, final int uid) {
+        sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
+    }
+
     private static int getNetdPermissionMask(String[] requestedPermissions,
                                              int[] requestedPermissionsFlags) {
         int permissions = 0;

tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java

@@ -76,7 +76,6 @@ import com.android.server.LocalServices;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.mockito.invocation.InvocationOnMock;
@@ -88,7 +87,6 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Set;
 
-
 @RunWith(AndroidJUnit4.class)
 @SmallTest
 public class PermissionMonitorTest {
@@ -117,7 +115,6 @@ public class PermissionMonitorTest {
     @Mock private PackageManagerInternal mMockPmi;
     @Mock private UserManager mUserManager;
 
-    private PackageManagerInternal.PackageListObserver mObserver;
     private PermissionMonitor mPermissionMonitor;
 
     @Before
@@ -139,11 +136,7 @@ public class PermissionMonitorTest {
                   /* observer */ null));
         when(mPackageManager.getInstalledPackages(anyInt())).thenReturn(/* empty app list */ null);
         mPermissionMonitor.startMonitoring();
-
+        verify(mMockPmi).getPackageList(mPermissionMonitor);
-        final ArgumentCaptor<PackageManagerInternal.PackageListObserver> observerCaptor =
-                ArgumentCaptor.forClass(PackageManagerInternal.PackageListObserver.class);
-        verify(mMockPmi).getPackageList(observerCaptor.capture());
-        mObserver = observerCaptor.getValue();
     }
 
     private boolean hasRestrictedNetworkPermission(String partition, int targetSdkVersion, int uid,
@@ -450,13 +443,13 @@ public class PermissionMonitorTest {
                 new int[]{MOCK_UID1});
 
         // Remove MOCK_UID1, expect no permission left for all user.
-        mPermissionMonitor.onPackageRemoved(MOCK_UID1);
+        mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
-        removePackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, MOCK_UID1);
+        removePackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_UID1);
         mNetdMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2}, new int[]{MOCK_UID1});
 
         // Remove SYSTEM_PACKAGE1, expect permission downgrade.
         when(mPackageManager.getPackagesForUid(anyInt())).thenReturn(new String[]{SYSTEM_PACKAGE2});
-        removePackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, SYSTEM_UID);
+        removePackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, SYSTEM_PACKAGE1, SYSTEM_UID);
         mNetdMonitor.expectPermission(NETWORK, new int[]{MOCK_USER1, MOCK_USER2},
                 new int[]{SYSTEM_UID});
 
@@ -465,7 +458,7 @@ public class PermissionMonitorTest {
 
         // Remove all packages, expect no permission left.
         when(mPackageManager.getPackagesForUid(anyInt())).thenReturn(new String[]{});
-        removePackageForUsers(new int[]{MOCK_USER2}, SYSTEM_UID);
+        removePackageForUsers(new int[]{MOCK_USER2}, SYSTEM_PACKAGE2, SYSTEM_UID);
         mNetdMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2},
                 new int[]{SYSTEM_UID, MOCK_UID1});
 
@@ -501,7 +494,8 @@ public class PermissionMonitorTest {
         reset(mNetdService);
 
         // When MOCK_UID1 package is uninstalled and reinstalled, expect Netd to be updated
-        mPermissionMonitor.onPackageRemoved(UserHandle.getUid(MOCK_USER1, MOCK_UID1));
+        mPermissionMonitor.onPackageRemoved(
+                MOCK_PACKAGE1, UserHandle.getUid(MOCK_USER1, MOCK_UID1));
         verify(mNetdService).firewallRemoveUidInterfaceRules(aryEq(new int[] {MOCK_UID1}));
         mPermissionMonitor.onPackageAdded(MOCK_PACKAGE1, UserHandle.getUid(MOCK_USER1, MOCK_UID1));
         verify(mNetdService).firewallAddUidInterfaceRules(eq("tun0"),
@@ -545,7 +539,8 @@ public class PermissionMonitorTest {
                 aryEq(new int[] {MOCK_UID1}));
 
         // Removed package should have its uid rules removed
-        mPermissionMonitor.onPackageRemoved(UserHandle.getUid(MOCK_USER1, MOCK_UID1));
+        mPermissionMonitor.onPackageRemoved(
+                MOCK_PACKAGE1, UserHandle.getUid(MOCK_USER1, MOCK_UID1));
         verify(mNetdService).firewallRemoveUidInterfaceRules(aryEq(new int[] {MOCK_UID1}));
     }
 
@@ -559,9 +554,9 @@ public class PermissionMonitorTest {
         }
     }
 
-    private void removePackageForUsers(int[] users, int uid) {
+    private void removePackageForUsers(int[] users, String packageName, int uid) {
         for (final int user : users) {
-            mPermissionMonitor.onPackageRemoved(UserHandle.getUid(user, uid));
+            mPermissionMonitor.onPackageRemoved(packageName, UserHandle.getUid(user, uid));
         }
     }
 
@@ -647,7 +642,7 @@ public class PermissionMonitorTest {
     private PackageInfo addPackage(String packageName, int uid, String[] permissions)
             throws Exception {
         PackageInfo packageInfo = setPackagePermissions(packageName, uid, permissions);
-        mObserver.onPackageAdded(packageName, uid);
+        mPermissionMonitor.onPackageAdded(packageName, uid);
         return packageInfo;
     }
 
@@ -678,7 +673,7 @@ public class PermissionMonitorTest {
         when(mPackageManager.getPackageInfo(eq(MOCK_PACKAGE2), anyInt())).thenReturn(packageInfo2);
         when(mPackageManager.getPackagesForUid(MOCK_UID1))
               .thenReturn(new String[]{MOCK_PACKAGE1, MOCK_PACKAGE2});
-        mObserver.onPackageAdded(MOCK_PACKAGE2, MOCK_UID1);
+        mPermissionMonitor.onPackageAdded(MOCK_PACKAGE2, MOCK_UID1);
         mNetdServiceMonitor.expectPermission(INetd.PERMISSION_INTERNET
                 | INetd.PERMISSION_UPDATE_DEVICE_STATS, new int[]{MOCK_UID1});
     }
@@ -692,7 +687,7 @@ public class PermissionMonitorTest {
                 | INetd.PERMISSION_UPDATE_DEVICE_STATS, new int[]{MOCK_UID1});
 
         when(mPackageManager.getPackagesForUid(MOCK_UID1)).thenReturn(new String[]{});
-        mObserver.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
+        mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
         mNetdServiceMonitor.expectPermission(INetd.PERMISSION_UNINSTALLED, new int[]{MOCK_UID1});
     }
 
@@ -705,7 +700,7 @@ public class PermissionMonitorTest {
                 | INetd.PERMISSION_UPDATE_DEVICE_STATS, new int[]{MOCK_UID1});
 
         when(mPackageManager.getPackagesForUid(MOCK_UID1)).thenReturn(new String[]{});
-        mObserver.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
+        mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
         mNetdServiceMonitor.expectPermission(INetd.PERMISSION_UNINSTALLED, new int[]{MOCK_UID1});
 
         addPackage(MOCK_PACKAGE1, MOCK_UID1, new String[] {INTERNET});
@@ -719,10 +714,7 @@ public class PermissionMonitorTest {
         addPackage(MOCK_PACKAGE1, MOCK_UID1, new String[] {});
         mNetdServiceMonitor.expectPermission(INetd.PERMISSION_NONE, new int[]{MOCK_UID1});
 
-        // When updating a package, the broadcast receiver gets two broadcasts (a remove and then an
+        addPackage(MOCK_PACKAGE1, MOCK_UID1, new String[] {INTERNET});
-        // add), but the observer sees only one callback (an update).
-        setPackagePermissions(MOCK_PACKAGE1, MOCK_UID1, new String[] {INTERNET});
-        mObserver.onPackageChanged(MOCK_PACKAGE1, MOCK_UID1);
         mNetdServiceMonitor.expectPermission(INetd.PERMISSION_INTERNET, new int[]{MOCK_UID1});
     }
 
@@ -740,7 +732,7 @@ public class PermissionMonitorTest {
         when(mPackageManager.getPackagesForUid(MOCK_UID1)).thenReturn(new String[]{
                 MOCK_PACKAGE2});
 
-        mObserver.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
+        mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
         mNetdServiceMonitor.expectPermission(INetd.PERMISSION_INTERNET, new int[]{MOCK_UID1});
     }