Merge "Add CTS tests for ApfCapabilities static APIs."
diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java
index 2c356e4..7ff954b 100644
--- a/core/java/android/net/LinkProperties.java
+++ b/core/java/android/net/LinkProperties.java
@@ -690,9 +690,9 @@
route.getMtu());
}
- private int findRouteIndexByDestination(RouteInfo route) {
+ private int findRouteIndexByRouteKey(RouteInfo route) {
for (int i = 0; i < mRoutes.size(); i++) {
- if (mRoutes.get(i).isSameDestinationAs(route)) {
+ if (mRoutes.get(i).getRouteKey().equals(route.getRouteKey())) {
return i;
}
}
@@ -701,11 +701,11 @@
/**
* Adds a {@link RouteInfo} to this {@code LinkProperties}, if a {@link RouteInfo}
- * with the same destination exists with different properties (e.g., different MTU),
- * it will be updated. If the {@link RouteInfo} had an interface name set and
- * that differs from the interface set for this {@code LinkProperties} an
- * {@link IllegalArgumentException} will be thrown. The proper
- * course is to add either un-named or properly named {@link RouteInfo}.
+ * with the same {@link RouteInfo.RouteKey} with different properties
+ * (e.g., different MTU), it will be updated. If the {@link RouteInfo} had an
+ * interface name set and that differs from the interface set for this
+ * {@code LinkProperties} an {@link IllegalArgumentException} will be thrown.
+ * The proper course is to add either un-named or properly named {@link RouteInfo}.
*
* @param route A {@link RouteInfo} to add to this object.
* @return {@code true} was added or updated, false otherwise.
@@ -719,7 +719,7 @@
}
route = routeWithInterface(route);
- int i = findRouteIndexByDestination(route);
+ int i = findRouteIndexByRouteKey(route);
if (i == -1) {
// Route was not present. Add it.
mRoutes.add(route);
diff --git a/core/java/android/net/Network.java b/core/java/android/net/Network.java
index c145b2b..3d641f5 100644
--- a/core/java/android/net/Network.java
+++ b/core/java/android/net/Network.java
@@ -27,6 +27,7 @@
import android.system.OsConstants;
import android.util.proto.ProtoOutputStream;
+import com.android.internal.annotations.GuardedBy;
import com.android.okhttp.internalandroidapi.Dns;
import com.android.okhttp.internalandroidapi.HttpURLConnectionFactory;
@@ -64,15 +65,15 @@
* The unique id of the network.
* @hide
*/
- @SystemApi
+ @UnsupportedAppUsage
public final int netId;
// Objects used to perform per-network operations such as getSocketFactory
// and openConnection, and a lock to protect access to them.
private volatile NetworkBoundSocketFactory mNetworkBoundSocketFactory = null;
- // mLock should be used to control write access to mUrlConnectionFactory.
- // maybeInitUrlConnectionFactory() must be called prior to reading this field.
- private volatile HttpURLConnectionFactory mUrlConnectionFactory;
+ // mUrlConnectionFactory is initialized lazily when it is first needed.
+ @GuardedBy("mLock")
+ private HttpURLConnectionFactory mUrlConnectionFactory;
private final Object mLock = new Object();
// Default connection pool values. These are evaluated at startup, just
@@ -170,6 +171,17 @@
}
/**
+ * Get the unique id of the network.
+ *
+ * @hide
+ */
+ @TestApi
+ @SystemApi
+ public int getNetId() {
+ return netId;
+ }
+
+ /**
* Returns a netid marked with the Private DNS bypass flag.
*
* This flag must be kept in sync with the NETID_USE_LOCAL_NAMESERVERS flag
@@ -284,36 +296,16 @@
return mNetworkBoundSocketFactory;
}
- // TODO: This creates a connection pool and host resolver for
- // every Network object, instead of one for every NetId. This is
- // suboptimal, because an app could potentially have more than one
- // Network object for the same NetId, causing increased memory footprint
- // and performance penalties due to lack of connection reuse (connection
- // setup time, congestion window growth time, etc.).
- //
- // Instead, investigate only having one connection pool and host resolver
- // for every NetId, perhaps by using a static HashMap of NetIds to
- // connection pools and host resolvers. The tricky part is deciding when
- // to remove a map entry; a WeakHashMap shouldn't be used because whether
- // a Network is referenced doesn't correlate with whether a new Network
- // will be instantiated in the near future with the same NetID. A good
- // solution would involve purging empty (or when all connections are timed
- // out) ConnectionPools.
- private void maybeInitUrlConnectionFactory() {
- synchronized (mLock) {
- if (mUrlConnectionFactory == null) {
- // Set configuration on the HttpURLConnectionFactory that will be good for all
- // connections created by this Network. Configuration that might vary is left
- // until openConnection() and passed as arguments.
- Dns dnsLookup = hostname -> Arrays.asList(Network.this.getAllByName(hostname));
- HttpURLConnectionFactory urlConnectionFactory = new HttpURLConnectionFactory();
- urlConnectionFactory.setDns(dnsLookup); // Let traffic go via dnsLookup
- // A private connection pool just for this Network.
- urlConnectionFactory.setNewConnectionPool(httpMaxConnections,
- httpKeepAliveDurationMs, TimeUnit.MILLISECONDS);
- mUrlConnectionFactory = urlConnectionFactory;
- }
- }
+ private static HttpURLConnectionFactory createUrlConnectionFactory(Dns dnsLookup) {
+ // Set configuration on the HttpURLConnectionFactory that will be good for all
+ // connections created by this Network. Configuration that might vary is left
+ // until openConnection() and passed as arguments.
+ HttpURLConnectionFactory urlConnectionFactory = new HttpURLConnectionFactory();
+ urlConnectionFactory.setDns(dnsLookup); // Let traffic go via dnsLookup
+ // A private connection pool just for this Network.
+ urlConnectionFactory.setNewConnectionPool(httpMaxConnections,
+ httpKeepAliveDurationMs, TimeUnit.MILLISECONDS);
+ return urlConnectionFactory;
}
/**
@@ -354,9 +346,31 @@
*/
public URLConnection openConnection(URL url, java.net.Proxy proxy) throws IOException {
if (proxy == null) throw new IllegalArgumentException("proxy is null");
- maybeInitUrlConnectionFactory();
+ // TODO: This creates a connection pool and host resolver for
+ // every Network object, instead of one for every NetId. This is
+ // suboptimal, because an app could potentially have more than one
+ // Network object for the same NetId, causing increased memory footprint
+ // and performance penalties due to lack of connection reuse (connection
+ // setup time, congestion window growth time, etc.).
+ //
+ // Instead, investigate only having one connection pool and host resolver
+ // for every NetId, perhaps by using a static HashMap of NetIds to
+ // connection pools and host resolvers. The tricky part is deciding when
+ // to remove a map entry; a WeakHashMap shouldn't be used because whether
+ // a Network is referenced doesn't correlate with whether a new Network
+ // will be instantiated in the near future with the same NetID. A good
+ // solution would involve purging empty (or when all connections are timed
+ // out) ConnectionPools.
+ final HttpURLConnectionFactory urlConnectionFactory;
+ synchronized (mLock) {
+ if (mUrlConnectionFactory == null) {
+ Dns dnsLookup = hostname -> Arrays.asList(getAllByName(hostname));
+ mUrlConnectionFactory = createUrlConnectionFactory(dnsLookup);
+ }
+ urlConnectionFactory = mUrlConnectionFactory;
+ }
SocketFactory socketFactory = getSocketFactory();
- return mUrlConnectionFactory.openConnection(url, socketFactory, proxy);
+ return urlConnectionFactory.openConnection(url, socketFactory, proxy);
}
/**
diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java
index ae5c24e..64e9350 100644
--- a/core/java/android/net/NetworkCapabilities.java
+++ b/core/java/android/net/NetworkCapabilities.java
@@ -412,6 +412,20 @@
| (1 << NET_CAPABILITY_PARTIAL_CONNECTIVITY);
/**
+ * Capabilities that are allowed for test networks. This list must be set so that it is safe
+ * for an unprivileged user to create a network with these capabilities via shell. As such,
+ * it must never contain capabilities that are generally useful to the system, such as
+ * INTERNET, IMS, SUPL, etc.
+ */
+ private static final long TEST_NETWORKS_ALLOWED_CAPABILITIES =
+ (1 << NET_CAPABILITY_NOT_METERED)
+ | (1 << NET_CAPABILITY_NOT_RESTRICTED)
+ | (1 << NET_CAPABILITY_NOT_VPN)
+ | (1 << NET_CAPABILITY_NOT_ROAMING)
+ | (1 << NET_CAPABILITY_NOT_CONGESTED)
+ | (1 << NET_CAPABILITY_NOT_SUSPENDED);
+
+ /**
* Adds the given capability to this {@code NetworkCapability} instance.
* Multiple capabilities may be applied sequentially. Note that when searching
* for a network to satisfy a request, all capabilities requested must be satisfied.
@@ -641,6 +655,21 @@
}
/**
+ * Test networks have strong restrictions on what capabilities they can have. Enforce these
+ * restrictions.
+ * @hide
+ */
+ public void restrictCapabilitesForTestNetwork() {
+ final long originalCapabilities = mNetworkCapabilities;
+ final NetworkSpecifier originalSpecifier = mNetworkSpecifier;
+ clearAll();
+ // Reset the transports to only contain TRANSPORT_TEST.
+ mTransportTypes = (1 << TRANSPORT_TEST);
+ mNetworkCapabilities = originalCapabilities & TEST_NETWORKS_ALLOWED_CAPABILITIES;
+ mNetworkSpecifier = originalSpecifier;
+ }
+
+ /**
* Representing the transport type. Apps should generally not care about transport. A
* request for a fast internet connection could be satisfied by a number of different
* transports. If any are specified here it will be satisfied a Network that matches
diff --git a/core/java/android/net/RouteInfo.java b/core/java/android/net/RouteInfo.java
index fec2df4..dbdaa4c 100644
--- a/core/java/android/net/RouteInfo.java
+++ b/core/java/android/net/RouteInfo.java
@@ -26,6 +26,7 @@
import android.os.Build;
import android.os.Parcel;
import android.os.Parcelable;
+import android.util.Pair;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -527,23 +528,27 @@
}
/**
- * Compares this RouteInfo object against the specified object and indicates if the
- * destinations of both routes are equal.
- * @return {@code true} if the route destinations are equal, {@code false} otherwise.
+ * A helper class that contains the destination and the gateway in a {@code RouteInfo},
+ * used by {@link ConnectivityService#updateRoutes} or
+ * {@link LinkProperties#addRoute} to calculate the list to be updated.
*
* @hide
*/
- public boolean isSameDestinationAs(@Nullable Object obj) {
- if (this == obj) return true;
-
- if (!(obj instanceof RouteInfo)) return false;
-
- RouteInfo target = (RouteInfo) obj;
-
- if (Objects.equals(mDestination, target.getDestination())) {
- return true;
+ public static class RouteKey extends Pair<IpPrefix, InetAddress> {
+ RouteKey(@NonNull IpPrefix destination, @Nullable InetAddress gateway) {
+ super(destination, gateway);
}
- return false;
+ }
+
+ /**
+ * Get {@code RouteKey} of this {@code RouteInfo}.
+ * @return a {@code RouteKey} object.
+ *
+ * @hide
+ */
+ @NonNull
+ public RouteKey getRouteKey() {
+ return new RouteKey(mDestination, mGateway);
}
/**
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 1ec4bc3..1309740 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -40,6 +40,7 @@
import static android.net.NetworkCapabilities.NET_CAPABILITY_PARTIAL_CONNECTIVITY;
import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED;
import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
+import static android.net.NetworkCapabilities.TRANSPORT_TEST;
import static android.net.NetworkCapabilities.TRANSPORT_VPN;
import static android.net.NetworkPolicyManager.RULE_NONE;
import static android.net.NetworkPolicyManager.uidRulesToString;
@@ -52,6 +53,7 @@
import static java.util.Map.Entry;
+import android.Manifest;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.AppOpsManager;
@@ -236,7 +238,6 @@
import java.util.StringJoiner;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicInteger;
-import java.util.function.Function;
/**
* @hide
@@ -2696,10 +2697,18 @@
switch (msg.what) {
case NetworkAgent.EVENT_NETWORK_CAPABILITIES_CHANGED: {
- final NetworkCapabilities networkCapabilities = (NetworkCapabilities) msg.obj;
+ NetworkCapabilities networkCapabilities = (NetworkCapabilities) msg.obj;
if (networkCapabilities.hasConnectivityManagedCapability()) {
Slog.wtf(TAG, "BUG: " + nai + " has CS-managed capability.");
}
+ if (networkCapabilities.hasTransport(TRANSPORT_TEST)) {
+ // Make sure the original object is not mutated. NetworkAgent normally
+ // makes a copy of the capabilities when sending the message through
+ // the Messenger, but if this ever changes, not making a defensive copy
+ // here will give attack vectors to clients using this code path.
+ networkCapabilities = new NetworkCapabilities(networkCapabilities);
+ networkCapabilities.restrictCapabilitesForTestNetwork();
+ }
updateCapabilities(nai.getCurrentScore(), nai, networkCapabilities);
break;
}
@@ -5772,7 +5781,16 @@
public Network registerNetworkAgent(Messenger messenger, NetworkInfo networkInfo,
LinkProperties linkProperties, NetworkCapabilities networkCapabilities,
int currentScore, NetworkAgentConfig networkAgentConfig, int providerId) {
- enforceNetworkFactoryPermission();
+ if (networkCapabilities.hasTransport(TRANSPORT_TEST)) {
+ enforceAnyPermissionOf(Manifest.permission.MANAGE_TEST_NETWORKS);
+ // Strictly, sanitizing here is unnecessary as the capabilities will be sanitized in
+ // the call to mixInCapabilities below anyway, but sanitizing here means the NAI never
+ // sees capabilities that may be malicious, which might prevent mistakes in the future.
+ networkCapabilities = new NetworkCapabilities(networkCapabilities);
+ networkCapabilities.restrictCapabilitesForTestNetwork();
+ } else {
+ enforceNetworkFactoryPermission();
+ }
LinkProperties lp = new LinkProperties(linkProperties);
lp.ensureDirectlyConnectedRoutes();
@@ -5982,12 +6000,12 @@
* @return true if routes changed between oldLp and newLp
*/
private boolean updateRoutes(LinkProperties newLp, LinkProperties oldLp, int netId) {
- Function<RouteInfo, IpPrefix> getDestination = (r) -> r.getDestination();
// compare the route diff to determine which routes have been updated
- CompareOrUpdateResult<IpPrefix, RouteInfo> routeDiff = new CompareOrUpdateResult<>(
- oldLp != null ? oldLp.getAllRoutes() : null,
- newLp != null ? newLp.getAllRoutes() : null,
- getDestination);
+ final CompareOrUpdateResult<RouteInfo.RouteKey, RouteInfo> routeDiff =
+ new CompareOrUpdateResult<>(
+ oldLp != null ? oldLp.getAllRoutes() : null,
+ newLp != null ? newLp.getAllRoutes() : null,
+ (r) -> r.getRouteKey());
// add routes before removing old in case it helps with continuous connectivity
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 0e592d9..83399b8 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -429,7 +429,6 @@
public Object getSystemService(String name) {
if (Context.CONNECTIVITY_SERVICE.equals(name)) return mCm;
if (Context.NOTIFICATION_SERVICE.equals(name)) return mNotificationManager;
- if (Context.NETWORK_STACK_SERVICE.equals(name)) return mNetworkStack;
if (Context.USER_SERVICE.equals(name)) return mUserManager;
if (Context.ALARM_SERVICE.equals(name)) return mAlarmManager;
if (Context.LOCATION_SERVICE.equals(name)) return mLocationManager;
diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
index 71b72b8..23098ec 100644
--- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
+++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
@@ -46,6 +46,7 @@
import android.net.Network;
import android.net.NetworkUtils;
import android.os.Binder;
+import android.os.INetworkManagementService;
import android.os.ParcelFileDescriptor;
import android.system.Os;
import android.test.mock.MockContext;
@@ -135,6 +136,7 @@
};
INetd mMockNetd;
+ INetworkManagementService mNetworkManager;
PackageManager mMockPkgMgr;
IpSecService.IpSecServiceConfiguration mMockIpSecSrvConfig;
IpSecService mIpSecService;
@@ -160,9 +162,10 @@
@Before
public void setUp() throws Exception {
mMockNetd = mock(INetd.class);
+ mNetworkManager = mock(INetworkManagementService.class);
mMockPkgMgr = mock(PackageManager.class);
mMockIpSecSrvConfig = mock(IpSecService.IpSecServiceConfiguration.class);
- mIpSecService = new IpSecService(mMockContext, mMockIpSecSrvConfig);
+ mIpSecService = new IpSecService(mMockContext, mNetworkManager, mMockIpSecSrvConfig);
// Injecting mock netd
when(mMockIpSecSrvConfig.getNetdInstance()).thenReturn(mMockNetd);
@@ -609,6 +612,7 @@
anyInt(),
anyInt(),
anyInt());
+ verify(mNetworkManager).setInterfaceUp(createTunnelResp.interfaceName);
}
@Test
diff --git a/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java b/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java
index 22a2c94..788e4ef 100644
--- a/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java
+++ b/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java
@@ -31,6 +31,7 @@
import android.content.Context;
import android.os.Binder;
import android.os.IBinder;
+import android.os.INetworkManagementService;
import android.os.RemoteException;
import androidx.test.filters.SmallTest;
@@ -61,7 +62,8 @@
public void setUp() throws Exception {
mMockContext = mock(Context.class);
mMockIpSecSrvConfig = mock(IpSecService.IpSecServiceConfiguration.class);
- mIpSecService = new IpSecService(mMockContext, mMockIpSecSrvConfig);
+ mIpSecService = new IpSecService(
+ mMockContext, mock(INetworkManagementService.class), mMockIpSecSrvConfig);
}
private void assertResourceState(
diff --git a/tests/net/java/com/android/server/IpSecServiceTest.java b/tests/net/java/com/android/server/IpSecServiceTest.java
index 4a35015..536e983 100644
--- a/tests/net/java/com/android/server/IpSecServiceTest.java
+++ b/tests/net/java/com/android/server/IpSecServiceTest.java
@@ -42,6 +42,7 @@
import android.net.IpSecSpiResponse;
import android.net.IpSecUdpEncapResponse;
import android.os.Binder;
+import android.os.INetworkManagementService;
import android.os.ParcelFileDescriptor;
import android.os.Process;
import android.system.ErrnoException;
@@ -115,6 +116,7 @@
}
Context mMockContext;
+ INetworkManagementService mMockNetworkManager;
INetd mMockNetd;
IpSecService.IpSecServiceConfiguration mMockIpSecSrvConfig;
IpSecService mIpSecService;
@@ -122,9 +124,10 @@
@Before
public void setUp() throws Exception {
mMockContext = mock(Context.class);
+ mMockNetworkManager = mock(INetworkManagementService.class);
mMockNetd = mock(INetd.class);
mMockIpSecSrvConfig = mock(IpSecService.IpSecServiceConfiguration.class);
- mIpSecService = new IpSecService(mMockContext, mMockIpSecSrvConfig);
+ mIpSecService = new IpSecService(mMockContext, mMockNetworkManager, mMockIpSecSrvConfig);
// Injecting mock netd
when(mMockIpSecSrvConfig.getNetdInstance()).thenReturn(mMockNetd);
@@ -132,7 +135,7 @@
@Test
public void testIpSecServiceCreate() throws InterruptedException {
- IpSecService ipSecSrv = IpSecService.create(mMockContext);
+ IpSecService ipSecSrv = IpSecService.create(mMockContext, mMockNetworkManager);
assertNotNull(ipSecSrv);
}
@@ -604,8 +607,8 @@
@Test
public void testOpenUdpEncapSocketTagsSocket() throws Exception {
IpSecService.UidFdTagger mockTagger = mock(IpSecService.UidFdTagger.class);
- IpSecService testIpSecService =
- new IpSecService(mMockContext, mMockIpSecSrvConfig, mockTagger);
+ IpSecService testIpSecService = new IpSecService(
+ mMockContext, mMockNetworkManager, mMockIpSecSrvConfig, mockTagger);
IpSecUdpEncapResponse udpEncapResp =
testIpSecService.openUdpEncapsulationSocket(0, new Binder());