Merge changes from topic "rway_nullable_nc"

* changes:
  Allowing for null net caps in updateConfiguration
  Eth Management APIs to Support TEST Interfaces

5 files changed, 195 insertions, 8 deletions

diff --git a/java/com/android/server/ethernet/EthernetNetworkFactory.java b/java/com/android/server/ethernet/EthernetNetworkFactory.java
index 8ce27a6..875fc10 100644
--- a/java/com/android/server/ethernet/EthernetNetworkFactory.java
+++ b/java/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/java/com/android/server/ethernet/EthernetServiceImpl.java b/java/com/android/server/ethernet/EthernetServiceImpl.java
index f80f6a0..9987b3e 100644
--- a/java/com/android/server/ethernet/EthernetServiceImpl.java
+++ b/java/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(@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/java/com/android/server/ethernet/EthernetTracker.java b/java/com/android/server/ethernet/EthernetTracker.java
index 794b5d1..ea241e1 100644
--- a/java/com/android/server/ethernet/EthernetTracker.java
+++ b/java/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} */
@@ -230,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
@@ -238,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);
@@ -738,6 +743,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/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
index e74a5a3..e814c84 100644
--- a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
+++ b/tests/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,68 @@ public class EthernetServiceImplTest {
         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/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
index e1a1a8e..d86cc0f 100644
--- a/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
+++ b/tests/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);
+    }
 }