Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f3072fcd authored by lucaslin's avatar lucaslin
Browse files

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
parent 30c6383d

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

+26 −2
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ import android.content.Context; 
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 class ConnectivityService extends IConnectivityManager.Stub 
        }

    }



    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 class ConnectivityService extends IConnectivityManager.Stub 
     */

    @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 class ConnectivityService extends IConnectivityManager.Stub 
     */

    @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

+18 −0
Original line number Diff line number Diff line
@@ -1224,6 +1224,9 @@ public class ConnectivityServiceTest { 
                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 @@ public class ConnectivityServiceTest { 
                        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 class ConnectivityServiceTest { 
        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();