From 105962ed147ce8c3a842769d17bd875b743e957e Mon Sep 17 00:00:00 2001 From: James Mattis Date: Mon, 25 Apr 2022 16:11:07 -0700 Subject: [PATCH 1/2] Updating Eth CTS to account for existing ifaces Updating EthernetManagerTest to account for existing ethernet interfaces. Specifically, testCallbacks() assumed the test interface created would be the only ethernet interface on the device. This would cause the test to fail if other ethernet interfaces existed. Bug: 230305098 Test: atest CtsNetTestCases:android.net.cts.EthernetManagerTest Change-Id: I675c4ef5c2bd96d01a057bdfec1794d6bfcf82bb --- .../android/net/cts/EthernetManagerTest.kt | 102 +++++++++++------- 1 file changed, 65 insertions(+), 37 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt index 04434e545c..b12c4db74f 100644 --- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt +++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt @@ -15,49 +15,51 @@ */ package android.net.cts +import android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS import android.Manifest.permission.MANAGE_TEST_NETWORKS import android.Manifest.permission.NETWORK_SETTINGS +import android.content.Context import android.net.InetAddresses import android.net.IpConfiguration import android.net.MacAddress import android.net.TestNetworkInterface import android.net.TestNetworkManager -import android.platform.test.annotations.AppModeFull -import androidx.test.platform.app.InstrumentationRegistry -import androidx.test.runner.AndroidJUnit4 -import com.android.net.module.util.ArrayTrackRecord -import com.android.net.module.util.TrackRecord -import com.android.testutils.DevSdkIgnoreRule -import com.android.testutils.SC_V2 -import com.android.testutils.runAsShell -import org.junit.After -import org.junit.Before -import org.junit.Rule -import org.junit.Test -import android.content.Context -import org.junit.runner.RunWith -import kotlin.test.assertNull -import kotlin.test.fail import android.net.cts.EthernetManagerTest.EthernetStateListener.CallbackEntry.InterfaceStateChanged import android.os.Handler import android.os.HandlerExecutor import android.os.Looper +import android.platform.test.annotations.AppModeFull +import android.util.ArraySet +import androidx.test.platform.app.InstrumentationRegistry +import androidx.test.runner.AndroidJUnit4 +import com.android.net.module.util.ArrayTrackRecord +import com.android.net.module.util.TrackRecord +import com.android.networkstack.apishim.EthernetManagerShimImpl import com.android.networkstack.apishim.common.EthernetManagerShim.InterfaceStateListener +import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_CLIENT +import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_NONE import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_ABSENT import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_LINK_DOWN import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_LINK_UP -import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_CLIENT -import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_NONE -import com.android.networkstack.apishim.EthernetManagerShimImpl +import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.RouterAdvertisementResponder +import com.android.testutils.SC_V2 import com.android.testutils.TapPacketReader +import com.android.testutils.runAsShell import com.android.testutils.waitForIdle +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith import java.net.Inet6Address -import java.util.concurrent.Executor -import kotlin.test.assertFalse -import kotlin.test.assertEquals -import kotlin.test.assertTrue import java.net.NetworkInterface +import java.util.concurrent.Executor +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue +import kotlin.test.fail private const val TIMEOUT_MS = 1000L private const val NO_CALLBACK_TIMEOUT_MS = 200L @@ -141,10 +143,13 @@ class EthernetManagerTest { } fun expectCallback(iface: EthernetTestInterface, state: Int, role: Int) { - expectCallback(InterfaceStateChanged(iface.interfaceName, state, role, - if (state != STATE_ABSENT) DEFAULT_IP_CONFIGURATION else null)) + expectCallback(createChangeEvent(iface, state, role)) } + fun createChangeEvent(iface: EthernetTestInterface, state: Int, role: Int) = + InterfaceStateChanged(iface.interfaceName, state, role, + if (state != STATE_ABSENT) DEFAULT_IP_CONFIGURATION else null) + fun pollForNextCallback(): CallbackEntry { return events.poll(TIMEOUT_MS) ?: fail("Did not receive callback after ${TIMEOUT_MS}ms") } @@ -172,7 +177,9 @@ class EthernetManagerTest { } private fun addInterfaceStateListener(executor: Executor, listener: EthernetStateListener) { - em.addInterfaceStateListener(executor, listener) + runAsShell(CONNECTIVITY_USE_RESTRICTED_NETWORKS) { + em.addInterfaceStateListener(executor, listener) + } addedListeners.add(listener) } @@ -195,28 +202,27 @@ class EthernetManagerTest { } @Test - public fun testCallbacks() { + fun testCallbacks() { val executor = HandlerExecutor(Handler(Looper.getMainLooper())) // If an interface exists when the callback is registered, it is reported on registration. val iface = createInterface() - val listener = EthernetStateListener() - addInterfaceStateListener(executor, listener) - listener.expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT) + val listener1 = EthernetStateListener() + addInterfaceStateListener(executor, listener1) + validateListenerOnRegistration(listener1) // If an interface appears, existing callbacks see it. // TODO: fix the up/up/down/up callbacks and only send down/up. val iface2 = createInterface() - listener.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) - listener.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) - listener.expectCallback(iface2, STATE_LINK_DOWN, ROLE_CLIENT) - listener.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + listener1.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + listener1.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + listener1.expectCallback(iface2, STATE_LINK_DOWN, ROLE_CLIENT) + listener1.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) // Register a new listener, it should see state of all existing interfaces immediately. val listener2 = EthernetStateListener() addInterfaceStateListener(executor, listener2) - listener2.expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT) - listener2.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + validateListenerOnRegistration(listener2) // Removing interfaces first sends link down, then STATE_ABSENT/ROLE_NONE. removeInterface(iface) @@ -233,8 +239,30 @@ class EthernetManagerTest { } } + /** + * Validate all interfaces are returned for an EthernetStateListener upon registration. + */ + private fun validateListenerOnRegistration(listener: EthernetStateListener) { + // Get all tracked interfaces to validate on listener registration. Ordering and interface + // state (up/down) can't be validated for interfaces not created as part of testing. + val ifaces = em.getInterfaceList() + val polledIfaces = ArraySet() + for (i in ifaces) { + val event = (listener.pollForNextCallback() as InterfaceStateChanged) + val iface = event.iface + assertTrue(polledIfaces.add(iface), "Duplicate interface $iface returned") + assertTrue(ifaces.contains(iface), "Untracked interface $iface returned") + // If the event's iface was created in the test, additional criteria can be validated. + createdIfaces.find { it.interfaceName.equals(iface) }?.let { + assertEquals(event, listener.createChangeEvent(it, STATE_LINK_UP, ROLE_CLIENT)) + } + } + // Assert all callbacks are accounted for. + listener.assertNoCallback() + } + @Test - public fun testGetInterfaceList() { + fun testGetInterfaceList() { setIncludeTestInterfaces(true) // Create two test interfaces and check the return list contains the interface names. From 46491d7546e2238e9530539884326592d511e378 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Thu, 21 Apr 2022 15:31:52 -0700 Subject: [PATCH 2/2] Allow ethernet on automotive to set allowed UIDs Allow ethernet factories on automotive devices to set the allowed UIDs on NetworkCapabilities. Bug: 229419469 Test: atest FrameworksNetTests Change-Id: I03e7cda75f1c530e0d0e4a756330bc9847a96668 --- .../android/server/ConnectivityService.java | 1 + .../server/connectivity/NetworkAgentInfo.java | 16 ++-- .../server/ConnectivityServiceTest.java | 79 +++++++++++++------ 3 files changed, 68 insertions(+), 28 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 02b8e622bb..1d65f21e6e 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -7825,6 +7825,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } nai.declaredCapabilities = new NetworkCapabilities(nc); NetworkAgentInfo.restrictCapabilitiesFromNetworkAgent(nc, nai.creatorUid, + mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE), mCarrierPrivilegeAuthenticator); } diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index 323888ac32..466e7b1e2b 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -19,6 +19,7 @@ package com.android.server.connectivity; import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; +import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static android.net.NetworkCapabilities.transportNamesOf; @@ -1224,20 +1225,22 @@ public class NetworkAgentInfo implements Comparable, NetworkRa * * @param nc the capabilities to sanitize * @param creatorUid the UID of the process creating this network agent + * @param hasAutomotiveFeature true if this device has the automotive feature, false otherwise * @param authenticator the carrier privilege authenticator to check for telephony constraints */ public static void restrictCapabilitiesFromNetworkAgent(@NonNull final NetworkCapabilities nc, - final int creatorUid, @NonNull final CarrierPrivilegeAuthenticator authenticator) { + final int creatorUid, final boolean hasAutomotiveFeature, + @Nullable final CarrierPrivilegeAuthenticator authenticator) { if (nc.hasTransport(TRANSPORT_TEST)) { nc.restrictCapabilitiesForTestNetwork(creatorUid); } - if (!areAllowedUidsAcceptableFromNetworkAgent(nc, authenticator)) { + if (!areAllowedUidsAcceptableFromNetworkAgent(nc, hasAutomotiveFeature, authenticator)) { nc.setAllowedUids(new ArraySet<>()); } } private static boolean areAllowedUidsAcceptableFromNetworkAgent( - @NonNull final NetworkCapabilities nc, + @NonNull final NetworkCapabilities nc, final boolean hasAutomotiveFeature, @Nullable final CarrierPrivilegeAuthenticator carrierPrivilegeAuthenticator) { // NCs without access UIDs are fine. if (!nc.hasAllowedUids()) return true; @@ -1252,6 +1255,11 @@ public class NetworkAgentInfo implements Comparable, NetworkRa // access UIDs if (nc.hasTransport(TRANSPORT_TEST)) return true; + // Factories that make ethernet networks can allow UIDs for automotive devices. + if (nc.hasTransport(TRANSPORT_ETHERNET) && hasAutomotiveFeature) { + return true; + } + // Factories that make cell networks can allow the UID for the carrier service package. // This can only work in T where there is support for CarrierPrivilegeAuthenticator if (null != carrierPrivilegeAuthenticator @@ -1262,8 +1270,6 @@ public class NetworkAgentInfo implements Comparable, NetworkRa return true; } - // TODO : accept Railway callers - return false; } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 52cebeccda..6294bff7cd 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -15518,6 +15518,27 @@ public class ConnectivityServiceTest { } + @Test + public void testAutomotiveEthernetAllowedUids() throws Exception { + mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_GRANTED); + mServiceContext.setPermission(MANAGE_TEST_NETWORKS, PERMISSION_GRANTED); + + // In this test the automotive feature will be enabled. + mockHasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE, true); + + // Simulate a restricted ethernet network. + final NetworkCapabilities.Builder agentNetCaps = new NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_ETHERNET) + .addCapability(NET_CAPABILITY_INTERNET) + .addCapability(NET_CAPABILITY_NOT_SUSPENDED) + .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .removeCapability(NET_CAPABILITY_NOT_RESTRICTED); + + mEthernetNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_ETHERNET, + new LinkProperties(), agentNetCaps.build()); + validateAllowedUids(mEthernetNetworkAgent, TRANSPORT_ETHERNET, agentNetCaps); + } + @Test public void testCbsAllowedUids() throws Exception { mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_GRANTED); @@ -15527,6 +15548,24 @@ public class ConnectivityServiceTest { doReturn(true).when(mCarrierPrivilegeAuthenticator) .hasCarrierPrivilegeForNetworkCapabilities(eq(TEST_PACKAGE_UID), any()); + // Simulate a restricted telephony network. The telephony factory is entitled to set + // the access UID to the service package on any of its restricted networks. + final NetworkCapabilities.Builder agentNetCaps = new NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_CELLULAR) + .addCapability(NET_CAPABILITY_INTERNET) + .addCapability(NET_CAPABILITY_NOT_SUSPENDED) + .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .removeCapability(NET_CAPABILITY_NOT_RESTRICTED) + .setNetworkSpecifier(new TelephonyNetworkSpecifier(1 /* subid */)); + + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, + new LinkProperties(), agentNetCaps.build()); + validateAllowedUids(mCellNetworkAgent, TRANSPORT_CELLULAR, agentNetCaps); + } + + private void validateAllowedUids(final TestNetworkAgentWrapper testAgent, + @NetworkCapabilities.Transport final int transportUnderTest, + final NetworkCapabilities.Builder ncb) throws Exception { final ArraySet serviceUidSet = new ArraySet<>(); serviceUidSet.add(TEST_PACKAGE_UID); final ArraySet nonServiceUidSet = new ArraySet<>(); @@ -15537,40 +15576,34 @@ public class ConnectivityServiceTest { final TestNetworkCallback cb = new TestNetworkCallback(); - // Simulate a restricted telephony network. The telephony factory is entitled to set - // the access UID to the service package on any of its restricted networks. - final NetworkCapabilities.Builder ncb = new NetworkCapabilities.Builder() - .addTransportType(TRANSPORT_CELLULAR) - .addCapability(NET_CAPABILITY_INTERNET) - .addCapability(NET_CAPABILITY_NOT_SUSPENDED) - .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) - .removeCapability(NET_CAPABILITY_NOT_RESTRICTED) - .setNetworkSpecifier(new TelephonyNetworkSpecifier(1 /* subid */)); - + /* Test setting UIDs */ // Cell gets to set the service UID as access UID mCm.requestNetwork(new NetworkRequest.Builder() - .addTransportType(TRANSPORT_CELLULAR) + .addTransportType(transportUnderTest) .removeCapability(NET_CAPABILITY_NOT_RESTRICTED) .build(), cb); - mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, - new LinkProperties(), ncb.build()); - mCellNetworkAgent.connect(true); - cb.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + testAgent.connect(true); + cb.expectAvailableThenValidatedCallbacks(testAgent); ncb.setAllowedUids(serviceUidSet); - mCellNetworkAgent.setNetworkCapabilities(ncb.build(), true /* sendToCS */); + testAgent.setNetworkCapabilities(ncb.build(), true /* sendToCS */); if (SdkLevel.isAtLeastT()) { - cb.expectCapabilitiesThat(mCellNetworkAgent, + cb.expectCapabilitiesThat(testAgent, caps -> caps.getAllowedUids().equals(serviceUidSet)); } else { // S must ignore access UIDs. cb.assertNoCallback(TEST_CALLBACK_TIMEOUT_MS); } + /* Test setting UIDs is rejected when expected */ + if (TRANSPORT_ETHERNET == transportUnderTest) { + mockHasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE, false); + } + // ...but not to some other UID. Rejection sets UIDs to the empty set ncb.setAllowedUids(nonServiceUidSet); - mCellNetworkAgent.setNetworkCapabilities(ncb.build(), true /* sendToCS */); + testAgent.setNetworkCapabilities(ncb.build(), true /* sendToCS */); if (SdkLevel.isAtLeastT()) { - cb.expectCapabilitiesThat(mCellNetworkAgent, + cb.expectCapabilitiesThat(testAgent, caps -> caps.getAllowedUids().isEmpty()); } else { // S must ignore access UIDs. @@ -15579,18 +15612,18 @@ public class ConnectivityServiceTest { // ...and also not to multiple UIDs even including the service UID ncb.setAllowedUids(serviceUidSetPlus); - mCellNetworkAgent.setNetworkCapabilities(ncb.build(), true /* sendToCS */); + testAgent.setNetworkCapabilities(ncb.build(), true /* sendToCS */); cb.assertNoCallback(TEST_CALLBACK_TIMEOUT_MS); - mCellNetworkAgent.disconnect(); - cb.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); + testAgent.disconnect(); + cb.expectCallback(CallbackEntry.LOST, testAgent); mCm.unregisterNetworkCallback(cb); // Must be unset before touching the transports, because remove and add transport types // check the specifier on the builder immediately, contradicting normal builder semantics // TODO : fix the builder ncb.setNetworkSpecifier(null); - ncb.removeTransportType(TRANSPORT_CELLULAR); + ncb.removeTransportType(transportUnderTest); ncb.addTransportType(TRANSPORT_WIFI); // Wifi does not get to set access UID, even to the correct UID mCm.requestNetwork(new NetworkRequest.Builder()