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

Commit 71116233 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge "Refactor NetworkStackService for testability"

parents 6002e86e ea9f7e39
Loading
Loading
Loading
Loading
+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"],
+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();
        }
    }

+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);
        }

+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);
        }

+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();
    }

Loading