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

Revert "NetworkCapabilities: Embed location senstive TransportInfo"

Browse Source 
Revert "WifiLocationTest: Add test for Wifi TransportInfo"

Revert "WifiInfo: Embed location sensitive TransportInfo"

Revert submission 13274595

Reason for revert: DroidMonitor: Potential culprit for Bug 177390648 - verifying through Forrest before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted.
Reverted Changes:
Ibcf0c6711:WifiInfo: Embed location sensitive TransportInfo
Ic68546e2a:WifiService: Use WifiInfo for masking
Ie522d8c75:NetworkCapabilities: Embed location senstive Trans...
Iaf0fef999:WifiLocationTest: Add test for Wifi TransportInfo
I017de6d4c:ClientModeImpl: Fill info elements for connected b...
I384c9321e:WifiInfo: Add info elements for connected bssid

Change-Id: I4e96850b96dc5ea71d9bb6af2ff17343b8d09d68
This commit is contained in:
Rick Yiu
2021-01-13 08:36:22 +00:00
parent e9c34f86cb
commit a427afd7e3
7 changed files with 49 additions and 235 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
core/java/android/net/NetworkAgent.java
View File

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

35
core/java/android/net/NetworkCapabilities.java
View File

@@ -76,33 +76,12 @@ 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);
}
@@ -114,12 +93,6 @@ 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;
@@ -136,8 +109,6 @@ 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) {
@@ -146,11 +117,7 @@ public final class NetworkCapabilities implements Parcelable {
mLinkUpBandwidthKbps = nc.mLinkUpBandwidthKbps;
mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps;
mNetworkSpecifier = nc.mNetworkSpecifier;
if (nc.getTransportInfo() != null) {
setTransportInfo(nc.getTransportInfo().makeCopy(mParcelLocationSensitiveFields));
} else {
setTransportInfo(null);
}
mTransportInfo = nc.mTransportInfo;
mSignalStrength = nc.mSignalStrength;
setUids(nc.mUids); // Will make the defensive copy
setAdministratorUids(nc.getAdministratorUids());

38
core/java/android/net/TransportInfo.java
View File

@@ -16,48 +16,10 @@
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;
}
}

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

@@ -1557,7 +1557,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
if (nc != null) {
result.put(
nai.network,
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
maybeSanitizeLocationInfoForCaller(
nc, mDeps.getCallingUid(), callingPackageName));
}
@@ -1567,9 +1567,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
for (Network network : networks) {
nc = getNetworkCapabilitiesInternal(network);
if (nc != null) {
result.put(
network,
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
result.put(network, maybeSanitizeLocationInfoForCaller(
nc, mDeps.getCallingUid(), callingPackageName));
}
}
@@ -1651,7 +1649,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
public NetworkCapabilities getNetworkCapabilities(Network network, String callingPackageName) {
mAppOpsManager.checkPackage(mDeps.getCallingUid(), callingPackageName);
enforceAccessPermission();
return createWithLocationInfoSanitizedIfNecessaryWhenParceled(
return maybeSanitizeLocationInfoForCaller(
getNetworkCapabilitiesInternal(network),
mDeps.getCallingUid(), callingPackageName);
}
@@ -1672,51 +1670,37 @@ 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 createWithLocationInfoSanitizedIfNecessaryWhenParceled(
NetworkCapabilities maybeSanitizeLocationInfoForCaller(
@Nullable NetworkCapabilities nc, int callerUid, @NonNull String callerPkgName) {
if (nc == null) {
return null;
}
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()) {
final NetworkCapabilities newNc = new NetworkCapabilities(nc);
if (callerUid != newNc.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;
}
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);
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);
}
return newNc;
}
@@ -6855,7 +6839,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
networkAgent.networkCapabilities, nri.mPid, nri.mUid);
putParcelable(
bundle,
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
maybeSanitizeLocationInfoForCaller(
nc, nri.mUid, nri.request.getRequestorPackageName()));
putParcelable(bundle, linkPropertiesRestrictedForCallerPermissions(
networkAgent.linkProperties, nri.mPid, nri.mUid));
@@ -6874,7 +6858,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
networkAgent.networkCapabilities, nri.mPid, nri.mUid);
putParcelable(
bundle,
createWithLocationInfoSanitizedIfNecessaryWhenParceled(
maybeSanitizeLocationInfoForCaller(
netCap, nri.mUid, nri.request.getRequestorPackageName()));
break;
}

1
tests/net/common/Android.bp
View File

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

94
tests/net/common/java/android/net/NetworkCapabilitiesTest.java
View File

@@ -42,11 +42,9 @@ 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;
@@ -55,19 +53,18 @@ 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;
@@ -92,11 +89,10 @@ public class NetworkCapabilitiesTest {
private PeerHandle mPeerHandle = Mockito.mock(PeerHandle.class);
private boolean isAtLeastR() {
return SdkLevel.isAtLeastR();
}
private boolean isAtLeastS() {
return SdkLevel.isAtLeastS();
// 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;
}
@Test
@@ -328,59 +324,8 @@ 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 (isAtLeastS()) {
assertParcelSane(cap, 16);
} else if (isAtLeastR()) {
if (isAtLeastR()) {
assertParcelSane(cap, 15);
} else {
assertParcelSane(cap, 11);
@@ -694,23 +639,26 @@ public class NetworkCapabilitiesTest {
// Sequence 1: Transport + Transport + TransportInfo
NetworkCapabilities nc1 = new NetworkCapabilities();
nc1.addTransportType(TRANSPORT_CELLULAR).addTransportType(TRANSPORT_WIFI)
.setTransportInfo(new TestTransportInfo());
.setTransportInfo(new TransportInfo() {});
// Sequence 2: Transport + NetworkSpecifier + Transport
NetworkCapabilities nc2 = new NetworkCapabilities();
nc2.addTransportType(TRANSPORT_CELLULAR).setTransportInfo(new TestTransportInfo())
nc2.addTransportType(TRANSPORT_CELLULAR).setTransportInfo(new TransportInfo() {})
.addTransportType(TRANSPORT_WIFI);
}
@Test
public void testCombineTransportInfo() {
NetworkCapabilities nc1 = new NetworkCapabilities();
nc1.setTransportInfo(new TestTransportInfo());
nc1.setTransportInfo(new TransportInfo() {
// empty
});
NetworkCapabilities nc2 = new NetworkCapabilities();
// new TransportInfo so that object is not #equals to nc1's TransportInfo (that's where
// combine fails)
nc2.setTransportInfo(new TestTransportInfo());
nc2.setTransportInfo(new TransportInfo() {
// empty
});
try {
nc1.combineCapabilities(nc2);
@@ -813,7 +761,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(INVALID_UID, nc1.getOwnerUid());
assertEquals(Process.INVALID_UID, nc1.getOwnerUid());
// Test setAdministratorUids and getAdministratorUids.
final int[] administratorUids = {1001, 10001};
final NetworkCapabilities nc2 = new NetworkCapabilities.Builder()
@@ -958,16 +906,6 @@ 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)

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

@@ -202,7 +202,6 @@ 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;
@@ -7569,76 +7568,51 @@ public class ConnectivityServiceTest {
private int getOwnerUidNetCapsForCallerPermission(int ownerUid, int callerUid) {
final NetworkCapabilities netCap = new NetworkCapabilities().setOwnerUid(ownerUid);
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));
return mService
.maybeSanitizeLocationInfoForCaller(netCap, callerUid, mContext.getPackageName())
.getOwnerUid();
}
@Test
public void testCreateForCallerWithLocationInfoSanitizedWithFineLocationAfterQ()
throws Exception {
public void testMaybeSanitizeLocationInfoForCallerWithFineLocationAfterQ() 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 testCreateForCallerWithLocationInfoSanitizedWithCoarseLocationPreQ()
throws Exception {
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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testCreateForCallerWithLocationInfoSanitizedLocationOff() throws Exception {
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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testCreateForCallerWithLocationInfoSanitizedWrongUid() throws Exception {
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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
public void testCreateForCallerWithLocationInfoSanitizedWithCoarseLocationAfterQ()
throws Exception {
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);
@@ -7646,22 +7620,15 @@ 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 testCreateForCallerWithLocationInfoSanitizedWithoutLocationPermission()
throws Exception {
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));
verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType)