Validate or set the eth specifier on update Iface

When a calling the updateInterface API, if no eth specifier was set it
would be removed not allowing follow-up network requests to request by
ethernet specifier.

This CL makes sure that the eth specifier is always set as expected when
updateInterface is called by either validating the input or setting the
specifier if the passed one is null.

Bug: 236294399
Bug: 229207021
Test: atest FrameworksNetTests
:com.android.server.ethernet.EthernetNetworkFactoryTest

Change-Id: Ic05ca7fe4adaa94f79e59758569f7595ab4c4f54

service-t/src/com/android/server/ethernet/EthernetServiceImpl.java

@@ -16,12 +16,14 @@
 
 package com.android.server.ethernet;
 
+import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET;
 import static android.net.NetworkCapabilities.TRANSPORT_TEST;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.content.Context;
 import android.content.pm.PackageManager;
+import android.net.EthernetNetworkSpecifier;
 import android.net.EthernetNetworkUpdateRequest;
 import android.net.IEthernetManager;
 import android.net.IEthernetServiceListener;
@@ -29,6 +31,7 @@ import android.net.INetworkInterfaceOutcomeReceiver;
 import android.net.ITetheredInterfaceCallback;
 import android.net.IpConfiguration;
 import android.net.NetworkCapabilities;
+import android.net.NetworkSpecifier;
 import android.os.Binder;
 import android.os.Handler;
 import android.os.RemoteException;
@@ -216,19 +219,39 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
                 "EthernetServiceImpl");
     }
 
-    private void maybeValidateTestCapabilities(final String iface,
+    private void validateOrSetNetworkSpecifier(String iface, NetworkCapabilities nc) {
-            @Nullable final NetworkCapabilities nc) {
+        final NetworkSpecifier spec = nc.getNetworkSpecifier();
+        if (spec == null) {
+            nc.setNetworkSpecifier(new EthernetNetworkSpecifier(iface));
+            return;
+        }
+        if (!(spec instanceof EthernetNetworkSpecifier)) {
+            throw new IllegalArgumentException("Invalid specifier type for request.");
+        }
+        if (!((EthernetNetworkSpecifier) spec).getInterfaceName().matches(iface)) {
+            throw new IllegalArgumentException("Invalid interface name set on specifier.");
+        }
+    }
+
+    private void maybeValidateTestCapabilities(String iface, NetworkCapabilities nc) {
         if (!mTracker.isValidTestInterface(iface)) {
             return;
         }
-        // For test interfaces, only null or capabilities that include TRANSPORT_TEST are
+        if (!nc.hasTransport(TRANSPORT_TEST)) {
-        // allowed.
-        if (nc != null && !nc.hasTransport(TRANSPORT_TEST)) {
             throw new IllegalArgumentException(
                     "Updates to test interfaces must have NetworkCapabilities.TRANSPORT_TEST.");
         }
     }
 
+    private void maybeValidateEthernetTransport(String iface, NetworkCapabilities nc) {
+        if (mTracker.isValidTestInterface(iface)) {
+            return;
+        }
+        if (!nc.hasSingleTransport(TRANSPORT_ETHERNET)) {
+            throw new IllegalArgumentException("Invalid transport type for request.");
+        }
+    }
+
     private void enforceAdminPermission(final String iface, boolean enforceAutomotive,
             final String logMessage) {
         if (mTracker.isValidTestInterface(iface)) {
@@ -251,12 +274,17 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
 
         // TODO: validate that iface is listed in overlay config_ethernet_interfaces
         // only automotive devices are allowed to set the NetworkCapabilities using this API
-        enforceAdminPermission(iface, request.getNetworkCapabilities() != null,
+        final NetworkCapabilities nc = request.getNetworkCapabilities();
-                "updateConfiguration() with non-null capabilities");
+        enforceAdminPermission(
-        maybeValidateTestCapabilities(iface, request.getNetworkCapabilities());
+                iface, nc != null, "updateConfiguration() with non-null capabilities");
+        if (nc != null) {
+            validateOrSetNetworkSpecifier(iface, nc);
+            maybeValidateTestCapabilities(iface, nc);
+            maybeValidateEthernetTransport(iface, nc);
+        }
 
         mTracker.updateConfiguration(
-                iface, request.getIpConfiguration(), request.getNetworkCapabilities(), listener);
+                iface, request.getIpConfiguration(), nc, listener);
     }
 
     @Override

tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java

@@ -16,6 +16,7 @@
 
 package com.android.server.ethernet;
 
+import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET;
 import static android.net.NetworkCapabilities.TRANSPORT_TEST;
 
 import static org.junit.Assert.assertThrows;
@@ -35,10 +36,12 @@ import android.Manifest;
 import android.annotation.NonNull;
 import android.content.Context;
 import android.content.pm.PackageManager;
+import android.net.EthernetNetworkSpecifier;
 import android.net.EthernetNetworkUpdateRequest;
 import android.net.INetworkInterfaceOutcomeReceiver;
 import android.net.IpConfiguration;
 import android.net.NetworkCapabilities;
+import android.net.StringNetworkSpecifier;
 import android.os.Build;
 import android.os.Handler;
 
@@ -56,10 +59,14 @@ import org.junit.runner.RunWith;
 @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2)
 public class EthernetServiceImplTest {
     private static final String TEST_IFACE = "test123";
+    private static final NetworkCapabilities DEFAULT_CAPS = new NetworkCapabilities.Builder()
+            .addTransportType(TRANSPORT_ETHERNET)
+            .setNetworkSpecifier(new EthernetNetworkSpecifier(TEST_IFACE))
+            .build();
     private static final EthernetNetworkUpdateRequest UPDATE_REQUEST =
             new EthernetNetworkUpdateRequest.Builder()
                     .setIpConfiguration(new IpConfiguration())
-                    .setNetworkCapabilities(new NetworkCapabilities.Builder().build())
+                    .setNetworkCapabilities(DEFAULT_CAPS)
                     .build();
     private static final EthernetNetworkUpdateRequest UPDATE_REQUEST_WITHOUT_CAPABILITIES =
             new EthernetNetworkUpdateRequest.Builder()
@@ -67,7 +74,7 @@ public class EthernetServiceImplTest {
                     .build();
     private static final EthernetNetworkUpdateRequest UPDATE_REQUEST_WITHOUT_IP_CONFIG =
             new EthernetNetworkUpdateRequest.Builder()
-                    .setNetworkCapabilities(new NetworkCapabilities.Builder().build())
+                    .setNetworkCapabilities(DEFAULT_CAPS)
                     .build();
     private static final INetworkInterfaceOutcomeReceiver NULL_LISTENER = null;
     private EthernetServiceImpl mEthernetServiceImpl;
@@ -160,6 +167,41 @@ public class EthernetServiceImplTest {
         });
     }
 
+    @Test
+    public void testUpdateConfigurationRejectsWithInvalidSpecifierType() {
+        final StringNetworkSpecifier invalidSpecifierType = new StringNetworkSpecifier("123");
+        final EthernetNetworkUpdateRequest request =
+                new EthernetNetworkUpdateRequest.Builder()
+                        .setNetworkCapabilities(
+                                new NetworkCapabilities.Builder()
+                                        .addTransportType(TRANSPORT_ETHERNET)
+                                        .setNetworkSpecifier(invalidSpecifierType)
+                                        .build()
+                        ).build();
+        assertThrows(IllegalArgumentException.class, () -> {
+            mEthernetServiceImpl.updateConfiguration(
+                    "" /* iface */, request, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testUpdateConfigurationRejectsWithInvalidSpecifierName() {
+        final String ifaceToUpdate = "eth0";
+        final String ifaceOnSpecifier = "wlan0";
+        EthernetNetworkUpdateRequest request =
+                new EthernetNetworkUpdateRequest.Builder()
+                        .setNetworkCapabilities(
+                                new NetworkCapabilities.Builder()
+                                        .addTransportType(TRANSPORT_ETHERNET)
+                                        .setNetworkSpecifier(
+                                                new EthernetNetworkSpecifier(ifaceOnSpecifier))
+                                        .build()
+                        ).build();
+        assertThrows(IllegalArgumentException.class, () -> {
+            mEthernetServiceImpl.updateConfiguration(ifaceToUpdate, request, null /* listener */);
+        });
+    }
+
     @Test
     public void testUpdateConfigurationWithCapabilitiesWithAutomotiveFeature() {
         toggleAutomotiveFeature(false);
@@ -246,6 +288,24 @@ public class EthernetServiceImplTest {
                 eq(UPDATE_REQUEST.getNetworkCapabilities()), eq(NULL_LISTENER));
     }
 
+    @Test
+    public void testUpdateConfigurationAddsSpecifierWhenNotSet() {
+        final NetworkCapabilities nc = new NetworkCapabilities.Builder()
+                .addTransportType(TRANSPORT_ETHERNET).build();
+        final EthernetNetworkUpdateRequest requestSansSpecifier =
+                new EthernetNetworkUpdateRequest.Builder()
+                        .setNetworkCapabilities(nc)
+                        .build();
+        final NetworkCapabilities ncWithSpecifier = new NetworkCapabilities(nc)
+                .setNetworkSpecifier(new EthernetNetworkSpecifier(TEST_IFACE));
+
+        mEthernetServiceImpl.updateConfiguration(TEST_IFACE, requestSansSpecifier, NULL_LISTENER);
+        verify(mEthernetTracker).updateConfiguration(
+                eq(TEST_IFACE),
+                isNull(),
+                eq(ncWithSpecifier), eq(NULL_LISTENER));
+    }
+
     @Test
     public void testEnableInterface() {
         mEthernetServiceImpl.enableInterface(TEST_IFACE, NULL_LISTENER);