Merge "Stop using PackageManagerInternal in PermissionMonitor"

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

@@ -31,14 +31,17 @@ import static android.os.Process.SYSTEM_UID;
 import static com.android.net.module.util.CollectionUtils.toIntArray;
 
 import android.annotation.NonNull;
+import android.content.BroadcastReceiver;
 import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
-import android.content.pm.PackageManagerInternal;
 import android.net.INetd;
 import android.net.UidRange;
+import android.net.Uri;
 import android.os.Build;
 import android.os.RemoteException;
 import android.os.ServiceSpecificException;
@@ -54,7 +57,6 @@ import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.IndentingPrintWriter;
 import com.android.net.module.util.CollectionUtils;
-import com.android.server.LocalServices;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -71,7 +73,7 @@ import java.util.Set;
  *
  * @hide
  */
-public class PermissionMonitor implements PackageManagerInternal.PackageListObserver {
+public class PermissionMonitor {
     private static final String TAG = "PermissionMonitor";
     private static final boolean DBG = true;
     protected static final Boolean SYSTEM = Boolean.TRUE;
@@ -83,6 +85,7 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse
     private final SystemConfigManager mSystemConfigManager;
     private final INetd mNetd;
     private final Dependencies mDeps;
+    private final Context mContext;
 
     @GuardedBy("this")
     private final Set<UserHandle> mUsers = new HashSet<>();
@@ -102,6 +105,25 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse
     @GuardedBy("this")
     private final Set<Integer> mAllApps = new HashSet<>();
 
+    private BroadcastReceiver mIntentReceiver = new BroadcastReceiver() {
+        @Override
+        public void onReceive(Context context, Intent intent) {
+            final String action = intent.getAction();
+            final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1);
+            final Uri packageData = intent.getData();
+            final String packageName =
+                    packageData != null ? packageData.getSchemeSpecificPart() : null;
+
+            if (Intent.ACTION_PACKAGE_ADDED.equals(action)) {
+                onPackageAdded(packageName, uid);
+            } else if (Intent.ACTION_PACKAGE_REMOVED.equals(action)) {
+                onPackageRemoved(packageName, uid);
+            } else {
+                Log.wtf(TAG, "received unexpected intent: " + action);
+            }
+        }
+    };
+
     /**
      * Dependencies of PermissionMonitor, for injection in tests.
      */
@@ -127,6 +149,7 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse
         mSystemConfigManager = context.getSystemService(SystemConfigManager.class);
         mNetd = netd;
         mDeps = deps;
+        mContext = context;
     }
 
     // Intended to be called only once at startup, after the system is ready. Installs a broadcast
@@ -134,12 +157,14 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse
     public synchronized void startMonitoring() {
         log("Monitoring");
 
-        PackageManagerInternal pmi = LocalServices.getService(PackageManagerInternal.class);
+        final IntentFilter intentFilter = new IntentFilter();
-        if (pmi != null) {
+        intentFilter.addAction(Intent.ACTION_PACKAGE_ADDED);
-            pmi.getPackageList(this);
+        intentFilter.addAction(Intent.ACTION_PACKAGE_REMOVED);
-        } else {
+        intentFilter.addDataScheme("package");
-            loge("failed to get the PackageManagerInternal service");
+        mContext.createContextAsUser(UserHandle.ALL, 0 /* flags */).registerReceiver(
-        }
+                mIntentReceiver, intentFilter, null /* broadcastPermission */,
+                null /* scheduler */);
+
         List<PackageInfo> apps = mPackageManager.getInstalledPackages(GET_PERMISSIONS
                 | MATCH_ANY_USER);
         if (apps == null) {
@@ -347,9 +372,10 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse
      *
      * @hide
      */
-    @Override
     public synchronized void onPackageAdded(@NonNull final String packageName, final int uid) {
-        sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
+        // TODO: Netd is using appId for checking traffic permission. Correct the methods that are
+        //  using appId instead of uid actually
+        sendPackagePermissionsForUid(UserHandle.getAppId(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).
@@ -384,9 +410,10 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse
      *
      * @hide
      */
-    @Override
     public synchronized void onPackageRemoved(@NonNull final String packageName, final int uid) {
-        sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
+        // TODO: Netd is using appId for checking traffic permission. Correct the methods that are
+        //  using appId instead of uid actually
+        sendPackagePermissionsForUid(UserHandle.getAppId(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
@@ -432,19 +459,6 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse
         }
     }
 
-    /**
-     * 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

@@ -48,18 +48,22 @@ import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import android.content.BroadcastReceiver;
 import android.content.Context;
+import android.content.Intent;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
-import android.content.pm.PackageManagerInternal;
 import android.net.INetd;
 import android.net.UidRange;
+import android.net.Uri;
 import android.os.Build;
 import android.os.SystemConfigManager;
 import android.os.UserHandle;
@@ -70,12 +74,11 @@ import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
-import com.android.server.LocalServices;
-import com.android.server.pm.PackageList;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.AdditionalAnswers;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.mockito.invocation.InvocationOnMock;
@@ -112,7 +115,6 @@ public class PermissionMonitorTest {
     @Mock private Context mContext;
     @Mock private PackageManager mPackageManager;
     @Mock private INetd mNetdService;
-    @Mock private PackageManagerInternal mMockPmi;
     @Mock private UserManager mUserManager;
     @Mock private PermissionMonitor.Dependencies mDeps;
     @Mock private SystemConfigManager mSystemConfigManager;
@@ -131,16 +133,14 @@ public class PermissionMonitorTest {
         when(mContext.getSystemService(Context.SYSTEM_CONFIG_SERVICE))
                 .thenReturn(mSystemConfigManager);
         when(mSystemConfigManager.getSystemPermissionUids(anyString())).thenReturn(new int[0]);
+        final Context asUserCtx = mock(Context.class, AdditionalAnswers.delegatesTo(mContext));
+        doReturn(UserHandle.ALL).when(asUserCtx).getUser();
+        when(mContext.createContextAsUser(eq(UserHandle.ALL), anyInt())).thenReturn(asUserCtx);
 
         mPermissionMonitor = spy(new PermissionMonitor(mContext, mNetdService, mDeps));
 
-        LocalServices.removeServiceForTest(PackageManagerInternal.class);
-        LocalServices.addService(PackageManagerInternal.class, mMockPmi);
-        when(mMockPmi.getPackageList(any())).thenReturn(new PackageList(new ArrayList<String>(),
-                  /* observer */ null));
         when(mPackageManager.getInstalledPackages(anyInt())).thenReturn(/* empty app list */ null);
         mPermissionMonitor.startMonitoring();
-        verify(mMockPmi).getPackageList(mPermissionMonitor);
     }
 
     private boolean hasRestrictedNetworkPermission(String partition, int targetSdkVersion, int uid,
@@ -770,4 +770,32 @@ public class PermissionMonitorTest {
                 INetd.PERMISSION_INTERNET | INetd.PERMISSION_UPDATE_DEVICE_STATS,
                 new int[]{ MOCK_UID2 });
     }
+
+    @Test
+    public void testIntentReceiver() throws Exception {
+        final NetdServiceMonitor mNetdServiceMonitor = new NetdServiceMonitor(mNetdService);
+        final ArgumentCaptor<BroadcastReceiver> receiverCaptor =
+                ArgumentCaptor.forClass(BroadcastReceiver.class);
+        verify(mContext, times(1)).registerReceiver(receiverCaptor.capture(), any(), any(), any());
+        final BroadcastReceiver receiver = receiverCaptor.getValue();
+
+        // Verify receiving PACKAGE_ADDED intent.
+        final Intent addedIntent = new Intent(Intent.ACTION_PACKAGE_ADDED,
+                Uri.fromParts("package", MOCK_PACKAGE1, null /* fragment */));
+        addedIntent.putExtra(Intent.EXTRA_UID, MOCK_UID1);
+        setPackagePermissions(MOCK_PACKAGE1, MOCK_UID1,
+                new String[] { INTERNET, UPDATE_DEVICE_STATS });
+        receiver.onReceive(mContext, addedIntent);
+        mNetdServiceMonitor.expectPermission(INetd.PERMISSION_INTERNET
+                | INetd.PERMISSION_UPDATE_DEVICE_STATS, new int[] { MOCK_UID1 });
+
+        // Verify receiving PACKAGE_REMOVED intent.
+        when(mPackageManager.getPackagesForUid(MOCK_UID1)).thenReturn(null);
+        final Intent removedIntent = new Intent(Intent.ACTION_PACKAGE_REMOVED,
+                Uri.fromParts("package", MOCK_PACKAGE1, null /* fragment */));
+        removedIntent.putExtra(Intent.EXTRA_UID, MOCK_UID1);
+        receiver.onReceive(mContext, removedIntent);
+        mNetdServiceMonitor.expectPermission(INetd.PERMISSION_UNINSTALLED, new int[] { MOCK_UID1 });
+    }
+
 }