From b0cc8f09a4eb5e4a1c9c3bf073f9ac14dde60744 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Sat, 26 Feb 2022 22:16:46 -0800 Subject: [PATCH 1/2] 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); + } } From 3e12496b6028428c896d1e78ecd838508de42f34 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Thu, 3 Mar 2022 16:19:04 -0800 Subject: [PATCH 2/2] Allowing for null net caps in updateConfiguration Marking NetworkCapabilities as nullable in updateConfiguration and updating where needed to support this. This will allow callers of the ethernet network management updateConfiguration API to use it primarily for setting an ethernet network's IP configuration. Bug: 222565654 Bug: 220017952 Bug: 210485380 Test: atest EthernetServiceTests Change-Id: Ifd908639a00470e599fe1a15487cc6383a56b2f5 --- .../server/ethernet/EthernetNetworkFactory.java | 4 +++- .../android/server/ethernet/EthernetServiceImpl.java | 4 ++-- .../com/android/server/ethernet/EthernetTracker.java | 6 ++++-- .../server/ethernet/EthernetServiceImplTest.java | 12 ++++++++++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java index 8ce27a64b1..875fc102c0 100644 --- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java @@ -485,7 +485,9 @@ public class EthernetNetworkFactory extends NetworkFactory { } mIpConfig = ipConfig; - setCapabilities(capabilities); + if (null != capabilities) { + setCapabilities(capabilities); + } // Send an abort callback if a request is filed before the previous one has completed. maybeSendNetworkManagementCallbackForAbort(); // TODO: Update this logic to only do a restart if required. Although a restart may diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java index 7f77e5e633..9987b3e00c 100644 --- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java +++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java @@ -237,8 +237,8 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { logIfEthernetNotStarted(); } - private void validateTestCapabilities(@NonNull final NetworkCapabilities nc) { - if (nc.hasTransport(TRANSPORT_TEST)) { + private void validateTestCapabilities(@Nullable final NetworkCapabilities nc) { + if (null != nc && nc.hasTransport(TRANSPORT_TEST)) { return; } throw new IllegalArgumentException( diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index 9070a7e090..ea241e1d3f 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -233,7 +233,7 @@ public class EthernetTracker { @VisibleForTesting(visibility = PACKAGE) protected void updateConfiguration(@NonNull final String iface, @NonNull final IpConfiguration ipConfig, - @NonNull final NetworkCapabilities capabilities, + @Nullable final NetworkCapabilities capabilities, @Nullable final IEthernetNetworkManagementListener listener) { if (DBG) { Log.i(TAG, "updateConfiguration, iface: " + iface + ", capabilities: " + capabilities @@ -241,7 +241,9 @@ public class EthernetTracker { } final IpConfiguration localIpConfig = new IpConfiguration(ipConfig); writeIpConfiguration(iface, localIpConfig); - mNetworkCapabilities.put(iface, capabilities); + if (null != capabilities) { + mNetworkCapabilities.put(iface, capabilities); + } mHandler.post(() -> { mFactory.updateInterface(iface, localIpConfig, capabilities, listener); broadcastInterfaceStateChange(iface); diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java index 012f07aca1..e814c84f5b 100644 --- a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java +++ b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java @@ -247,6 +247,18 @@ public class EthernetServiceImplTest { verify(mEthernetTracker).disconnectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER)); } + @Test + public void testUpdateConfigurationRejectsTestRequestWithNullCapabilities() { + enableTestInterface(); + final EthernetNetworkUpdateRequest request = + new EthernetNetworkUpdateRequest + .Builder() + .setIpConfiguration(new IpConfiguration()).build(); + assertThrows(IllegalArgumentException.class, () -> { + mEthernetServiceImpl.updateConfiguration(TEST_IFACE, request, NULL_LISTENER); + }); + } + @Test public void testUpdateConfigurationRejectsInvalidTestRequest() { enableTestInterface();