Merge changes I0baf582f,I4bba01ba
* changes:
Block unpriviledged apps which create keepalives with null fd
Fix keepalive don't get removed when lower layer error
diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java
index 123564e..0c55934 100644
--- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java
+++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java
@@ -22,6 +22,7 @@
import static android.Manifest.permission.INTERNET;
import static android.Manifest.permission.NETWORK_STACK;
import static android.Manifest.permission.UPDATE_DEVICE_STATS;
+import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED;
import static android.content.pm.PackageManager.GET_PERMISSIONS;
import static android.content.pm.PackageManager.MATCH_ANY_USER;
import static android.os.Process.INVALID_UID;
@@ -42,13 +43,15 @@
import android.os.RemoteException;
import android.os.UserHandle;
import android.os.UserManager;
+import android.util.ArraySet;
import android.util.Log;
-import android.util.Slog;
+import android.util.SparseArray;
import android.util.SparseIntArray;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.ArrayUtils;
import com.android.server.LocalServices;
+import com.android.server.SystemConfig;
import java.util.ArrayList;
import java.util.HashMap;
@@ -83,41 +86,32 @@
private final Map<Integer, Boolean> mApps = new HashMap<>();
private class PackageListObserver implements PackageManagerInternal.PackageListObserver {
- @Override
- public void onPackageAdded(String packageName, int uid) {
- final PackageInfo app = getPackageInfo(packageName);
- if (app == null) {
- Slog.wtf(TAG, "Failed to get information of installed package: " + packageName);
- return;
- }
- if (uid == INVALID_UID) {
- Slog.wtf(TAG, "Failed to get the uid of installed package: " + packageName
- + "uid: " + uid);
- return;
- }
- if (app.requestedPermissions == null) {
- return;
- }
- sendPackagePermissionsForUid(uid,
- getNetdPermissionMask(app.requestedPermissions));
- }
- @Override
- public void onPackageRemoved(String packageName, int uid) {
+ private int getPermissionForUid(int uid) {
int permission = 0;
- // If there are still packages remain under the same uid, check the permission of the
- // remaining packages. We only remove the permission for a given uid when all packages
- // for that uid no longer have that permission.
+ // Check all the packages for this UID. The UID has the permission if any of the
+ // packages in it has the permission.
String[] packages = mPackageManager.getPackagesForUid(uid);
if (packages != null && packages.length > 0) {
for (String name : packages) {
final PackageInfo app = getPackageInfo(name);
if (app != null && app.requestedPermissions != null) {
- permission |= getNetdPermissionMask(app.requestedPermissions);
+ permission |= getNetdPermissionMask(app.requestedPermissions,
+ app.requestedPermissionsFlags);
}
}
}
- sendPackagePermissionsForUid(uid, permission);
+ return permission;
+ }
+
+ @Override
+ public void onPackageAdded(String packageName, int uid) {
+ sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
+ }
+
+ @Override
+ public void onPackageRemoved(String packageName, int uid) {
+ sendPackagePermissionsForUid(uid, getPermissionForUid(uid));
}
}
@@ -167,12 +161,9 @@
}
//TODO: unify the management of the permissions into one codepath.
- if (app.requestedPermissions != null) {
- int otherNetdPerms = getNetdPermissionMask(app.requestedPermissions);
- if (otherNetdPerms != 0) {
- netdPermsUids.put(uid, netdPermsUids.get(uid) | otherNetdPerms);
- }
- }
+ int otherNetdPerms = getNetdPermissionMask(app.requestedPermissions,
+ app.requestedPermissionsFlags);
+ netdPermsUids.put(uid, netdPermsUids.get(uid) | otherNetdPerms);
}
List<UserInfo> users = mUserManager.getUsers(true); // exclude dying users
@@ -182,6 +173,23 @@
}
}
+ final SparseArray<ArraySet<String>> systemPermission =
+ SystemConfig.getInstance().getSystemPermissions();
+ for (int i = 0; i < systemPermission.size(); i++) {
+ ArraySet<String> perms = systemPermission.valueAt(i);
+ int uid = systemPermission.keyAt(i);
+ int netdPermission = 0;
+ // Get the uids of native services that have UPDATE_DEVICE_STATS permission.
+ if (perms != null) {
+ netdPermission |= perms.contains(UPDATE_DEVICE_STATS)
+ ? INetd.PERMISSION_UPDATE_DEVICE_STATS : 0;
+ }
+ // For internet permission, the native services have their own selinux domains and
+ // sepolicy will control the socket creation during run time. netd cannot block the
+ // socket creation based on the permission information here.
+ netdPermission |= INetd.PERMISSION_INTERNET;
+ netdPermsUids.put(uid, netdPermsUids.get(uid) | netdPermission);
+ }
log("Users: " + mUsers.size() + ", Apps: " + mApps.size());
update(mUsers, mApps, true);
sendPackagePermissionsToNetd(netdPermsUids);
@@ -403,13 +411,17 @@
}
}
- private static int getNetdPermissionMask(String[] requestedPermissions) {
+ private static int getNetdPermissionMask(String[] requestedPermissions,
+ int[] requestedPermissionsFlags) {
int permissions = 0;
- for (String permissionName : requestedPermissions) {
- if (permissionName.equals(INTERNET)) {
+ if (requestedPermissions == null || requestedPermissionsFlags == null) return permissions;
+ for (int i = 0; i < requestedPermissions.length; i++) {
+ if (requestedPermissions[i].equals(INTERNET)
+ && ((requestedPermissionsFlags[i] & REQUESTED_PERMISSION_GRANTED) != 0)) {
permissions |= INetd.PERMISSION_INTERNET;
}
- if (permissionName.equals(UPDATE_DEVICE_STATS)) {
+ if (requestedPermissions[i].equals(UPDATE_DEVICE_STATS)
+ && ((requestedPermissionsFlags[i] & REQUESTED_PERMISSION_GRANTED) != 0)) {
permissions |= INetd.PERMISSION_UPDATE_DEVICE_STATS;
}
}
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 8740f07..6f48da3 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -212,7 +212,6 @@
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.function.Consumer;
import java.util.function.Predicate;
/**
@@ -499,7 +498,6 @@
try {
doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected();
doAnswer(validateAnswer).when(mNetworkMonitor).forceReevaluation(anyInt());
- doAnswer(validateAnswer).when(mNetworkMonitor).setAcceptPartialConnectivity();
} catch (RemoteException e) {
fail(e.getMessage());
}
@@ -2554,8 +2552,7 @@
verifyActiveNetwork(TRANSPORT_CELLULAR);
}
- // TODO(b/128426024): deflake and re-enable
- // @Test
+ @Test
public void testPartialConnectivity() {
// Register network callback.
NetworkRequest request = new NetworkRequest.Builder()
@@ -2579,20 +2576,24 @@
assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
callback.assertNoCallback();
+ // With HTTPS probe disabled, NetworkMonitor should pass the network validation with http
+ // probe.
+ mWiFiNetworkAgent.setNetworkValid();
// If the user chooses yes to use this partial connectivity wifi, switch the default
// network to wifi and check if wifi becomes valid or not.
mCm.setAcceptPartialConnectivity(mWiFiNetworkAgent.getNetwork(), true /* accept */,
false /* always */);
- // With https probe disabled, NetworkMonitor should pass the network validation with http
- // probe.
- mWiFiNetworkAgent.setNetworkValid();
+ // If user accepts partial connectivity network,
+ // NetworkMonitor#setAcceptPartialConnectivity() should be called too.
waitForIdle();
try {
- verify(mWiFiNetworkAgent.mNetworkMonitor,
- timeout(TIMEOUT_MS).times(1)).setAcceptPartialConnectivity();
+ verify(mWiFiNetworkAgent.mNetworkMonitor, times(1)).setAcceptPartialConnectivity();
} catch (RemoteException e) {
fail(e.getMessage());
}
+ // Need a trigger point to let NetworkMonitor tell ConnectivityService that network is
+ // validated.
+ mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
callback.expectCallback(CallbackState.LOSING, mCellNetworkAgent);
NetworkCapabilities nc = callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED,
mWiFiNetworkAgent);
@@ -2622,6 +2623,15 @@
// acceptUnvalidated is also used as setting for accepting partial networks.
mWiFiNetworkAgent.explicitlySelected(true /* acceptUnvalidated */);
mWiFiNetworkAgent.connect(true);
+ // If user accepted partial connectivity network before,
+ // NetworkMonitor#setAcceptPartialConnectivity() will be called in
+ // ConnectivityService#updateNetworkInfo().
+ waitForIdle();
+ try {
+ verify(mWiFiNetworkAgent.mNetworkMonitor, times(1)).setAcceptPartialConnectivity();
+ } catch (RemoteException e) {
+ fail(e.getMessage());
+ }
callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
callback.expectCallback(CallbackState.LOSING, mCellNetworkAgent);
nc = callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
@@ -2636,23 +2646,33 @@
// NET_CAPABILITY_PARTIAL_CONNECTIVITY.
mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);
mWiFiNetworkAgent.explicitlySelected(true /* acceptUnvalidated */);
+ // Current design cannot send multi-testResult from NetworkMonitor to ConnectivityService.
+ // So, if user accepts partial connectivity, NetworkMonitor will send PARTIAL_CONNECTIVITY
+ // to ConnectivityService first then send VALID. Once NetworkMonitor support
+ // multi-testResult, this test case also need to be changed to meet the new design.
mWiFiNetworkAgent.connectWithPartialConnectivity();
- callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
- // TODO: If the user accepted partial connectivity, we shouldn't switch to wifi until
- // NetworkMonitor detects partial connectivity
- assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
- mWiFiNetworkAgent.setNetworkValid();
+ // If user accepted partial connectivity network before,
+ // NetworkMonitor#setAcceptPartialConnectivity() will be called in
+ // ConnectivityService#updateNetworkInfo().
waitForIdle();
try {
- verify(mWiFiNetworkAgent.mNetworkMonitor,
- timeout(TIMEOUT_MS).times(1)).setAcceptPartialConnectivity();
+ verify(mWiFiNetworkAgent.mNetworkMonitor, times(1)).setAcceptPartialConnectivity();
} catch (RemoteException e) {
fail(e.getMessage());
}
+ callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
callback.expectCallback(CallbackState.LOSING, mCellNetworkAgent);
- callback.expectCapabilitiesWith(NET_CAPABILITY_PARTIAL_CONNECTIVITY, mWiFiNetworkAgent);
- // Wifi should be the default network.
+ // TODO: If the user accepted partial connectivity, we shouldn't switch to wifi until
+ // NetworkMonitor detects partial connectivity
assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
+ callback.expectCapabilitiesWith(NET_CAPABILITY_PARTIAL_CONNECTIVITY, mWiFiNetworkAgent);
+ mWiFiNetworkAgent.setNetworkValid();
+ // Need a trigger point to let NetworkMonitor tell ConnectivityService that network is
+ // validated.
+ mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
+ callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
+ mWiFiNetworkAgent.disconnect();
+ callback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent);
}
@Test
@@ -4022,8 +4042,13 @@
callback3.expectStopped();
}
+ @FunctionalInterface
+ private interface ThrowingConsumer<T> {
+ void accept(T t) throws Exception;
+ }
+
// Helper method to prepare the executor and run test
- private void runTestWithSerialExecutors(Consumer<Executor> functor) {
+ private void runTestWithSerialExecutors(ThrowingConsumer<Executor> functor) throws Exception {
final ExecutorService executorSingleThread = Executors.newSingleThreadExecutor();
final Executor executorInline = (Runnable r) -> r.run();
functor.accept(executorSingleThread);
@@ -4032,15 +4057,9 @@
}
@Test
- public void testNattSocketKeepalives() {
- runTestWithSerialExecutors(executor -> {
- try {
- doTestNattSocketKeepalivesWithExecutor(executor);
- doTestNattSocketKeepalivesFdWithExecutor(executor);
- } catch (Exception e) {
- fail(e.getMessage());
- }
- });
+ public void testNattSocketKeepalives() throws Exception {
+ runTestWithSerialExecutors(executor -> doTestNattSocketKeepalivesWithExecutor(executor));
+ runTestWithSerialExecutors(executor -> doTestNattSocketKeepalivesFdWithExecutor(executor));
}
private void doTestNattSocketKeepalivesWithExecutor(Executor executor) throws Exception {
@@ -4215,14 +4234,8 @@
}
@Test
- public void testTcpSocketKeepalives() {
- runTestWithSerialExecutors(executor -> {
- try {
- doTestTcpSocketKeepalivesWithExecutor(executor);
- } catch (Exception e) {
- fail(e.getMessage());
- }
- });
+ public void testTcpSocketKeepalives() throws Exception {
+ runTestWithSerialExecutors(executor -> doTestTcpSocketKeepalivesWithExecutor(executor));
}
private void doTestTcpSocketKeepalivesWithExecutor(Executor executor) throws Exception {