Allow all device types to call updateConfiguration am: f95a298286 am: bbca93ad0f am: 258107eba3

Original change: https://android-review.googlesource.com/c/platform/frameworks/opt/net/ethernet/+/2013099

Change-Id: I45b96406319f6bfcbd185253e142e100036ac189

2 files changed, 59 insertions, 34 deletions

diff --git a/java/com/android/server/ethernet/EthernetServiceImpl.java b/java/com/android/server/ethernet/EthernetServiceImpl.java
index 9987b3e..782ee0f 100644
--- a/java/com/android/server/ethernet/EthernetServiceImpl.java
+++ b/java/com/android/server/ethernet/EthernetServiceImpl.java
@@ -215,45 +215,34 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
                 "EthernetServiceImpl");
     }
 
-    /**
-     * Validate the state of ethernet for APIs tied to network management.
-     *
-     * @param iface the ethernet interface name to operate on.
-     * @param methodName the name of the calling method.
-     */
-    private void validateNetworkManagementState(@NonNull final String iface,
-            final @NonNull String methodName) {
-        Objects.requireNonNull(iface, "Pass a non-null iface.");
-        Objects.requireNonNull(methodName, "Pass a non-null methodName.");
-
-        // 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;
+        // For test capabilities, only null or capabilities that include TRANSPORT_TEST are allowed.
+        if (nc != null && !nc.hasTransport(TRANSPORT_TEST)) {
+            throw new IllegalArgumentException(
+                    "Updates to test interfaces must have NetworkCapabilities.TRANSPORT_TEST.");
         }
-        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) {
-        validateNetworkManagementState(iface, "updateConfiguration()");
+        Objects.requireNonNull(iface);
+        Objects.requireNonNull(request);
+        // TODO: rename to throwIfEthernetNotStarted.
+        logIfEthernetNotStarted();
+
         if (mTracker.isValidTestInterface(iface)) {
+            enforceManageTestNetworksPermission();
             validateTestCapabilities(request.getNetworkCapabilities());
             // TODO: use NetworkCapabilities#restrictCapabilitiesForTestNetwork when available on a
             //  local NetworkCapabilities copy to pass to mTracker.updateConfiguration.
+        } else {
+            enforceNetworkManagementPermission();
+            if (request.getNetworkCapabilities() != null) {
+                // only automotive devices are allowed to set the NetworkCapabilities using this API
+                enforceAutomotiveDevice("updateConfiguration() with non-null capabilities");
+            }
         }
         // TODO: validate that iface is listed in overlay config_ethernet_interfaces
 
@@ -265,7 +254,17 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
     public void connectNetwork(@NonNull final String iface,
             @Nullable final IEthernetNetworkManagementListener listener) {
         Log.i(TAG, "connectNetwork called with: iface=" + iface + ", listener=" + listener);
-        validateNetworkManagementState(iface, "connectNetwork()");
+        Objects.requireNonNull(iface);
+        logIfEthernetNotStarted();
+
+        if (mTracker.isValidTestInterface(iface)) {
+            enforceManageTestNetworksPermission();
+        } else {
+            // only automotive devices are allowed to use this API.
+            enforceNetworkManagementPermission();
+            enforceAutomotiveDevice("connectNetwork()");
+        }
+
         mTracker.connectNetwork(iface, listener);
     }
 
@@ -273,7 +272,17 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
     public void disconnectNetwork(@NonNull final String iface,
             @Nullable final IEthernetNetworkManagementListener listener) {
         Log.i(TAG, "disconnectNetwork called with: iface=" + iface + ", listener=" + listener);
-        validateNetworkManagementState(iface, "disconnectNetwork()");
+        Objects.requireNonNull(iface);
+        logIfEthernetNotStarted();
+
+        if (mTracker.isValidTestInterface(iface)) {
+            enforceManageTestNetworksPermission();
+        } else {
+            // only automotive devices are allowed to use this API.
+            enforceNetworkManagementPermission();
+            enforceAutomotiveDevice("disconnectNetwork()");
+        }
+
         mTracker.disconnectNetwork(iface, listener);
     }
 }
diff --git a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
index e814c84..175a97d 100644
--- a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
+++ b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertThrows;
 
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.verify;
@@ -55,6 +56,10 @@ public class EthernetServiceImplTest {
                     .setIpConfiguration(new IpConfiguration())
                     .setNetworkCapabilities(new NetworkCapabilities.Builder().build())
                     .build();
+    private static final EthernetNetworkUpdateRequest UPDATE_REQUEST_WITHOUT_CAPABILITIES =
+            new EthernetNetworkUpdateRequest.Builder()
+                    .setIpConfiguration(new IpConfiguration())
+                    .build();
     private static final IEthernetNetworkManagementListener NULL_LISTENER = null;
     private EthernetServiceImpl mEthernetServiceImpl;
     @Mock private Context mContext;
@@ -136,7 +141,7 @@ public class EthernetServiceImplTest {
     }
 
     @Test
-    public void testUpdateConfigurationRejectsWithoutAutomotiveFeature() {
+    public void testUpdateConfigurationWithCapabilitiesRejectsWithoutAutomotiveFeature() {
         toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
             mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER);
@@ -144,6 +149,16 @@ public class EthernetServiceImplTest {
     }
 
     @Test
+    public void testUpdateConfigurationWithCapabilitiesWithAutomotiveFeature() {
+        toggleAutomotiveFeature(false);
+        mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST_WITHOUT_CAPABILITIES,
+                NULL_LISTENER);
+        verify(mEthernetTracker).updateConfiguration(eq(TEST_IFACE),
+                eq(UPDATE_REQUEST_WITHOUT_CAPABILITIES.getIpConfiguration()),
+                eq(UPDATE_REQUEST_WITHOUT_CAPABILITIES.getNetworkCapabilities()), isNull());
+    }
+
+    @Test
     public void testConnectNetworkRejectsWithoutAutomotiveFeature() {
         toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
@@ -248,15 +263,16 @@ public class EthernetServiceImplTest {
     }
 
     @Test
-    public void testUpdateConfigurationRejectsTestRequestWithNullCapabilities() {
+    public void testUpdateConfigurationAcceptsTestRequestWithNullCapabilities() {
         enableTestInterface();
         final EthernetNetworkUpdateRequest request =
                 new EthernetNetworkUpdateRequest
                         .Builder()
                         .setIpConfiguration(new IpConfiguration()).build();
-        assertThrows(IllegalArgumentException.class, () -> {
-            mEthernetServiceImpl.updateConfiguration(TEST_IFACE, request, NULL_LISTENER);
-        });
+        mEthernetServiceImpl.updateConfiguration(TEST_IFACE, request, NULL_LISTENER);
+        verify(mEthernetTracker).updateConfiguration(eq(TEST_IFACE),
+                eq(request.getIpConfiguration()),
+                eq(request.getNetworkCapabilities()), isNull());
     }
 
     @Test