Disallow NetworkAgents from changing the owner UID.
The current behaviour with regards to changing the owner UID is
bizarre and arguably incorrect. A NetworkAgent can change the
owner to whatever other app it wants, regardless of signatures,
at any time. This includes, for example, transferring ownership
to another UID and then recovering it.
Fortunately no existing NetworkAgent appears to do this:
- ClientModeImpl sets it to the UID of the app that created the
configuration. It doesn't look like it can change while the
network is connected.
- Vpn sets it to the UID of the VPN owner. That also can't change.
- Telephony does not appear to set it at all, it only sets the
administrator UIDs (and updates them whenever it gets
EVENT_CARRIER_PRIVILEGED_UIDS_CHANGED).
Disallow this now before code is written that depends on it.
Bug: 175188445
Test: modified tests in ConnectivityServiceTest
Change-Id: I638e29fda2481ec3bf4fff562ea66a73322881df
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 65903af..6b3e8a9 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -6355,6 +6355,11 @@
*/
private void processCapabilitiesFromAgent(NetworkAgentInfo nai, NetworkCapabilities nc) {
nai.declaredMetered = !nc.hasCapability(NET_CAPABILITY_NOT_METERED);
+ if (nai.networkCapabilities.getOwnerUid() != nc.getOwnerUid()) {
+ Log.e(TAG, nai.toShortString() + ": ignoring attempt to change owner from "
+ + nai.networkCapabilities.getOwnerUid() + " to " + nc.getOwnerUid());
+ nc.setOwnerUid(nai.networkCapabilities.getOwnerUid());
+ }
}
/** Modifies |caps| based on the capabilities of the specified underlying networks. */
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 2aade42..a4b6e31 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -1958,7 +1958,7 @@
}
@Test
- public void testOwnerUidChangeBug() throws Exception {
+ public void testOwnerUidCannotChange() throws Exception {
// Owner UIDs are not visible without location permission.
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION);
@@ -1973,37 +1973,19 @@
waitForIdle();
// Send ConnectivityService an update to the mWiFiNetworkAgent's capabilities that changes
- // its owner UID.
+ // the owner UID and an unrelated capability.
NetworkCapabilities agentCapabilities = mWiFiNetworkAgent.getNetworkCapabilities();
assertEquals(originalOwnerUid, agentCapabilities.getOwnerUid());
agentCapabilities.setOwnerUid(42);
- mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true);
- waitForIdle();
-
- // Check that the owner UID is updated.
- // The owner UID is -1 because it is visible only to the UID that owns the network.
- NetworkCapabilities nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
- assertEquals(-1, nc.getOwnerUid());
-
- // Make an unrelated change to the capabilities and check it. The owner UID remains -1.
assertFalse(agentCapabilities.hasCapability(NET_CAPABILITY_NOT_CONGESTED));
agentCapabilities.addCapability(NET_CAPABILITY_NOT_CONGESTED);
mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true);
waitForIdle();
- nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
- assertEquals(-1, nc.getOwnerUid());
- assertTrue(nc.hasCapability(NET_CAPABILITY_NOT_CONGESTED));
- // Set the owner back to originalOwnerUid, update the capabilities, and check that it is
- // visible again.
- // TODO: should this even be possible?
- agentCapabilities.setOwnerUid(originalOwnerUid);
- agentCapabilities.removeCapability(NET_CAPABILITY_NOT_CONGESTED);
- mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true);
- waitForIdle();
-
- nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
+ // Check that the capability change has been applied but the owner UID is not modified.
+ NetworkCapabilities nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
assertEquals(originalOwnerUid, nc.getOwnerUid());
+ assertTrue(nc.hasCapability(NET_CAPABILITY_NOT_CONGESTED));
}
@Test