NetworkCapabilities: Embed location senstive TransportInfo

Changes:
i) Add a new constructor for NetworkCapabilities which accepts whether
location sensitive fields need to be parceled or not. Defalts to false
on the other constructor. This boolean should only be set on the copy of
NetworkCapabilities when sent to apps that hold location permission.
(Similar to how sensitive fields are handled in LinkProperties)
ii) Add a new makeCopy() method in the TransportInfo interface which
accepts whether location sensitive fields need to be parceled or not.
iii) Migrate the existing NetworkCapabilities owner UID masking to use
this new mechanism (instead of existing masking in ConnectivityService).
iv) Always set parcelLocationSensitiveFields to true in the NetworkAgent
surface (since that is a privileged surface from the transports to the
connectivity service)
v) Add a hasSensitiveFields() in TransportInfo interface to avoid
perfoming location permission checks for location insensitive
TrasnsportInfo.

Also, migrate to the new SdkLevel util for isAtLeastR() & isAtLeastS()
checks.

Bug: 162602799
Test: atest android.net
Test: atest com.android.server
Change-Id: Ie522d8c75a82ae521ccfd5165823d0c72642e651
Merged-In: Ie522d8c75a82ae521ccfd5165823d0c72642e651
This commit is contained in:
Roshan Pius
2020-12-28 09:01:49 -08:00
parent d895e5a193
commit 9ed1462e82
7 changed files with 235 additions and 49 deletions

View File

@@ -408,7 +408,8 @@ public abstract class NetworkAgent {
throw new IllegalArgumentException();
}
mInitialConfiguration = new InitialConfiguration(context, new NetworkCapabilities(nc),
mInitialConfiguration = new InitialConfiguration(context,
new NetworkCapabilities(nc, /* parcelLocationSensitiveFields */ true),
new LinkProperties(lp), score, config, ni);
}
@@ -818,7 +819,9 @@ public abstract class NetworkAgent {
Objects.requireNonNull(networkCapabilities);
mBandwidthUpdatePending.set(false);
mLastBwRefreshTime = System.currentTimeMillis();
final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities);
final NetworkCapabilities nc =
new NetworkCapabilities(networkCapabilities,
/* parcelLocationSensitiveFields */ true);
queueOrSendMessage(reg -> reg.sendNetworkCapabilities(nc));
}

View File

@@ -76,12 +76,33 @@ public final class NetworkCapabilities implements Parcelable {
*/
private String mRequestorPackageName;
/**
* Indicates whether parceling should preserve fields that are set based on permissions of
* the process receiving the {@link NetworkCapabilities}.
*/
private final boolean mParcelLocationSensitiveFields;
public NetworkCapabilities() {
mParcelLocationSensitiveFields = false;
clearAll();
mNetworkCapabilities = DEFAULT_CAPABILITIES;
}
public NetworkCapabilities(NetworkCapabilities nc) {
this(nc, false /* parcelLocationSensitiveFields */);
}
/**
* Make a copy of NetworkCapabilities.
*
* @param nc Original NetworkCapabilities
* @param parcelLocationSensitiveFields Whether to parcel location sensitive data or not.
* @hide
*/
@SystemApi
public NetworkCapabilities(
@Nullable NetworkCapabilities nc, boolean parcelLocationSensitiveFields) {
mParcelLocationSensitiveFields = parcelLocationSensitiveFields;
if (nc != null) {
set(nc);
}
@@ -93,6 +114,12 @@ public final class NetworkCapabilities implements Parcelable {
* @hide
*/
public void clearAll() {
// Ensures that the internal copies maintained by the connectivity stack does not set
// this bit.
if (mParcelLocationSensitiveFields) {
throw new UnsupportedOperationException(
"Cannot clear NetworkCapabilities when parcelLocationSensitiveFields is set");
}
mNetworkCapabilities = mTransportTypes = mUnwantedNetworkCapabilities = 0;
mLinkUpBandwidthKbps = mLinkDownBandwidthKbps = LINK_BANDWIDTH_UNSPECIFIED;
mNetworkSpecifier = null;
@@ -109,6 +136,8 @@ public final class NetworkCapabilities implements Parcelable {
/**
* Set all contents of this object to the contents of a NetworkCapabilities.
*
* @param nc Original NetworkCapabilities
* @hide
*/
public void set(@NonNull NetworkCapabilities nc) {
@@ -117,7 +146,11 @@ public final class NetworkCapabilities implements Parcelable {
mLinkUpBandwidthKbps = nc.mLinkUpBandwidthKbps;
mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps;
mNetworkSpecifier = nc.mNetworkSpecifier;
mTransportInfo = nc.mTransportInfo;
if (nc.getTransportInfo() != null) {
setTransportInfo(nc.getTransportInfo().makeCopy(mParcelLocationSensitiveFields));
} else {
setTransportInfo(null);
}
mSignalStrength = nc.mSignalStrength;
setUids(nc.mUids); // Will make the defensive copy
setAdministratorUids(nc.getAdministratorUids());

View File

@@ -16,10 +16,48 @@
package android.net;
import android.annotation.NonNull;
import android.annotation.SystemApi;
/**
* A container for transport-specific capabilities which is returned by
* {@link NetworkCapabilities#getTransportInfo()}. Specific networks
* may provide concrete implementations of this interface.
* @see android.net.wifi.aware.WifiAwareNetworkInfo
* @see android.net.wifi.WifiInfo
*/
public interface TransportInfo {
/**
* Create a copy of a {@link TransportInfo} that will preserve location sensitive fields that
* were set based on the permissions of the process that originally received it.
*
* <p>By default {@link TransportInfo} does not preserve such fields during parceling, as
* they should not be shared outside of the process that receives them without appropriate
* checks.
*
* @param parcelLocationSensitiveFields Whether the location sensitive fields should be kept
* when parceling
* @return Copy of this instance.
* @hide
*/
@SystemApi
@NonNull
default TransportInfo makeCopy(boolean parcelLocationSensitiveFields) {
return this;
}
/**
* Returns whether this TransportInfo type has location sensitive fields or not (helps
* to determine whether to perform a location permission check or not before sending to
* apps).
*
* @return {@code true} if this instance contains location sensitive info, {@code false}
* otherwise.
* @hide
*/
@SystemApi
default boolean hasLocationSensitiveFields() {
return false;
}
}

View File

@@ -1557,7 +1557,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
if (nc != null) {
result.put(
nai.network,
maybeSanitizeLocationInfoForCaller(
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
nc, mDeps.getCallingUid(), callingPackageName));
}
@@ -1567,7 +1567,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
for (Network network : networks) {
nc = getNetworkCapabilitiesInternal(network);
if (nc != null) {
result.put(network, maybeSanitizeLocationInfoForCaller(
result.put(
network,
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
nc, mDeps.getCallingUid(), callingPackageName));
}
}
@@ -1649,7 +1651,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
public NetworkCapabilities getNetworkCapabilities(Network network, String callingPackageName) {
mAppOpsManager.checkPackage(mDeps.getCallingUid(), callingPackageName);
enforceAccessPermission();
return maybeSanitizeLocationInfoForCaller(
return createWithLocationInfoSanitizedIfNecessaryWhenParceled(
getNetworkCapabilitiesInternal(network),
mDeps.getCallingUid(), callingPackageName);
}
@@ -1670,37 +1672,51 @@ public class ConnectivityService extends IConnectivityManager.Stub
return newNc;
}
private boolean hasLocationPermission(int callerUid, @NonNull String callerPkgName) {
final long token = Binder.clearCallingIdentity();
try {
return mLocationPermissionChecker.checkLocationPermission(
callerPkgName, null /* featureId */, callerUid, null /* message */);
} finally {
Binder.restoreCallingIdentity(token);
}
}
@VisibleForTesting
@Nullable
NetworkCapabilities maybeSanitizeLocationInfoForCaller(
NetworkCapabilities createWithLocationInfoSanitizedIfNecessaryWhenParceled(
@Nullable NetworkCapabilities nc, int callerUid, @NonNull String callerPkgName) {
if (nc == null) {
return null;
}
final NetworkCapabilities newNc = new NetworkCapabilities(nc);
if (callerUid != newNc.getOwnerUid()) {
Boolean hasLocationPermission = null;
final NetworkCapabilities newNc;
// Avoid doing location permission check if the transport info has no location sensitive
// data.
if (nc.getTransportInfo() != null && nc.getTransportInfo().hasLocationSensitiveFields()) {
hasLocationPermission = hasLocationPermission(callerUid, callerPkgName);
newNc = new NetworkCapabilities(nc, hasLocationPermission);
} else {
newNc = new NetworkCapabilities(nc, false /* parcelLocationSensitiveFields */);
}
// Reset owner uid if not destined for the owner app.
if (callerUid != nc.getOwnerUid()) {
newNc.setOwnerUid(INVALID_UID);
return newNc;
}
// Allow VPNs to see ownership of their own VPN networks - not location sensitive.
if (nc.hasTransport(TRANSPORT_VPN)) {
// Owner UIDs already checked above. No need to re-check.
return newNc;
}
final long token = Binder.clearCallingIdentity();
try {
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);
}
} finally {
Binder.restoreCallingIdentity(token);
if (hasLocationPermission == null) {
// Location permission not checked yet, check now for masking owner UID.
hasLocationPermission = hasLocationPermission(callerUid, callerPkgName);
}
// Reset owner uid if the app has no location permission.
if (!hasLocationPermission) {
newNc.setOwnerUid(INVALID_UID);
}
return newNc;
}
@@ -6839,7 +6855,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
networkAgent.networkCapabilities, nri.mPid, nri.mUid);
putParcelable(
bundle,
maybeSanitizeLocationInfoForCaller(
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
nc, nri.mUid, nri.request.getRequestorPackageName()));
putParcelable(bundle, linkPropertiesRestrictedForCallerPermissions(
networkAgent.linkProperties, nri.mPid, nri.mUid));
@@ -6858,7 +6874,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
networkAgent.networkCapabilities, nri.mPid, nri.mUid);
putParcelable(
bundle,
maybeSanitizeLocationInfoForCaller(
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
netCap, nri.mUid, nri.request.getRequestorPackageName()));
break;
}

View File

@@ -24,6 +24,7 @@ java_library {
"androidx.test.rules",
"junit",
"mockito-target-minus-junit4",
"modules-utils-build",
"net-tests-utils",
"net-utils-framework-common",
"platform-test-annotations",

View File

@@ -42,9 +42,11 @@ import static android.net.NetworkCapabilities.TRANSPORT_VPN;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI_AWARE;
import static android.net.NetworkCapabilities.UNRESTRICTED_CAPABILITIES;
import static android.os.Process.INVALID_UID;
import static com.android.testutils.ParcelUtils.assertParcelSane;
import static com.android.testutils.ParcelUtils.assertParcelingIsLossless;
import static com.android.testutils.ParcelUtils.parcelingRoundTrip;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
@@ -53,18 +55,19 @@ import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeTrue;
import android.net.wifi.WifiInfo;
import android.net.wifi.aware.DiscoverySession;
import android.net.wifi.aware.PeerHandle;
import android.net.wifi.aware.WifiAwareNetworkSpecifier;
import android.os.Build;
import android.os.Process;
import android.test.suitebuilder.annotation.SmallTest;
import android.util.ArraySet;
import androidx.core.os.BuildCompat;
import androidx.test.runner.AndroidJUnit4;
import com.android.modules.utils.build.SdkLevel;
import com.android.testutils.DevSdkIgnoreRule;
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
@@ -89,10 +92,11 @@ public class NetworkCapabilitiesTest {
private PeerHandle mPeerHandle = Mockito.mock(PeerHandle.class);
private boolean isAtLeastR() {
// BuildCompat.isAtLeastR() is used to check the Android version before releasing Android R.
// Build.VERSION.SDK_INT > Build.VERSION_CODES.Q is used to check the Android version after
// releasing Android R.
return BuildCompat.isAtLeastR() || Build.VERSION.SDK_INT > Build.VERSION_CODES.Q;
return SdkLevel.isAtLeastR();
}
private boolean isAtLeastS() {
return SdkLevel.isAtLeastS();
}
@Test
@@ -324,8 +328,59 @@ public class NetworkCapabilitiesTest {
testParcelSane(netCap);
}
private NetworkCapabilities createNetworkCapabilitiesWithWifiInfo() {
// uses a real WifiInfo to test parceling of sensitive data.
final WifiInfo wifiInfo = new WifiInfo.Builder()
.setSsid("sssid1234".getBytes())
.setBssid("00:11:22:33:44:55")
.build();
return new NetworkCapabilities()
.addCapability(NET_CAPABILITY_INTERNET)
.addCapability(NET_CAPABILITY_EIMS)
.addCapability(NET_CAPABILITY_NOT_METERED)
.setSSID(TEST_SSID)
.setTransportInfo(wifiInfo)
.setRequestorPackageName("com.android.test")
.setRequestorUid(9304);
}
@Test
public void testParcelNetworkCapabilitiesWithLocationSensitiveFields() {
assumeTrue(isAtLeastS());
final NetworkCapabilities netCap = createNetworkCapabilitiesWithWifiInfo();
final NetworkCapabilities netCapWithLocationSensitiveFields =
new NetworkCapabilities(netCap, true);
assertParcelingIsLossless(netCapWithLocationSensitiveFields);
testParcelSane(netCapWithLocationSensitiveFields);
assertEquals(netCapWithLocationSensitiveFields,
parcelingRoundTrip(netCapWithLocationSensitiveFields));
}
@Test
public void testParcelNetworkCapabilitiesWithoutLocationSensitiveFields() {
assumeTrue(isAtLeastS());
final NetworkCapabilities netCap = createNetworkCapabilitiesWithWifiInfo();
final NetworkCapabilities netCapWithoutLocationSensitiveFields =
new NetworkCapabilities(netCap, false);
final NetworkCapabilities sanitizedNetCap =
new NetworkCapabilities(netCapWithoutLocationSensitiveFields);
final WifiInfo sanitizedWifiInfo = new WifiInfo.Builder()
.setSsid(new byte[0])
.setBssid(WifiInfo.DEFAULT_MAC_ADDRESS)
.build();
sanitizedNetCap.setTransportInfo(sanitizedWifiInfo);
assertEquals(sanitizedNetCap, parcelingRoundTrip(netCapWithoutLocationSensitiveFields));
}
private void testParcelSane(NetworkCapabilities cap) {
if (isAtLeastR()) {
if (isAtLeastS()) {
assertParcelSane(cap, 16);
} else if (isAtLeastR()) {
assertParcelSane(cap, 15);
} else {
assertParcelSane(cap, 11);
@@ -639,26 +694,23 @@ public class NetworkCapabilitiesTest {
// Sequence 1: Transport + Transport + TransportInfo
NetworkCapabilities nc1 = new NetworkCapabilities();
nc1.addTransportType(TRANSPORT_CELLULAR).addTransportType(TRANSPORT_WIFI)
.setTransportInfo(new TransportInfo() {});
.setTransportInfo(new TestTransportInfo());
// Sequence 2: Transport + NetworkSpecifier + Transport
NetworkCapabilities nc2 = new NetworkCapabilities();
nc2.addTransportType(TRANSPORT_CELLULAR).setTransportInfo(new TransportInfo() {})
nc2.addTransportType(TRANSPORT_CELLULAR).setTransportInfo(new TestTransportInfo())
.addTransportType(TRANSPORT_WIFI);
}
@Test
public void testCombineTransportInfo() {
NetworkCapabilities nc1 = new NetworkCapabilities();
nc1.setTransportInfo(new TransportInfo() {
// empty
});
nc1.setTransportInfo(new TestTransportInfo());
NetworkCapabilities nc2 = new NetworkCapabilities();
// new TransportInfo so that object is not #equals to nc1's TransportInfo (that's where
// combine fails)
nc2.setTransportInfo(new TransportInfo() {
// empty
});
nc2.setTransportInfo(new TestTransportInfo());
try {
nc1.combineCapabilities(nc2);
@@ -761,7 +813,7 @@ public class NetworkCapabilitiesTest {
// Test default owner uid.
// If the owner uid is not set, the default value should be Process.INVALID_UID.
final NetworkCapabilities nc1 = new NetworkCapabilities.Builder().build();
assertEquals(Process.INVALID_UID, nc1.getOwnerUid());
assertEquals(INVALID_UID, nc1.getOwnerUid());
// Test setAdministratorUids and getAdministratorUids.
final int[] administratorUids = {1001, 10001};
final NetworkCapabilities nc2 = new NetworkCapabilities.Builder()
@@ -906,6 +958,16 @@ public class NetworkCapabilitiesTest {
private class TestTransportInfo implements TransportInfo {
TestTransportInfo() {
}
@Override
public TransportInfo makeCopy(boolean parcelLocationSensitiveFields) {
return this;
}
@Override
public boolean hasLocationSensitiveFields() {
return false;
}
}
@Test @IgnoreUpTo(Build.VERSION_CODES.Q)

View File

@@ -202,6 +202,7 @@ import android.net.metrics.IpConnectivityLog;
import android.net.shared.NetworkMonitorUtils;
import android.net.shared.PrivateDnsConfig;
import android.net.util.MultinetworkPolicyTracker;
import android.net.wifi.WifiInfo;
import android.os.BadParcelableException;
import android.os.Binder;
import android.os.Build;
@@ -7568,51 +7569,76 @@ public class ConnectivityServiceTest {
private int getOwnerUidNetCapsForCallerPermission(int ownerUid, int callerUid) {
final NetworkCapabilities netCap = new NetworkCapabilities().setOwnerUid(ownerUid);
return mService
.maybeSanitizeLocationInfoForCaller(netCap, callerUid, mContext.getPackageName())
.getOwnerUid();
return mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled(
netCap, callerUid, mContext.getPackageName()).getOwnerUid();
}
private void verifyWifiInfoCopyNetCapsForCallerPermission(
int callerUid, boolean shouldMakeCopyWithLocationSensitiveFieldsParcelable) {
final WifiInfo wifiInfo = mock(WifiInfo.class);
when(wifiInfo.hasLocationSensitiveFields()).thenReturn(true);
final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(wifiInfo);
mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled(
netCap, callerUid, mContext.getPackageName());
verify(wifiInfo).makeCopy(eq(shouldMakeCopyWithLocationSensitiveFieldsParcelable));
}
@Test
public void testMaybeSanitizeLocationInfoForCallerWithFineLocationAfterQ() throws Exception {
public void testCreateForCallerWithLocationInfoSanitizedWithFineLocationAfterQ()
throws Exception {
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION);
final int myUid = Process.myUid();
assertEquals(myUid, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationPreQ() throws Exception {
public void testCreateForCallerWithLocationInfoSanitizedWithCoarseLocationPreQ()
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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testMaybeSanitizeLocationInfoForCallerLocationOff() throws Exception {
public void testCreateForCallerWithLocationInfoSanitizedLocationOff() 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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testMaybeSanitizeLocationInfoForCallerWrongUid() throws Exception {
public void testCreateForCallerWithLocationInfoSanitizedWrongUid() 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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationAfterQ() throws Exception {
public void testCreateForCallerWithLocationInfoSanitizedWithCoarseLocationAfterQ()
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);
@@ -7620,15 +7646,22 @@ public class ConnectivityServiceTest {
// Test that without the location permission, the owner field is sanitized.
final int myUid = Process.myUid();
assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testMaybeSanitizeLocationInfoForCallerWithoutLocationPermission() throws Exception {
public void testCreateForCallerWithLocationInfoSanitizedWithoutLocationPermission()
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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType)