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

Commit 200cad09 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge changes Ic8231b18,I30009f88 into pi-dev

* changes:
  Fix: VPNs update caps upon underlying network disconnect.
  Add tests for setUnderlyingNetworks.
parents 3a95d0bb 6b65ec77
Loading
Loading
Loading
Loading
+25 −7
Original line number Diff line number Diff line
@@ -2503,6 +2503,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
                ensureNetworkTransitionWakelock(nai.name());
            }
            mLegacyTypeTracker.remove(nai, wasDefault);
            if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) {
                updateAllVpnsCapabilities();
            }
            rematchAllNetworksAndRequests(null, 0);
            mLingerMonitor.noteDisconnect(nai);
            if (nai.created) {
@@ -3778,6 +3781,26 @@ public class ConnectivityService extends IConnectivityManager.Stub
        }
    }

    /**
     * Ask all VPN objects to recompute and update their capabilities.
     *
     * When underlying networks change, VPNs may have to update capabilities to reflect things
     * like the metered bit, their transports, and so on. This asks the VPN objects to update
     * their capabilities, and as this will cause them to send messages to the ConnectivityService
     * 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() {
        synchronized (mVpns) {
            for (int i = 0; i < mVpns.size(); i++) {
                final Vpn vpn = mVpns.valueAt(i);
                vpn.updateCapabilities();
            }
        }
    }

    @Override
    public boolean updateLockdownVpn() {
        if (Binder.getCallingUid() != Process.SYSTEM_UID) {
@@ -4935,12 +4958,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
        if (!newNc.hasTransport(TRANSPORT_VPN)) {
            // Tell VPNs about updated capabilities, since they may need to
            // bubble those changes through.
            synchronized (mVpns) {
                for (int i = 0; i < mVpns.size(); i++) {
                    final Vpn vpn = mVpns.valueAt(i);
                    vpn.updateCapabilities();
                }
            }
            updateAllVpnsCapabilities();
        }
    }

+8 −4
Original line number Diff line number Diff line
@@ -172,10 +172,13 @@ public class Vpn {
    private PendingIntent mStatusIntent;
    private volatile boolean mEnableTeardown = true;
    private final INetworkManagementService mNetd;
    private VpnConfig mConfig;
    private NetworkAgent mNetworkAgent;
    @VisibleForTesting
    protected VpnConfig mConfig;
    @VisibleForTesting
    protected NetworkAgent mNetworkAgent;
    private final Looper mLooper;
    private final NetworkCapabilities mNetworkCapabilities;
    @VisibleForTesting
    protected final NetworkCapabilities mNetworkCapabilities;
    private final SystemServices mSystemServices;

    /**
@@ -1071,7 +1074,8 @@ public class Vpn {

    // Returns true if the VPN has been established and the calling UID is its owner. Used to check
    // that a call to mutate VPN state is admissible.
    private boolean isCallerEstablishedOwnerLocked() {
    @VisibleForTesting
    protected boolean isCallerEstablishedOwnerLocked() {
        return isRunningLocked() && Binder.getCallingUid() == mOwnerUID;
    }

+217 −16
Original line number Diff line number Diff line
@@ -112,6 +112,7 @@ import android.net.NetworkUtils;
import android.net.RouteInfo;
import android.net.StringNetworkSpecifier;
import android.net.UidRange;
import android.net.VpnService;
import android.net.captiveportal.CaptivePortalProbeResult;
import android.net.metrics.IpConnectivityLog;
import android.net.util.MultinetworkPolicyTracker;
@@ -134,6 +135,7 @@ import android.test.mock.MockContentResolver;
import android.util.ArraySet;
import android.util.Log;

import com.android.internal.net.VpnConfig;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.WakeupMessage;
import com.android.internal.util.test.BroadcastInterceptingContext;
@@ -197,13 +199,13 @@ public class ConnectivityServiceTest {
    private MockNetworkAgent mWiFiNetworkAgent;
    private MockNetworkAgent mCellNetworkAgent;
    private MockNetworkAgent mEthernetNetworkAgent;
    private MockVpn mMockVpn;
    private Context mContext;

    @Mock IpConnectivityMetrics.Logger mMetricsService;
    @Mock DefaultNetworkMetrics mDefaultNetworkMetrics;
    @Mock INetworkManagementService mNetworkManagementService;
    @Mock INetworkStatsService mStatsService;
    @Mock Vpn mMockVpn;

    private ArgumentCaptor<String[]> mStringArrayCaptor = ArgumentCaptor.forClass(String[].class);

@@ -479,6 +481,14 @@ public class ConnectivityServiceTest {
            mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities);
        }

        public void setNetworkCapabilities(NetworkCapabilities nc,
                boolean sendToConnectivityService) {
            mNetworkCapabilities.set(nc);
            if (sendToConnectivityService) {
                mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities);
            }
        }

        public void connectWithoutInternet() {
            mNetworkInfo.setDetailedState(DetailedState.CONNECTED, null, null);
            mNetworkAgent.sendNetworkInfo(mNetworkInfo);
@@ -594,6 +604,10 @@ public class ConnectivityServiceTest {
            return mRedirectUrl;
        }

        public NetworkAgent getNetworkAgent() {
            return mNetworkAgent;
        }

        public NetworkCapabilities getNetworkCapabilities() {
            return mNetworkCapabilities;
        }
@@ -726,6 +740,87 @@ public class ConnectivityServiceTest {
        }
    }

    private static Looper startHandlerThreadAndReturnLooper() {
        final HandlerThread handlerThread = new HandlerThread("MockVpnThread");
        handlerThread.start();
        return handlerThread.getLooper();
    }

    private class MockVpn extends Vpn {
        // TODO : the interactions between this mock and the mock network agent are too
        // hard to get right at this moment, because it's unclear in which case which
        // target needs to get a method call or both, and in what order. It's because
        // MockNetworkAgent wants to manage its own NetworkCapabilities, but the Vpn
        // parent class of MockVpn agent wants that responsibility.
        // That being said inside the test it should be possible to make the interactions
        // harder to get wrong with precise speccing, judicious comments, helper methods
        // and a few sprinkled assertions.

        private boolean mConnected = false;
        // Careful ! This is different from mNetworkAgent, because MockNetworkAgent does
        // not inherit from NetworkAgent.
        private MockNetworkAgent mMockNetworkAgent;

        public MockVpn(int userId) {
            super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService,
                    userId);
        }

        public void setNetworkAgent(MockNetworkAgent agent) {
            waitForIdle(agent, TIMEOUT_MS);
            mMockNetworkAgent = agent;
            mNetworkAgent = agent.getNetworkAgent();
            mNetworkCapabilities.set(agent.getNetworkCapabilities());
        }

        public void setUids(Set<UidRange> uids) {
            mNetworkCapabilities.setUids(uids);
            updateCapabilities();
        }

        @Override
        public int getNetId() {
            return mMockNetworkAgent.getNetwork().netId;
        }

        @Override
        public boolean appliesToUid(int uid) {
            return mConnected;  // Trickery to simplify testing.
        }

        @Override
        protected boolean isCallerEstablishedOwnerLocked() {
            return mConnected;  // Similar trickery
        }

        public void connect() {
            mNetworkCapabilities.set(mMockNetworkAgent.getNetworkCapabilities());
            mConnected = true;
            mConfig = new VpnConfig();
        }

        @Override
        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();
        }

        private void copyCapabilitiesToNetworkAgent() {
            if (null != mMockNetworkAgent) {
                mMockNetworkAgent.setNetworkCapabilities(mNetworkCapabilities,
                        false /* sendToConnectivityService */);
            }
        }

        public void disconnect() {
            mConnected = false;
            mConfig = null;
        }
    }

    private class FakeWakeupMessage extends WakeupMessage {
        private static final int UNREASONABLY_LONG_WAIT = 1000;

@@ -894,10 +989,12 @@ public class ConnectivityServiceTest {

        public void mockVpn(int uid) {
            synchronized (mVpns) {
                int userId = UserHandle.getUserId(uid);
                mMockVpn = new MockVpn(userId);
                // This has no effect unless the VPN is actually connected, because things like
                // getActiveNetworkForUidInternal call getNetworkAgentInfoForNetId on the VPN
                // netId, and check if that network is actually connected.
                mVpns.put(UserHandle.getUserId(Process.myUid()), mMockVpn);
                mVpns.put(userId, mMockVpn);
            }
        }

@@ -927,7 +1024,6 @@ public class ConnectivityServiceTest {

        MockitoAnnotations.initMocks(this);
        when(mMetricsService.defaultNetworkMetrics()).thenReturn(mDefaultNetworkMetrics);
        when(mMockVpn.appliesToUid(Process.myUid())).thenReturn(true);

        // InstrumentationTestRunner prepares a looper, but AndroidJUnitRunner does not.
        // http://b/25897652 .
@@ -1549,7 +1645,8 @@ public class ConnectivityServiceTest {

        void expectCapabilitiesLike(Predicate<NetworkCapabilities> fn, MockNetworkAgent agent) {
            CallbackInfo cbi = expectCallback(CallbackState.NETWORK_CAPABILITIES, agent);
            assertTrue(fn.test((NetworkCapabilities) cbi.arg));
            assertTrue("Received capabilities don't match expectations : " + cbi.arg,
                    fn.test((NetworkCapabilities) cbi.arg));
        }

        void assertNoCallback() {
@@ -2577,9 +2674,10 @@ public class ConnectivityServiceTest {
        final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);
        final ArraySet<UidRange> ranges = new ArraySet<>();
        ranges.add(new UidRange(uid, uid));
        when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId);
        vpnNetworkAgent.setUids(ranges);
        mMockVpn.setNetworkAgent(vpnNetworkAgent);
        mMockVpn.setUids(ranges);
        vpnNetworkAgent.connect(true);
        mMockVpn.connect();
        defaultNetworkCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent);
        assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());

@@ -4108,9 +4206,10 @@ public class ConnectivityServiceTest {
        final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);
        final ArraySet<UidRange> ranges = new ArraySet<>();
        ranges.add(new UidRange(uid, uid));
        when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId);
        vpnNetworkAgent.setUids(ranges);
        mMockVpn.setNetworkAgent(vpnNetworkAgent);
        mMockVpn.setUids(ranges);
        vpnNetworkAgent.connect(false);
        mMockVpn.connect();

        genericNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent);
        genericNotVpnNetworkCallback.assertNoCallback();
@@ -4142,7 +4241,7 @@ public class ConnectivityServiceTest {
        defaultCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent);

        ranges.add(new UidRange(uid, uid));
        vpnNetworkAgent.setUids(ranges);
        mMockVpn.setUids(ranges);

        genericNetworkCallback.expectAvailableCallbacksValidated(vpnNetworkAgent);
        genericNotVpnNetworkCallback.assertNoCallback();
@@ -4192,9 +4291,10 @@ public class ConnectivityServiceTest {
        MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);
        final ArraySet<UidRange> ranges = new ArraySet<>();
        ranges.add(new UidRange(uid, uid));
        when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId);
        vpnNetworkAgent.setUids(ranges);
        mMockVpn.setNetworkAgent(vpnNetworkAgent);
        mMockVpn.setUids(ranges);
        vpnNetworkAgent.connect(true /* validated */, false /* hasInternet */);
        mMockVpn.connect();

        defaultCallback.assertNoCallback();
        assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
@@ -4203,9 +4303,10 @@ public class ConnectivityServiceTest {
        defaultCallback.assertNoCallback();

        vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);
        when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId);
        vpnNetworkAgent.setUids(ranges);
        mMockVpn.setNetworkAgent(vpnNetworkAgent);
        mMockVpn.setUids(ranges);
        vpnNetworkAgent.connect(true /* validated */, true /* hasInternet */);
        mMockVpn.connect();
        defaultCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent);
        assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());

@@ -4214,13 +4315,113 @@ public class ConnectivityServiceTest {
        defaultCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent);

        vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);
        when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId);
        ranges.clear();
        vpnNetworkAgent.setUids(ranges);

        mMockVpn.setNetworkAgent(vpnNetworkAgent);
        mMockVpn.setUids(ranges);
        vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */);
        mMockVpn.connect();
        defaultCallback.assertNoCallback();

        mCm.unregisterNetworkCallback(defaultCallback);
    }

    @Test
    public void testVpnSetUnderlyingNetworks() {
        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));
        // For safety reasons a VPN without underlying networks is considered metered.
        assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_METERED));

        // Connect cell and use it as an underlying network.
        mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR);
        mCellNetworkAgent.connect(true);

        mService.setUnderlyingNetworksForVpn(
                new Network[] { mCellNetworkAgent.getNetwork() });

        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
                && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
                && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                vpnNetworkAgent);

        mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);
        mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED);
        mWiFiNetworkAgent.connect(true);

        mService.setUnderlyingNetworksForVpn(
                new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() });

        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
                && caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
                && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                vpnNetworkAgent);

        // Don't disconnect, but note the VPN is not using wifi any more.
        mService.setUnderlyingNetworksForVpn(
                new Network[] { mCellNetworkAgent.getNetwork() });

        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
                && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
                && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                vpnNetworkAgent);

        // Use Wifi but not cell. Note the VPN is now unmetered.
        mService.setUnderlyingNetworksForVpn(
                new Network[] { mWiFiNetworkAgent.getNetwork() });

        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
                && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
                && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                vpnNetworkAgent);

        // Use both again.
        mService.setUnderlyingNetworksForVpn(
                new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() });

        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
                && caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
                && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                vpnNetworkAgent);

        // Disconnect cell. Receive update without even removing the dead network from the
        // underlying networks – it's dead anyway. Not metered any more.
        mCellNetworkAgent.disconnect();
        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
                && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
                && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                vpnNetworkAgent);

        // Disconnect wifi too. No underlying networks should mean this is now metered,
        // unfortunately a discrepancy in the current implementation has this unmetered.
        // TODO : fix this.
        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();
    }
}