Refactor Module connectors for testing

Move getModuleServiceIntent to a dependencies object to allow clients to
create test versions of the class that return test intents and avoid the
permissions checks.

Refactor dependencies in NetworkStackClient in a dependencies object for
the same test purposes.

Test: manual: built, booted, WiFi working
Change-Id: I9b115f4cd26f36eee5c669226ea6296b8d7d2d06
This commit is contained in:
Remi NGUYEN VAN
2019-08-27 21:12:46 +09:00
parent 7a71de717b
commit 91e39d9a45
2 changed files with 120 additions and 55 deletions

View File

@@ -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 =

View File

@@ -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;
@@ -50,6 +51,9 @@ public class NetworkStackClient {
private static NetworkStackClient sInstance;
@NonNull
private final Dependencies mDependencies;
@NonNull
@GuardedBy("mPendingNetStackRequests")
private final ArrayList<NetworkStackCallback> mPendingNetStackRequests = new ArrayList<>();
@@ -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