diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4c8fe4c347..e1bcb0c21c 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/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)) { diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java index f0b7150dd8..c5aa8d5361 100644 --- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java +++ b/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 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; diff --git a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java index 76e3e2fced..5eea0e86eb 100644 --- a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java +++ b/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(); - - final ArgumentCaptor observerCaptor = - ArgumentCaptor.forClass(PackageManagerInternal.PackageListObserver.class); - verify(mMockPmi).getPackageList(observerCaptor.capture()); - mObserver = observerCaptor.getValue(); + verify(mMockPmi).getPackageList(mPermissionMonitor); } 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); - removePackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, MOCK_UID1); + mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, 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 - // add), but the observer sees only one callback (an update). - setPackagePermissions(MOCK_PACKAGE1, MOCK_UID1, new String[] {INTERNET}); - mObserver.onPackageChanged(MOCK_PACKAGE1, MOCK_UID1); + addPackage(MOCK_PACKAGE1, MOCK_UID1, new String[] {INTERNET}); 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}); }