Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ea9f7e39 authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN
Browse files

Refactor NetworkStackService for testability 

Refactor some methods to allow integration testing between
ConnectivityService and NetworkStack. The integration tests override
some NetworkStack methods to mock permission checks or NetworkMonitor
network requests.

Test: atest NetworkStackTests
Change-Id: Ib5b4458f0b4d1423759e1e4016ab961d3ced7b48
parent 967dd0a2

Android.bp

+6 −0
Original line number Diff line number Diff line
@@ -125,6 +125,12 @@ android_app { 
    required: ["NetworkPermissionConfig"],

}



android_library {

    name: "TestNetworkStackLib",

    defaults: ["NetworkStackAppCommon"],

    manifest: "AndroidManifest.xml",

}



genrule {

    name: "statslog-networkstack-java-gen",

    tools: ["stats-log-api-gen"],

src/android/net/dhcp/DhcpServer.java

+2 −4
Original line number Diff line number Diff line
@@ -20,8 +20,6 @@ import static android.net.dhcp.DhcpPacket.DHCP_CLIENT; 
import static android.net.dhcp.DhcpPacket.DHCP_HOST_NAME;

import static android.net.dhcp.DhcpPacket.DHCP_SERVER;

import static android.net.dhcp.DhcpPacket.ENCAP_BOOTP;

import static android.net.dhcp.IDhcpServer.STATUS_INVALID_ARGUMENT;

import static android.net.dhcp.IDhcpServer.STATUS_SUCCESS;

import static android.net.shared.Inet4AddressUtils.getBroadcastAddress;

import static android.net.shared.Inet4AddressUtils.getPrefixMaskAsInet4Address;

import static android.system.OsConstants.AF_INET;
@@ -36,7 +34,7 @@ import static com.android.internal.util.TrafficStatsConstants.TAG_SYSTEM_DHCP_SE 
import static com.android.server.util.NetworkStackConstants.INFINITE_LEASE;

import static com.android.server.util.NetworkStackConstants.IPV4_ADDR_ALL;

import static com.android.server.util.NetworkStackConstants.IPV4_ADDR_ANY;

import static com.android.server.util.PermissionUtil.checkNetworkStackCallingPermission;

import static com.android.server.util.PermissionUtil.enforceNetworkStackCallingPermission;



import static java.lang.Integer.toUnsignedLong;
@@ -213,7 +211,7 @@ public class DhcpServer extends IDhcpServer.Stub { 


        @Override

        public void checkCaller() {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

        }

    }

src/android/net/ip/IpClient.java

+14 −14
Original line number Diff line number Diff line
@@ -19,7 +19,7 @@ package android.net.ip; 
import static android.net.RouteInfo.RTN_UNICAST;

import static android.net.shared.IpConfigurationParcelableUtil.toStableParcelable;



import static com.android.server.util.PermissionUtil.checkNetworkStackCallingPermission;

import static com.android.server.util.PermissionUtil.enforceNetworkStackCallingPermission;



import android.annotation.NonNull;

import android.content.Context;
@@ -515,67 +515,67 @@ public class IpClient extends StateMachine { 
    class IpClientConnector extends IIpClient.Stub {

        @Override

        public void completedPreDhcpAction() {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.completedPreDhcpAction();

        }

        @Override

        public void confirmConfiguration() {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.confirmConfiguration();

        }

        @Override

        public void readPacketFilterComplete(byte[] data) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.readPacketFilterComplete(data);

        }

        @Override

        public void shutdown() {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.shutdown();

        }

        @Override

        public void startProvisioning(ProvisioningConfigurationParcelable req) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.startProvisioning(ProvisioningConfiguration.fromStableParcelable(req));

        }

        @Override

        public void stop() {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.stop();

        }

        @Override

        public void setL2KeyAndGroupHint(String l2Key, String groupHint) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.setL2KeyAndGroupHint(l2Key, groupHint);

        }

        @Override

        public void setTcpBufferSizes(String tcpBufferSizes) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.setTcpBufferSizes(tcpBufferSizes);

        }

        @Override

        public void setHttpProxy(ProxyInfo proxyInfo) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.setHttpProxy(proxyInfo);

        }

        @Override

        public void setMulticastFilter(boolean enabled) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.setMulticastFilter(enabled);

        }

        @Override

        public void addKeepalivePacketFilter(int slot, TcpKeepalivePacketDataParcelable pkt) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.addKeepalivePacketFilter(slot, pkt);

        }

        @Override

        public void addNattKeepalivePacketFilter(int slot, NattKeepalivePacketDataParcelable pkt) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.addNattKeepalivePacketFilter(slot, pkt);

        }

        @Override

        public void removeKeepalivePacketFilter(int slot) {

            checkNetworkStackCallingPermission();

            enforceNetworkStackCallingPermission();

            IpClient.this.removeKeepalivePacketFilter(slot);

        }

src/com/android/server/NetworkStackService.java

+58 −25
Original line number Diff line number Diff line
@@ -21,14 +21,12 @@ import static android.net.dhcp.IDhcpServer.STATUS_SUCCESS; 
import static android.net.dhcp.IDhcpServer.STATUS_UNKNOWN_ERROR;



import static com.android.server.util.PermissionUtil.checkDumpPermission;

import static com.android.server.util.PermissionUtil.checkNetworkStackCallingPermission;



import android.annotation.NonNull;

import android.annotation.Nullable;

import android.app.Service;

import android.content.Context;

import android.content.Intent;

import android.net.ConnectivityManager;

import android.net.IIpMemoryStore;

import android.net.IIpMemoryStoreCallbacks;

import android.net.INetd;
@@ -50,10 +48,13 @@ import android.net.util.SharedLog; 
import android.os.IBinder;

import android.os.RemoteException;



import androidx.annotation.VisibleForTesting;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.util.IndentingPrintWriter;

import com.android.server.connectivity.NetworkMonitor;

import com.android.server.connectivity.ipmemorystore.IpMemoryStoreService;

import com.android.server.util.PermissionUtil;



import java.io.FileDescriptor;

import java.io.PrintWriter;
@@ -81,7 +82,8 @@ public class NetworkStackService extends Service { 
     */

    public static synchronized IBinder makeConnector(Context context) {

        if (sConnector == null) {

            sConnector = new NetworkStackConnector(context);

            sConnector = new NetworkStackConnector(

                    context, new NetworkStackConnector.PermissionChecker());

        }

        return sConnector;

    }
@@ -103,13 +105,17 @@ public class NetworkStackService extends Service { 
        IIpMemoryStore getIpMemoryStoreService();

    }



    private static class NetworkStackConnector extends INetworkStackConnector.Stub

    /**

     * Connector implementing INetworkStackConnector for clients.

     */

    @VisibleForTesting

    public static class NetworkStackConnector extends INetworkStackConnector.Stub

            implements NetworkStackServiceManager {

        private static final int NUM_VALIDATION_LOG_LINES = 20;

        private final Context mContext;

        private final PermissionChecker mPermChecker;

        private final INetd mNetd;

        private final NetworkObserverRegistry mObserverRegistry;

        private final ConnectivityManager mCm;

        @GuardedBy("mIpClients")

        private final ArrayList<WeakReference<IpClient>> mIpClients = new ArrayList<>();

        private final IpMemoryStoreService mIpMemoryStoreService;
@@ -127,6 +133,19 @@ public class NetworkStackService extends Service { 
        /** Whether different versions have been observed on interfaces provided by the system */

        private volatile boolean mConflictingSystemAidlVersions = false;



        /**

         * 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) {
@@ -138,12 +157,14 @@ public class NetworkStackService extends Service { 
            return log;

        }



        NetworkStackConnector(Context context) {

        @VisibleForTesting

        public NetworkStackConnector(

                @NonNull Context context, @NonNull PermissionChecker permChecker) {

            mContext = context;

            mPermChecker = permChecker;

            mNetd = INetd.Stub.asInterface(

                    (IBinder) context.getSystemService(Context.NETD_SERVICE));

            mObserverRegistry = new NetworkObserverRegistry();

            mCm = context.getSystemService(ConnectivityManager.class);

            mIpMemoryStoreService = new IpMemoryStoreService(context);



            try {
@@ -166,7 +187,7 @@ public class NetworkStackService extends Service { 
        @Override

        public void makeDhcpServer(@NonNull String ifName, @NonNull DhcpServingParamsParcel params,

                @NonNull IDhcpServerCallbacks cb) throws RemoteException {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            updateSystemAidlVersion(cb.getInterfaceVersion());

            final DhcpServer server;

            try {
@@ -189,16 +210,16 @@ public class NetworkStackService extends Service { 
        @Override

        public void makeNetworkMonitor(Network network, String name, INetworkMonitorCallbacks cb)

                throws RemoteException {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            updateSystemAidlVersion(cb.getInterfaceVersion());

            final SharedLog log = addValidationLogs(network, name);

            final NetworkMonitor nm = new NetworkMonitor(mContext, cb, network, log);

            cb.onNetworkMonitorCreated(new NetworkMonitorImpl(nm));

            cb.onNetworkMonitorCreated(new NetworkMonitorConnector(nm, mPermChecker));

        }



        @Override

        public void makeIpClient(String ifName, IIpClientCallbacks cb) throws RemoteException {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            updateSystemAidlVersion(cb.getInterfaceVersion());

            final IpClient ipClient = new IpClient(mContext, ifName, cb, mObserverRegistry, this);
@@ -224,7 +245,7 @@ public class NetworkStackService extends Service { 
        @Override

        public void fetchIpMemoryStore(@NonNull final IIpMemoryStoreCallbacks cb)

                throws RemoteException {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            updateSystemAidlVersion(cb.getInterfaceVersion());

            cb.onIpMemoryStoreFetched(mIpMemoryStoreService);

        }
@@ -290,82 +311,94 @@ public class NetworkStackService extends Service { 
            fout.println("SystemServerConflicts: " + mConflictingSystemAidlVersions);

        }



        /**

         * Get the version of the AIDL interface.

         */

        @Override

        public int getInterfaceVersion() {

            return this.VERSION;

        }

    }



    private static class NetworkMonitorImpl extends INetworkMonitor.Stub {

    /**

     * Proxy for {@link NetworkMonitor} that implements {@link INetworkMonitor}.

     */

    @VisibleForTesting

    public static class NetworkMonitorConnector extends INetworkMonitor.Stub {

        @NonNull

        private final NetworkMonitor mNm;

        @NonNull

        private final NetworkStackConnector.PermissionChecker mPermChecker;



        NetworkMonitorImpl(NetworkMonitor nm) {

        public NetworkMonitorConnector(@NonNull NetworkMonitor nm,

                @NonNull NetworkStackConnector.PermissionChecker permChecker) {

            mNm = nm;

            mPermChecker = permChecker;

        }



        @Override

        public void start() {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.start();

        }



        @Override

        public void launchCaptivePortalApp() {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.launchCaptivePortalApp();

        }



        @Override

        public void notifyCaptivePortalAppFinished(int response) {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.notifyCaptivePortalAppFinished(response);

        }



        @Override

        public void setAcceptPartialConnectivity() {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.setAcceptPartialConnectivity();

        }



        @Override

        public void forceReevaluation(int uid) {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.forceReevaluation(uid);

        }



        @Override

        public void notifyPrivateDnsChanged(PrivateDnsConfigParcel config) {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.notifyPrivateDnsSettingsChanged(PrivateDnsConfig.fromParcel(config));

        }



        @Override

        public void notifyDnsResponse(int returnCode) {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.notifyDnsResponse(returnCode);

        }



        @Override

        public void notifyNetworkConnected(LinkProperties lp, NetworkCapabilities nc) {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.notifyNetworkConnected(lp, nc);

        }



        @Override

        public void notifyNetworkDisconnected() {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.notifyNetworkDisconnected();

        }



        @Override

        public void notifyLinkPropertiesChanged(LinkProperties lp) {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.notifyLinkPropertiesChanged(lp);

        }



        @Override

        public void notifyNetworkCapabilitiesChanged(NetworkCapabilities nc) {

            checkNetworkStackCallingPermission();

            mPermChecker.enforceNetworkStackCallingPermission();

            mNm.notifyNetworkCapabilitiesChanged(nc);

        }

src/com/android/server/connectivity/NetworkMonitor.java

+12 −4
Original line number Diff line number Diff line
@@ -379,7 +379,7 @@ public class NetworkMonitor extends StateMachine { 
    }



    @VisibleForTesting

    protected NetworkMonitor(Context context, INetworkMonitorCallbacks cb, Network network,

    public NetworkMonitor(Context context, INetworkMonitorCallbacks cb, Network network,

            IpConnectivityLog logger, SharedLog validationLogs,

            Dependencies deps, DataStallStatsUtils detectionStatsUtils) {

        // Add suffix indicating which NetworkMonitor we're talking about.
@@ -1843,8 +1843,7 @@ public class NetworkMonitor extends StateMachine { 
            latencyBroadcast.putExtra(NetworkMonitorUtils.EXTRA_RESPONSE_TIMESTAMP_MS,

                    responseTimestampMs);

        }

        mContext.sendBroadcastAsUser(latencyBroadcast, UserHandle.CURRENT,

                NetworkMonitorUtils.PERMISSION_ACCESS_NETWORK_CONDITIONS);

        mDependencies.sendNetworkConditionsBroadcast(mContext, latencyBroadcast);

    }



    private void logNetworkEvent(int evtype) {
@@ -1889,7 +1888,7 @@ public class NetworkMonitor extends StateMachine { 
    }



    @VisibleForTesting

    static class Dependencies {

    public static class Dependencies {

        public Network getPrivateDnsBypassNetwork(Network network) {

            return new OneAddressPerFamilyNetwork(network);

        }
@@ -1948,6 +1947,15 @@ public class NetworkMonitor extends StateMachine { 
            return NetworkStackUtils.getDeviceConfigPropertyInt(namespace, name, defaultValue);

        }



        /**

         * Send a broadcast indicating network conditions.

         */

        public void sendNetworkConditionsBroadcast(@NonNull Context context,

                @NonNull Intent broadcast) {

            context.sendBroadcastAsUser(broadcast, UserHandle.CURRENT,

                    NetworkMonitorUtils.PERMISSION_ACCESS_NETWORK_CONDITIONS);

        }



        public static final Dependencies DEFAULT = new Dependencies();

    }