From b0cc8f09a4eb5e4a1c9c3bf073f9ac14dde60744 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Sat, 26 Feb 2022 22:16:46 -0800 Subject: [PATCH] Eth Management APIs to Support TEST Interfaces Updating Ethernet Network Management APIs to allow support for test interfaces when the caller has the MANAGE_TEST_NETWORKS permission, test interfaces are being tracked in ethernet and if updating a network's capabilities, they include the TEST transport. Bug: 210487893 Test: atest EthernetServiceTests atest CtsNetTestCasesLatestSdk :android.net.cts.EthernetManagerTest Change-Id: I0e0bc9632d9b3d5d61f23e74150586f42c0b5bd2 --- .../server/ethernet/EthernetServiceImpl.java | 34 ++++++- .../server/ethernet/EthernetTracker.java | 14 +++ .../ethernet/EthernetServiceImplTest.java | 92 +++++++++++++++++++ .../server/ethernet/EthernetTrackerTest.java | 41 ++++++++- 4 files changed, 176 insertions(+), 5 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java index f80f6a05b8..7f77e5e633 100644 --- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java +++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java @@ -16,6 +16,8 @@ package com.android.server.ethernet; +import static android.net.NetworkCapabilities.TRANSPORT_TEST; + import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; @@ -26,6 +28,7 @@ import android.net.IEthernetNetworkManagementListener; import android.net.ITetheredInterfaceCallback; import android.net.EthernetNetworkUpdateRequest; import android.net.IpConfiguration; +import android.net.NetworkCapabilities; import android.os.Binder; import android.os.Handler; import android.os.RemoteException; @@ -206,6 +209,12 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { "EthernetServiceImpl"); } + private void enforceManageTestNetworksPermission() { + mContext.enforceCallingOrSelfPermission( + android.Manifest.permission.MANAGE_TEST_NETWORKS, + "EthernetServiceImpl"); + } + /** * Validate the state of ethernet for APIs tied to network management. * @@ -217,18 +226,35 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { Objects.requireNonNull(iface, "Pass a non-null iface."); Objects.requireNonNull(methodName, "Pass a non-null methodName."); - enforceAutomotiveDevice(methodName); - enforceNetworkManagementPermission(); + // Only bypass the permission/device checks if this is a valid test interface. + if (mTracker.isValidTestInterface(iface)) { + enforceManageTestNetworksPermission(); + Log.i(TAG, "Ethernet network management API used with test interface " + iface); + } else { + enforceAutomotiveDevice(methodName); + enforceNetworkManagementPermission(); + } logIfEthernetNotStarted(); } + private void validateTestCapabilities(@NonNull final NetworkCapabilities nc) { + if (nc.hasTransport(TRANSPORT_TEST)) { + return; + } + throw new IllegalArgumentException( + "Updates to test interfaces must have NetworkCapabilities.TRANSPORT_TEST."); + } + @Override public void updateConfiguration(@NonNull final String iface, @NonNull final EthernetNetworkUpdateRequest request, @Nullable final IEthernetNetworkManagementListener listener) { - Log.i(TAG, "updateConfiguration called with: iface=" + iface - + ", request=" + request + ", listener=" + listener); validateNetworkManagementState(iface, "updateConfiguration()"); + if (mTracker.isValidTestInterface(iface)) { + validateTestCapabilities(request.getNetworkCapabilities()); + // TODO: use NetworkCapabilities#restrictCapabilitiesForTestNetwork when available on a + // local NetworkCapabilities copy to pass to mTracker.updateConfiguration. + } // TODO: validate that iface is listed in overlay config_ethernet_interfaces mTracker.updateConfiguration( diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index 794b5d1b90..9070a7e090 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -86,6 +86,9 @@ public class EthernetTracker { * if setIncludeTestInterfaces is true, any test interfaces. */ private String mIfaceMatch; + /** + * Track test interfaces if true, don't track otherwise. + */ private boolean mIncludeTestInterfaces = false; /** Mapping between {iface name | mac address} -> {NetworkCapabilities} */ @@ -738,6 +741,17 @@ public class EthernetTracker { Log.d(TAG, "Interface match regexp set to '" + mIfaceMatch + "'"); } + /** + * Validate if a given interface is valid for testing. + * + * @param iface the name of the interface to validate. + * @return {@code true} if test interfaces are enabled and the given {@code iface} has a test + * interface prefix, {@code false} otherwise. + */ + public boolean isValidTestInterface(@NonNull final String iface) { + return mIncludeTestInterfaces && iface.matches(TEST_IFACE_REGEXP); + } + private void postAndWaitForRunnable(Runnable r) { final ConditionVariable cv = new ConditionVariable(); if (mHandler.post(() -> { diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java index e74a5a366c..012f07aca1 100644 --- a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java +++ b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java @@ -16,6 +16,8 @@ package com.android.server.ethernet; +import static android.net.NetworkCapabilities.TRANSPORT_TEST; + import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.anyString; @@ -23,6 +25,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.Manifest; import android.annotation.NonNull; @@ -162,6 +165,12 @@ public class EthernetServiceImplTest { eq(Manifest.permission.MANAGE_ETHERNET_NETWORKS), anyString()); } + private void denyManageTestNetworksPermission() { + doThrow(new SecurityException("")).when(mContext) + .enforceCallingOrSelfPermission( + eq(Manifest.permission.MANAGE_TEST_NETWORKS), anyString()); + } + @Test public void testUpdateConfigurationRejectsWithoutManageEthPermission() { denyManageEthPermission(); @@ -186,6 +195,37 @@ public class EthernetServiceImplTest { }); } + private void enableTestInterface() { + when(mEthernetTracker.isValidTestInterface(eq(TEST_IFACE))).thenReturn(true); + } + + @Test + public void testUpdateConfigurationRejectsTestRequestWithoutTestPermission() { + enableTestInterface(); + denyManageTestNetworksPermission(); + assertThrows(SecurityException.class, () -> { + mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER); + }); + } + + @Test + public void testConnectNetworkRejectsTestRequestWithoutTestPermission() { + enableTestInterface(); + denyManageTestNetworksPermission(); + assertThrows(SecurityException.class, () -> { + mEthernetServiceImpl.connectNetwork(TEST_IFACE, NULL_LISTENER); + }); + } + + @Test + public void testDisconnectNetworkRejectsTestRequestWithoutTestPermission() { + enableTestInterface(); + denyManageTestNetworksPermission(); + assertThrows(SecurityException.class, () -> { + mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER); + }); + } + @Test public void testUpdateConfiguration() { mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER); @@ -206,4 +246,56 @@ public class EthernetServiceImplTest { mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER); verify(mEthernetTracker).disconnectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER)); } + + @Test + public void testUpdateConfigurationRejectsInvalidTestRequest() { + enableTestInterface(); + assertThrows(IllegalArgumentException.class, () -> { + mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER); + }); + } + + private EthernetNetworkUpdateRequest createTestNetworkUpdateRequest() { + final NetworkCapabilities nc = new NetworkCapabilities + .Builder(UPDATE_REQUEST.getNetworkCapabilities()) + .addTransportType(TRANSPORT_TEST).build(); + + return new EthernetNetworkUpdateRequest + .Builder(UPDATE_REQUEST) + .setNetworkCapabilities(nc).build(); + } + + @Test + public void testUpdateConfigurationForTestRequestDoesNotRequireAutoOrEthernetPermission() { + enableTestInterface(); + toggleAutomotiveFeature(false); + denyManageEthPermission(); + final EthernetNetworkUpdateRequest request = createTestNetworkUpdateRequest(); + + mEthernetServiceImpl.updateConfiguration(TEST_IFACE, request, NULL_LISTENER); + verify(mEthernetTracker).updateConfiguration( + eq(TEST_IFACE), + eq(request.getIpConfiguration()), + eq(request.getNetworkCapabilities()), eq(NULL_LISTENER)); + } + + @Test + public void testConnectNetworkForTestRequestDoesNotRequireAutoOrNetPermission() { + enableTestInterface(); + toggleAutomotiveFeature(false); + denyManageEthPermission(); + + mEthernetServiceImpl.connectNetwork(TEST_IFACE, NULL_LISTENER); + verify(mEthernetTracker).connectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER)); + } + + @Test + public void testDisconnectNetworkForTestRequestDoesNotRequireAutoOrNetPermission() { + enableTestInterface(); + toggleAutomotiveFeature(false); + denyManageEthPermission(); + + mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER); + verify(mEthernetTracker).disconnectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER)); + } } diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java index e1a1a8e7e9..d86cc0f60d 100644 --- a/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -16,8 +16,12 @@ package com.android.server.ethernet; +import static android.net.TestNetworkManager.TEST_TAP_PREFIX; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -39,6 +43,7 @@ import android.net.LinkAddress; import android.net.NetworkCapabilities; import android.net.StaticIpConfiguration; import android.os.HandlerThread; +import android.os.RemoteException; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -71,10 +76,11 @@ public class EthernetTrackerTest { @Mock Resources mResources; @Before - public void setUp() { + public void setUp() throws RemoteException { MockitoAnnotations.initMocks(this); initMockResources(); when(mFactory.updateInterfaceLinkState(anyString(), anyBoolean(), any())).thenReturn(false); + when(mNetd.interfaceGetList()).thenReturn(new String[0]); mHandlerThread = new HandlerThread(THREAD_NAME); mHandlerThread.start(); tracker = new EthernetTracker(mContext, mHandlerThread.getThreadHandler(), mFactory, mNetd); @@ -355,4 +361,37 @@ public class EthernetTrackerTest { verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */), eq(NULL_LISTENER)); } + + @Test + public void testIsValidTestInterfaceIsFalseWhenTestInterfacesAreNotIncluded() { + final String validIfaceName = TEST_TAP_PREFIX + "123"; + tracker.setIncludeTestInterfaces(false); + waitForIdle(); + + final boolean isValidTestInterface = tracker.isValidTestInterface(validIfaceName); + + assertFalse(isValidTestInterface); + } + + @Test + public void testIsValidTestInterfaceIsFalseWhenTestInterfaceNameIsInvalid() { + final String invalidIfaceName = "123" + TEST_TAP_PREFIX; + tracker.setIncludeTestInterfaces(true); + waitForIdle(); + + final boolean isValidTestInterface = tracker.isValidTestInterface(invalidIfaceName); + + assertFalse(isValidTestInterface); + } + + @Test + public void testIsValidTestInterfaceIsTrueWhenTestInterfacesIncludedAndValidName() { + final String validIfaceName = TEST_TAP_PREFIX + "123"; + tracker.setIncludeTestInterfaces(true); + waitForIdle(); + + final boolean isValidTestInterface = tracker.isValidTestInterface(validIfaceName); + + assertTrue(isValidTestInterface); + } }