LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Disallow NetworkAgents from changing the owner UID.

Browse Source 
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
This commit is contained in:
Lorenzo Colitti
2020-12-09 18:30:52 +09:00
parent 46fd589d89
commit 9340134026
2 changed files with 10 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
services/core/java/com/android/server/ConnectivityService.java
View File

@@ -6355,6 +6355,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
*/
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. */

28
tests/net/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -1958,7 +1958,7 @@ public class ConnectivityServiceTest {
}
@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 @@ public class ConnectivityServiceTest {
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