Clean Up NetworkManagementService Tests

Cleaning up tests, so I can easily add more for restricted networking
mode.
I merged the NetworkManagementInternalTests with the
NetworkManagementServiceTests.

Test: atest NetworkManagementServiceTest
Change-Id: If8c3cc1883cfb2524eeb78e23165fc868130f0e7

services/core/java/com/android/server/NetworkManagementService.java

@@ -88,7 +88,6 @@ import android.util.SparseBooleanArray;
 import android.util.SparseIntArray;
 
 import com.android.internal.annotations.GuardedBy;
-import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.app.IBatteryStats;
 import com.android.internal.util.DumpUtils;
 import com.android.internal.util.FrameworkStatsLog;
@@ -122,7 +121,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
      * Helper class that encapsulates NetworkManagementService dependencies and makes them
      * easier to mock in unit tests.
      */
-    static class SystemServices {
+    static class Dependencies {
         public IBinder getService(String name) {
             return ServiceManager.getService(name);
         }
@@ -132,6 +131,10 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
         public INetd getNetd() {
             return NetdService.get();
         }
+
+        public int getCallingUid() {
+            return Binder.getCallingUid();
+        }
     }
 
     private static final String TAG = "NetworkManagement";
@@ -157,7 +160,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
 
     private final Handler mDaemonHandler;
 
-    private final SystemServices mServices;
+    private final Dependencies mDeps;
 
     private INetd mNetdService;
 
@@ -254,33 +257,32 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
      * @param context  Binder context for this service
      */
     private NetworkManagementService(
-            Context context, SystemServices services) {
+            Context context, Dependencies deps) {
         mContext = context;
-        mServices = services;
+        mDeps = deps;
 
         mDaemonHandler = new Handler(FgThread.get().getLooper());
 
         mNetdUnsolicitedEventListener = new NetdUnsolicitedEventListener();
 
-        mServices.registerLocalService(new LocalService());
+        mDeps.registerLocalService(new LocalService());
 
         synchronized (mTetheringStatsProviders) {
             mTetheringStatsProviders.put(new NetdTetheringStatsProvider(), "netd");
         }
     }
 
-    @VisibleForTesting
+    private NetworkManagementService() {
-    NetworkManagementService() {
         mContext = null;
         mDaemonHandler = null;
-        mServices = null;
+        mDeps = null;
         mNetdUnsolicitedEventListener = null;
     }
 
-    static NetworkManagementService create(Context context, SystemServices services)
+    static NetworkManagementService create(Context context, Dependencies deps)
             throws InterruptedException {
         final NetworkManagementService service =
-                new NetworkManagementService(context, services);
+                new NetworkManagementService(context, deps);
         if (DBG) Slog.d(TAG, "Creating NetworkManagementService");
         if (DBG) Slog.d(TAG, "Connecting native netd service");
         service.connectNativeNetdService();
@@ -289,7 +291,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
     }
 
     public static NetworkManagementService create(Context context) throws InterruptedException {
-        return create(context, new SystemServices());
+        return create(context, new Dependencies());
     }
 
     public void systemReady() {
@@ -310,7 +312,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
                 return mBatteryStats;
             }
             mBatteryStats =
-                    IBatteryStats.Stub.asInterface(mServices.getService(BatteryStats.SERVICE_NAME));
+                    IBatteryStats.Stub.asInterface(mDeps.getService(BatteryStats.SERVICE_NAME));
             return mBatteryStats;
         }
     }
@@ -511,7 +513,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
     }
 
     private void connectNativeNetdService() {
-        mNetdService = mServices.getNetd();
+        mNetdService = mDeps.getNetd();
         try {
             mNetdService.registerUnsolicitedEventListener(mNetdUnsolicitedEventListener);
             if (DBG) Slog.d(TAG, "Register unsolicited event listener");
@@ -1448,7 +1450,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
 
     @Override
     public void setUidCleartextNetworkPolicy(int uid, int policy) {
-        if (Binder.getCallingUid() != uid) {
+        if (mDeps.getCallingUid() != uid) {
             NetworkStack.checkNetworkStackPermission(mContext);
         }
 
@@ -1862,8 +1864,8 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
         return rule;
     }
 
-    private static void enforceSystemUid() {
+    private void enforceSystemUid() {
-        final int uid = Binder.getCallingUid();
+        final int uid = mDeps.getCallingUid();
         if (uid != Process.SYSTEM_UID) {
             throw new SecurityException("Only available to AID_SYSTEM");
         }
@@ -2150,60 +2152,10 @@ public class NetworkManagementService extends INetworkManagementService.Stub {
         }
     }
 
-    @VisibleForTesting
+    private class LocalService extends NetworkManagementInternal {
-    class LocalService extends NetworkManagementInternal {
         @Override
         public boolean isNetworkRestrictedForUid(int uid) {
             return isNetworkRestrictedInternal(uid);
         }
     }
-
-    @VisibleForTesting
-    Injector getInjector() {
-        return new Injector();
-    }
-
-    @VisibleForTesting
-    class Injector {
-        void setDataSaverMode(boolean dataSaverMode) {
-            mDataSaverMode = dataSaverMode;
-        }
-
-        void setFirewallChainState(int chain, boolean state) {
-            NetworkManagementService.this.setFirewallChainState(chain, state);
-        }
-
-        void setFirewallRule(int chain, int uid, int rule) {
-            synchronized (mRulesLock) {
-                getUidFirewallRulesLR(chain).put(uid, rule);
-            }
-        }
-
-        void setUidOnMeteredNetworkList(boolean denylist, int uid, boolean enable) {
-            synchronized (mRulesLock) {
-                if (denylist) {
-                    mUidRejectOnMetered.put(uid, enable);
-                } else {
-                    mUidAllowOnMetered.put(uid, enable);
-                }
-            }
-        }
-
-        void reset() {
-            synchronized (mRulesLock) {
-                setDataSaverMode(false);
-                final int[] chains = {
-                        FIREWALL_CHAIN_DOZABLE,
-                        FIREWALL_CHAIN_STANDBY,
-                        FIREWALL_CHAIN_POWERSAVE
-                };
-                for (int chain : chains) {
-                    setFirewallChainState(chain, false);
-                    getUidFirewallRulesLR(chain).clear();
-                }
-                mUidAllowOnMetered.clear();
-                mUidRejectOnMetered.clear();
-            }
-        }
-    }
 }

services/tests/servicestests/src/com/android/server/NetworkManagementInternalTest.java

@@ -1,144 +0,0 @@
-/*
- * Copyright (C) 2017 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;
-
-import static android.net.INetd.FIREWALL_CHAIN_DOZABLE;
-import static android.net.INetd.FIREWALL_CHAIN_POWERSAVE;
-import static android.net.INetd.FIREWALL_CHAIN_STANDBY;
-import static android.net.INetd.FIREWALL_RULE_ALLOW;
-import static android.net.INetd.FIREWALL_RULE_DENY;
-import static android.net.NetworkPolicyManager.FIREWALL_RULE_DEFAULT;
-import static android.util.DebugUtils.valueToString;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-import android.net.NetworkPolicyManager;
-import android.util.ArrayMap;
-
-import androidx.test.filters.SmallTest;
-import androidx.test.runner.AndroidJUnit4;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-import java.util.function.BiFunction;
-
-/**
- * Test class for {@link NetworkManagementInternal}.
- *
- * To run the tests, use
- *
- * runtest -c com.android.server.NetworkManagementInternalTest frameworks-services
- *
- * or the following steps:
- *
- * Build: m FrameworksServicesTests
- * Install: adb install -r \
- *     ${ANDROID_PRODUCT_OUT}/data/app/FrameworksServicesTests/FrameworksServicesTests.apk
- * Run: adb shell am instrument -e class com.android.server.NetworkManagementInternalTest -w \
- *     com.android.frameworks.servicestests/androidx.test.runner.AndroidJUnitRunner
- */
-@SmallTest
-@RunWith(AndroidJUnit4.class)
-public class NetworkManagementInternalTest {
-    private static final int TEST_UID = 111;
-
-    private NetworkManagementService.Injector mInjector;
-    private NetworkManagementInternal mNmi;
-
-    @Before
-    public void setUp() {
-        final NetworkManagementService service = new NetworkManagementService();
-        mInjector = service.getInjector();
-        mNmi = service.new LocalService();
-    }
-
-    @Test
-    public void testIsNetworkRestrictedForUid() {
-        // No firewall chains enabled
-        assertFalse(mNmi.isNetworkRestrictedForUid(TEST_UID));
-
-        // Restrict usage of mobile data in background
-        mInjector.setUidOnMeteredNetworkList(true, TEST_UID, true);
-        assertTrue("Should be true since mobile data usage is restricted",
-                mNmi.isNetworkRestrictedForUid(TEST_UID));
-        mInjector.reset();
-
-        // Data saver is on and uid is not allowlisted
-        mInjector.setDataSaverMode(true);
-        mInjector.setUidOnMeteredNetworkList(false, TEST_UID, false);
-        assertTrue("Should be true since data saver is on and the uid is not whitelisted",
-                mNmi.isNetworkRestrictedForUid(TEST_UID));
-        mInjector.reset();
-
-        // Data saver is on and uid is allowlisted
-        mInjector.setDataSaverMode(true);
-        mInjector.setUidOnMeteredNetworkList(false, TEST_UID, true);
-        assertFalse("Should be false since data saver is on and the uid is whitelisted",
-                mNmi.isNetworkRestrictedForUid(TEST_UID));
-        mInjector.reset();
-
-        final ArrayMap<Integer, ArrayMap<Integer, Boolean>> expected = new ArrayMap<>();
-        // Dozable chain
-        final ArrayMap<Integer, Boolean> isRestrictedForDozable = new ArrayMap<>();
-        isRestrictedForDozable.put(FIREWALL_RULE_DEFAULT, true);
-        isRestrictedForDozable.put(FIREWALL_RULE_ALLOW, false);
-        isRestrictedForDozable.put(FIREWALL_RULE_DENY, true);
-        expected.put(FIREWALL_CHAIN_DOZABLE, isRestrictedForDozable);
-        // Powersaver chain
-        final ArrayMap<Integer, Boolean> isRestrictedForPowerSave = new ArrayMap<>();
-        isRestrictedForPowerSave.put(FIREWALL_RULE_DEFAULT, true);
-        isRestrictedForPowerSave.put(FIREWALL_RULE_ALLOW, false);
-        isRestrictedForPowerSave.put(FIREWALL_RULE_DENY, true);
-        expected.put(FIREWALL_CHAIN_POWERSAVE, isRestrictedForPowerSave);
-        // Standby chain
-        final ArrayMap<Integer, Boolean> isRestrictedForStandby = new ArrayMap<>();
-        isRestrictedForStandby.put(FIREWALL_RULE_DEFAULT, false);
-        isRestrictedForStandby.put(FIREWALL_RULE_ALLOW, false);
-        isRestrictedForStandby.put(FIREWALL_RULE_DENY, true);
-        expected.put(FIREWALL_CHAIN_STANDBY, isRestrictedForStandby);
-
-        final int[] chains = {
-                FIREWALL_CHAIN_STANDBY,
-                FIREWALL_CHAIN_POWERSAVE,
-                FIREWALL_CHAIN_DOZABLE
-        };
-        final int[] states = {
-                FIREWALL_RULE_ALLOW,
-                FIREWALL_RULE_DENY,
-                FIREWALL_RULE_DEFAULT
-        };
-        BiFunction<Integer, Integer, String> errorMsg = (chain, state) -> {
-            return String.format("Unexpected value for chain: %s and state: %s",
-                    valueToString(NetworkPolicyManager.class, "FIREWALL_CHAIN_", chain),
-                    valueToString(NetworkPolicyManager.class, "FIREWALL_RULE_", state));
-        };
-        for (int chain : chains) {
-            final ArrayMap<Integer, Boolean> expectedValues = expected.get(chain);
-            mInjector.setFirewallChainState(chain, true);
-            for (int state : states) {
-                mInjector.setFirewallRule(chain, TEST_UID, state);
-                assertEquals(errorMsg.apply(chain, state),
-                        expectedValues.get(state), mNmi.isNetworkRestrictedForUid(TEST_UID));
-            }
-            mInjector.reset();
-        }
-    }
-}

tests/net/java/com/android/server/NetworkManagementServiceTest.java

@@ -16,6 +16,12 @@
 
 package com.android.server;
 
+import static android.util.DebugUtils.valueToString;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
@@ -29,15 +35,19 @@ import android.content.Context;
 import android.net.INetd;
 import android.net.INetdUnsolicitedEventListener;
 import android.net.LinkAddress;
+import android.net.NetworkPolicyManager;
 import android.os.BatteryStats;
 import android.os.Binder;
 import android.os.IBinder;
+import android.os.Process;
+import android.os.RemoteException;
 import android.test.suitebuilder.annotation.SmallTest;
+import android.util.ArrayMap;
 
 import androidx.test.runner.AndroidJUnit4;
 
 import com.android.internal.app.IBatteryStats;
-import com.android.server.NetworkManagementService.SystemServices;
+import com.android.server.NetworkManagementService.Dependencies;
 import com.android.server.net.BaseNetworkObserver;
 
 import org.junit.After;
@@ -49,13 +59,14 @@ import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.util.function.BiFunction;
+
 /**
  * Tests for {@link NetworkManagementService}.
  */
 @RunWith(AndroidJUnit4.class)
 @SmallTest
 public class NetworkManagementServiceTest {
-
     private NetworkManagementService mNMService;
 
     @Mock private Context mContext;
@@ -66,7 +77,9 @@ public class NetworkManagementServiceTest {
     @Captor
     private ArgumentCaptor<INetdUnsolicitedEventListener> mUnsolListenerCaptor;
 
-    private final SystemServices mServices = new SystemServices() {
+    private final MockDependencies mDeps = new MockDependencies();
+
+    private final class MockDependencies extends Dependencies {
         @Override
         public IBinder getService(String name) {
             switch (name) {
@@ -76,14 +89,21 @@ public class NetworkManagementServiceTest {
                     throw new UnsupportedOperationException("Unknown service " + name);
             }
         }
+
         @Override
         public void registerLocalService(NetworkManagementInternal nmi) {
         }
+
         @Override
         public INetd getNetd() {
             return mNetdService;
         }
-    };
+
+        @Override
+        public int getCallingUid() {
+            return Process.SYSTEM_UID;
+        }
+    }
 
     @Before
     public void setUp() throws Exception {
@@ -91,7 +111,7 @@ public class NetworkManagementServiceTest {
         doNothing().when(mNetdService)
                 .registerUnsolicitedEventListener(mUnsolListenerCaptor.capture());
         // Start the service and wait until it connects to our socket.
-        mNMService = NetworkManagementService.create(mContext, mServices);
+        mNMService = NetworkManagementService.create(mContext, mDeps);
     }
 
     @After
@@ -192,4 +212,98 @@ public class NetworkManagementServiceTest {
         // Make sure nothing else was called.
         verifyNoMoreInteractions(observer);
     }
+
+    @Test
+    public void testFirewallEnabled() {
+        mNMService.setFirewallEnabled(true);
+        assertTrue(mNMService.isFirewallEnabled());
+
+        mNMService.setFirewallEnabled(false);
+        assertFalse(mNMService.isFirewallEnabled());
+    }
+
+    private static final int TEST_UID = 111;
+
+    @Test
+    public void testNetworkRestrictedDefault() {
+        assertFalse(mNMService.isNetworkRestricted(TEST_UID));
+    }
+
+    @Test
+    public void testMeteredNetworkRestrictions() throws RemoteException {
+        // Make sure the mocked netd method returns true.
+        doReturn(true).when(mNetdService).bandwidthEnableDataSaver(anyBoolean());
+
+        // Restrict usage of mobile data in background
+        mNMService.setUidMeteredNetworkDenylist(TEST_UID, true);
+        assertTrue("Should be true since mobile data usage is restricted",
+                mNMService.isNetworkRestricted(TEST_UID));
+
+        mNMService.setDataSaverModeEnabled(true);
+        verify(mNetdService).bandwidthEnableDataSaver(true);
+
+        mNMService.setUidMeteredNetworkDenylist(TEST_UID, false);
+        assertTrue("Should be true since data saver is on and the uid is not allowlisted",
+                mNMService.isNetworkRestricted(TEST_UID));
+
+        mNMService.setUidMeteredNetworkAllowlist(TEST_UID, true);
+        assertFalse("Should be false since data saver is on and the uid is allowlisted",
+                mNMService.isNetworkRestricted(TEST_UID));
+
+        // remove uid from allowlist and turn datasaver off again
+        mNMService.setUidMeteredNetworkAllowlist(TEST_UID, false);
+        mNMService.setDataSaverModeEnabled(false);
+        verify(mNetdService).bandwidthEnableDataSaver(false);
+        assertFalse("Network should not be restricted when data saver is off",
+                mNMService.isNetworkRestricted(TEST_UID));
+    }
+
+    @Test
+    public void testFirewallChains() {
+        final ArrayMap<Integer, ArrayMap<Integer, Boolean>> expected = new ArrayMap<>();
+        // Dozable chain
+        final ArrayMap<Integer, Boolean> isRestrictedForDozable = new ArrayMap<>();
+        isRestrictedForDozable.put(NetworkPolicyManager.FIREWALL_RULE_DEFAULT, true);
+        isRestrictedForDozable.put(INetd.FIREWALL_RULE_ALLOW, false);
+        isRestrictedForDozable.put(INetd.FIREWALL_RULE_DENY, true);
+        expected.put(INetd.FIREWALL_CHAIN_DOZABLE, isRestrictedForDozable);
+        // Powersaver chain
+        final ArrayMap<Integer, Boolean> isRestrictedForPowerSave = new ArrayMap<>();
+        isRestrictedForPowerSave.put(NetworkPolicyManager.FIREWALL_RULE_DEFAULT, true);
+        isRestrictedForPowerSave.put(INetd.FIREWALL_RULE_ALLOW, false);
+        isRestrictedForPowerSave.put(INetd.FIREWALL_RULE_DENY, true);
+        expected.put(INetd.FIREWALL_CHAIN_POWERSAVE, isRestrictedForPowerSave);
+        // Standby chain
+        final ArrayMap<Integer, Boolean> isRestrictedForStandby = new ArrayMap<>();
+        isRestrictedForStandby.put(NetworkPolicyManager.FIREWALL_RULE_DEFAULT, false);
+        isRestrictedForStandby.put(INetd.FIREWALL_RULE_ALLOW, false);
+        isRestrictedForStandby.put(INetd.FIREWALL_RULE_DENY, true);
+        expected.put(INetd.FIREWALL_CHAIN_STANDBY, isRestrictedForStandby);
+
+        final int[] chains = {
+                INetd.FIREWALL_CHAIN_STANDBY,
+                INetd.FIREWALL_CHAIN_POWERSAVE,
+                INetd.FIREWALL_CHAIN_DOZABLE
+        };
+        final int[] states = {
+                INetd.FIREWALL_RULE_ALLOW,
+                INetd.FIREWALL_RULE_DENY,
+                NetworkPolicyManager.FIREWALL_RULE_DEFAULT
+        };
+        BiFunction<Integer, Integer, String> errorMsg = (chain, state) -> {
+            return String.format("Unexpected value for chain: %s and state: %s",
+                    valueToString(INetd.class, "FIREWALL_CHAIN_", chain),
+                    valueToString(INetd.class, "FIREWALL_RULE_", state));
+        };
+        for (int chain : chains) {
+            final ArrayMap<Integer, Boolean> expectedValues = expected.get(chain);
+            mNMService.setFirewallChainEnabled(chain, true);
+            for (int state : states) {
+                mNMService.setFirewallUidRule(chain, TEST_UID, state);
+                assertEquals(errorMsg.apply(chain, state),
+                        expectedValues.get(state), mNMService.isNetworkRestricted(TEST_UID));
+            }
+            mNMService.setFirewallChainEnabled(chain, false);
+        }
+    }
 }