Merge "Refactor Module connectors for testing"

am: 288c75d055

Change-Id: Ie022cbb6a7f900f637647b00eef38baee28bb34d

2 files changed, 120 insertions, 55 deletions

diff --git a/services/net/java/android/net/ConnectivityModuleConnector.java b/services/net/java/android/net/ConnectivityModuleConnector.java
index e84dac258cc5..19ade3394de7 100644
--- a/services/net/java/android/net/ConnectivityModuleConnector.java
+++ b/services/net/java/android/net/ConnectivityModuleConnector.java
@@ -38,6 +38,7 @@ import android.util.ArraySet;
 import android.util.Slog;
 
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 
 import java.io.File;
 import java.io.PrintWriter;
@@ -76,8 +77,17 @@ public class ConnectivityModuleConnector {
     private final SharedLog mLog = new SharedLog(TAG);
     @GuardedBy("mHealthListeners")
     private final ArraySet<ConnectivityModuleHealthListener> mHealthListeners = new ArraySet<>();
+    @NonNull
+    private final Dependencies mDeps;
 
-    private ConnectivityModuleConnector() { }
+    private ConnectivityModuleConnector() {
+        this(new DependenciesImpl());
+    }
+
+    @VisibleForTesting
+    ConnectivityModuleConnector(@NonNull Dependencies deps) {
+        mDeps = deps;
+    }
 
     /**
      * Get the {@link ConnectivityModuleConnector} singleton instance.
@@ -124,6 +134,59 @@ public class ConnectivityModuleConnector {
         void onModuleServiceConnected(@NonNull IBinder iBinder);
     }
 
+    /**
+     * Interface used to determine the intent to use to bind to the module. Useful for testing.
+     */
+    @VisibleForTesting
+    protected interface Dependencies {
+        /**
+         * Determine the intent to use to bind to the module.
+         * @return null if the intent could not be resolved, the intent otherwise.
+         */
+        @Nullable
+        Intent getModuleServiceIntent(
+                @NonNull PackageManager pm, @NonNull String serviceIntentBaseAction,
+                @NonNull String servicePermissionName, boolean inSystemProcess);
+    }
+
+    private static class DependenciesImpl implements Dependencies {
+        @Nullable
+        @Override
+        public Intent getModuleServiceIntent(
+                @NonNull PackageManager pm, @NonNull String serviceIntentBaseAction,
+                @NonNull String servicePermissionName, boolean inSystemProcess) {
+            final Intent intent =
+                    new Intent(inSystemProcess
+                            ? serviceIntentBaseAction + IN_PROCESS_SUFFIX
+                            : serviceIntentBaseAction);
+            final ComponentName comp = intent.resolveSystemService(pm, 0);
+            if (comp == null) {
+                return null;
+            }
+            intent.setComponent(comp);
+
+            final int uid;
+            try {
+                uid = pm.getPackageUidAsUser(comp.getPackageName(), UserHandle.USER_SYSTEM);
+            } catch (PackageManager.NameNotFoundException e) {
+                throw new SecurityException(
+                        "Could not check network stack UID; package not found.", e);
+            }
+
+            final int expectedUid =
+                    inSystemProcess ? Process.SYSTEM_UID : Process.NETWORK_STACK_UID;
+            if (uid != expectedUid) {
+                throw new SecurityException("Invalid network stack UID: " + uid);
+            }
+
+            if (!inSystemProcess) {
+                checkModuleServicePermission(pm, comp, servicePermissionName);
+            }
+
+            return intent;
+        }
+    }
+
 
     /**
      * Add a {@link ConnectivityModuleHealthListener} to listen to network stack health events.
@@ -156,13 +219,13 @@ public class ConnectivityModuleConnector {
         final PackageManager pm = mContext.getPackageManager();
 
         // Try to bind in-process if the device was shipped with an in-process version
-        Intent intent = getModuleServiceIntent(pm, serviceIntentBaseAction, servicePermissionName,
-                true /* inSystemProcess */);
+        Intent intent = mDeps.getModuleServiceIntent(pm, serviceIntentBaseAction,
+                servicePermissionName, true /* inSystemProcess */);
 
         // Otherwise use the updatable module version
         if (intent == null) {
-            intent = getModuleServiceIntent(pm, serviceIntentBaseAction, servicePermissionName,
-                false /* inSystemProcess */);
+            intent = mDeps.getModuleServiceIntent(pm, serviceIntentBaseAction,
+                    servicePermissionName, false /* inSystemProcess */);
             log("Starting networking module in network_stack process");
         } else {
             log("Starting networking module in system_server process");
@@ -219,41 +282,7 @@ public class ConnectivityModuleConnector {
         }
     }
 
-    @Nullable
-    private Intent getModuleServiceIntent(
-            @NonNull PackageManager pm, @NonNull String serviceIntentBaseAction,
-            @NonNull String servicePermissionName, boolean inSystemProcess) {
-        final Intent intent =
-                new Intent(inSystemProcess
-                        ? serviceIntentBaseAction + IN_PROCESS_SUFFIX
-                        : serviceIntentBaseAction);
-        final ComponentName comp = intent.resolveSystemService(pm, 0);
-        if (comp == null) {
-            return null;
-        }
-        intent.setComponent(comp);
-
-        int uid = -1;
-        try {
-            uid = pm.getPackageUidAsUser(comp.getPackageName(), UserHandle.USER_SYSTEM);
-        } catch (PackageManager.NameNotFoundException e) {
-            logWtf("Networking module package not found", e);
-            // Fall through
-        }
-
-        final int expectedUid = inSystemProcess ? Process.SYSTEM_UID : Process.NETWORK_STACK_UID;
-        if (uid != expectedUid) {
-            throw new SecurityException("Invalid network stack UID: " + uid);
-        }
-
-        if (!inSystemProcess) {
-            checkModuleServicePermission(pm, comp, servicePermissionName);
-        }
-
-        return intent;
-    }
-
-    private void checkModuleServicePermission(
+    private static void checkModuleServicePermission(
             @NonNull PackageManager pm, @NonNull ComponentName comp,
             @NonNull String servicePermissionName) {
         final int hasPermission =
diff --git a/services/net/java/android/net/NetworkStackClient.java b/services/net/java/android/net/NetworkStackClient.java
index abb46664e40d..69e24063210d 100644
--- a/services/net/java/android/net/NetworkStackClient.java
+++ b/services/net/java/android/net/NetworkStackClient.java
@@ -35,6 +35,7 @@ import android.os.UserHandle;
 import android.util.Slog;
 
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 
 import java.io.PrintWriter;
 import java.util.ArrayList;
@@ -51,6 +52,9 @@ public class NetworkStackClient {
     private static NetworkStackClient sInstance;
 
     @NonNull
+    private final Dependencies mDependencies;
+
+    @NonNull
     @GuardedBy("mPendingNetStackRequests")
     private final ArrayList<NetworkStackCallback> mPendingNetStackRequests = new ArrayList<>();
     @Nullable
@@ -66,7 +70,50 @@ public class NetworkStackClient {
         void onNetworkStackConnected(INetworkStackConnector connector);
     }
 
-    private NetworkStackClient() { }
+    @VisibleForTesting
+    protected NetworkStackClient(@NonNull Dependencies dependencies) {
+        mDependencies = dependencies;
+    }
+
+    private NetworkStackClient() {
+        this(new DependenciesImpl());
+    }
+
+    @VisibleForTesting
+    protected interface Dependencies {
+        void addToServiceManager(@NonNull IBinder service);
+        void checkCallerUid();
+        ConnectivityModuleConnector getConnectivityModuleConnector();
+    }
+
+    private static class DependenciesImpl implements Dependencies {
+        @Override
+        public void addToServiceManager(@NonNull IBinder service) {
+            ServiceManager.addService(Context.NETWORK_STACK_SERVICE, service,
+                    false /* allowIsolated */, DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL);
+        }
+
+        @Override
+        public void checkCallerUid() {
+            final int caller = Binder.getCallingUid();
+            // This is a client lib so "caller" is the current UID in most cases. The check is done
+            // here in the caller's process just to provide a nicer error message to clients; more
+            // generic checks are also done in NetworkStackService.
+            // See PermissionUtil in NetworkStack for the actual check on the service side - the
+            // checks here should be kept in sync with PermissionUtil.
+            if (caller != Process.SYSTEM_UID
+                    && caller != Process.NETWORK_STACK_UID
+                    && !UserHandle.isSameApp(caller, Process.BLUETOOTH_UID)) {
+                throw new SecurityException(
+                        "Only the system server should try to bind to the network stack.");
+            }
+        }
+
+        @Override
+        public ConnectivityModuleConnector getConnectivityModuleConnector() {
+            return ConnectivityModuleConnector.getInstance();
+        }
+    }
 
     /**
      * Get the NetworkStackClient singleton instance.
@@ -150,9 +197,7 @@ public class NetworkStackClient {
 
     private void registerNetworkStackService(@NonNull IBinder service) {
         final INetworkStackConnector connector = INetworkStackConnector.Stub.asInterface(service);
-
-        ServiceManager.addService(Context.NETWORK_STACK_SERVICE, service, false /* allowIsolated */,
-                DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL);
+        mDependencies.addToServiceManager(service);
         log("Network stack service registered");
 
         final ArrayList<NetworkStackCallback> requests;
@@ -185,7 +230,7 @@ public class NetworkStackClient {
      * started.
      */
     public void start() {
-        ConnectivityModuleConnector.getInstance().startModuleService(
+        mDependencies.getConnectivityModuleConnector().startModuleService(
                 INetworkStackConnector.class.getName(), PERMISSION_MAINLINE_NETWORK_STACK,
                 new NetworkStackConnection());
         log("Network stack service start requested");
@@ -251,16 +296,7 @@ public class NetworkStackClient {
     }
 
     private void requestConnector(@NonNull NetworkStackCallback request) {
-        // TODO: PID check.
-        final int caller = Binder.getCallingUid();
-        if (caller != Process.SYSTEM_UID
-                && caller != Process.NETWORK_STACK_UID
-                && !UserHandle.isSameApp(caller, Process.BLUETOOTH_UID)
-                && !UserHandle.isSameApp(caller, Process.PHONE_UID)) {
-            // Don't even attempt to obtain the connector and give a nice error message
-            throw new SecurityException(
-                    "Only the system server should try to bind to the network stack.");
-        }
+        mDependencies.checkCallerUid();
 
         if (!mWasSystemServerInitialized) {
             // The network stack is not being started in this process, e.g. this process is not