From 88cef1a2425873c9fcb90bcc4acbba3ef4582888 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Tue, 24 Mar 2020 19:00:09 +0800 Subject: [PATCH 1/3] Fix NetworkCapabilitiesTest fail on Q device The getSSID() has changed to getSsid() in Android R, adding isAtLeastR() to prevent NetworkCapabilitiesTest fail on Android Q. Bug: 151322799 Test: Run "atest CtsNetTestCasesLatestSdk:NetworkCapabilitiesTest" on Android Q & R device. Change-Id: I602ae32dae1ad29fe3293c541fa6d2cef01b81d3 --- .../java/android/net/NetworkCapabilitiesTest.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index b2e8c378d9..916c339811 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -463,7 +463,9 @@ public class NetworkCapabilitiesTest { nc1.setSSID(TEST_SSID); nc2.combineCapabilities(nc1); - assertTrue(TEST_SSID.equals(nc2.getSsid())); + if (isAtLeastR()) { + assertTrue(TEST_SSID.equals(nc2.getSsid())); + } // Because they now have the same SSID, the following call should not throw nc2.combineCapabilities(nc1); @@ -601,12 +603,16 @@ public class NetworkCapabilitiesTest { // from nc2. assertFalse(nc2.hasCapability(NET_CAPABILITY_NOT_ROAMING)); assertTrue(nc2.hasUnwantedCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(TEST_SSID.equals(nc2.getSsid())); + if (isAtLeastR()) { + assertTrue(TEST_SSID.equals(nc2.getSsid())); + } nc1.setSSID(DIFFERENT_TEST_SSID); nc2.set(nc1); assertEquals(nc1, nc2); - assertTrue(DIFFERENT_TEST_SSID.equals(nc2.getSsid())); + if (isAtLeastR()) { + assertTrue(DIFFERENT_TEST_SSID.equals(nc2.getSsid())); + } nc1.setUids(uidRange(10, 13)); nc2.set(nc1); // Overwrites, as opposed to combineCapabilities From 4b1aada0aa3340877cd276b02a5fffb566a67d45 Mon Sep 17 00:00:00 2001 From: Aaron Huang Date: Wed, 25 Mar 2020 13:36:38 +0800 Subject: [PATCH 2/3] API review: make exception class and Builder final - InvalidPacketException exception class should be final - NetworkCapabilities.Builder should be final Bug: 152203926 Test: atest FrameworksNetTests Change-Id: If9b799151aff6d41c9bcd8bb86c65a58e46bad73 --- core/java/android/net/InvalidPacketException.java | 2 +- core/java/android/net/NetworkCapabilities.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/InvalidPacketException.java b/core/java/android/net/InvalidPacketException.java index b3b0f11a77..1873d778c0 100644 --- a/core/java/android/net/InvalidPacketException.java +++ b/core/java/android/net/InvalidPacketException.java @@ -27,7 +27,7 @@ import java.lang.annotation.RetentionPolicy; * @hide */ @SystemApi -public class InvalidPacketException extends Exception { +public final class InvalidPacketException extends Exception { private final int mError; // Must match SocketKeepalive#ERROR_INVALID_IP_ADDRESS. diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index fcfcebdf08..05d7860ff8 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -2000,7 +2000,7 @@ public final class NetworkCapabilities implements Parcelable { */ @SystemApi @TestApi - public static class Builder { + public static final class Builder { private final NetworkCapabilities mCaps; /** From d22cb127b974d698b3a2220f9528e065fb082b21 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 25 Mar 2020 10:33:55 +0000 Subject: [PATCH 3/3] Allow MANAGE_TEST_NETWORKS to register limited agents This puts in force some restrictions against test networks, and in exchange relaxes the restrictions around registering a network agent that provides a test network. Test networks can only ever have transport TEST, and have only a few capabilities available to them. This is useful in particular to test CTS. See aosp/1253423 for first, basic usage of this capability. Test: IpSecManagerTunnelTest Test: new CTS aosp/1253423 Bug: 139268426 Change-Id: Ibd162792a7ab02fcbb06130f21a825a386678c05 (cherry picked from commit 2c129e97cca2234ee6dd079a9c07df0c530d8b36) --- .../java/android/net/NetworkCapabilities.java | 29 +++++++++++++++++++ .../android/server/ConnectivityService.java | 23 +++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 05d7860ff8..af9414c566 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -414,6 +414,20 @@ public final class NetworkCapabilities implements Parcelable { | (1 << NET_CAPABILITY_FOREGROUND) | (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. * Note that when searching for a network to satisfy a request, all capabilities @@ -645,6 +659,21 @@ public final class NetworkCapabilities implements Parcelable { } } + /** + * 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 diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ec84ae73d6..76a8e1474a 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_NOT_VPN; 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; @@ -50,6 +51,7 @@ import static android.system.OsConstants.IPPROTO_UDP; import static java.util.Map.Entry; +import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; import android.app.AppOpsManager; @@ -2702,10 +2704,18 @@ public class ConnectivityService extends IConnectivityManager.Stub 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; } @@ -5778,7 +5788,16 @@ public class ConnectivityService extends IConnectivityManager.Stub 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();