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

Commit 11008a78 authored by Jeff Davidson's avatar Jeff Davidson
Browse files

Don't enforce control permission when preparing consented VPN. 

If a VPN app requests to be prepared and has already obtained user
consent, there is no need to additionally enforce the control
permission. We only need to enforce the control permission when a VPN
is first being prepared, where such a preparation would bypass user
consent.

Also ensure that in this case, the VPN being prepared matches the
calling app. Otherwise an app could prepare another pre-consented VPN,
which is not particularly dangerous but is likely unexpected.

Finally, remove misleading comment in ConnectivityService#prepareVpn.
This method IS called from VpnService.prepare(), not only from
system-privileged apps.

Bug: 18442887
Change-Id: Ic3227c6c1c74312697f0576d3811b06692a4edff
parent 8ccf071a

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

+1 −1
Original line number Diff line number Diff line
@@ -2813,7 +2813,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    }



    /**

     * Prepare for a VPN application. This method is used by system-privileged apps.

     * Prepare for a VPN application.

     * Permissions are checked in Vpn class.

     * @hide

     */

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

+46 −51
Original line number Diff line number Diff line
@@ -215,20 +215,11 @@ public class Vpn { 
     * @return true if the operation is succeeded.

     */

    public synchronized boolean prepare(String oldPackage, String newPackage) {

        // Return false if the package does not match.

        if (oldPackage != null && getAppUid(oldPackage, mUserHandle) != mOwnerUID) {

            // The package doesn't match. If this VPN was not previously authorized, return false

            // to force user authorization. Otherwise, revoke the VPN anyway.

            // The package doesn't match. We return false (to obtain user consent) unless the user

            // has already consented to that VPN package.

            if (!oldPackage.equals(VpnConfig.LEGACY_VPN) && isVpnUserPreConsented(oldPackage)) {

                long token = Binder.clearCallingIdentity();

                try {

                    // This looks bizarre, but it is what ConfirmDialog in VpnDialogs is doing when

                    // the user clicks through to allow the VPN to consent. So we are emulating the

                    // action of the dialog without actually showing it.

                    prepare(null, oldPackage);

                } finally {

                    Binder.restoreCallingIdentity(token);

                }

                prepareInternal(oldPackage);

                return true;

            }

            return false;
@@ -243,6 +234,14 @@ public class Vpn { 
        // Check if the caller is authorized.

        enforceControlPermission();



        prepareInternal(newPackage);

        return true;

    }



    /** Prepare the VPN for the given package. Does not perform permission checks. */

    private void prepareInternal(String newPackage) {

        long token = Binder.clearCallingIdentity();

        try {

            // Reset the interface.

            if (mInterface != null) {

                mStatusIntent = null;
@@ -267,30 +266,26 @@ public class Vpn { 
                mLegacyVpnRunner = null;

            }



        long token = Binder.clearCallingIdentity();

            try {

                mNetd.denyProtect(mOwnerUID);

            } catch (Exception e) {

                Log.wtf(TAG, "Failed to disallow UID " + mOwnerUID + " to call protect() " + e);

        } finally {

            Binder.restoreCallingIdentity(token);

            }



            Log.i(TAG, "Switched from " + mPackage + " to " + newPackage);

            mPackage = newPackage;

            mOwnerUID = getAppUid(newPackage, mUserHandle);

        token = Binder.clearCallingIdentity();

            try {

                mNetd.allowProtect(mOwnerUID);

            } catch (Exception e) {

                Log.wtf(TAG, "Failed to allow UID " + mOwnerUID + " to call protect() " + e);

        } finally {

            Binder.restoreCallingIdentity(token);

            }

            mConfig = null;



            updateState(DetailedState.IDLE, "prepare");

        return true;

        } finally {

            Binder.restoreCallingIdentity(token);

        }

    }



    /**