diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 1a9ff25f33..3c351ba1ff 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -3261,11 +3261,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } case NetworkAgent.EVENT_UNDERLYING_NETWORKS_CHANGED: { // TODO: prevent loops, e.g., if a network declares itself as underlying. - if (!nai.supportsUnderlyingNetworks()) { - Log.wtf(TAG, "Non-virtual networks cannot have underlying networks"); - break; - } - final List underlying = (List) arg.second; if (isLegacyLockdownNai(nai) @@ -5304,12 +5299,12 @@ public class ConnectivityService extends IConnectivityManager.Stub * information, e.g underlying ifaces. */ private UnderlyingNetworkInfo createVpnInfo(NetworkAgentInfo nai) { - if (!nai.isVPN()) return null; - Network[] underlyingNetworks = nai.declaredUnderlyingNetworks; // see VpnService.setUnderlyingNetworks()'s javadoc about how to interpret // the underlyingNetworks list. - if (underlyingNetworks == null) { + // TODO: stop using propagateUnderlyingCapabilities here, for example, by always + // initializing NetworkAgentInfo#declaredUnderlyingNetworks to an empty array. + if (underlyingNetworks == null && nai.propagateUnderlyingCapabilities()) { final NetworkAgentInfo defaultNai = getDefaultNetworkForUid( nai.networkCapabilities.getOwnerUid()); if (defaultNai != null) { @@ -5358,7 +5353,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private boolean hasUnderlyingNetwork(NetworkAgentInfo nai, Network network) { // TODO: support more than one level of underlying networks, either via a fixed-depth search // (e.g., 2 levels of underlying networks), or via loop detection, or.... - if (!nai.supportsUnderlyingNetworks()) return false; + if (!nai.propagateUnderlyingCapabilities()) return false; final Network[] underlying = underlyingNetworksOrDefault( nai.networkCapabilities.getOwnerUid(), nai.declaredUnderlyingNetworks); return CollectionUtils.contains(underlying, network); @@ -7331,7 +7326,7 @@ public class ConnectivityService extends IConnectivityManager.Stub newNc.addCapability(NET_CAPABILITY_NOT_ROAMING); } - if (nai.supportsUnderlyingNetworks()) { + if (nai.propagateUnderlyingCapabilities()) { applyUnderlyingCapabilities(nai.declaredUnderlyingNetworks, nai.declaredCapabilities, newNc); } @@ -8473,7 +8468,7 @@ public class ConnectivityService extends IConnectivityManager.Stub networkAgent.networkCapabilities.addCapability(NET_CAPABILITY_FOREGROUND); if (!createNativeNetwork(networkAgent)) return; - if (networkAgent.supportsUnderlyingNetworks()) { + if (networkAgent.propagateUnderlyingCapabilities()) { // Initialize the network's capabilities to their starting values according to the // underlying networks. This ensures that the capabilities are correct before // anything happens to the network. @@ -9446,7 +9441,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private boolean ownsVpnRunningOverNetwork(int uid, Network network) { for (NetworkAgentInfo virtual : mNetworkAgentInfos) { - if (virtual.supportsUnderlyingNetworks() + if (virtual.propagateUnderlyingCapabilities() && virtual.networkCapabilities.getOwnerUid() == uid && CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)) { return true; diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index 18becd45f6..bbf523a29d 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -157,8 +157,8 @@ public class NetworkAgentInfo implements Comparable, NetworkRa @NonNull public NetworkCapabilities networkCapabilities; @NonNull public final NetworkAgentConfig networkAgentConfig; - // Underlying networks declared by the agent. Only set if supportsUnderlyingNetworks is true. - // The networks in this list might be declared by a VPN app using setUnderlyingNetworks and are + // Underlying networks declared by the agent. + // The networks in this list might be declared by a VPN using setUnderlyingNetworks and are // not guaranteed to be current or correct, or even to exist. // // This array is read and iterated on multiple threads with no locking so its contents must @@ -168,7 +168,7 @@ public class NetworkAgentInfo implements Comparable, NetworkRa // The capabilities originally announced by the NetworkAgent, regardless of any capabilities // that were added or removed due to this network's underlying networks. - // Only set if #supportsUnderlyingNetworks is true. + // Only set if #propagateUnderlyingCapabilities is true. public @Nullable NetworkCapabilities declaredCapabilities; // Indicates if netd has been told to create this Network. From this point on the appropriate @@ -898,8 +898,11 @@ public class NetworkAgentInfo implements Comparable, NetworkRa return networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN); } - /** Whether this network might have underlying networks. Currently only true for VPNs. */ - public boolean supportsUnderlyingNetworks() { + /** + * Whether this network should propagate the capabilities from its underlying networks. + * Currently only true for VPNs. + */ + public boolean propagateUnderlyingCapabilities() { return isVPN(); } diff --git a/tests/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/integration/util/com/android/server/NetworkAgentWrapper.java index 17db17923f..95ea401acd 100644 --- a/tests/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/tests/integration/util/com/android/server/NetworkAgentWrapper.java @@ -55,6 +55,7 @@ import com.android.net.module.util.ArrayTrackRecord; import com.android.testutils.HandlerUtils; import com.android.testutils.TestableNetworkCallback; +import java.util.List; import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -255,6 +256,15 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { } } + public void setUnderlyingNetworks(List underlyingNetworks) { + mNetworkAgent.setUnderlyingNetworks(underlyingNetworks); + } + + public void setOwnerUid(int uid) { + mNetworkCapabilities.setOwnerUid(uid); + mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); + } + public void connect() { if (!mConnected.compareAndSet(false /* expect */, true /* update */)) { // compareAndSet returns false when the value couldn't be updated because it did not diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 6d7d3d2abe..cec38827ca 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -6029,7 +6029,7 @@ public class ConnectivityServiceTest { verify(mStatsManager, atLeastOnce()).notifyNetworkStatus(networksCaptor.capture(), any(List.class), eq(defaultIface), vpnInfosCaptor.capture()); - assertSameElements(networksCaptor.getValue(), networks); + assertSameElements(networks, networksCaptor.getValue()); List infos = vpnInfosCaptor.getValue(); if (vpnUid != null) { @@ -6233,6 +6233,77 @@ public class ConnectivityServiceTest { reset(mStatsManager); } + @Test + public void testNonVpnUnderlyingNetworks() throws Exception { + // Ensure wifi and cellular are not torn down. + for (int transport : new int[]{TRANSPORT_CELLULAR, TRANSPORT_WIFI}) { + final NetworkRequest request = new NetworkRequest.Builder() + .addTransportType(transport) + .removeCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .build(); + mCm.requestNetwork(request, new NetworkCallback()); + } + + // Connect a VCN-managed wifi network. + final LinkProperties wifiLp = new LinkProperties(); + wifiLp.setInterfaceName(WIFI_IFNAME); + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, wifiLp); + mWiFiNetworkAgent.removeCapability(NET_CAPABILITY_NOT_VCN_MANAGED); + mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED); + mWiFiNetworkAgent.connect(true /* validated */); + + final List none = List.of(); + expectNotifyNetworkStatus(none, null); // Wifi is not the default network + + // Create a virtual network based on the wifi network. + final int ownerUid = 10042; + NetworkCapabilities nc = new NetworkCapabilities.Builder() + .setOwnerUid(ownerUid) + .setAdministratorUids(new int[]{ownerUid}) + .build(); + final String vcnIface = "ipsec42"; + final LinkProperties lp = new LinkProperties(); + lp.setInterfaceName(vcnIface); + final TestNetworkAgentWrapper vcn = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, lp, nc); + vcn.setUnderlyingNetworks(List.of(mWiFiNetworkAgent.getNetwork())); + vcn.connect(false /* validated */); + + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(callback); + callback.expectAvailableCallbacksUnvalidated(vcn); + + // The underlying wifi network's capabilities are not propagated to the virtual network, + // but NetworkStatsService is informed of the underlying interface. + nc = mCm.getNetworkCapabilities(vcn.getNetwork()); + assertFalse(nc.hasTransport(TRANSPORT_WIFI)); + assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_METERED)); + final List onlyVcn = List.of(vcn.getNetwork()); + expectNotifyNetworkStatus(onlyVcn, vcnIface, ownerUid, vcnIface, List.of(WIFI_IFNAME)); + + // Add NOT_METERED to the underlying network, check that it is not propagated. + mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED); + callback.assertNoCallback(); + nc = mCm.getNetworkCapabilities(vcn.getNetwork()); + assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_METERED)); + + // Switch underlying networks. + final LinkProperties cellLp = new LinkProperties(); + cellLp.setInterfaceName(MOBILE_IFNAME); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); + mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_VCN_MANAGED); + mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_ROAMING); + mCellNetworkAgent.connect(false /* validated */); + vcn.setUnderlyingNetworks(List.of(mCellNetworkAgent.getNetwork())); + + // The underlying capability changes do not propagate to the virtual network, but + // NetworkStatsService is informed of the new underlying interface. + callback.assertNoCallback(); + nc = mCm.getNetworkCapabilities(vcn.getNetwork()); + assertFalse(nc.hasTransport(TRANSPORT_WIFI)); + assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + expectNotifyNetworkStatus(onlyVcn, vcnIface, ownerUid, vcnIface, List.of(MOBILE_IFNAME)); + } + @Test public void testBasicDnsConfigurationPushed() throws Exception { setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com");