Merge "Validate or set the eth specifier on update Iface" am: 85694dc277

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2128511

Change-Id: I5ebaff5ea668a2d23ab27a3200119a99e04b1eaf
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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);