diff options
| -rw-r--r-- | src/com/android/server/NetworkStackService.java | 93 | ||||
| -rw-r--r-- | src/com/android/server/util/PermissionUtil.java | 2 | ||||
| -rw-r--r-- | tests/unit/src/com/android/server/NetworkStackServiceTest.kt | 192 |
3 files changed, 263 insertions, 24 deletions
diff --git a/src/com/android/server/NetworkStackService.java b/src/com/android/server/NetworkStackService.java index 5ab2744..3a702e5 100644 --- a/src/com/android/server/NetworkStackService.java +++ b/src/com/android/server/NetworkStackService.java @@ -46,6 +46,7 @@ import android.net.util.SharedLog; import android.os.Build; import android.os.HandlerThread; import android.os.IBinder; +import android.os.Looper; import android.os.RemoteException; import android.util.ArraySet; @@ -86,8 +87,7 @@ public class NetworkStackService extends Service { */ public static synchronized IBinder makeConnector(Context context) { if (sConnector == null) { - sConnector = new NetworkStackConnector( - context, new NetworkStackConnector.PermissionChecker()); + sConnector = new NetworkStackConnector(context); } return sConnector; } @@ -115,6 +115,58 @@ public class NetworkStackService extends Service { } /** + * Permission checking dependency of the connector, useful for testing. + */ + public static class PermissionChecker { + /** + * @see PermissionUtil#enforceNetworkStackCallingPermission() + */ + public void enforceNetworkStackCallingPermission() { + PermissionUtil.enforceNetworkStackCallingPermission(); + } + } + + /** + * Dependencies of {@link NetworkStackConnector}, useful for testing. + */ + public static class Dependencies { + /** @see IpMemoryStoreService */ + @NonNull + public IpMemoryStoreService makeIpMemoryStoreService(@NonNull Context context) { + return new IpMemoryStoreService(context); + } + + /** @see NetworkStackNotifier */ + @NonNull + public NetworkStackNotifier makeNotifier(@NonNull Context context, @NonNull Looper looper) { + return new NetworkStackNotifier(context, looper); + } + + /** @see DhcpServer */ + @NonNull + public DhcpServer makeDhcpServer(@NonNull Context context, @NonNull String ifName, + @NonNull DhcpServingParams params, @NonNull SharedLog log) { + return new DhcpServer(context, ifName, params, log); + } + + /** @see NetworkMonitor */ + @NonNull + public NetworkMonitor makeNetworkMonitor(@NonNull Context context, + @NonNull INetworkMonitorCallbacks cb, @NonNull Network network, + @NonNull SharedLog log, @NonNull NetworkStackServiceManager nsServiceManager) { + return new NetworkMonitor(context, cb, network, log, nsServiceManager); + } + + /** @see IpClient */ + @NonNull + public IpClient makeIpClient(@NonNull Context context, @NonNull String ifName, + @NonNull IIpClientCallbacks cb, @NonNull NetworkObserverRegistry observerRegistry, + @NonNull NetworkStackServiceManager nsServiceManager) { + return new IpClient(context, ifName, cb, observerRegistry, nsServiceManager); + } + } + + /** * Connector implementing INetworkStackConnector for clients. */ @VisibleForTesting @@ -123,6 +175,7 @@ public class NetworkStackService extends Service { private static final int NUM_VALIDATION_LOG_LINES = 20; private final Context mContext; private final PermissionChecker mPermChecker; + private final Dependencies mDeps; private final INetd mNetd; private final NetworkObserverRegistry mObserverRegistry; @GuardedBy("mIpClients") @@ -142,19 +195,6 @@ public class NetworkStackService extends Service { private final ArraySet<Integer> mFrameworkAidlVersions = new ArraySet<>(1); private final int mNetdAidlVersion; - /** - * Permission checking dependency of the connector, useful for testing. - */ - @VisibleForTesting - public static class PermissionChecker { - /** - * @see PermissionUtil#enforceNetworkStackCallingPermission() - */ - public void enforceNetworkStackCallingPermission() { - PermissionUtil.enforceNetworkStackCallingPermission(); - } - } - private SharedLog addValidationLogs(Network network, String name) { final SharedLog log = new SharedLog(NUM_VALIDATION_LOG_LINES, network + " - " + name); synchronized (mValidationLogs) { @@ -166,21 +206,27 @@ public class NetworkStackService extends Service { return log; } + NetworkStackConnector(@NonNull Context context) { + this(context, new PermissionChecker(), new Dependencies()); + } + @VisibleForTesting public NetworkStackConnector( - @NonNull Context context, @NonNull PermissionChecker permChecker) { + @NonNull Context context, @NonNull PermissionChecker permChecker, + @NonNull Dependencies deps) { mContext = context; mPermChecker = permChecker; + mDeps = deps; mNetd = INetd.Stub.asInterface( (IBinder) context.getSystemService(Context.NETD_SERVICE)); mObserverRegistry = new NetworkObserverRegistry(); - mIpMemoryStoreService = new IpMemoryStoreService(context); + mIpMemoryStoreService = mDeps.makeIpMemoryStoreService(context); // NetworkStackNotifier only shows notifications relevant for API level > Q if (ShimUtils.isReleaseOrDevelopmentApiAbove(Build.VERSION_CODES.Q)) { final HandlerThread notifierThread = new HandlerThread( NetworkStackNotifier.class.getSimpleName()); notifierThread.start(); - mNotifier = new NetworkStackNotifier(context, notifierThread.getLooper()); + mNotifier = mDeps.makeNotifier(context, notifierThread.getLooper()); } else { mNotifier = null; } @@ -217,7 +263,7 @@ public class NetworkStackService extends Service { updateSystemAidlVersion(cb.getInterfaceVersion()); final DhcpServer server; try { - server = new DhcpServer( + server = mDeps.makeDhcpServer( mContext, ifName, DhcpServingParams.fromParcelableObject(params), @@ -240,7 +286,7 @@ public class NetworkStackService extends Service { mPermChecker.enforceNetworkStackCallingPermission(); updateSystemAidlVersion(cb.getInterfaceVersion()); final SharedLog log = addValidationLogs(network, name); - final NetworkMonitor nm = new NetworkMonitor(mContext, cb, network, log, this); + final NetworkMonitor nm = mDeps.makeNetworkMonitor(mContext, cb, network, log, this); cb.onNetworkMonitorCreated(new NetworkMonitorConnector(nm, mPermChecker)); } @@ -248,7 +294,8 @@ public class NetworkStackService extends Service { public void makeIpClient(String ifName, IIpClientCallbacks cb) throws RemoteException { mPermChecker.enforceNetworkStackCallingPermission(); updateSystemAidlVersion(cb.getInterfaceVersion()); - final IpClient ipClient = new IpClient(mContext, ifName, cb, mObserverRegistry, this); + final IpClient ipClient = mDeps.makeIpClient( + mContext, ifName, cb, mObserverRegistry, this); synchronized (mIpClients) { final Iterator<WeakReference<IpClient>> it = mIpClients.iterator(); @@ -371,10 +418,10 @@ public class NetworkStackService extends Service { @NonNull private final NetworkMonitor mNm; @NonNull - private final NetworkStackConnector.PermissionChecker mPermChecker; + private final PermissionChecker mPermChecker; public NetworkMonitorConnector(@NonNull NetworkMonitor nm, - @NonNull NetworkStackConnector.PermissionChecker permChecker) { + @NonNull PermissionChecker permChecker) { mNm = nm; mPermChecker = permChecker; } diff --git a/src/com/android/server/util/PermissionUtil.java b/src/com/android/server/util/PermissionUtil.java index 28dad25..3dff715 100644 --- a/src/com/android/server/util/PermissionUtil.java +++ b/src/com/android/server/util/PermissionUtil.java @@ -73,7 +73,7 @@ public final class PermissionUtil { */ public static void checkDumpPermission() { final int caller = getCallingUid(); - if (caller != Process.SYSTEM_UID && caller != Process.ROOT_UID + if (caller != Process.myUid() && caller != Process.SYSTEM_UID && caller != Process.ROOT_UID && caller != Process.SHELL_UID) { throw new SecurityException("No dump permissions for caller: " + caller); } diff --git a/tests/unit/src/com/android/server/NetworkStackServiceTest.kt b/tests/unit/src/com/android/server/NetworkStackServiceTest.kt new file mode 100644 index 0000000..52556bc --- /dev/null +++ b/tests/unit/src/com/android/server/NetworkStackServiceTest.kt @@ -0,0 +1,192 @@ +/* + * Copyright (C) 2020 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 android.content.Context +import android.net.IIpMemoryStoreCallbacks +import android.net.INetd +import android.net.INetworkMonitorCallbacks +import android.net.INetworkStackConnector +import android.net.InetAddresses.parseNumericAddress +import android.net.Network +import android.net.dhcp.DhcpServer +import android.net.dhcp.DhcpServingParamsParcel +import android.net.dhcp.IDhcpServer +import android.net.dhcp.IDhcpServerCallbacks +import android.net.ip.IIpClientCallbacks +import android.net.ip.IpClient +import android.net.shared.Inet4AddressUtils.inet4AddressToIntHTH +import android.os.Build +import android.os.IBinder +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SmallTest +import com.android.server.NetworkStackService.Dependencies +import com.android.server.NetworkStackService.NetworkStackConnector +import com.android.server.NetworkStackService.PermissionChecker +import com.android.server.connectivity.NetworkMonitor +import com.android.server.connectivity.ipmemorystore.IpMemoryStoreService +import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.doReturn +import org.mockito.Mockito.eq +import org.mockito.Mockito.mock +import org.mockito.Mockito.spy +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import java.io.FileDescriptor +import java.io.PrintWriter +import java.io.StringWriter +import java.net.Inet4Address +import kotlin.reflect.KVisibility +import kotlin.reflect.full.declaredMemberFunctions +import kotlin.test.assertEquals + +private val TEST_NETD_VERSION = 9991001 +private val TEST_NETD_HASH = "test_netd_hash" + +private val TEST_IPMEMORYSTORE_VERSION = 9991002 +private val TEST_IPMEMORYSTORE_HASH = "test_ipmemorystore_hash" + +private val TEST_IFACE = "test_iface" + +@RunWith(AndroidJUnit4::class) +@SmallTest +class NetworkStackServiceTest { + @Rule @JvmField + val ignoreRule = DevSdkIgnoreRule() + + private val permChecker = mock(PermissionChecker::class.java) + private val mockIpMemoryStoreService = mock(IpMemoryStoreService::class.java) + private val mockDhcpServer = mock(DhcpServer::class.java) + private val mockNetworkMonitor = mock(NetworkMonitor::class.java) + private val mockIpClient = mock(IpClient::class.java) + private val deps = mock(Dependencies::class.java).apply { + doReturn(mockIpMemoryStoreService).`when`(this).makeIpMemoryStoreService(any()) + doReturn(mockDhcpServer).`when`(this).makeDhcpServer(any(), any(), any(), any()) + doReturn(mockNetworkMonitor).`when`(this).makeNetworkMonitor(any(), any(), any(), any(), + any()) + doReturn(mockIpClient).`when`(this).makeIpClient(any(), any(), any(), any(), any()) + } + private val netd = mock(INetd::class.java).apply { + doReturn(TEST_NETD_VERSION).`when`(this).interfaceVersion + doReturn(TEST_NETD_HASH).`when`(this).interfaceHash + } + private val netdBinder = mock(IBinder::class.java).apply { + doReturn(netd).`when`(this).queryLocalInterface(any()) + } + private val context = mock(Context::class.java).apply { + doReturn(netdBinder).`when`(this).getSystemService(Context.NETD_SERVICE) + } + + private val connector = NetworkStackConnector(context, permChecker, deps) + + @Test + fun testDumpVersion_Q() { + prepareDumpVersionTest() + + val dumpsysOut = StringWriter() + connector.dump(FileDescriptor(), PrintWriter(dumpsysOut, true /* autoFlush */), + arrayOf("version") /* args */) + + assertEquals("NetworkStack version:\n" + + "NetworkStackConnector: ${INetworkStackConnector.VERSION}\n" + + "SystemServer: {9990001, 9990002, 9990003, 9990004, 9990005}\n" + + "Netd: $TEST_NETD_VERSION\n\n", + dumpsysOut.toString()) + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.Q) + fun testDumpVersion() { + // TODO: log interface hash on R+ and test it + } + + fun prepareDumpVersionTest() { + // Call each method on INetworkStackConnector and verify that it notes down the version of + // the remote. + // Call fetchIpMemoryStore + val mockIpMemoryStoreCb = mock(IIpMemoryStoreCallbacks::class.java) + doReturn(9990001).`when`(mockIpMemoryStoreCb).interfaceVersion + doReturn("fetch_ipmemorystore_hash").`when`(mockIpMemoryStoreCb).interfaceHash + + connector.fetchIpMemoryStore(mockIpMemoryStoreCb) + // IpMemoryStore was created at initialization time + verify(mockIpMemoryStoreCb).onIpMemoryStoreFetched(any()) + + // Call makeDhcpServer + val testParams = DhcpServingParamsParcel() + testParams.linkMtu = 1500 + testParams.dhcpLeaseTimeSecs = 3600L + testParams.serverAddr = inet4AddressToIntHTH( + parseNumericAddress("192.168.1.1") as Inet4Address) + testParams.serverAddrPrefixLength = 24 + + val mockDhcpCb = mock(IDhcpServerCallbacks::class.java) + doReturn(9990002).`when`(mockDhcpCb).interfaceVersion + doReturn("dhcp_server_hash").`when`(mockDhcpCb).interfaceHash + + connector.makeDhcpServer(TEST_IFACE, testParams, mockDhcpCb) + + verify(deps).makeDhcpServer(any(), eq(TEST_IFACE), any(), any()) + verify(mockDhcpCb).onDhcpServerCreated(eq(IDhcpServer.STATUS_SUCCESS), any()) + + // Call makeNetworkMonitor + // Use a spy of INetworkMonitorCallbacks and not a mock, as mockito can't create a mock on Q + // because of the missing CaptivePortalData class that is an argument of one of the methods + val mockNetworkMonitorCb = spy(INetworkMonitorCallbacks.Stub.asInterface( + mock(IBinder::class.java))) + doReturn(9990003).`when`(mockNetworkMonitorCb).interfaceVersion + doReturn("networkmonitor_hash").`when`(mockNetworkMonitorCb).interfaceHash + + connector.makeNetworkMonitor(Network(123), "test_nm", mockNetworkMonitorCb) + + verify(deps).makeNetworkMonitor(any(), any(), eq(Network(123)), any(), any()) + verify(mockNetworkMonitorCb).onNetworkMonitorCreated(any()) + + // Call makeIpClient + // Use a spy of IIpClientCallbacks instead of a mock, as mockito cannot create a mock on Q + // because of the missing CaptivePortalData class that is an argument on one of the methods + val mockIpClientCb = mock(IIpClientCallbacks::class.java) + doReturn(9990004).`when`(mockIpClientCb).interfaceVersion + doReturn("ipclient_hash").`when`(mockIpClientCb).interfaceHash + + connector.makeIpClient(TEST_IFACE, mockIpClientCb) + + verify(deps).makeIpClient(any(), eq(TEST_IFACE), any(), any(), any()) + verify(mockIpClientCb).onIpClientCreated(any()) + + // Call some methods one more time with a shared version number and hash to verify no + // duplicates are reported + doReturn(9990005).`when`(mockIpMemoryStoreCb).interfaceVersion + doReturn("multiple_use_hash").`when`(mockIpMemoryStoreCb).interfaceHash + connector.fetchIpMemoryStore(mockIpMemoryStoreCb) + verify(mockIpMemoryStoreCb, times(2)).onIpMemoryStoreFetched(any()) + + doReturn(9990005).`when`(mockDhcpCb).interfaceVersion + doReturn("multiple_use_hash").`when`(mockDhcpCb).interfaceHash + connector.makeDhcpServer(TEST_IFACE, testParams, mockDhcpCb) + verify(mockDhcpCb, times(2)).onDhcpServerCreated(eq(IDhcpServer.STATUS_SUCCESS), any()) + + // Verify all methods were covered by the test (4 methods + getVersion + getHash) + assertEquals(6, INetworkStackConnector::class.declaredMemberFunctions.count { + it.visibility == KVisibility.PUBLIC + }) + } +}
\ No newline at end of file |