Eth Service updates to validate net mgmt calls am: cc1dac61b3 am: 24e202722d

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

Change-Id: Ic887b3229960bee4516066d8199a8a03d714f9d6

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

@@ -33,6 +33,7 @@ import android.os.RemoteException;
 import android.util.Log;
 import android.util.PrintWriterPrinter;
 
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.IndentingPrintWriter;
 import com.android.net.module.util.PermissionUtils;
 
@@ -49,7 +50,8 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
     private static final String TAG = "EthernetServiceImpl";
 
     private final Context mContext;
-    private final AtomicBoolean mStarted = new AtomicBoolean(false);
+    @VisibleForTesting
+    final AtomicBoolean mStarted = new AtomicBoolean(false);
 
     private Handler mHandler;
     private EthernetTracker mTracker;
@@ -70,6 +72,17 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
                 "ConnectivityService");
     }
 
+    private void enforceAutomotiveDevice(final @NonNull String methodName) {
+        PermissionUtils.enforceSystemFeature(mContext, PackageManager.FEATURE_AUTOMOTIVE,
+                methodName + " is only available on automotive devices.");
+    }
+
+    private void enforceInterfaceIsTracked(final @NonNull String iface) {
+        if(!mTracker.isTrackingInterface(iface)) {
+            throw new UnsupportedOperationException("The given iface is not currently tracked.");
+        }
+    }
+
     private boolean checkUseRestrictedNetworksPermission() {
         return mContext.checkCallingOrSelfPermission(
                 android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS)
@@ -89,6 +102,12 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
         mStarted.set(true);
     }
 
+    private void logIfEthernetNotStarted() {
+        if (!mStarted.get()) {
+            throw new IllegalStateException("System isn't ready to change ethernet configurations");
+        }
+    }
+
     @Override
     public String[] getAvailableInterfaces() throws RemoteException {
         enforceAccessPermission();
@@ -116,9 +135,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
      */
     @Override
     public void setConfiguration(String iface, IpConfiguration config) {
-        if (!mStarted.get()) {
+        logIfEthernetNotStarted();
-            Log.w(TAG, "System isn't ready enough to change ethernet configuration");
-        }
 
         PermissionUtils.enforceNetworkStackPermission(mContext);
 
@@ -214,23 +231,44 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
         pw.decreaseIndent();
     }
 
+    /**
+     * 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) {
+        logIfEthernetNotStarted();
+
+        // TODO: add permission check here for MANAGE_INTERNAL_NETWORKS when it's available.
+        Objects.requireNonNull(iface, "Pass a non-null iface.");
+        Objects.requireNonNull(methodName, "Pass a non-null methodName.");
+        enforceAutomotiveDevice(methodName);
+        enforceInterfaceIsTracked(iface);
+    }
+
     @Override
     public void updateConfiguration(@NonNull final String iface,
             @NonNull final InternalNetworkUpdateRequest request,
             @Nullable final IInternalNetworkManagementListener listener) {
         Log.i(TAG, "updateConfiguration called with: iface=" + iface
                 + ", request=" + request + ", listener=" + listener);
+        validateNetworkManagementState(iface, "updateConfiguration()");
+        // TODO: validate that iface is listed in overlay config_ethernet_interfaces
     }
 
     @Override
     public void connectNetwork(@NonNull final String iface,
             @Nullable final IInternalNetworkManagementListener listener) {
         Log.i(TAG, "connectNetwork called with: iface=" + iface + ", listener=" + listener);
+        validateNetworkManagementState(iface, "connectNetwork()");
     }
 
     @Override
     public void disconnectNetwork(@NonNull final String iface,
             @Nullable final IInternalNetworkManagementListener listener) {
         Log.i(TAG, "disconnectNetwork called with: iface=" + iface + ", listener=" + listener);
+        validateNetworkManagementState(iface, "disconnectNetwork()");
     }
 }

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

@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.ethernet;
+
+import static org.junit.Assert.assertThrows;
+
+import static org.mockito.Mockito.doReturn;
+
+import android.content.Context;
+import android.content.pm.PackageManager;
+import android.net.InternalNetworkUpdateRequest;
+import android.net.IpConfiguration;
+import android.net.StaticIpConfiguration;
+
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public class EthernetServiceImplTest {
+    private EthernetServiceImpl mEthernetServiceImpl;
+    @Mock private Context mContext;
+    @Mock private PackageManager mPackageManager;
+
+    @Before
+    public void setup() {
+        MockitoAnnotations.initMocks(this);
+        doReturn(mPackageManager).when(mContext).getPackageManager();
+        mEthernetServiceImpl = new EthernetServiceImpl(mContext);
+        mEthernetServiceImpl.mStarted.set(true);
+    }
+
+    @Test
+    public void testSetConfigurationRejectsWhenEthNotStarted() {
+        mEthernetServiceImpl.mStarted.set(false);
+        assertThrows(IllegalStateException.class, () -> {
+            mEthernetServiceImpl.setConfiguration("" /* iface */, new IpConfiguration());
+        });
+    }
+
+    @Test
+    public void testUpdateConfigurationRejectsWhenEthNotStarted() {
+        mEthernetServiceImpl.mStarted.set(false);
+        assertThrows(IllegalStateException.class, () -> {
+            final InternalNetworkUpdateRequest r =
+                    new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null);
+
+            mEthernetServiceImpl.updateConfiguration("" /* iface */, r, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testConnectNetworkRejectsWhenEthNotStarted() {
+        mEthernetServiceImpl.mStarted.set(false);
+        assertThrows(IllegalStateException.class, () -> {
+            mEthernetServiceImpl.connectNetwork("" /* iface */, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testDisconnectNetworkRejectsWhenEthNotStarted() {
+        mEthernetServiceImpl.mStarted.set(false);
+        assertThrows(IllegalStateException.class, () -> {
+            mEthernetServiceImpl.disconnectNetwork("" /* iface */, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testUpdateConfigurationRejectsNullIface() {
+        assertThrows(NullPointerException.class, () -> {
+            final InternalNetworkUpdateRequest r =
+                    new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null);
+
+            mEthernetServiceImpl.updateConfiguration(null /* iface */, r, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testConnectNetworkRejectsNullIface() {
+        assertThrows(NullPointerException.class, () -> {
+            mEthernetServiceImpl.connectNetwork(null /* iface */, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testDisconnectNetworkRejectsNullIface() {
+        assertThrows(NullPointerException.class, () -> {
+            mEthernetServiceImpl.disconnectNetwork(null /* iface */, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testUpdateConfigurationRejectsWithoutAutomotiveFeature() {
+        disableAutomotiveFeature();
+        assertThrows(UnsupportedOperationException.class, () -> {
+            final InternalNetworkUpdateRequest r =
+                    new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null);
+
+            mEthernetServiceImpl.updateConfiguration("" /* iface */, r, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testConnectNetworkRejectsWithoutAutomotiveFeature() {
+        disableAutomotiveFeature();
+        assertThrows(UnsupportedOperationException.class, () -> {
+            mEthernetServiceImpl.connectNetwork("" /* iface */, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testDisconnectNetworkRejectsWithoutAutomotiveFeature() {
+        disableAutomotiveFeature();
+        assertThrows(UnsupportedOperationException.class, () -> {
+            mEthernetServiceImpl.disconnectNetwork("" /* iface */, null /* listener */);
+        });
+    }
+
+    private void disableAutomotiveFeature() {
+        doReturn(false).when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE);
+    }
+}