diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index f1b5d3298b..0c46462f98 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -1279,7 +1279,8 @@ public class ConnectivityManager {
@UnsupportedAppUsage
public NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(int userId) {
try {
- return mService.getDefaultNetworkCapabilitiesForUser(userId);
+ return mService.getDefaultNetworkCapabilitiesForUser(
+ userId, mContext.getOpPackageName());
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -1361,7 +1362,7 @@ public class ConnectivityManager {
@Nullable
public NetworkCapabilities getNetworkCapabilities(@Nullable Network network) {
try {
- return mService.getNetworkCapabilities(network);
+ return mService.getNetworkCapabilities(network, mContext.getOpPackageName());
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -4036,10 +4037,9 @@ public class ConnectivityManager {
@NonNull PendingIntent operation) {
printStackTrace();
checkPendingIntentNotNull(operation);
- final String callingPackageName = mContext.getOpPackageName();
try {
mService.pendingRequestForNetwork(
- request.networkCapabilities, operation, callingPackageName);
+ request.networkCapabilities, operation, mContext.getOpPackageName());
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
} catch (ServiceSpecificException e) {
@@ -4151,10 +4151,9 @@ public class ConnectivityManager {
@NonNull PendingIntent operation) {
printStackTrace();
checkPendingIntentNotNull(operation);
- final String callingPackageName = mContext.getOpPackageName();
try {
mService.pendingListenForNetwork(
- request.networkCapabilities, operation, callingPackageName);
+ request.networkCapabilities, operation, mContext.getOpPackageName());
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
} catch (ServiceSpecificException e) {
diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl
index 3a55461a77..14345608e9 100644
--- a/core/java/android/net/IConnectivityManager.aidl
+++ b/core/java/android/net/IConnectivityManager.aidl
@@ -59,7 +59,8 @@ interface IConnectivityManager
NetworkInfo[] getAllNetworkInfo();
Network getNetworkForType(int networkType);
Network[] getAllNetworks();
- NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(int userId);
+ NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(
+ int userId, String callingPackageName);
boolean isNetworkSupported(int networkType);
@@ -68,7 +69,7 @@ interface IConnectivityManager
LinkProperties getLinkPropertiesForType(int networkType);
LinkProperties getLinkProperties(in Network network);
- NetworkCapabilities getNetworkCapabilities(in Network network);
+ NetworkCapabilities getNetworkCapabilities(in Network network, String callingPackageName);
@UnsupportedAppUsage
NetworkState[] getAllNetworkState();
diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java
index ef4a9e5f3b..873d6e9146 100644
--- a/core/java/android/net/NetworkCapabilities.java
+++ b/core/java/android/net/NetworkCapabilities.java
@@ -830,6 +830,23 @@ public final class NetworkCapabilities implements Parcelable {
* This field keeps track of the UID of the app that created this network and is in charge of
* its lifecycle. This could be the UID of apps such as the Wifi network suggestor, the running
* VPN, or Carrier Service app managing a cellular data connection.
+ *
+ *
For NetworkCapability instances being sent from ConnectivityService, this value MUST be
+ * reset to Process.INVALID_UID unless all the following conditions are met:
+ *
+ *
+ * - The destination app is the network owner
+ *
- The destination app has the ACCESS_FINE_LOCATION permission granted
+ *
- The user's location toggle is on
+ *
+ *
+ * This is because the owner UID is location-sensitive. The apps that request a network could
+ * know where the device is if they can tell for sure the system has connected to the network
+ * they requested.
+ *
+ * This is populated by the network agents and for the NetworkCapabilities instance sent by
+ * an app to the System Server, the value MUST be reset to Process.INVALID_UID by the system
+ * server.
*/
private int mOwnerUid = Process.INVALID_UID;
@@ -842,7 +859,16 @@ public final class NetworkCapabilities implements Parcelable {
}
/**
- * Retrieves the UID of the owner app.
+ * Retrieves the UID of the app that owns this network.
+ *
+ *
For user privacy reasons, this field will only be populated if:
+ *
+ *
+ * - The calling app is the network owner
+ *
- The calling app has the ACCESS_FINE_LOCATION permission granted
+ *
- The user's location toggle is on
+ *
+ *
*/
public int getOwnerUid() {
return mOwnerUid;
@@ -880,8 +906,9 @@ public final class NetworkCapabilities implements Parcelable {
* @param administratorUids the UIDs to be set as administrators of this Network.
* @hide
*/
+ @NonNull
@SystemApi
- public @NonNull NetworkCapabilities setAdministratorUids(
+ public NetworkCapabilities setAdministratorUids(
@NonNull final List administratorUids) {
mAdministratorUids.clear();
mAdministratorUids.addAll(administratorUids);
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 52a2ca974d..1eb77a4963 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -1536,7 +1536,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
@Override
- public NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(int userId) {
+ public NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(
+ int userId, String callingPackageName) {
// The basic principle is: if an app's traffic could possibly go over a
// network, without the app doing anything multinetwork-specific,
// (hence, by "default"), then include that network's capabilities in
@@ -1558,7 +1559,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
NetworkAgentInfo nai = getDefaultNetwork();
NetworkCapabilities nc = getNetworkCapabilitiesInternal(nai);
if (nc != null) {
- result.put(nai.network, nc);
+ result.put(
+ nai.network,
+ maybeSanitizeLocationInfoForCaller(
+ nc, Binder.getCallingUid(), callingPackageName));
}
synchronized (mVpns) {
@@ -1568,10 +1572,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
Network[] networks = vpn.getUnderlyingNetworks();
if (networks != null) {
for (Network network : networks) {
- nai = getNetworkAgentInfoForNetwork(network);
- nc = getNetworkCapabilitiesInternal(nai);
+ nc = getNetworkCapabilitiesInternal(network);
if (nc != null) {
- result.put(network, nc);
+ result.put(
+ network,
+ maybeSanitizeLocationInfoForCaller(
+ nc, Binder.getCallingUid(), callingPackageName));
}
}
}
@@ -1638,20 +1644,26 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
}
+ private NetworkCapabilities getNetworkCapabilitiesInternal(Network network) {
+ return getNetworkCapabilitiesInternal(getNetworkAgentInfoForNetwork(network));
+ }
+
private NetworkCapabilities getNetworkCapabilitiesInternal(NetworkAgentInfo nai) {
if (nai == null) return null;
synchronized (nai) {
if (nai.networkCapabilities == null) return null;
return networkCapabilitiesRestrictedForCallerPermissions(
- nai.networkCapabilities,
- Binder.getCallingPid(), Binder.getCallingUid());
+ nai.networkCapabilities, Binder.getCallingPid(), Binder.getCallingUid());
}
}
@Override
- public NetworkCapabilities getNetworkCapabilities(Network network) {
+ public NetworkCapabilities getNetworkCapabilities(Network network, String callingPackageName) {
+ mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackageName);
enforceAccessPermission();
- return getNetworkCapabilitiesInternal(getNetworkAgentInfoForNetwork(network));
+ return maybeSanitizeLocationInfoForCaller(
+ getNetworkCapabilitiesInternal(network),
+ Binder.getCallingUid(), callingPackageName);
}
@VisibleForTesting
@@ -1667,20 +1679,34 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
newNc.setAdministratorUids(Collections.EMPTY_LIST);
- maybeSanitizeLocationInfoForCaller(newNc, callerUid);
-
return newNc;
}
- private void maybeSanitizeLocationInfoForCaller(
- NetworkCapabilities nc, int callerUid) {
- // TODO(b/142072839): Conditionally reset the owner UID if the following
- // conditions are not met:
- // 1. The destination app is the network owner
- // 2. The destination app has the ACCESS_COARSE_LOCATION permission granted
- // if target SDK<29 or otherwise has the ACCESS_FINE_LOCATION permission granted
- // 3. The user's location toggle is on
- nc.setOwnerUid(INVALID_UID);
+ @VisibleForTesting
+ @Nullable
+ NetworkCapabilities maybeSanitizeLocationInfoForCaller(
+ @Nullable NetworkCapabilities nc, int callerUid, @NonNull String callerPkgName) {
+ if (nc == null) {
+ return null;
+ }
+ final NetworkCapabilities newNc = new NetworkCapabilities(nc);
+ if (callerUid != newNc.getOwnerUid()) {
+ newNc.setOwnerUid(INVALID_UID);
+ return newNc;
+ }
+
+ Binder.withCleanCallingIdentity(
+ () -> {
+ if (!mLocationPermissionChecker.checkLocationPermission(
+ callerPkgName, null /* featureId */, callerUid, null /* message */)) {
+ // Caller does not have the requisite location permissions. Reset the
+ // owner's UID in the NetworkCapabilities.
+ newNc.setOwnerUid(INVALID_UID);
+ }
+ }
+ );
+
+ return newNc;
}
private LinkProperties linkPropertiesRestrictedForCallerPermissions(
@@ -1755,7 +1781,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
public boolean isActiveNetworkMetered() {
enforceAccessPermission();
- final NetworkCapabilities caps = getNetworkCapabilities(getActiveNetwork());
+ final NetworkCapabilities caps = getNetworkCapabilitiesInternal(getActiveNetwork());
if (caps != null) {
return !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED);
} else {
@@ -5322,8 +5348,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
public String toString() {
- return "uid/pid:" + mUid + "/" + mPid + " " + request +
- (mPendingIntent == null ? "" : " to trigger " + mPendingIntent);
+ return "uid/pid:" + mUid + "/" + mPid + " " + request
+ + (mPendingIntent == null ? "" : " to trigger " + mPendingIntent);
}
}
@@ -6416,8 +6442,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
switch (notificationType) {
case ConnectivityManager.CALLBACK_AVAILABLE: {
- putParcelable(bundle, networkCapabilitiesRestrictedForCallerPermissions(
- networkAgent.networkCapabilities, nri.mPid, nri.mUid));
+ final NetworkCapabilities nc =
+ networkCapabilitiesRestrictedForCallerPermissions(
+ networkAgent.networkCapabilities, nri.mPid, nri.mUid);
+ putParcelable(
+ bundle,
+ maybeSanitizeLocationInfoForCaller(
+ nc, nri.mUid, nri.request.getRequestorPackageName()));
putParcelable(bundle, linkPropertiesRestrictedForCallerPermissions(
networkAgent.linkProperties, nri.mPid, nri.mUid));
// For this notification, arg1 contains the blocked status.
@@ -6430,9 +6461,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
case ConnectivityManager.CALLBACK_CAP_CHANGED: {
// networkAgent can't be null as it has been accessed a few lines above.
- final NetworkCapabilities nc = networkCapabilitiesRestrictedForCallerPermissions(
- networkAgent.networkCapabilities, nri.mPid, nri.mUid);
- putParcelable(bundle, nc);
+ final NetworkCapabilities netCap =
+ networkCapabilitiesRestrictedForCallerPermissions(
+ networkAgent.networkCapabilities, nri.mPid, nri.mUid);
+ putParcelable(
+ bundle,
+ maybeSanitizeLocationInfoForCaller(
+ netCap, nri.mUid, nri.request.getRequestorPackageName()));
break;
}
case ConnectivityManager.CALLBACK_IP_CHANGED: {
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 77147c8a35..86ba8afe0c 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -1180,6 +1180,10 @@ public class ConnectivityServiceTest {
Arrays.asList(new UserInfo[] {
new UserInfo(VPN_USER, "", 0),
}));
+ final ApplicationInfo applicationInfo = new ApplicationInfo();
+ applicationInfo.targetSdkVersion = Build.VERSION_CODES.Q;
+ when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
+ .thenReturn(applicationInfo);
// InstrumentationTestRunner prepares a looper, but AndroidJUnitRunner does not.
// http://b/25897652 .
@@ -3041,7 +3045,7 @@ public class ConnectivityServiceTest {
networkCapabilities.addTransportType(TRANSPORT_WIFI)
.setNetworkSpecifier(new MatchAllNetworkSpecifier());
mService.requestNetwork(networkCapabilities, null, 0, null,
- ConnectivityManager.TYPE_WIFI, TEST_PACKAGE_NAME);
+ ConnectivityManager.TYPE_WIFI, mContext.getPackageName());
});
class NonParcelableSpecifier extends NetworkSpecifier {
@@ -6438,17 +6442,89 @@ public class ConnectivityServiceTest {
assertEquals(wifiLp, mService.getActiveLinkProperties());
}
+ private void setupLocationPermissions(
+ int targetSdk, boolean locationToggle, String op, String perm) throws Exception {
+ final ApplicationInfo applicationInfo = new ApplicationInfo();
+ applicationInfo.targetSdkVersion = targetSdk;
+ when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
+ .thenReturn(applicationInfo);
+
+ when(mLocationManager.isLocationEnabledForUser(any())).thenReturn(locationToggle);
+
+ if (op != null) {
+ when(mAppOpsManager.noteOp(eq(op), eq(Process.myUid()), eq(mContext.getPackageName())))
+ .thenReturn(AppOpsManager.MODE_ALLOWED);
+ }
+
+ if (perm != null) {
+ mServiceContext.setPermission(perm, PERMISSION_GRANTED);
+ }
+ }
+
+ private int getOwnerUidNetCapsForCallerPermission(int ownerUid, int callerUid) {
+ final NetworkCapabilities netCap = new NetworkCapabilities().setOwnerUid(ownerUid);
+
+ return mService
+ .maybeSanitizeLocationInfoForCaller(netCap, callerUid, mContext.getPackageName())
+ .getOwnerUid();
+ }
+
@Test
- public void testNetworkCapabilitiesRestrictedForCallerPermissions() {
- int callerUid = Process.myUid();
- final NetworkCapabilities originalNc = new NetworkCapabilities();
- originalNc.setOwnerUid(callerUid);
+ public void testMaybeSanitizeLocationInfoForCallerWithFineLocationAfterQ() throws Exception {
+ setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
+ Manifest.permission.ACCESS_FINE_LOCATION);
- final NetworkCapabilities newNc =
- mService.networkCapabilitiesRestrictedForCallerPermissions(
- originalNc, Process.myPid(), callerUid);
+ final int myUid = Process.myUid();
+ assertEquals(myUid, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+ }
- assertEquals(Process.INVALID_UID, newNc.getOwnerUid());
+ @Test
+ public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationPreQ() throws Exception {
+ setupLocationPermissions(Build.VERSION_CODES.P, true, AppOpsManager.OPSTR_COARSE_LOCATION,
+ Manifest.permission.ACCESS_COARSE_LOCATION);
+
+ final int myUid = Process.myUid();
+ assertEquals(myUid, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+ }
+
+ @Test
+ public void testMaybeSanitizeLocationInfoForCallerLocationOff() throws Exception {
+ // Test that even with fine location permission, and UIDs matching, the UID is sanitized.
+ setupLocationPermissions(Build.VERSION_CODES.Q, false, AppOpsManager.OPSTR_FINE_LOCATION,
+ Manifest.permission.ACCESS_FINE_LOCATION);
+
+ final int myUid = Process.myUid();
+ assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+ }
+
+ @Test
+ public void testMaybeSanitizeLocationInfoForCallerWrongUid() throws Exception {
+ // Test that even with fine location permission, not being the owner leads to sanitization.
+ setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
+ Manifest.permission.ACCESS_FINE_LOCATION);
+
+ final int myUid = Process.myUid();
+ assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid + 1, myUid));
+ }
+
+ @Test
+ public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationAfterQ() throws Exception {
+ // Test that not having fine location permission leads to sanitization.
+ setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_COARSE_LOCATION,
+ Manifest.permission.ACCESS_COARSE_LOCATION);
+
+ // Test that without the location permission, the owner field is sanitized.
+ final int myUid = Process.myUid();
+ assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+ }
+
+ @Test
+ public void testMaybeSanitizeLocationInfoForCallerWithoutLocationPermission() throws Exception {
+ setupLocationPermissions(Build.VERSION_CODES.Q, true, null /* op */, null /* perm */);
+
+ // Test that without the location permission, the owner field is sanitized.
+ final int myUid = Process.myUid();
+ assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
}
private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType)
@@ -6734,21 +6810,6 @@ public class ConnectivityServiceTest {
mContext.getOpPackageName()));
}
- private void setupLocationPermissions(
- int targetSdk, boolean locationToggle, String op, String perm) throws Exception {
- final ApplicationInfo applicationInfo = new ApplicationInfo();
- applicationInfo.targetSdkVersion = targetSdk;
- when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
- .thenReturn(applicationInfo);
-
- when(mLocationManager.isLocationEnabledForUser(any())).thenReturn(locationToggle);
-
- when(mAppOpsManager.noteOp(eq(op), eq(Process.myUid()), eq(mContext.getPackageName())))
- .thenReturn(AppOpsManager.MODE_ALLOWED);
-
- mServiceContext.setPermission(perm, PERMISSION_GRANTED);
- }
-
private void setUpConnectivityDiagnosticsCallback() throws Exception {
final NetworkRequest request = new NetworkRequest.Builder().build();
when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder);