Merge changes from topic "rway_nullable_nc"
* changes:
Allowing for null net caps in updateConfiguration
Eth Management APIs to Support TEST Interfaces
diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
index 8ce27a6..875fc10 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 @@
}
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 f80f6a0..9987b3e 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.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 @@
"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 @@
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(@Nullable final NetworkCapabilities nc) {
+ if (null != nc && 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 794b5d1..ea241e1 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 @@
* 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} */
@@ -230,7 +233,7 @@
@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
@@ -238,7 +241,9 @@
}
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);
@@ -738,6 +743,17 @@
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 e74a5a3..e814c84 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.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 @@
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 @@
});
}
+ 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,68 @@
mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER);
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();
+ 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 e1a1a8e..d86cc0f 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.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 @@
@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 @@
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);
+ }
}