Make sure that only the owner can call stopVpnProfile()

In stopVpnProfile(), it doesn't check if the caller's package
name is the same as the given one, so any app has chance to stop
the VPN profile of other apps.

Bug: 191382886
Test: atest FrameworksNetTests CtsNetTestCases \
      CtsHostsideNetworkTests:HostsideVpnTests
Change-Id: Ib0a6e9ed191ff8c8bd55ce9902d894b6a339ace2
Merged-In: I254ffd1c08ec058d594b4ea55cbae5505f8497cc
(cherry picked from commit f3072fcd46112bad7c5f6ddd4cc35d2c67f00d11)

Author: lucaslin

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

@@ -73,6 +73,7 @@
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.PackageManager;
+import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.res.Configuration;
 import android.database.ContentObserver;
 import android.net.CaptivePortal;
@@ -4607,6 +4608,25 @@ public void deleteVpnProfile(@NonNull String packageName) {
         }
     }
 
+    private int getAppUid(final String app, final int userId) {
+        final PackageManager pm = mContext.getPackageManager();
+        final long token = Binder.clearCallingIdentity();
+        try {
+            return pm.getPackageUidAsUser(app, userId);
+        } catch (NameNotFoundException e) {
+            return -1;
+        } finally {
+            Binder.restoreCallingIdentity(token);
+        }
+    }
+
+    private void verifyCallingUidAndPackage(String packageName, int callingUid) {
+        final int userId = UserHandle.getUserId(callingUid);
+        if (getAppUid(packageName, userId) != callingUid) {
+            throw new SecurityException(packageName + " does not belong to uid " + callingUid);
+        }
+    }
+
     /**
      * Starts the VPN based on the stored profile for the given package
      *
@@ -4618,7 +4638,9 @@ public void deleteVpnProfile(@NonNull String packageName) {
      */
     @Override
     public void startVpnProfile(@NonNull String packageName) {
-        final int user = UserHandle.getUserId(Binder.getCallingUid());
+        final int callingUid = Binder.getCallingUid();
+        verifyCallingUidAndPackage(packageName, callingUid);
+        final int user = UserHandle.getUserId(callingUid);
         synchronized (mVpns) {
             throwIfLockdownEnabled();
             mVpns.get(user).startVpnProfile(packageName, mKeyStore);
@@ -4635,7 +4657,9 @@ public void startVpnProfile(@NonNull String packageName) {
      */
     @Override
     public void stopVpnProfile(@NonNull String packageName) {
-        final int user = UserHandle.getUserId(Binder.getCallingUid());
+        final int callingUid = Binder.getCallingUid();
+        verifyCallingUidAndPackage(packageName, callingUid);
+        final int user = UserHandle.getUserId(callingUid);
         synchronized (mVpns) {
             mVpns.get(user).stopVpnProfile(packageName);
         }

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

@@ -1224,6 +1224,9 @@ public void setUp() throws Exception {
                 Arrays.asList(new UserInfo[] {
                         new UserInfo(VPN_USER, "", 0),
                 }));
+        final int userId = UserHandle.getCallingUserId();
+        final UserInfo primaryUser = new UserInfo(userId, "", UserInfo.FLAG_PRIMARY);
+        doReturn(primaryUser).when(mUserManager).getUserInfo(eq(userId));
         final ApplicationInfo applicationInfo = new ApplicationInfo();
         applicationInfo.targetSdkVersion = Build.VERSION_CODES.Q;
         when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
@@ -1368,6 +1371,9 @@ private void mockDefaultPackages() throws Exception {
                         buildPackageInfo(/* SYSTEM */ false, APP2_UID),
                         buildPackageInfo(/* SYSTEM */ false, VPN_UID)
                 }));
+        final int userId = UserHandle.getCallingUserId();
+        when(mPackageManager.getPackageUidAsUser(TEST_PACKAGE_NAME, userId))
+                .thenReturn(Process.myUid());
     }
 
     private void verifyActiveNetwork(int transport) {
@@ -7068,6 +7074,18 @@ public void testVpnHandoverChangesInterfaceFilteringRule() throws Exception {
         assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID);
     }
 
+    @Test
+    public void testStartVpnProfileFromDiffPackage() throws Exception {
+        final String notMyVpnPkg = "com.not.my.vpn";
+        assertThrows(SecurityException.class, () -> mService.startVpnProfile(notMyVpnPkg));
+    }
+
+    @Test
+    public void testStopVpnProfileFromDiffPackage() throws Exception {
+        final String notMyVpnPkg = "com.not.my.vpn";
+        assertThrows(SecurityException.class, () -> mService.stopVpnProfile(notMyVpnPkg));
+    }
+
     @Test
     public void testUidUpdateChangesInterfaceFilteringRule() throws Exception {
         LinkProperties lp = new LinkProperties();