diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 4f46736c08..4166c2c4f2 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -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)); } diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 49cec29d39..1a37fb9fb6 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -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()); diff --git a/core/java/android/net/TransportInfo.java b/core/java/android/net/TransportInfo.java index aa4bbb0511..b78d3feccf 100644 --- a/core/java/android/net/TransportInfo.java +++ b/core/java/android/net/TransportInfo.java @@ -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. - * - *

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; - } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b73cc65ec1..397eeb2339 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -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; } diff --git a/tests/net/common/Android.bp b/tests/net/common/Android.bp index c271f49ee5..373aac604b 100644 --- a/tests/net/common/Android.bp +++ b/tests/net/common/Android.bp @@ -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", diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index 5d0e016d50..6b7ea66df2 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -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) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index bdec64d637..3433880f3c 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -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)