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

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 1b7912e704..6b45abde74 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1569,7 +1569,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nc != null) { result.put( nai.network, - maybeSanitizeLocationInfoForCaller( + createWithLocationInfoSanitizedIfNecessaryWhenParceled( nc, mDeps.getCallingUid(), callingPackageName)); } @@ -1579,7 +1579,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)); } } @@ -1661,7 +1663,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); } @@ -1682,37 +1684,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; } @@ -6976,7 +6992,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)); @@ -6995,7 +7011,7 @@ public class ConnectivityService extends IConnectivityManager.Stub networkAgent.networkCapabilities, nri.mPid, nri.mUid); putParcelable( bundle, - maybeSanitizeLocationInfoForCaller( + createWithLocationInfoSanitizedIfNecessaryWhenParceled( netCap, nri.mUid, nri.request.getRequestorPackageName())); break; } diff --git a/tests/net/common/Android.bp b/tests/net/common/Android.bp index 373aac604b..c271f49ee5 100644 --- a/tests/net/common/Android.bp +++ b/tests/net/common/Android.bp @@ -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", diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index 6b7ea66df2..5d0e016d50 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -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) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 2a0414fc69..b6e6816ca5 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -201,6 +201,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; @@ -7802,51 +7803,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); @@ -7854,15 +7880,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)