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

Commit 0aa677c7 authored by Varun Anand's avatar Varun Anand Committed by android-build-team Robot
Browse files

Revert "Update VPN capabilities when its underlying network set is null." 

This reverts commit ad8805a6.

Bug: 126245192

Reason for revert: This change can lead to a deadlock that was fixed in http://ag/6580635. However, platform PMs think that fixing this is risky enough as this is not a recent problem and has been in the field for 3/4 of the year.

Note: The merged-in tag is used to avoid this change from getting merged into pi-dev-plus-aosp. This is to avoid merge conflicts since we mostly work in aosp/master which merges into pi-dev-plus-aosp.

Change-Id: I3814bcec87efb059f50f00617406501aaeac3b4d
Merged-In: Id0abc4d304bb096e92479a118168690ccce634ed
(cherry picked from commit 6a050c7c)
parent d374204d

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

+5 −50
Original line number Diff line number Diff line
@@ -867,8 +867,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 


        mPermissionMonitor = new PermissionMonitor(mContext, mNetd);



        // Set up the listener for user state for creating user VPNs.

        // Should run on mHandler to avoid any races.

        //set up the listener for user state for creating user VPNs

        IntentFilter intentFilter = new IntentFilter();

        intentFilter.addAction(Intent.ACTION_USER_STARTED);

        intentFilter.addAction(Intent.ACTION_USER_STOPPED);
@@ -876,11 +875,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        intentFilter.addAction(Intent.ACTION_USER_REMOVED);

        intentFilter.addAction(Intent.ACTION_USER_UNLOCKED);

        mContext.registerReceiverAsUser(

                mUserIntentReceiver,

                UserHandle.ALL,

                intentFilter,

                null /* broadcastPermission */,

                mHandler);

                mUserIntentReceiver, UserHandle.ALL, intentFilter, null, null);

        mContext.registerReceiverAsUser(mUserPresentReceiver, UserHandle.SYSTEM,

                new IntentFilter(Intent.ACTION_USER_PRESENT), null, null);
@@ -3820,27 +3815,17 @@ public class ConnectivityService extends IConnectivityManager.Stub 
     * handler thread through their agent, this is asynchronous. When the capabilities objects

     * are computed they will be up-to-date as they are computed synchronously from here and

     * this is running on the ConnectivityService thread.

     * TODO : Fix this and call updateCapabilities inline to remove out-of-order events.

     */

    private void updateAllVpnsCapabilities() {

        Network defaultNetwork = getNetwork(getDefaultNetwork());

        synchronized (mVpns) {

            for (int i = 0; i < mVpns.size(); i++) {

                final Vpn vpn = mVpns.valueAt(i);

                NetworkCapabilities nc = vpn.updateCapabilities(defaultNetwork);

                updateVpnCapabilities(vpn, nc);

                vpn.updateCapabilities();

            }

        }

    }



    private void updateVpnCapabilities(Vpn vpn, @Nullable NetworkCapabilities nc) {

        ensureRunningOnConnectivityServiceThread();

        NetworkAgentInfo vpnNai = getNetworkAgentInfoForNetId(vpn.getNetId());

        if (vpnNai == null || nc == null) {

            return;

        }

        updateCapabilities(vpnNai.getCurrentScore(), vpnNai, nc);

    }



    @Override

    public boolean updateLockdownVpn() {

        if (Binder.getCallingUid() != Process.SYSTEM_UID) {
@@ -4147,27 +4132,21 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    }



    private void onUserAdded(int userId) {

        Network defaultNetwork = getNetwork(getDefaultNetwork());

        synchronized (mVpns) {

            final int vpnsSize = mVpns.size();

            for (int i = 0; i < vpnsSize; i++) {

                Vpn vpn = mVpns.valueAt(i);

                vpn.onUserAdded(userId);

                NetworkCapabilities nc = vpn.updateCapabilities(defaultNetwork);

                updateVpnCapabilities(vpn, nc);

            }

        }

    }



    private void onUserRemoved(int userId) {

        Network defaultNetwork = getNetwork(getDefaultNetwork());

        synchronized (mVpns) {

            final int vpnsSize = mVpns.size();

            for (int i = 0; i < vpnsSize; i++) {

                Vpn vpn = mVpns.valueAt(i);

                vpn.onUserRemoved(userId);

                NetworkCapabilities nc = vpn.updateCapabilities(defaultNetwork);

                updateVpnCapabilities(vpn, nc);

            }

        }

    }
@@ -4186,7 +4165,6 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    private BroadcastReceiver mUserIntentReceiver = new BroadcastReceiver() {

        @Override

        public void onReceive(Context context, Intent intent) {

            ensureRunningOnConnectivityServiceThread();

            final String action = intent.getAction();

            final int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL);

            if (userId == UserHandle.USER_NULL) return;
@@ -4672,19 +4650,6 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        return getNetworkForRequest(mDefaultRequest.requestId);

    }



    @Nullable

    private Network getNetwork(@Nullable NetworkAgentInfo nai) {

        return nai != null ? nai.network : null;

    }



    private void ensureRunningOnConnectivityServiceThread() {

        if (mHandler.getLooper().getThread() != Thread.currentThread()) {

            throw new IllegalStateException(

                    "Not running on ConnectivityService thread: "

                            + Thread.currentThread().getName());

        }

    }



    private boolean isDefaultNetwork(NetworkAgentInfo nai) {

        return nai == getDefaultNetwork();

    }
@@ -5232,8 +5197,6 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        updateTcpBufferSizes(newNetwork);

        mDnsManager.setDefaultDnsSystemProperties(newNetwork.linkProperties.getDnsServers());

        notifyIfacesChangedForNetworkStats();

        // Fix up the NetworkCapabilities of any VPNs that don't specify underlying networks.

        updateAllVpnsCapabilities();

    }



    private void processListenRequests(NetworkAgentInfo nai, boolean capabilitiesChanged) {
@@ -5667,10 +5630,6 @@ public class ConnectivityService extends IConnectivityManager.Stub 
            // doing.

            updateSignalStrengthThresholds(networkAgent, "CONNECT", null);



            if (networkAgent.isVPN()) {

                updateAllVpnsCapabilities();

            }



            // Consider network even though it is not yet validated.

            final long now = SystemClock.elapsedRealtime();

            rematchNetworkAndRequests(networkAgent, ReapUnvalidatedNetworks.REAP, now);
@@ -5864,11 +5823,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 
            success = mVpns.get(user).setUnderlyingNetworks(networks);

        }

        if (success) {

            mHandler.post(() -> {

                // Update VPN's capabilities based on updated underlying network set.

                updateAllVpnsCapabilities();

                notifyIfacesChangedForNetworkStats();

            });

            mHandler.post(() -> notifyIfacesChangedForNetworkStats());

        }

        return success;

    }

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

+14 −52
Original line number Diff line number Diff line
@@ -273,7 +273,7 @@ public class Vpn { 
        mNetworkCapabilities = new NetworkCapabilities();

        mNetworkCapabilities.addTransportType(NetworkCapabilities.TRANSPORT_VPN);

        mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN);

        updateCapabilities(null /* defaultNetwork */);

        updateCapabilities();



        loadAlwaysOnPackage();

    }
@@ -300,39 +300,18 @@ public class Vpn { 
        updateAlwaysOnNotification(detailedState);

    }



    /**

     * Updates {@link #mNetworkCapabilities} based on current underlying networks and returns a

     * defensive copy.

     *

     * <p>Does not propagate updated capabilities to apps.

     *

     * @param defaultNetwork underlying network for VPNs following platform's default

     */

    public synchronized NetworkCapabilities updateCapabilities(

            @Nullable Network defaultNetwork) {

        if (mConfig == null) {

            // VPN is not running.

            return null;

        }



        Network[] underlyingNetworks = mConfig.underlyingNetworks;

        if (underlyingNetworks == null && defaultNetwork != null) {

            // null underlying networks means to track the default.

            underlyingNetworks = new Network[] { defaultNetwork };

        }



        applyUnderlyingCapabilities(

                mContext.getSystemService(ConnectivityManager.class),

                underlyingNetworks,

    public void updateCapabilities() {

        final Network[] underlyingNetworks = (mConfig != null) ? mConfig.underlyingNetworks : null;

        updateCapabilities(mContext.getSystemService(ConnectivityManager.class), underlyingNetworks,

                mNetworkCapabilities);



        return new NetworkCapabilities(mNetworkCapabilities);

        if (mNetworkAgent != null) {

            mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities);

        }

    }



    @VisibleForTesting

    public static void applyUnderlyingCapabilities(

            ConnectivityManager cm,

            Network[] underlyingNetworks,

    public static void updateCapabilities(ConnectivityManager cm, Network[] underlyingNetworks,

            NetworkCapabilities caps) {

        int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN };

        int downKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED;
@@ -344,7 +323,6 @@ public class Vpn { 
        boolean hadUnderlyingNetworks = false;

        if (null != underlyingNetworks) {

            for (Network underlying : underlyingNetworks) {

                // TODO(b/124469351): Get capabilities directly from ConnectivityService instead.

                final NetworkCapabilities underlyingCaps = cm.getNetworkCapabilities(underlying);

                if (underlyingCaps == null) continue;

                hadUnderlyingNetworks = true;
@@ -1015,8 +993,9 @@ public class Vpn { 
    }



    /**

     * Establish a VPN network and return the file descriptor of the VPN interface. This methods

     * returns {@code null} if the application is revoked or not prepared.

     * Establish a VPN network and return the file descriptor of the VPN

     * interface. This methods returns {@code null} if the application is

     * revoked or not prepared.

     *

     * @param config The parameters to configure the network.

     * @return The file descriptor of the VPN interface.
@@ -1263,11 +1242,6 @@ public class Vpn { 
        return ranges;

    }



    /**

     * Updates UID ranges for this VPN and also updates its internal capabilities.

     *

     * <p>Should be called on primary ConnectivityService thread.

     */

    public void onUserAdded(int userHandle) {

        // If the user is restricted tie them to the parent user's VPN

        UserInfo user = UserManager.get(mContext).getUserInfo(userHandle);
@@ -1278,9 +1252,8 @@ public class Vpn { 
                    try {

                        addUserToRanges(existingRanges, userHandle, mConfig.allowedApplications,

                                mConfig.disallowedApplications);

                        // ConnectivityService will call {@link #updateCapabilities} and apply

                        // those for VPN network.

                        mNetworkCapabilities.setUids(existingRanges);

                        updateCapabilities();

                    } catch (Exception e) {

                        Log.wtf(TAG, "Failed to add restricted user to owner", e);

                    }
@@ -1290,11 +1263,6 @@ public class Vpn { 
        }

    }



    /**

     * Updates UID ranges for this VPN and also updates its capabilities.

     *

     * <p>Should be called on primary ConnectivityService thread.

     */

    public void onUserRemoved(int userHandle) {

        // clean up if restricted

        UserInfo user = UserManager.get(mContext).getUserInfo(userHandle);
@@ -1306,9 +1274,8 @@ public class Vpn { 
                        final List<UidRange> removedRanges =

                            uidRangesForUser(userHandle, existingRanges);

                        existingRanges.removeAll(removedRanges);

                        // ConnectivityService will call {@link #updateCapabilities} and

                        // apply those for VPN network.

                        mNetworkCapabilities.setUids(existingRanges);

                        updateCapabilities();

                    } catch (Exception e) {

                        Log.wtf(TAG, "Failed to remove restricted user to owner", e);

                    }
@@ -1516,12 +1483,6 @@ public class Vpn { 
        return success;

    }



    /**

     * Updates underlying network set.

     *

     * <p>Note: Does not updates capabilities. Call {@link #updateCapabilities} from

     * ConnectivityService thread to get updated capabilities.

     */

    public synchronized boolean setUnderlyingNetworks(Network[] networks) {

        if (!isCallerEstablishedOwnerLocked()) {

            return false;
@@ -1538,6 +1499,7 @@ public class Vpn { 
                }

            }

        }

        updateCapabilities();

        return true;

    }

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

+17 −98
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; 
import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OFF;

import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC;

import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_PROVIDER_HOSTNAME;

import static android.net.ConnectivityManager.NETID_UNSET;

import static android.net.ConnectivityManager.TYPE_ETHERNET;

import static android.net.ConnectivityManager.TYPE_MOBILE;

import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA;
@@ -776,14 +775,11 @@ public class ConnectivityServiceTest { 


        public void setUids(Set<UidRange> uids) {

            mNetworkCapabilities.setUids(uids);

            updateCapabilities(null /* defaultNetwork */);

            updateCapabilities();

        }



        @Override

        public int getNetId() {

            if (mMockNetworkAgent == null) {

                return NETID_UNSET;

            }

            return mMockNetworkAgent.getNetwork().netId;

        }
@@ -804,13 +800,12 @@ public class ConnectivityServiceTest { 
        }



        @Override

        public NetworkCapabilities updateCapabilities(Network defaultNetwork) {

            if (!mConnected) return null;

            super.updateCapabilities(defaultNetwork);

            // Because super.updateCapabilities will update the capabilities of the agent but

            // not the mock agent, the mock agent needs to know about them.

        public void updateCapabilities() {

            if (!mConnected) return;

            super.updateCapabilities();

            // Because super.updateCapabilities will update the capabilities of the agent but not

            // the mock agent, the mock agent needs to know about them.

            copyCapabilitiesToNetworkAgent();

            return new NetworkCapabilities(mNetworkCapabilities);

        }



        private void copyCapabilitiesToNetworkAgent() {
@@ -4223,7 +4218,6 @@ public class ConnectivityServiceTest { 
        mMockVpn.setUids(ranges);

        vpnNetworkAgent.connect(false);

        mMockVpn.connect();

        mMockVpn.setUnderlyingNetworks(new Network[0]);



        genericNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent);

        genericNotVpnNetworkCallback.assertNoCallback();
@@ -4256,7 +4250,6 @@ public class ConnectivityServiceTest { 


        ranges.add(new UidRange(uid, uid));

        mMockVpn.setUids(ranges);

        vpnNetworkAgent.setUids(ranges);



        genericNetworkCallback.expectAvailableCallbacksValidated(vpnNetworkAgent);

        genericNotVpnNetworkCallback.assertNoCallback();
@@ -4290,11 +4283,12 @@ public class ConnectivityServiceTest { 
    }



    @Test

    public void testVpnWithoutInternet() {

    public void testVpnWithAndWithoutInternet() {

        final int uid = Process.myUid();



        final TestNetworkCallback defaultCallback = new TestNetworkCallback();

        mCm.registerDefaultNetworkCallback(defaultCallback);

        defaultCallback.assertNoCallback();



        mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);

        mWiFiNetworkAgent.connect(true);
@@ -4316,30 +4310,11 @@ public class ConnectivityServiceTest { 
        vpnNetworkAgent.disconnect();

        defaultCallback.assertNoCallback();



        mCm.unregisterNetworkCallback(defaultCallback);

    }



    @Test

    public void testVpnWithInternet() {

        final int uid = Process.myUid();



        final TestNetworkCallback defaultCallback = new TestNetworkCallback();

        mCm.registerDefaultNetworkCallback(defaultCallback);



        mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);

        mWiFiNetworkAgent.connect(true);



        defaultCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent);

        assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());



        MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);

        final ArraySet<UidRange> ranges = new ArraySet<>();

        ranges.add(new UidRange(uid, uid));

        vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);

        mMockVpn.setNetworkAgent(vpnNetworkAgent);

        mMockVpn.setUids(ranges);

        vpnNetworkAgent.connect(true /* validated */, true /* hasInternet */);

        mMockVpn.connect();



        defaultCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent);

        assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
@@ -4347,6 +4322,14 @@ public class ConnectivityServiceTest { 
        defaultCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent);

        defaultCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent);



        vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);

        ranges.clear();

        mMockVpn.setNetworkAgent(vpnNetworkAgent);

        mMockVpn.setUids(ranges);

        vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */);

        mMockVpn.connect();

        defaultCallback.assertNoCallback();



        mCm.unregisterNetworkCallback(defaultCallback);

    }
@@ -4447,68 +4430,4 @@ public class ConnectivityServiceTest { 


        mMockVpn.disconnect();

    }



    @Test

    public void testNullUnderlyingNetworks() {

        final int uid = Process.myUid();



        final TestNetworkCallback vpnNetworkCallback = new TestNetworkCallback();

        final NetworkRequest vpnNetworkRequest = new NetworkRequest.Builder()

                .removeCapability(NET_CAPABILITY_NOT_VPN)

                .addTransportType(TRANSPORT_VPN)

                .build();

        NetworkCapabilities nc;

        mCm.registerNetworkCallback(vpnNetworkRequest, vpnNetworkCallback);

        vpnNetworkCallback.assertNoCallback();



        final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);

        final ArraySet<UidRange> ranges = new ArraySet<>();

        ranges.add(new UidRange(uid, uid));

        mMockVpn.setNetworkAgent(vpnNetworkAgent);

        mMockVpn.connect();

        mMockVpn.setUids(ranges);

        vpnNetworkAgent.connect(true /* validated */, false /* hasInternet */);



        vpnNetworkCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent);

        nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork());

        assertTrue(nc.hasTransport(TRANSPORT_VPN));

        assertFalse(nc.hasTransport(TRANSPORT_CELLULAR));

        assertFalse(nc.hasTransport(TRANSPORT_WIFI));

        // By default, VPN is set to track default network (i.e. its underlying networks is null).

        // In case of no default network, VPN is considered metered.

        assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_METERED));



        // Connect to Cell; Cell is the default network.

        mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR);

        mCellNetworkAgent.connect(true);



        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)

                && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)

                && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),

                vpnNetworkAgent);



        // Connect to WiFi; WiFi is the new default.

        mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);

        mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED);

        mWiFiNetworkAgent.connect(true);



        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)

                && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)

                && caps.hasCapability(NET_CAPABILITY_NOT_METERED),

                vpnNetworkAgent);



        // Disconnect Cell. The default network did not change, so there shouldn't be any changes in

        // the capabilities.

        mCellNetworkAgent.disconnect();



        // Disconnect wifi too. Now we have no default network.

        mWiFiNetworkAgent.disconnect();



        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)

                && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)

                && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),

                vpnNetworkAgent);



        mMockVpn.disconnect();

    }

}

tests/net/java/com/android/server/connectivity/VpnTest.java

+4 −8
Original line number Diff line number Diff line
@@ -457,8 +457,7 @@ public class VpnTest { 


        final NetworkCapabilities caps = new NetworkCapabilities();



        Vpn.applyUnderlyingCapabilities(

                mConnectivityManager, new Network[] {}, caps);

        Vpn.updateCapabilities(mConnectivityManager, new Network[] { }, caps);

        assertTrue(caps.hasTransport(TRANSPORT_VPN));

        assertFalse(caps.hasTransport(TRANSPORT_CELLULAR));

        assertFalse(caps.hasTransport(TRANSPORT_WIFI));
@@ -468,8 +467,7 @@ public class VpnTest { 
        assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING));

        assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED));



        Vpn.applyUnderlyingCapabilities(

                mConnectivityManager, new Network[] {mobile}, caps);

        Vpn.updateCapabilities(mConnectivityManager, new Network[] { mobile }, caps);

        assertTrue(caps.hasTransport(TRANSPORT_VPN));

        assertTrue(caps.hasTransport(TRANSPORT_CELLULAR));

        assertFalse(caps.hasTransport(TRANSPORT_WIFI));
@@ -479,8 +477,7 @@ public class VpnTest { 
        assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING));

        assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED));



        Vpn.applyUnderlyingCapabilities(

                mConnectivityManager, new Network[] {wifi}, caps);

        Vpn.updateCapabilities(mConnectivityManager, new Network[] { wifi }, caps);

        assertTrue(caps.hasTransport(TRANSPORT_VPN));

        assertFalse(caps.hasTransport(TRANSPORT_CELLULAR));

        assertTrue(caps.hasTransport(TRANSPORT_WIFI));
@@ -490,8 +487,7 @@ public class VpnTest { 
        assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING));

        assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED));



        Vpn.applyUnderlyingCapabilities(

                mConnectivityManager, new Network[] {mobile, wifi}, caps);

        Vpn.updateCapabilities(mConnectivityManager, new Network[] { mobile, wifi }, caps);

        assertTrue(caps.hasTransport(TRANSPORT_VPN));

        assertTrue(caps.hasTransport(TRANSPORT_CELLULAR));

        assertTrue(caps.hasTransport(TRANSPORT_WIFI));