From 8156c4ea31568375c28bcd60b3c23e652a5b5331 Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Fri, 19 Mar 2021 00:45:39 +0000 Subject: [PATCH 1/3] Revert^2 "Replace the usage of UidRange" 1b5c01b06f4bec0a14dc1a83f59b629c9182cb41 UidRange is used in a shared way between ConnectivityService and VPN through the use of NetworkCapabilities. UidRange will be part of the ConnectivityService mainline but Vpn.java will stay in the framework. We need a way to replace the APIs using UidRange, or to make UidRange system API. The only really relevant surface here is NetworkCapabilities#{setUids, getUids}. The need for UidRange could be replaced by an integer Range, so replace the usage of UidRange by a integer Range in NetworkCapabilities#{setUids, getUids} and update the relevant callers. Bug: 172183305 Test: atest FrameworksNetTests CtsNetTestCasesLatestSdk Change-Id: I0f679fb5fb8f4fe26461ca4912ca1fdfe7f43c9e Merged-In: I4e5aec6ef1ea02e038fcd7ed117a3b67b69c5cb9 --- .../src/android/net/NetworkCapabilities.java | 31 ++-- framework/src/android/net/NetworkRequest.java | 5 +- framework/src/android/net/UidRange.java | 31 ++++ .../android/server/ConnectivityService.java | 22 +-- .../android/net/NetworkCapabilitiesTest.java | 173 ++++++++++-------- .../android/server/NetworkAgentWrapper.java | 4 +- .../server/ConnectivityServiceTest.java | 60 +++--- .../android/server/connectivity/VpnTest.java | 133 ++++++++------ 8 files changed, 274 insertions(+), 185 deletions(-) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index 058f3c999d..ba9f21b261 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -32,6 +32,7 @@ import android.os.Parcelable; import android.os.Process; import android.text.TextUtils; import android.util.ArraySet; +import android.util.Range; import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.VisibleForTesting; @@ -153,7 +154,7 @@ public final class NetworkCapabilities implements Parcelable { setTransportInfo(null); } mSignalStrength = nc.mSignalStrength; - setUids(nc.mUids); // Will make the defensive copy + mUids = (nc.mUids == null) ? null : new ArraySet<>(nc.mUids); setAdministratorUids(nc.getAdministratorUids()); mOwnerUid = nc.mOwnerUid; mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; @@ -1458,9 +1459,8 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ public @NonNull NetworkCapabilities setSingleUid(int uid) { - final ArraySet identity = new ArraySet<>(1); - identity.add(new UidRange(uid, uid)); - setUids(identity); + mUids = new ArraySet<>(1); + mUids.add(new UidRange(uid, uid)); return this; } @@ -1469,12 +1469,8 @@ public final class NetworkCapabilities implements Parcelable { * This makes a copy of the set so that callers can't modify it after the call. * @hide */ - public @NonNull NetworkCapabilities setUids(Set uids) { - if (null == uids) { - mUids = null; - } else { - mUids = new ArraySet<>(uids); - } + public @NonNull NetworkCapabilities setUids(@Nullable Set> uids) { + mUids = UidRange.fromIntRanges(uids); return this; } @@ -1483,8 +1479,19 @@ public final class NetworkCapabilities implements Parcelable { * This returns a copy of the set so that callers can't modify the original object. * @hide */ - public @Nullable Set getUids() { - return null == mUids ? null : new ArraySet<>(mUids); + public @Nullable Set> getUids() { + return UidRange.toIntRanges(mUids); + } + + /** + * Get the list of UIDs this network applies to. + * This returns a copy of the set so that callers can't modify the original object. + * @hide + */ + public @Nullable Set getUidRanges() { + if (mUids == null) return null; + + return new ArraySet<>(mUids); } /** diff --git a/framework/src/android/net/NetworkRequest.java b/framework/src/android/net/NetworkRequest.java index dbe3ecc4d7..4ebbf06c51 100644 --- a/framework/src/android/net/NetworkRequest.java +++ b/framework/src/android/net/NetworkRequest.java @@ -45,6 +45,7 @@ import android.os.Parcel; import android.os.Parcelable; import android.os.Process; import android.text.TextUtils; +import android.util.Range; import android.util.proto.ProtoOutputStream; import java.util.Arrays; @@ -277,11 +278,11 @@ public class NetworkRequest implements Parcelable { * Set the watched UIDs for this request. This will be reset and wiped out unless * the calling app holds the CHANGE_NETWORK_STATE permission. * - * @param uids The watched UIDs as a set of UidRanges, or null for everything. + * @param uids The watched UIDs as a set of {@code Range}, or null for everything. * @return The builder to facilitate chaining. * @hide */ - public Builder setUids(Set uids) { + public Builder setUids(@Nullable Set> uids) { mNetworkCapabilities.setUids(uids); return this; } diff --git a/framework/src/android/net/UidRange.java b/framework/src/android/net/UidRange.java index 26518d32ed..bc67c745c9 100644 --- a/framework/src/android/net/UidRange.java +++ b/framework/src/android/net/UidRange.java @@ -20,8 +20,11 @@ import android.annotation.Nullable; import android.os.Parcel; import android.os.Parcelable; import android.os.UserHandle; +import android.util.ArraySet; +import android.util.Range; import java.util.Collection; +import java.util.Set; /** * An inclusive range of UIDs. @@ -149,4 +152,32 @@ public final class UidRange implements Parcelable { } return false; } + + /** + * Convert a set of {@code Range} to a set of {@link UidRange}. + */ + @Nullable + public static ArraySet fromIntRanges(@Nullable Set> ranges) { + if (null == ranges) return null; + + final ArraySet uids = new ArraySet<>(); + for (Range range : ranges) { + uids.add(new UidRange(range.getLower(), range.getUpper())); + } + return uids; + } + + /** + * Convert a set of {@link UidRange} to a set of {@code Range}. + */ + @Nullable + public static ArraySet> toIntRanges(@Nullable Set ranges) { + if (null == ranges) return null; + + final ArraySet> uids = new ArraySet<>(); + for (UidRange range : ranges) { + uids.add(new Range(range.start, range.stop)); + } + return uids; + } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d99da057c3..e2aa6e76ef 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1343,7 +1343,7 @@ public class ConnectivityService extends IConnectivityManager.Stub netCap.addCapability(NET_CAPABILITY_INTERNET); netCap.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); netCap.removeCapability(NET_CAPABILITY_NOT_VPN); - netCap.setUids(Collections.singleton(uids)); + netCap.setUids(UidRange.toIntRanges(Collections.singleton(uids))); return netCap; } @@ -2903,7 +2903,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (0 == defaultRequest.mRequests.size()) { pw.println("none, this should never occur."); } else { - pw.println(defaultRequest.mRequests.get(0).networkCapabilities.getUids()); + pw.println(defaultRequest.mRequests.get(0).networkCapabilities.getUidRanges()); } pw.decreaseIndent(); pw.decreaseIndent(); @@ -5320,9 +5320,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private Set getUids() { // networkCapabilities.getUids() returns a defensive copy. // multilayer requests will all have the same uids so return the first one. - final Set uids = null == mRequests.get(0).networkCapabilities.getUids() - ? new ArraySet<>() : mRequests.get(0).networkCapabilities.getUids(); - return uids; + final Set uids = mRequests.get(0).networkCapabilities.getUidRanges(); + return (null == uids) ? new ArraySet<>() : uids; } NetworkRequestInfo(@NonNull final NetworkRequest r, @Nullable final PendingIntent pi, @@ -6128,7 +6127,7 @@ public class ConnectivityService extends IConnectivityManager.Stub for (final NetworkRequestInfo nri : mDefaultNetworkRequests) { // Currently, all network requests will have the same uids therefore checking the first // one is sufficient. If/when uids are tracked at the nri level, this can change. - final Set uids = nri.mRequests.get(0).networkCapabilities.getUids(); + final Set uids = nri.mRequests.get(0).networkCapabilities.getUidRanges(); if (null == uids) { continue; } @@ -6569,7 +6568,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } - final Set ranges = nai.networkCapabilities.getUids(); + final Set ranges = nai.networkCapabilities.getUidRanges(); final int vpnAppUid = nai.networkCapabilities.getOwnerUid(); // TODO: this create a window of opportunity for apps to receive traffic between the time // when the old rules are removed and the time when new rules are added. To fix this, @@ -6934,8 +6933,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private void updateUids(NetworkAgentInfo nai, NetworkCapabilities prevNc, NetworkCapabilities newNc) { - Set prevRanges = null == prevNc ? null : prevNc.getUids(); - Set newRanges = null == newNc ? null : newNc.getUids(); + Set prevRanges = null == prevNc ? null : prevNc.getUidRanges(); + Set newRanges = null == newNc ? null : newNc.getUidRanges(); if (null == prevRanges) prevRanges = new ArraySet<>(); if (null == newRanges) newRanges = new ArraySet<>(); final Set prevRangesCopy = new ArraySet<>(prevRanges); @@ -9266,7 +9265,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final ArrayList nrs = new ArrayList<>(); nrs.add(createNetworkRequest(NetworkRequest.Type.REQUEST, pref.capabilities)); nrs.add(createDefaultRequest()); - setNetworkRequestUids(nrs, pref.capabilities.getUids()); + setNetworkRequestUids(nrs, UidRange.fromIntRanges(pref.capabilities.getUids())); final NetworkRequestInfo nri = new NetworkRequestInfo(nrs); result.add(nri); } @@ -9482,9 +9481,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private static void setNetworkRequestUids(@NonNull final List requests, @NonNull final Set uids) { - final Set ranges = new ArraySet<>(uids); for (final NetworkRequest req : requests) { - req.networkCapabilities.setUids(ranges); + req.networkCapabilities.setUids(UidRange.toIntRanges(uids)); } } diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index 0dfec75922..a7ad695641 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -69,6 +69,7 @@ import android.net.wifi.aware.WifiAwareNetworkSpecifier; import android.os.Build; import android.test.suitebuilder.annotation.SmallTest; import android.util.ArraySet; +import android.util.Range; import androidx.test.runner.AndroidJUnit4; @@ -240,72 +241,93 @@ public class NetworkCapabilitiesTest { @Test public void testSetUids() { final NetworkCapabilities netCap = new NetworkCapabilities(); - final Set uids = new ArraySet<>(); - uids.add(new UidRange(50, 100)); - uids.add(new UidRange(3000, 4000)); - netCap.setUids(uids); - assertTrue(netCap.appliesToUid(50)); - assertTrue(netCap.appliesToUid(80)); - assertTrue(netCap.appliesToUid(100)); + // Null uids match all UIDs + netCap.setUids(null); + assertTrue(netCap.appliesToUid(10)); + assertTrue(netCap.appliesToUid(200)); assertTrue(netCap.appliesToUid(3000)); - assertTrue(netCap.appliesToUid(3001)); - assertFalse(netCap.appliesToUid(10)); - assertFalse(netCap.appliesToUid(25)); - assertFalse(netCap.appliesToUid(49)); - assertFalse(netCap.appliesToUid(101)); - assertFalse(netCap.appliesToUid(2000)); - assertFalse(netCap.appliesToUid(100000)); - + assertTrue(netCap.appliesToUid(10010)); assertTrue(netCap.appliesToUidRange(new UidRange(50, 100))); assertTrue(netCap.appliesToUidRange(new UidRange(70, 72))); assertTrue(netCap.appliesToUidRange(new UidRange(3500, 3912))); - assertFalse(netCap.appliesToUidRange(new UidRange(1, 100))); - assertFalse(netCap.appliesToUidRange(new UidRange(49, 100))); - assertFalse(netCap.appliesToUidRange(new UidRange(1, 10))); - assertFalse(netCap.appliesToUidRange(new UidRange(60, 101))); - assertFalse(netCap.appliesToUidRange(new UidRange(60, 3400))); - - NetworkCapabilities netCap2 = new NetworkCapabilities(); - // A new netcap object has null UIDs, so anything will satisfy it. - assertTrue(netCap2.satisfiedByUids(netCap)); - // Still not equal though. - assertFalse(netCap2.equalsUids(netCap)); - netCap2.setUids(uids); - assertTrue(netCap2.satisfiedByUids(netCap)); - assertTrue(netCap.equalsUids(netCap2)); - assertTrue(netCap2.equalsUids(netCap)); - - uids.add(new UidRange(600, 700)); - netCap2.setUids(uids); - assertFalse(netCap2.satisfiedByUids(netCap)); - assertFalse(netCap.appliesToUid(650)); - assertTrue(netCap2.appliesToUid(650)); - netCap.combineCapabilities(netCap2); - assertTrue(netCap2.satisfiedByUids(netCap)); - assertTrue(netCap.appliesToUid(650)); - assertFalse(netCap.appliesToUid(500)); - - assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); - netCap.combineCapabilities(new NetworkCapabilities()); - assertTrue(netCap.appliesToUid(500)); assertTrue(netCap.appliesToUidRange(new UidRange(1, 100000))); - assertFalse(netCap2.appliesToUid(500)); - assertFalse(netCap2.appliesToUidRange(new UidRange(1, 100000))); - assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); + + if (isAtLeastS()) { + final Set> uids = new ArraySet<>(); + uids.add(uidRange(50, 100)); + uids.add(uidRange(3000, 4000)); + netCap.setUids(uids); + assertTrue(netCap.appliesToUid(50)); + assertTrue(netCap.appliesToUid(80)); + assertTrue(netCap.appliesToUid(100)); + assertTrue(netCap.appliesToUid(3000)); + assertTrue(netCap.appliesToUid(3001)); + assertFalse(netCap.appliesToUid(10)); + assertFalse(netCap.appliesToUid(25)); + assertFalse(netCap.appliesToUid(49)); + assertFalse(netCap.appliesToUid(101)); + assertFalse(netCap.appliesToUid(2000)); + assertFalse(netCap.appliesToUid(100000)); + + assertTrue(netCap.appliesToUidRange(new UidRange(50, 100))); + assertTrue(netCap.appliesToUidRange(new UidRange(70, 72))); + assertTrue(netCap.appliesToUidRange(new UidRange(3500, 3912))); + assertFalse(netCap.appliesToUidRange(new UidRange(1, 100))); + assertFalse(netCap.appliesToUidRange(new UidRange(49, 100))); + assertFalse(netCap.appliesToUidRange(new UidRange(1, 10))); + assertFalse(netCap.appliesToUidRange(new UidRange(60, 101))); + assertFalse(netCap.appliesToUidRange(new UidRange(60, 3400))); + + NetworkCapabilities netCap2 = new NetworkCapabilities(); + // A new netcap object has null UIDs, so anything will satisfy it. + assertTrue(netCap2.satisfiedByUids(netCap)); + // Still not equal though. + assertFalse(netCap2.equalsUids(netCap)); + netCap2.setUids(uids); + assertTrue(netCap2.satisfiedByUids(netCap)); + assertTrue(netCap.equalsUids(netCap2)); + assertTrue(netCap2.equalsUids(netCap)); + + uids.add(uidRange(600, 700)); + netCap2.setUids(uids); + assertFalse(netCap2.satisfiedByUids(netCap)); + assertFalse(netCap.appliesToUid(650)); + assertTrue(netCap2.appliesToUid(650)); + netCap.combineCapabilities(netCap2); + assertTrue(netCap2.satisfiedByUids(netCap)); + assertTrue(netCap.appliesToUid(650)); + assertFalse(netCap.appliesToUid(500)); + + assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); + netCap.combineCapabilities(new NetworkCapabilities()); + assertTrue(netCap.appliesToUid(500)); + assertTrue(netCap.appliesToUidRange(new UidRange(1, 100000))); + assertFalse(netCap2.appliesToUid(500)); + assertFalse(netCap2.appliesToUidRange(new UidRange(1, 100000))); + assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); + + // Null uids satisfies everything. + netCap.setUids(null); + assertTrue(netCap2.satisfiedByUids(netCap)); + assertTrue(netCap.satisfiedByUids(netCap2)); + netCap2.setUids(null); + assertTrue(netCap2.satisfiedByUids(netCap)); + assertTrue(netCap.satisfiedByUids(netCap2)); + } } @Test public void testParcelNetworkCapabilities() { - final Set uids = new ArraySet<>(); - uids.add(new UidRange(50, 100)); - uids.add(new UidRange(3000, 4000)); + final Set> uids = new ArraySet<>(); + uids.add(uidRange(50, 100)); + uids.add(uidRange(3000, 4000)); final NetworkCapabilities netCap = new NetworkCapabilities() .addCapability(NET_CAPABILITY_INTERNET) - .setUids(uids) .addCapability(NET_CAPABILITY_EIMS) .addCapability(NET_CAPABILITY_NOT_METERED); if (isAtLeastS()) { netCap.setSubIds(Set.of(TEST_SUBID1, TEST_SUBID2)); + netCap.setUids(uids); } else if (isAtLeastR()) { netCap.setOwnerUid(123); netCap.setAdministratorUids(new int[] {5, 11}); @@ -540,12 +562,16 @@ public class NetworkCapabilitiesTest { assertFalse(nc1.satisfiedByNetworkCapabilities(nc2)); } - private ArraySet uidRange(int from, int to) { - final ArraySet range = new ArraySet<>(1); - range.add(new UidRange(from, to)); + private ArraySet> uidRanges(int from, int to) { + final ArraySet> range = new ArraySet<>(1); + range.add(uidRange(from, to)); return range; } + private Range uidRange(int from, int to) { + return new Range(from, to); + } + @Test @IgnoreUpTo(Build.VERSION_CODES.Q) public void testSetAdministratorUids() { NetworkCapabilities nc = @@ -601,23 +627,23 @@ public class NetworkCapabilitiesTest { } catch (IllegalStateException expected) {} nc1.setSSID(TEST_SSID); - nc1.setUids(uidRange(10, 13)); - assertNotEquals(nc1, nc2); - nc2.combineCapabilities(nc1); // Everything + 10~13 is still everything. - assertNotEquals(nc1, nc2); - nc1.combineCapabilities(nc2); // 10~13 + everything is everything. - assertEquals(nc1, nc2); - nc1.setUids(uidRange(10, 13)); - nc2.setUids(uidRange(20, 23)); - assertNotEquals(nc1, nc2); - nc1.combineCapabilities(nc2); - assertTrue(nc1.appliesToUid(12)); - assertFalse(nc2.appliesToUid(12)); - assertTrue(nc1.appliesToUid(22)); - assertTrue(nc2.appliesToUid(22)); - - // Verify the subscription id list can be combined only when they are equal. if (isAtLeastS()) { + nc1.setUids(uidRanges(10, 13)); + assertNotEquals(nc1, nc2); + nc2.combineCapabilities(nc1); // Everything + 10~13 is still everything. + assertNotEquals(nc1, nc2); + nc1.combineCapabilities(nc2); // 10~13 + everything is everything. + assertEquals(nc1, nc2); + nc1.setUids(uidRanges(10, 13)); + nc2.setUids(uidRanges(20, 23)); + assertNotEquals(nc1, nc2); + nc1.combineCapabilities(nc2); + assertTrue(nc1.appliesToUid(12)); + assertFalse(nc2.appliesToUid(12)); + assertTrue(nc1.appliesToUid(22)); + assertTrue(nc2.appliesToUid(22)); + + // Verify the subscription id list can be combined only when they are equal. nc1.setSubIds(Set.of(TEST_SUBID1, TEST_SUBID2)); nc2.setSubIds(Set.of(TEST_SUBID2)); assertThrows(IllegalStateException.class, () -> nc2.combineCapabilities(nc1)); @@ -773,8 +799,11 @@ public class NetworkCapabilitiesTest { if (isAtLeastR()) { assertTrue(DIFFERENT_TEST_SSID.equals(nc2.getSsid())); } - - nc1.setUids(uidRange(10, 13)); + if (isAtLeastS()) { + nc1.setUids(uidRanges(10, 13)); + } else { + nc1.setUids(null); + } nc2.set(nc1); // Overwrites, as opposed to combineCapabilities assertEquals(nc1, nc2); diff --git a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java index 01d8186c7d..e2d43cbb8e 100644 --- a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java @@ -44,11 +44,11 @@ import android.net.NetworkProvider; import android.net.NetworkSpecifier; import android.net.QosFilter; import android.net.SocketKeepalive; -import android.net.UidRange; import android.os.ConditionVariable; import android.os.HandlerThread; import android.os.Message; import android.util.Log; +import android.util.Range; import com.android.net.module.util.ArrayTrackRecord; import com.android.server.connectivity.ConnectivityConstants; @@ -222,7 +222,7 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); } - public void setUids(Set uids) { + public void setUids(Set> uids) { mNetworkCapabilities.setUids(uids); mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 8b20a51b44..6cb5305ea1 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -266,6 +266,7 @@ import android.text.TextUtils; import android.util.ArraySet; import android.util.Log; import android.util.Pair; +import android.util.Range; import android.util.SparseArray; import androidx.test.InstrumentationRegistry; @@ -1158,7 +1159,7 @@ public class ConnectivityServiceTest { } public void setUids(Set uids) { - mNetworkCapabilities.setUids(uids); + mNetworkCapabilities.setUids(UidRange.toIntRanges(uids)); if (mAgentRegistered) { mMockNetworkAgent.setNetworkCapabilities(mNetworkCapabilities, true); } @@ -1463,6 +1464,8 @@ public class ConnectivityServiceTest { } private static final int PRIMARY_USER = 0; + private static final UidRange PRIMARY_UIDRANGE = + UidRange.createForUser(UserHandle.of(PRIMARY_USER)); private static final int APP1_UID = UserHandle.getUid(PRIMARY_USER, 10100); private static final int APP2_UID = UserHandle.getUid(PRIMARY_USER, 10101); private static final int VPN_UID = UserHandle.getUid(PRIMARY_USER, 10043); @@ -6944,7 +6947,7 @@ public class ConnectivityServiceTest { final int uid = Process.myUid(); NetworkCapabilities nc = mCm.getNetworkCapabilities(mMockVpn.getNetwork()); assertNotNull("nc=" + nc, nc.getUids()); - assertEquals(nc.getUids(), uidRangesForUids(uid)); + assertEquals(nc.getUids(), UidRange.toIntRanges(uidRangesForUids(uid))); assertVpnTransportInfo(nc, VpnManager.TYPE_VPN_SERVICE); // Set an underlying network and expect to see the VPN transports change. @@ -6969,10 +6972,13 @@ public class ConnectivityServiceTest { // Expect that the VPN UID ranges contain both |uid| and the UID range for the newly-added // restricted user. + final UidRange rRange = UidRange.createForUser(UserHandle.of(RESTRICTED_USER)); + final Range restrictUidRange = new Range(rRange.start, rRange.stop); + final Range singleUidRange = new Range(uid, uid); callback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.getUids().size() == 2 - && caps.getUids().contains(new UidRange(uid, uid)) - && caps.getUids().contains(createUidRange(RESTRICTED_USER)) + && caps.getUids().contains(singleUidRange) + && caps.getUids().contains(restrictUidRange) && caps.hasTransport(TRANSPORT_VPN) && caps.hasTransport(TRANSPORT_WIFI)); @@ -6981,8 +6987,8 @@ public class ConnectivityServiceTest { callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); callback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.getUids().size() == 2 - && caps.getUids().contains(new UidRange(uid, uid)) - && caps.getUids().contains(createUidRange(RESTRICTED_USER)) + && caps.getUids().contains(singleUidRange) + && caps.getUids().contains(restrictUidRange) && caps.hasTransport(TRANSPORT_VPN) && !caps.hasTransport(TRANSPORT_WIFI)); @@ -6996,7 +7002,7 @@ public class ConnectivityServiceTest { // change made just before that (i.e., loss of TRANSPORT_WIFI) is preserved. callback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.getUids().size() == 1 - && caps.getUids().contains(new UidRange(uid, uid)) + && caps.getUids().contains(singleUidRange) && caps.hasTransport(TRANSPORT_VPN) && !caps.hasTransport(TRANSPORT_WIFI)); } @@ -7654,7 +7660,7 @@ public class ConnectivityServiceTest { assertNotNull(underlying); mMockVpn.setVpnType(VpnManager.TYPE_VPN_LEGACY); // The legacy lockdown VPN only supports userId 0. - final Set ranges = Collections.singleton(createUidRange(PRIMARY_USER)); + final Set ranges = Collections.singleton(PRIMARY_UIDRANGE); mMockVpn.registerAgent(ranges); mMockVpn.setUnderlyingNetworks(new Network[]{underlying}); mMockVpn.connect(true); @@ -8616,7 +8622,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); + final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); mMockVpn.establish(lp, VPN_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, VPN_UID); @@ -8644,7 +8650,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); + final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); mMockVpn.establish(lp, Process.SYSTEM_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, Process.SYSTEM_UID); @@ -8660,7 +8666,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix("192.0.2.0/24"), null, "tun0")); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); + final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); mMockVpn.establish(lp, Process.SYSTEM_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, Process.SYSTEM_UID); @@ -8675,7 +8681,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); + final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); mMockVpn.establish(lp, VPN_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, VPN_UID); @@ -8727,7 +8733,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), RTN_UNREACHABLE)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); // The uid range needs to cover the test app so the network is visible to it. - final UidRange vpnRange = createUidRange(PRIMARY_USER); + final UidRange vpnRange = PRIMARY_UIDRANGE; final Set vpnRanges = Collections.singleton(vpnRange); mMockVpn.establish(lp, VPN_UID, vpnRanges); assertVpnUidRangesUpdated(true, vpnRanges, VPN_UID); @@ -9008,7 +9014,7 @@ public class ConnectivityServiceTest { private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType) throws Exception { - final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); + final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); mMockVpn.setVpnType(vpnType); mMockVpn.establish(new LinkProperties(), vpnOwnerUid, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, vpnOwnerUid); @@ -9568,7 +9574,7 @@ public class ConnectivityServiceTest { lp.setInterfaceName("tun0"); lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); - final UidRange vpnRange = createUidRange(PRIMARY_USER); + final UidRange vpnRange = PRIMARY_UIDRANGE; Set vpnRanges = Collections.singleton(vpnRange); mMockVpn.establish(lp, VPN_UID, vpnRanges); assertVpnUidRangesUpdated(true, vpnRanges, VPN_UID); @@ -9766,7 +9772,7 @@ public class ConnectivityServiceTest { .thenReturn(hasFeature); } - private UidRange getNriFirstUidRange( + private Range getNriFirstUidRange( @NonNull final ConnectivityService.NetworkRequestInfo nri) { return nri.mRequests.get(0).networkCapabilities.getUids().iterator().next(); } @@ -9949,11 +9955,11 @@ public class ConnectivityServiceTest { pref)); // Sort by uid to access nris by index - nris.sort(Comparator.comparingInt(nri -> getNriFirstUidRange(nri).start)); - assertEquals(TEST_PACKAGE_UID, getNriFirstUidRange(nris.get(0)).start); - assertEquals(TEST_PACKAGE_UID, getNriFirstUidRange(nris.get(0)).stop); - assertEquals(testPackageNameUid2, getNriFirstUidRange(nris.get(1)).start); - assertEquals(testPackageNameUid2, getNriFirstUidRange(nris.get(1)).stop); + nris.sort(Comparator.comparingInt(nri -> getNriFirstUidRange(nri).getLower())); + assertEquals(TEST_PACKAGE_UID, (int) getNriFirstUidRange(nris.get(0)).getLower()); + assertEquals(TEST_PACKAGE_UID, (int) getNriFirstUidRange(nris.get(0)).getUpper()); + assertEquals(testPackageNameUid2, (int) getNriFirstUidRange(nris.get(1)).getLower()); + assertEquals(testPackageNameUid2, (int) getNriFirstUidRange(nris.get(1)).getUpper()); } @Test @@ -9983,17 +9989,17 @@ public class ConnectivityServiceTest { // UIDs for all users and all managed packages should be present. // Two users each with two packages. final int expectedUidSize = 2; - final List uids = + final List> uids = new ArrayList<>(nris.get(0).mRequests.get(0).networkCapabilities.getUids()); assertEquals(expectedUidSize, uids.size()); // Sort by uid to access nris by index - uids.sort(Comparator.comparingInt(uid -> uid.start)); + uids.sort(Comparator.comparingInt(uid -> uid.getLower())); final int secondUserTestPackageUid = UserHandle.getUid(secondUser, TEST_PACKAGE_UID); - assertEquals(TEST_PACKAGE_UID, uids.get(0).start); - assertEquals(TEST_PACKAGE_UID, uids.get(0).stop); - assertEquals(secondUserTestPackageUid, uids.get(1).start); - assertEquals(secondUserTestPackageUid, uids.get(1).stop); + assertEquals(TEST_PACKAGE_UID, (int) uids.get(0).getLower()); + assertEquals(TEST_PACKAGE_UID, (int) uids.get(0).getUpper()); + assertEquals(secondUserTestPackageUid, (int) uids.get(1).getLower()); + assertEquals(secondUserTestPackageUid, (int) uids.get(1).getUpper()); } @Test diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 11fcea60d9..6ad4900989 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -23,6 +23,7 @@ import static android.content.pm.UserInfo.FLAG_RESTRICTED; import static android.net.ConnectivityManager.NetworkCallback; import static android.net.INetd.IF_STATE_DOWN; import static android.net.INetd.IF_STATE_UP; +import static android.os.UserHandle.PER_USER_RANGE; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -74,7 +75,6 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo.DetailedState; import android.net.RouteInfo; -import android.net.UidRange; import android.net.UidRangeParcel; import android.net.VpnManager; import android.net.VpnService; @@ -181,8 +181,7 @@ public class VpnTest { mPackages.put(PKGS[i], PKG_UIDS[i]); } } - private static final UidRange PRI_USER_RANGE = - UidRange.createForUser(UserHandle.of(primaryUser.id)); + private static final Range PRI_USER_RANGE = uidRangeForUser(primaryUser.id); @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Context mContext; @Mock private UserManager mUserManager; @@ -260,6 +259,21 @@ public class VpnTest { .thenReturn(tunnelResp); } + private Set> rangeSet(Range ... ranges) { + final Set> range = new ArraySet<>(); + for (Range r : ranges) range.add(r); + + return range; + } + + private static Range uidRangeForUser(int userId) { + return new Range(userId * PER_USER_RANGE, (userId + 1) * PER_USER_RANGE - 1); + } + + private Range uidRange(int start, int stop) { + return new Range(start, stop); + } + @Test public void testRestrictedProfilesAreAddedToVpn() { setMockedUsers(primaryUser, secondaryUser, restrictedProfileA, restrictedProfileB); @@ -268,12 +282,10 @@ public class VpnTest { // Assume the user can have restricted profiles. doReturn(true).when(mUserManager).canHaveRestrictedProfile(); - final Set ranges = + final Set> ranges = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, null, null); - assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { - PRI_USER_RANGE, UidRange.createForUser(UserHandle.of(restrictedProfileA.id)) - })), ranges); + assertEquals(rangeSet(PRI_USER_RANGE, uidRangeForUser(restrictedProfileA.id)), ranges); } @Test @@ -281,10 +293,10 @@ public class VpnTest { setMockedUsers(primaryUser, managedProfileA); final Vpn vpn = createVpn(primaryUser.id); - final Set ranges = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, + final Set> ranges = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, null, null); - assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { PRI_USER_RANGE })), ranges); + assertEquals(rangeSet(PRI_USER_RANGE), ranges); } @Test @@ -292,35 +304,38 @@ public class VpnTest { setMockedUsers(primaryUser, restrictedProfileA, managedProfileA); final Vpn vpn = createVpn(primaryUser.id); - final Set ranges = new ArraySet<>(); + final Set> ranges = new ArraySet<>(); vpn.addUserToRanges(ranges, primaryUser.id, null, null); - assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { PRI_USER_RANGE })), ranges); + assertEquals(rangeSet(PRI_USER_RANGE), ranges); } @Test public void testUidAllowAndDenylist() throws Exception { final Vpn vpn = createVpn(primaryUser.id); - final UidRange user = PRI_USER_RANGE; + final Range user = PRI_USER_RANGE; + final int userStart = user.getLower(); + final int userStop = user.getUpper(); final String[] packages = {PKGS[0], PKGS[1], PKGS[2]}; // Allowed list - final Set allow = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, - Arrays.asList(packages), null); - assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { - new UidRange(user.start + PKG_UIDS[0], user.start + PKG_UIDS[0]), - new UidRange(user.start + PKG_UIDS[1], user.start + PKG_UIDS[2]) - })), allow); + final Set> allow = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, + Arrays.asList(packages), null /* disallowedApplications */); + assertEquals(rangeSet( + uidRange(userStart + PKG_UIDS[0], userStart + PKG_UIDS[0]), + uidRange(userStart + PKG_UIDS[1], userStart + PKG_UIDS[2])), + allow); // Denied list - final Set disallow = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, - null, Arrays.asList(packages)); - assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { - new UidRange(user.start, user.start + PKG_UIDS[0] - 1), - new UidRange(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[1] - 1), - /* Empty range between UIDS[1] and UIDS[2], should be excluded, */ - new UidRange(user.start + PKG_UIDS[2] + 1, user.stop) - })), disallow); + final Set> disallow = + vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, + null /* allowedApplications */, Arrays.asList(packages)); + assertEquals(rangeSet( + uidRange(userStart, userStart + PKG_UIDS[0] - 1), + uidRange(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[1] - 1), + /* Empty range between UIDS[1] and UIDS[2], should be excluded, */ + uidRange(userStart + PKG_UIDS[2] + 1, userStop)), + disallow); } @Test @@ -350,84 +365,86 @@ public class VpnTest { @Test public void testLockdownChangingPackage() throws Exception { final Vpn vpn = createVpn(primaryUser.id); - final UidRange user = PRI_USER_RANGE; - + final Range user = PRI_USER_RANGE; + final int userStart = user.getLower(); + final int userStop = user.getUpper(); // Set always-on without lockdown. assertTrue(vpn.setAlwaysOnPackage(PKGS[1], false, null)); // Set always-on with lockdown. assertTrue(vpn.setAlwaysOnPackage(PKGS[1], true, null)); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), - new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) + new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), + new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) })); // Switch to another app. assertTrue(vpn.setAlwaysOnPackage(PKGS[3], true, null)); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), - new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) + new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), + new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start, user.start + PKG_UIDS[3] - 1), - new UidRangeParcel(user.start + PKG_UIDS[3] + 1, user.stop) + new UidRangeParcel(userStart, userStart + PKG_UIDS[3] - 1), + new UidRangeParcel(userStart + PKG_UIDS[3] + 1, userStop) })); } @Test public void testLockdownAllowlist() throws Exception { final Vpn vpn = createVpn(primaryUser.id); - final UidRange user = PRI_USER_RANGE; - + final Range user = PRI_USER_RANGE; + final int userStart = user.getLower(); + final int userStop = user.getUpper(); // Set always-on with lockdown and allow app PKGS[2] from lockdown. assertTrue(vpn.setAlwaysOnPackage( PKGS[1], true, Collections.singletonList(PKGS[2]))); - verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), - new UidRangeParcel(user.start + PKG_UIDS[2] + 1, user.stop) + verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { + new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), + new UidRangeParcel(userStart + PKG_UIDS[2] + 1, userStop) })); // Change allowed app list to PKGS[3]. assertTrue(vpn.setAlwaysOnPackage( PKGS[1], true, Collections.singletonList(PKGS[3]))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[2] + 1, user.stop) + new UidRangeParcel(userStart + PKG_UIDS[2] + 1, userStop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.start + PKG_UIDS[3] - 1), - new UidRangeParcel(user.start + PKG_UIDS[3] + 1, user.stop) + new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStart + PKG_UIDS[3] - 1), + new UidRangeParcel(userStart + PKG_UIDS[3] + 1, userStop) })); // Change the VPN app. assertTrue(vpn.setAlwaysOnPackage( PKGS[0], true, Collections.singletonList(PKGS[3]))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), - new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.start + PKG_UIDS[3] - 1) + new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), + new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStart + PKG_UIDS[3] - 1) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start, user.start + PKG_UIDS[0] - 1), - new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[3] - 1) + new UidRangeParcel(userStart, userStart + PKG_UIDS[0] - 1), + new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[3] - 1) })); // Remove the list of allowed packages. assertTrue(vpn.setAlwaysOnPackage(PKGS[0], true, null)); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[3] - 1), - new UidRangeParcel(user.start + PKG_UIDS[3] + 1, user.stop) + new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[3] - 1), + new UidRangeParcel(userStart + PKG_UIDS[3] + 1, userStop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.stop), + new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStop), })); // Add the list of allowed packages. assertTrue(vpn.setAlwaysOnPackage( PKGS[0], true, Collections.singletonList(PKGS[1]))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.stop) + new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[1] - 1), - new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) + new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[1] - 1), + new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) })); // Try allowing a package with a comma, should be rejected. @@ -439,12 +456,12 @@ public class VpnTest { assertTrue(vpn.setAlwaysOnPackage( PKGS[0], true, Arrays.asList("com.foo.app", PKGS[2], "com.bar.app"))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[1] - 1), - new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) + new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[1] - 1), + new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[2] - 1), - new UidRangeParcel(user.start + PKG_UIDS[2] + 1, user.stop) + new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[2] - 1), + new UidRangeParcel(userStart + PKG_UIDS[2] + 1, userStop) })); } @@ -452,7 +469,7 @@ public class VpnTest { public void testLockdownRuleRepeatability() throws Exception { final Vpn vpn = createVpn(primaryUser.id); final UidRangeParcel[] primaryUserRangeParcel = new UidRangeParcel[] { - new UidRangeParcel(PRI_USER_RANGE.start, PRI_USER_RANGE.stop)}; + new UidRangeParcel(PRI_USER_RANGE.getLower(), PRI_USER_RANGE.getUpper())}; // Given legacy lockdown is already enabled, vpn.setLockdown(true); verify(mConnectivityManager, times(1)).setRequireVpnForUids(true, @@ -484,7 +501,7 @@ public class VpnTest { public void testLockdownRuleReversibility() throws Exception { final Vpn vpn = createVpn(primaryUser.id); final UidRangeParcel[] entireUser = { - new UidRangeParcel(PRI_USER_RANGE.start, PRI_USER_RANGE.stop) + new UidRangeParcel(PRI_USER_RANGE.getLower(), PRI_USER_RANGE.getUpper()) }; final UidRangeParcel[] exceptPkg0 = { new UidRangeParcel(entireUser[0].start, entireUser[0].start + PKG_UIDS[0] - 1), From c2ea3ab97d37dd265078a3206fcb2dc116112fb7 Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Fri, 19 Mar 2021 00:45:39 +0000 Subject: [PATCH 2/3] Revert "Revert "Expose uids related APIs in NetworkRequest and N..." Revert^2 "Add shims for NetworkRequest" b72b3ca768fc25ef72dc78f1396b08447b8ef5c6 NetworkRequest is moving into the incoming connectivity mainline module. The hidden setUids becomes inaccessible outside the module. Shims for support cts in different API levels will need to use it to verify the behavior of NetworkRequest. Thus, expose it to the API surface. Also, VPN uses getUids and setUids to control network capabilities. Networkcapabilities is a part of incoming connectivity mainline module but VPN is not. Thus, exposing these two methods are needed to allow VPN to continue using it. Test: make update-api Bug: 172183305 Change-Id: I4b8e1aa558e3459a932535f9901f4ae86b0ecb67 Merged-In: I107c329d4d7130d488772166eae8b5e7aaa2ff04 --- framework/api/module-lib-current.txt | 9 ++++++++ .../src/android/net/NetworkCapabilities.java | 21 +++++++++++++++++++ framework/src/android/net/NetworkRequest.java | 4 ++++ 3 files changed, 34 insertions(+) diff --git a/framework/api/module-lib-current.txt b/framework/api/module-lib-current.txt index bb296476c7..6339094341 100644 --- a/framework/api/module-lib-current.txt +++ b/framework/api/module-lib-current.txt @@ -27,9 +27,18 @@ package android.net { } public final class NetworkCapabilities implements android.os.Parcelable { + method @Nullable public java.util.Set> getUids(); field public static final int TRANSPORT_TEST = 7; // 0x7 } + public static final class NetworkCapabilities.Builder { + method @NonNull public android.net.NetworkCapabilities.Builder setUids(@Nullable java.util.Set>); + } + + public static class NetworkRequest.Builder { + method @NonNull public android.net.NetworkRequest.Builder setUids(@Nullable java.util.Set>); + } + public class ParseException extends java.lang.RuntimeException { ctor public ParseException(@NonNull String); ctor public ParseException(@NonNull String, @NonNull Throwable); diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index ba9f21b261..182bc7141a 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -22,6 +22,7 @@ import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; +import android.annotation.SuppressLint; import android.annotation.SystemApi; import android.compat.annotation.UnsupportedAppUsage; import android.net.ConnectivityManager.NetworkCallback; @@ -1477,8 +1478,13 @@ public final class NetworkCapabilities implements Parcelable { /** * Get the list of UIDs this network applies to. * This returns a copy of the set so that callers can't modify the original object. + * + * @return the list of UIDs this network applies to. If {@code null}, then the network applies + * to all UIDs. * @hide */ + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + @SuppressLint("NullableCollection") public @Nullable Set> getUids() { return UidRange.toIntRanges(mUids); } @@ -2661,6 +2667,21 @@ public final class NetworkCapabilities implements Parcelable { return this; } + /** + * Set the list of UIDs this network applies to. + * + * @param uids the list of UIDs this network applies to, or {@code null} if this network + * applies to all UIDs. + * @return this builder + * @hide + */ + @NonNull + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + public Builder setUids(@Nullable Set> uids) { + mCaps.setUids(uids); + return this; + } + /** * Builds the instance of the capabilities. * diff --git a/framework/src/android/net/NetworkRequest.java b/framework/src/android/net/NetworkRequest.java index 4ebbf06c51..cf131f0df6 100644 --- a/framework/src/android/net/NetworkRequest.java +++ b/framework/src/android/net/NetworkRequest.java @@ -36,6 +36,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_TEST; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; +import android.annotation.SuppressLint; import android.annotation.SystemApi; import android.compat.annotation.UnsupportedAppUsage; import android.net.NetworkCapabilities.NetCapability; @@ -282,6 +283,9 @@ public class NetworkRequest implements Parcelable { * @return The builder to facilitate chaining. * @hide */ + @NonNull + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + @SuppressLint("MissingGetterMatchingBuilder") public Builder setUids(@Nullable Set> uids) { mNetworkCapabilities.setUids(uids); return this; From 98f59ecb99a753e204cc1069390014b66cf239dd Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 23 Feb 2021 08:47:39 -0800 Subject: [PATCH 3/3] TransportInfo: Add a generic redaction mechanism This replaces the existing mechanism for redacting location sensitive fields with a more extensible mechanism. Currently supported redactions are for the following permissions: i. ACCESS_FINE_LOCATION ii. LOCAL_MAC_ADDRESS iii. NETWORK_SETTINGS Also, removed WifiInfo from ConnectivityServiceTest to reduce cross dependencies on wifi code. Bug: 156867433 Bug: 162602799 Test: atest android.net Test: atest com.android.server Change-Id: I2bb980c624667a55c1383f13ab71b9b97ed6eeab --- framework/api/module-lib-current.txt | 11 + framework/api/system-current.txt | 6 - framework/src/android/net/NetworkAgent.java | 5 +- .../src/android/net/NetworkCapabilities.java | 111 +++++++- framework/src/android/net/TransportInfo.java | 48 ++-- .../android/server/ConnectivityService.java | 172 +++++++++--- .../android/net/NetworkCapabilitiesTest.java | 129 +++++---- .../server/ConnectivityServiceTest.java | 259 +++++++++++++++--- 8 files changed, 570 insertions(+), 171 deletions(-) diff --git a/framework/api/module-lib-current.txt b/framework/api/module-lib-current.txt index 6339094341..f484c3cdb0 100644 --- a/framework/api/module-lib-current.txt +++ b/framework/api/module-lib-current.txt @@ -27,7 +27,13 @@ package android.net { } public final class NetworkCapabilities implements android.os.Parcelable { + ctor public NetworkCapabilities(@Nullable android.net.NetworkCapabilities, long); method @Nullable public java.util.Set> getUids(); + field public static final long REDACT_ALL = -1L; // 0xffffffffffffffffL + field public static final long REDACT_FOR_ACCESS_FINE_LOCATION = 1L; // 0x1L + field public static final long REDACT_FOR_LOCAL_MAC_ADDRESS = 2L; // 0x2L + field public static final long REDACT_FOR_NETWORK_SETTINGS = 4L; // 0x4L + field public static final long REDACT_NONE = 0L; // 0x0L field public static final int TRANSPORT_TEST = 7; // 0x7 } @@ -79,6 +85,11 @@ package android.net { field @NonNull public static final android.os.Parcelable.Creator CREATOR; } + public interface TransportInfo { + method public default long getApplicableRedactions(); + method @NonNull public default android.net.TransportInfo makeCopy(long); + } + public final class VpnTransportInfo implements android.os.Parcelable android.net.TransportInfo { ctor public VpnTransportInfo(int); method public int describeContents(); diff --git a/framework/api/system-current.txt b/framework/api/system-current.txt index 4dca411cca..5cab0bcf1c 100644 --- a/framework/api/system-current.txt +++ b/framework/api/system-current.txt @@ -263,7 +263,6 @@ package android.net { } public final class NetworkCapabilities implements android.os.Parcelable { - ctor public NetworkCapabilities(@Nullable android.net.NetworkCapabilities, boolean); method @NonNull public int[] getAdministratorUids(); method @Nullable public String getSsid(); method @NonNull public int[] getTransportTypes(); @@ -437,11 +436,6 @@ package android.net { field public final int tcpWindowScale; } - public interface TransportInfo { - method public default boolean hasLocationSensitiveFields(); - method @NonNull public default android.net.TransportInfo makeCopy(boolean); - } - } package android.net.apf { diff --git a/framework/src/android/net/NetworkAgent.java b/framework/src/android/net/NetworkAgent.java index b3ab0ee8bd..c21aff9639 100644 --- a/framework/src/android/net/NetworkAgent.java +++ b/framework/src/android/net/NetworkAgent.java @@ -434,7 +434,7 @@ public abstract class NetworkAgent { } mInitialConfiguration = new InitialConfiguration(context, - new NetworkCapabilities(nc, /* parcelLocationSensitiveFields */ true), + new NetworkCapabilities(nc, NetworkCapabilities.REDACT_NONE), new LinkProperties(lp), score, config, ni); } @@ -878,8 +878,7 @@ public abstract class NetworkAgent { mBandwidthUpdatePending.set(false); mLastBwRefreshTime = System.currentTimeMillis(); final NetworkCapabilities nc = - new NetworkCapabilities(networkCapabilities, - /* parcelLocationSensitiveFields */ true); + new NetworkCapabilities(networkCapabilities, NetworkCapabilities.REDACT_NONE); queueOrSendMessage(reg -> reg.sendNetworkCapabilities(nc)); } diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index 182bc7141a..add9a15db4 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -19,6 +19,7 @@ package android.net; import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIVATE; import android.annotation.IntDef; +import android.annotation.LongDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; @@ -65,6 +66,68 @@ import java.util.StringJoiner; public final class NetworkCapabilities implements Parcelable { private static final String TAG = "NetworkCapabilities"; + /** + * Mechanism to support redaction of fields in NetworkCapabilities that are guarded by specific + * app permissions. + **/ + /** + * Don't redact any fields since the receiving app holds all the necessary permissions. + * + * @hide + */ + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + public static final long REDACT_NONE = 0; + + /** + * Redact any fields that need {@link android.Manifest.permission#ACCESS_FINE_LOCATION} + * permission since the receiving app does not hold this permission or the location toggle + * is off. + * + * @see android.Manifest.permission#ACCESS_FINE_LOCATION + * @hide + */ + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + public static final long REDACT_FOR_ACCESS_FINE_LOCATION = 1 << 0; + + /** + * Redact any fields that need {@link android.Manifest.permission#LOCAL_MAC_ADDRESS} + * permission since the receiving app does not hold this permission. + * + * @see android.Manifest.permission#LOCAL_MAC_ADDRESS + * @hide + */ + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + public static final long REDACT_FOR_LOCAL_MAC_ADDRESS = 1 << 1; + + /** + * + * Redact any fields that need {@link android.Manifest.permission#NETWORK_SETTINGS} + * permission since the receiving app does not hold this permission. + * + * @see android.Manifest.permission#NETWORK_SETTINGS + * @hide + */ + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + public static final long REDACT_FOR_NETWORK_SETTINGS = 1 << 2; + + /** + * Redact all fields in this object that require any relevant permission. + * @hide + */ + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + public static final long REDACT_ALL = -1L; + + /** @hide */ + @LongDef(flag = true, prefix = { "REDACT_" }, value = { + REDACT_NONE, + REDACT_FOR_ACCESS_FINE_LOCATION, + REDACT_FOR_LOCAL_MAC_ADDRESS, + REDACT_FOR_NETWORK_SETTINGS, + REDACT_ALL + }) + @Retention(RetentionPolicy.SOURCE) + public @interface RedactionType {} + // Set to true when private DNS is broken. private boolean mPrivateDnsBroken; @@ -79,32 +142,31 @@ 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}. + * Indicates what fields should be redacted from this instance. */ - private final boolean mParcelLocationSensitiveFields; + private final @RedactionType long mRedactions; public NetworkCapabilities() { - mParcelLocationSensitiveFields = false; + mRedactions = REDACT_ALL; clearAll(); mNetworkCapabilities = DEFAULT_CAPABILITIES; } public NetworkCapabilities(NetworkCapabilities nc) { - this(nc, false /* parcelLocationSensitiveFields */); + this(nc, REDACT_ALL); } /** * Make a copy of NetworkCapabilities. * * @param nc Original NetworkCapabilities - * @param parcelLocationSensitiveFields Whether to parcel location sensitive data or not. + * @param redactions bitmask of redactions that needs to be performed on this new instance of + * {@link NetworkCapabilities}. * @hide */ - @SystemApi - public NetworkCapabilities( - @Nullable NetworkCapabilities nc, boolean parcelLocationSensitiveFields) { - mParcelLocationSensitiveFields = parcelLocationSensitiveFields; + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + public NetworkCapabilities(@Nullable NetworkCapabilities nc, @RedactionType long redactions) { + mRedactions = redactions; if (nc != null) { set(nc); } @@ -116,11 +178,13 @@ 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) { + // Ensures that the internal copies maintained by the connectivity stack does not set it to + // anything other than |REDACT_ALL|. + if (mRedactions != REDACT_ALL) { + // This is needed because the current redaction mechanism relies on redaction while + // parceling. throw new UnsupportedOperationException( - "Cannot clear NetworkCapabilities when parcelLocationSensitiveFields is set"); + "Cannot clear NetworkCapabilities when mRedactions is set"); } mNetworkCapabilities = mTransportTypes = mUnwantedNetworkCapabilities = 0; mLinkUpBandwidthKbps = mLinkDownBandwidthKbps = LINK_BANDWIDTH_UNSPECIFIED; @@ -150,7 +214,7 @@ public final class NetworkCapabilities implements Parcelable { mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps; mNetworkSpecifier = nc.mNetworkSpecifier; if (nc.getTransportInfo() != null) { - setTransportInfo(nc.getTransportInfo().makeCopy(mParcelLocationSensitiveFields)); + setTransportInfo(nc.getTransportInfo().makeCopy(mRedactions)); } else { setTransportInfo(null); } @@ -2350,6 +2414,23 @@ public final class NetworkCapabilities implements Parcelable { } } + /** + * Returns a bitmask of all the applicable redactions (based on the permissions held by the + * receiving app) to be performed on this object. + * + * @return bitmask of redactions applicable on this instance. + * @hide + */ + public @RedactionType long getApplicableRedactions() { + // Currently, there are no fields redacted in NetworkCapabilities itself, so we just + // passthrough the redactions required by the embedded TransportInfo. If this changes + // in the future, modify this method. + if (mTransportInfo == null) { + return NetworkCapabilities.REDACT_NONE; + } + return mTransportInfo.getApplicableRedactions(); + } + /** * Builder class for NetworkCapabilities. * diff --git a/framework/src/android/net/TransportInfo.java b/framework/src/android/net/TransportInfo.java index aa4bbb0511..fa889eabb8 100644 --- a/framework/src/android/net/TransportInfo.java +++ b/framework/src/android/net/TransportInfo.java @@ -29,35 +29,47 @@ import android.annotation.SystemApi; 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. + * Create a copy of a {@link TransportInfo} with some fields redacted based on the permissions + * held by the receiving app. * - *

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. + *

+ * Usage by connectivity stack: + *

    + *
  • Connectivity stack will invoke {@link #getApplicableRedactions()} to find the list + * of redactions that are required by this {@link TransportInfo} instance.
    • + *
  • Connectivity stack then loops through each bit in the bitmask returned and checks if the + * receiving app holds the corresponding permission. + *
      + *
    • If the app holds the corresponding permission, the bit is cleared from the + * |redactions| bitmask.
      • + *
    • If the app does not hold the corresponding permission, the bit is retained in the + * |redactions| bitmask.
      • + *
    + *
  • Connectivity stack then invokes {@link #makeCopy(long)} with the necessary |redactions| + * to create a copy to send to the corresponding app.
    • + *
+ *

* - * @param parcelLocationSensitiveFields Whether the location sensitive fields should be kept - * when parceling - * @return Copy of this instance. + * @param redactions bitmask of redactions that needs to be performed on this instance. + * @return Copy of this instance with the necessary redactions. * @hide */ - @SystemApi + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) @NonNull - default TransportInfo makeCopy(boolean parcelLocationSensitiveFields) { + default TransportInfo makeCopy(@NetworkCapabilities.RedactionType long redactions) { 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). + * Returns a bitmask of all the applicable redactions (based on the permissions held by the + * receiving app) to be performed on this TransportInfo. * - * @return {@code true} if this instance contains location sensitive info, {@code false} - * otherwise. + * @return bitmask of redactions applicable on this instance. + * @see #makeCopy(long) * @hide */ - @SystemApi - default boolean hasLocationSensitiveFields() { - return false; + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + default @NetworkCapabilities.RedactionType long getApplicableRedactions() { + return NetworkCapabilities.REDACT_NONE; } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e2aa6e76ef..5347f9bb9f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -69,6 +69,9 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_OEM_PAID; import static android.net.NetworkCapabilities.NET_CAPABILITY_OEM_PRIVATE; import static android.net.NetworkCapabilities.NET_CAPABILITY_PARTIAL_CONNECTIVITY; import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; +import static android.net.NetworkCapabilities.REDACT_FOR_ACCESS_FINE_LOCATION; +import static android.net.NetworkCapabilities.REDACT_FOR_LOCAL_MAC_ADDRESS; +import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static android.net.NetworkCapabilities.TRANSPORT_VPN; @@ -1779,7 +1782,8 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.network, createWithLocationInfoSanitizedIfNecessaryWhenParceled( nc, false /* includeLocationSensitiveInfo */, - mDeps.getCallingUid(), callingPackageName, callingAttributionTag)); + getCallingPid(), mDeps.getCallingUid(), callingPackageName, + callingAttributionTag)); } } @@ -1794,7 +1798,7 @@ public class ConnectivityService extends IConnectivityManager.Stub createWithLocationInfoSanitizedIfNecessaryWhenParceled( nc, false /* includeLocationSensitiveInfo */, - mDeps.getCallingUid(), callingPackageName, + getCallingPid(), mDeps.getCallingUid(), callingPackageName, callingAttributionTag)); } } @@ -1877,7 +1881,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return createWithLocationInfoSanitizedIfNecessaryWhenParceled( getNetworkCapabilitiesInternal(network), false /* includeLocationSensitiveInfo */, - mDeps.getCallingUid(), callingPackageName, callingAttributionTag); + getCallingPid(), mDeps.getCallingUid(), callingPackageName, callingAttributionTag); } @VisibleForTesting @@ -1896,40 +1900,137 @@ public class ConnectivityService extends IConnectivityManager.Stub return newNc; } - private boolean hasLocationPermission(int callerUid, @NonNull String callerPkgName, - @Nullable String callingAttributionTag) { - final long token = Binder.clearCallingIdentity(); - try { - return mLocationPermissionChecker.checkLocationPermission( - callerPkgName, callingAttributionTag, callerUid, null /* message */); - } finally { - Binder.restoreCallingIdentity(token); + /** + * Wrapper used to cache the permission check results performed for the corresponding + * app. This avoid performing multiple permission checks for different fields in + * NetworkCapabilities. + * Note: This wrapper does not support any sort of invalidation and thus must not be + * persistent or long-lived. It may only be used for the time necessary to + * compute the redactions required by one particular NetworkCallback or + * synchronous call. + */ + private class RedactionPermissionChecker { + private final int mCallingPid; + private final int mCallingUid; + @NonNull private final String mCallingPackageName; + @Nullable private final String mCallingAttributionTag; + + private Boolean mHasLocationPermission = null; + private Boolean mHasLocalMacAddressPermission = null; + private Boolean mHasSettingsPermission = null; + + RedactionPermissionChecker(int callingPid, int callingUid, + @NonNull String callingPackageName, @Nullable String callingAttributionTag) { + mCallingPid = callingPid; + mCallingUid = callingUid; + mCallingPackageName = callingPackageName; + mCallingAttributionTag = callingAttributionTag; } + + private boolean hasLocationPermissionInternal() { + final long token = Binder.clearCallingIdentity(); + try { + return mLocationPermissionChecker.checkLocationPermission( + mCallingPackageName, mCallingAttributionTag, mCallingUid, + null /* message */); + } finally { + Binder.restoreCallingIdentity(token); + } + } + + /** + * Returns whether the app holds location permission or not (might return cached result + * if the permission was already checked before). + */ + public boolean hasLocationPermission() { + if (mHasLocationPermission == null) { + // If there is no cached result, perform the check now. + mHasLocationPermission = hasLocationPermissionInternal(); + } + return mHasLocationPermission; + } + + /** + * Returns whether the app holds local mac address permission or not (might return cached + * result if the permission was already checked before). + */ + public boolean hasLocalMacAddressPermission() { + if (mHasLocalMacAddressPermission == null) { + // If there is no cached result, perform the check now. + mHasLocalMacAddressPermission = + checkLocalMacAddressPermission(mCallingPid, mCallingUid); + } + return mHasLocalMacAddressPermission; + } + + /** + * Returns whether the app holds settings permission or not (might return cached + * result if the permission was already checked before). + */ + public boolean hasSettingsPermission() { + if (mHasSettingsPermission == null) { + // If there is no cached result, perform the check now. + mHasSettingsPermission = checkSettingsPermission(mCallingPid, mCallingUid); + } + return mHasSettingsPermission; + } + } + + private static boolean shouldRedact(@NetworkCapabilities.RedactionType long redactions, + @NetworkCapabilities.NetCapability long redaction) { + return (redactions & redaction) != 0; + } + + /** + * Use the provided |applicableRedactions| to check the receiving app's + * permissions and clear/set the corresponding bit in the returned bitmask. The bitmask + * returned will be used to ensure the necessary redactions are performed by NetworkCapabilities + * before being sent to the corresponding app. + */ + private @NetworkCapabilities.RedactionType long retrieveRequiredRedactions( + @NetworkCapabilities.RedactionType long applicableRedactions, + @NonNull RedactionPermissionChecker redactionPermissionChecker, + boolean includeLocationSensitiveInfo) { + long redactions = applicableRedactions; + if (shouldRedact(redactions, REDACT_FOR_ACCESS_FINE_LOCATION)) { + if (includeLocationSensitiveInfo + && redactionPermissionChecker.hasLocationPermission()) { + redactions &= ~REDACT_FOR_ACCESS_FINE_LOCATION; + } + } + if (shouldRedact(redactions, REDACT_FOR_LOCAL_MAC_ADDRESS)) { + if (redactionPermissionChecker.hasLocalMacAddressPermission()) { + redactions &= ~REDACT_FOR_LOCAL_MAC_ADDRESS; + } + } + if (shouldRedact(redactions, REDACT_FOR_NETWORK_SETTINGS)) { + if (redactionPermissionChecker.hasSettingsPermission()) { + redactions &= ~REDACT_FOR_NETWORK_SETTINGS; + } + } + return redactions; } @VisibleForTesting @Nullable NetworkCapabilities createWithLocationInfoSanitizedIfNecessaryWhenParceled( @Nullable NetworkCapabilities nc, boolean includeLocationSensitiveInfo, - int callerUid, @NonNull String callerPkgName, @Nullable String callingAttributionTag) { + int callingPid, int callingUid, @NonNull String callingPkgName, + @Nullable String callingAttributionTag) { 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 (includeLocationSensitiveInfo - && nc.getTransportInfo() != null - && nc.getTransportInfo().hasLocationSensitiveFields()) { - hasLocationPermission = - hasLocationPermission(callerUid, callerPkgName, callingAttributionTag); - newNc = new NetworkCapabilities(nc, hasLocationPermission); - } else { - newNc = new NetworkCapabilities(nc, false /* parcelLocationSensitiveFields */); - } + final RedactionPermissionChecker redactionPermissionChecker = + new RedactionPermissionChecker(callingPid, callingUid, callingPkgName, + callingAttributionTag); + final long redactions = retrieveRequiredRedactions( + nc.getApplicableRedactions(), redactionPermissionChecker, + includeLocationSensitiveInfo); + final NetworkCapabilities newNc = new NetworkCapabilities(nc, redactions); // Reset owner uid if not destined for the owner app. - if (callerUid != nc.getOwnerUid()) { + if (callingUid != nc.getOwnerUid()) { newNc.setOwnerUid(INVALID_UID); return newNc; } @@ -1938,23 +2039,17 @@ public class ConnectivityService extends IConnectivityManager.Stub // Owner UIDs already checked above. No need to re-check. return newNc; } - // If the caller does not want location sensitive data & target SDK >= S, then mask info. - // Else include the owner UID iff the caller has location permission to provide backwards + // If the calling does not want location sensitive data & target SDK >= S, then mask info. + // Else include the owner UID iff the calling has location permission to provide backwards // compatibility for older apps. if (!includeLocationSensitiveInfo && isTargetSdkAtleast( - Build.VERSION_CODES.S, callerUid, callerPkgName)) { + Build.VERSION_CODES.S, callingUid, callingPkgName)) { newNc.setOwnerUid(INVALID_UID); return newNc; } - - if (hasLocationPermission == null) { - // Location permission not checked yet, check now for masking owner UID. - hasLocationPermission = - hasLocationPermission(callerUid, callerPkgName, callingAttributionTag); - } // Reset owner uid if the app has no location permission. - if (!hasLocationPermission) { + if (!redactionPermissionChecker.hasLocationPermission()) { newNc.setOwnerUid(INVALID_UID); } return newNc; @@ -2469,6 +2564,11 @@ public class ConnectivityService extends IConnectivityManager.Stub mContext.enforceCallingOrSelfPermission(KeepaliveTracker.PERMISSION, "ConnectivityService"); } + private boolean checkLocalMacAddressPermission(int pid, int uid) { + return PERMISSION_GRANTED == mContext.checkPermission( + Manifest.permission.LOCAL_MAC_ADDRESS, pid, uid); + } + private void sendConnectedBroadcast(NetworkInfo info) { sendGeneralBroadcast(info, CONNECTIVITY_ACTION); } @@ -7170,7 +7270,7 @@ public class ConnectivityService extends IConnectivityManager.Stub putParcelable( bundle, createWithLocationInfoSanitizedIfNecessaryWhenParceled( - nc, includeLocationSensitiveInfo, nri.mUid, + nc, includeLocationSensitiveInfo, nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag)); putParcelable(bundle, linkPropertiesRestrictedForCallerPermissions( @@ -7191,7 +7291,7 @@ public class ConnectivityService extends IConnectivityManager.Stub putParcelable( bundle, createWithLocationInfoSanitizedIfNecessaryWhenParceled( - netCap, includeLocationSensitiveInfo, nri.mUid, + netCap, includeLocationSensitiveInfo, nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag)); break; diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index a7ad695641..d40b88ca59 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -35,6 +35,9 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_OEM_PRIVATE; import static android.net.NetworkCapabilities.NET_CAPABILITY_PARTIAL_CONNECTIVITY; import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static android.net.NetworkCapabilities.NET_CAPABILITY_WIFI_P2P; +import static android.net.NetworkCapabilities.REDACT_FOR_ACCESS_FINE_LOCATION; +import static android.net.NetworkCapabilities.REDACT_FOR_LOCAL_MAC_ADDRESS; +import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; import static android.net.NetworkCapabilities.RESTRICTED_CAPABILITIES; import static android.net.NetworkCapabilities.SIGNAL_STRENGTH_UNSPECIFIED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; @@ -51,7 +54,6 @@ import static com.android.testutils.MiscAsserts.assertEmpty; import static com.android.testutils.MiscAsserts.assertThrows; 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; @@ -62,7 +64,6 @@ 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; @@ -352,55 +353,6 @@ 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, 17); @@ -411,6 +363,45 @@ public class NetworkCapabilitiesTest { } } + private static NetworkCapabilities createNetworkCapabilitiesWithTransportInfo() { + return new NetworkCapabilities() + .addCapability(NET_CAPABILITY_INTERNET) + .addCapability(NET_CAPABILITY_EIMS) + .addCapability(NET_CAPABILITY_NOT_METERED) + .setSSID(TEST_SSID) + .setTransportInfo(new TestTransportInfo()) + .setRequestorPackageName("com.android.test") + .setRequestorUid(9304); + } + + @Test + public void testNetworkCapabilitiesCopyWithNoRedactions() { + assumeTrue(isAtLeastS()); + + final NetworkCapabilities netCap = createNetworkCapabilitiesWithTransportInfo(); + final NetworkCapabilities netCapWithNoRedactions = + new NetworkCapabilities(netCap, NetworkCapabilities.REDACT_NONE); + TestTransportInfo testTransportInfo = + (TestTransportInfo) netCapWithNoRedactions.getTransportInfo(); + assertFalse(testTransportInfo.locationRedacted); + assertFalse(testTransportInfo.localMacAddressRedacted); + assertFalse(testTransportInfo.settingsRedacted); + } + + @Test + public void testNetworkCapabilitiesCopyWithoutLocationSensitiveFields() { + assumeTrue(isAtLeastS()); + + final NetworkCapabilities netCap = createNetworkCapabilitiesWithTransportInfo(); + final NetworkCapabilities netCapWithNoRedactions = + new NetworkCapabilities(netCap, REDACT_FOR_ACCESS_FINE_LOCATION); + TestTransportInfo testTransportInfo = + (TestTransportInfo) netCapWithNoRedactions.getTransportInfo(); + assertTrue(testTransportInfo.locationRedacted); + assertFalse(testTransportInfo.localMacAddressRedacted); + assertFalse(testTransportInfo.settingsRedacted); + } + @Test public void testOemPaid() { NetworkCapabilities nc = new NetworkCapabilities(); @@ -1062,18 +1053,42 @@ public class NetworkCapabilitiesTest { } catch (IllegalArgumentException e) { } } - private class TestTransportInfo implements TransportInfo { + /** + * Test TransportInfo to verify redaction mechanism. + */ + private static class TestTransportInfo implements TransportInfo { + public final boolean locationRedacted; + public final boolean localMacAddressRedacted; + public final boolean settingsRedacted; + TestTransportInfo() { + locationRedacted = false; + localMacAddressRedacted = false; + settingsRedacted = false; + } + + TestTransportInfo(boolean locationRedacted, + boolean localMacAddressRedacted, + boolean settingsRedacted) { + this.locationRedacted = locationRedacted; + this.localMacAddressRedacted = + localMacAddressRedacted; + this.settingsRedacted = settingsRedacted; } @Override - public TransportInfo makeCopy(boolean parcelLocationSensitiveFields) { - return this; + public TransportInfo makeCopy(@NetworkCapabilities.RedactionType long redactions) { + return new TestTransportInfo( + (redactions & NetworkCapabilities.REDACT_FOR_ACCESS_FINE_LOCATION) != 0, + (redactions & REDACT_FOR_LOCAL_MAC_ADDRESS) != 0, + (redactions & REDACT_FOR_NETWORK_SETTINGS) != 0 + ); } @Override - public boolean hasLocationSensitiveFields() { - return false; + public @NetworkCapabilities.RedactionType long getApplicableRedactions() { + return REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_LOCAL_MAC_ADDRESS + | REDACT_FOR_NETWORK_SETTINGS; } } @@ -1084,7 +1099,7 @@ public class NetworkCapabilitiesTest { final int requestUid = 10100; final int[] administratorUids = {ownerUid, 10001}; final TelephonyNetworkSpecifier specifier = new TelephonyNetworkSpecifier(1); - final TestTransportInfo transportInfo = new TestTransportInfo(); + final TransportInfo transportInfo = new TransportInfo() {}; final String ssid = "TEST_SSID"; final String packageName = "com.google.test.networkcapabilities"; final NetworkCapabilities nc = new NetworkCapabilities.Builder() diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 6cb5305ea1..4c7d947e0b 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -82,6 +82,10 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_TRUSTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static android.net.NetworkCapabilities.NET_CAPABILITY_WIFI_P2P; import static android.net.NetworkCapabilities.NET_CAPABILITY_XCAP; +import static android.net.NetworkCapabilities.REDACT_FOR_ACCESS_FINE_LOCATION; +import static android.net.NetworkCapabilities.REDACT_FOR_LOCAL_MAC_ADDRESS; +import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; +import static android.net.NetworkCapabilities.REDACT_NONE; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; import static android.net.NetworkCapabilities.TRANSPORT_VPN; @@ -235,7 +239,6 @@ import android.net.resolv.aidl.PrivateDnsValidationEventParcel; 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; @@ -8838,29 +8841,34 @@ public class ConnectivityServiceTest { final NetworkCapabilities netCap = new NetworkCapabilities().setOwnerUid(ownerUid); return mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( - netCap, includeLocationSensitiveInfo, callerUid, + netCap, includeLocationSensitiveInfo, Process.myUid(), callerUid, mContext.getPackageName(), getAttributionTag()) .getOwnerUid(); } - private void verifyWifiInfoCopyNetCapsPermission( + private void verifyTransportInfoCopyNetCapsPermission( int callerUid, boolean includeLocationSensitiveInfo, boolean shouldMakeCopyWithLocationSensitiveFieldsParcelable) { - final WifiInfo wifiInfo = mock(WifiInfo.class); - when(wifiInfo.hasLocationSensitiveFields()).thenReturn(true); - final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(wifiInfo); + final TransportInfo transportInfo = mock(TransportInfo.class); + when(transportInfo.getApplicableRedactions()).thenReturn(REDACT_FOR_ACCESS_FINE_LOCATION); + final NetworkCapabilities netCap = + new NetworkCapabilities().setTransportInfo(transportInfo); mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( - netCap, includeLocationSensitiveInfo, callerUid, + netCap, includeLocationSensitiveInfo, Process.myPid(), callerUid, mContext.getPackageName(), getAttributionTag()); - verify(wifiInfo).makeCopy(eq(shouldMakeCopyWithLocationSensitiveFieldsParcelable)); + if (shouldMakeCopyWithLocationSensitiveFieldsParcelable) { + verify(transportInfo).makeCopy(REDACT_NONE); + } else { + verify(transportInfo).makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION); + } } - private void verifyOwnerUidAndWifiInfoNetCapsPermission( + private void verifyOwnerUidAndTransportInfoNetCapsPermission( boolean shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag, boolean shouldInclLocationSensitiveOwnerUidWithIncludeFlag, - boolean shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag, - boolean shouldInclLocationSensitiveWifiInfoWithIncludeFlag) { + boolean shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag, + boolean shouldInclLocationSensitiveTransportInfoWithIncludeFlag) { final int myUid = Process.myUid(); final int expectedOwnerUidWithoutIncludeFlag = @@ -8874,13 +8882,13 @@ public class ConnectivityServiceTest { assertEquals(expectedOwnerUidWithIncludeFlag, getOwnerUidNetCapsPermission( myUid, myUid, true /* includeLocationSensitiveInfo */)); - verifyWifiInfoCopyNetCapsPermission(myUid, + verifyTransportInfoCopyNetCapsPermission(myUid, false, /* includeLocationSensitiveInfo */ - shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag); + shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag); - verifyWifiInfoCopyNetCapsPermission(myUid, + verifyTransportInfoCopyNetCapsPermission(myUid, true, /* includeLocationSensitiveInfo */ - shouldInclLocationSensitiveWifiInfoWithIncludeFlag); + shouldInclLocationSensitiveTransportInfoWithIncludeFlag); } @@ -8890,15 +8898,15 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - verifyOwnerUidAndWifiInfoNetCapsPermission( + verifyOwnerUidAndTransportInfoNetCapsPermission( // Ensure that we include owner uid even if the request asks to remove it since the // app has necessary permissions and targetSdk < S. true, /* shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag */ true, /* shouldInclLocationSensitiveOwnerUidWithIncludeFlag */ - false, /* shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag */ + false, /* shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag */ // Ensure that we remove location info if the request asks to remove it even if the // app has necessary permissions. - true /* shouldInclLocationSensitiveWifiInfoWithIncludeFlag */ + true /* shouldInclLocationSensitiveTransportInfoWithIncludeFlag */ ); } @@ -8908,15 +8916,15 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.R, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - verifyOwnerUidAndWifiInfoNetCapsPermission( + verifyOwnerUidAndTransportInfoNetCapsPermission( // Ensure that we include owner uid even if the request asks to remove it since the // app has necessary permissions and targetSdk < S. true, /* shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag */ true, /* shouldInclLocationSensitiveOwnerUidWithIncludeFlag */ - false, /* shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag */ + false, /* shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag */ // Ensure that we remove location info if the request asks to remove it even if the // app has necessary permissions. - true /* shouldInclLocationSensitiveWifiInfoWithIncludeFlag */ + true /* shouldInclLocationSensitiveTransportInfoWithIncludeFlag */ ); } @@ -8927,15 +8935,15 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.S, true, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - verifyOwnerUidAndWifiInfoNetCapsPermission( + verifyOwnerUidAndTransportInfoNetCapsPermission( // Ensure that we owner UID if the request asks us to remove it even if the app // has necessary permissions since targetSdk >= S. false, /* shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag */ true, /* shouldInclLocationSensitiveOwnerUidWithIncludeFlag */ - false, /* shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag */ + false, /* shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag */ // Ensure that we remove location info if the request asks to remove it even if the // app has necessary permissions. - true /* shouldInclLocationSensitiveWifiInfoWithIncludeFlag */ + true /* shouldInclLocationSensitiveTransportInfoWithIncludeFlag */ ); } @@ -8945,15 +8953,15 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.P, true, AppOpsManager.OPSTR_COARSE_LOCATION, Manifest.permission.ACCESS_COARSE_LOCATION); - verifyOwnerUidAndWifiInfoNetCapsPermission( + verifyOwnerUidAndTransportInfoNetCapsPermission( // Ensure that we owner UID if the request asks us to remove it even if the app // has necessary permissions since targetSdk >= S. true, /* shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag */ true, /* shouldInclLocationSensitiveOwnerUidWithIncludeFlag */ - false, /* shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag */ + false, /* shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag */ // Ensure that we remove location info if the request asks to remove it even if the // app has necessary permissions. - true /* shouldInclLocationSensitiveWifiInfoWithIncludeFlag */ + true /* shouldInclLocationSensitiveTransportInfoWithIncludeFlag */ ); } @@ -8963,11 +8971,11 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, false, AppOpsManager.OPSTR_FINE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION); - verifyOwnerUidAndWifiInfoNetCapsPermission( + verifyOwnerUidAndTransportInfoNetCapsPermission( false, /* shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag */ false, /* shouldInclLocationSensitiveOwnerUidWithIncludeFlag */ - false, /* shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag */ - false /* shouldInclLocationSensitiveWifiInfoWithIncludeFlag */ + false, /* shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag */ + false /* shouldInclLocationSensitiveTransportInfoWithIncludeFlag */ ); } @@ -8990,11 +8998,11 @@ public class ConnectivityServiceTest { setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_COARSE_LOCATION, Manifest.permission.ACCESS_COARSE_LOCATION); - verifyOwnerUidAndWifiInfoNetCapsPermission( + verifyOwnerUidAndTransportInfoNetCapsPermission( false, /* shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag */ false, /* shouldInclLocationSensitiveOwnerUidWithIncludeFlag */ - false, /* shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag */ - false /* shouldInclLocationSensitiveWifiInfoWithIncludeFlag */ + false, /* shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag */ + false /* shouldInclLocationSensitiveTransportInfoWithIncludeFlag */ ); } @@ -9004,14 +9012,193 @@ public class ConnectivityServiceTest { // Test that not having fine location permission leads to sanitization. setupLocationPermissions(Build.VERSION_CODES.Q, true, null /* op */, null /* perm */); - verifyOwnerUidAndWifiInfoNetCapsPermission( + verifyOwnerUidAndTransportInfoNetCapsPermission( false, /* shouldInclLocationSensitiveOwnerUidWithoutIncludeFlag */ false, /* shouldInclLocationSensitiveOwnerUidWithIncludeFlag */ - false, /* shouldInclLocationSensitiveWifiInfoWithoutIncludeFlag */ - false /* shouldInclLocationSensitiveWifiInfoWithIncludeFlag */ + false, /* shouldInclLocationSensitiveTransportInfoWithoutIncludeFlag */ + false /* shouldInclLocationSensitiveTransportInfoWithIncludeFlag */ ); } + @Test + public void testCreateForCallerWithLocalMacAddressSanitizedWithLocalMacAddressPermission() + throws Exception { + mServiceContext.setPermission(Manifest.permission.LOCAL_MAC_ADDRESS, PERMISSION_GRANTED); + + final TransportInfo transportInfo = mock(TransportInfo.class); + when(transportInfo.getApplicableRedactions()) + .thenReturn(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_LOCAL_MAC_ADDRESS); + final NetworkCapabilities netCap = + new NetworkCapabilities().setTransportInfo(transportInfo); + + mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + netCap, false /* includeLocationSensitiveInfoInTransportInfo */, + Process.myPid(), Process.myUid(), + mContext.getPackageName(), getAttributionTag()); + // don't redact MAC_ADDRESS fields, only location sensitive fields. + verify(transportInfo).makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION); + } + + @Test + public void testCreateForCallerWithLocalMacAddressSanitizedWithoutLocalMacAddressPermission() + throws Exception { + mServiceContext.setPermission(Manifest.permission.LOCAL_MAC_ADDRESS, PERMISSION_DENIED); + + final TransportInfo transportInfo = mock(TransportInfo.class); + when(transportInfo.getApplicableRedactions()) + .thenReturn(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_LOCAL_MAC_ADDRESS); + final NetworkCapabilities netCap = + new NetworkCapabilities().setTransportInfo(transportInfo); + + mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + netCap, false /* includeLocationSensitiveInfoInTransportInfo */, + Process.myPid(), Process.myUid(), + mContext.getPackageName(), getAttributionTag()); + // redact both MAC_ADDRESS & location sensitive fields. + verify(transportInfo).makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION + | REDACT_FOR_LOCAL_MAC_ADDRESS); + } + + @Test + public void testCreateForCallerWithLocalMacAddressSanitizedWithSettingsPermission() + throws Exception { + mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED); + + final TransportInfo transportInfo = mock(TransportInfo.class); + when(transportInfo.getApplicableRedactions()) + .thenReturn(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_NETWORK_SETTINGS); + final NetworkCapabilities netCap = + new NetworkCapabilities().setTransportInfo(transportInfo); + + mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + netCap, false /* includeLocationSensitiveInfoInTransportInfo */, + Process.myPid(), Process.myUid(), + mContext.getPackageName(), getAttributionTag()); + // don't redact NETWORK_SETTINGS fields, only location sensitive fields. + verify(transportInfo).makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION); + } + + @Test + public void testCreateForCallerWithLocalMacAddressSanitizedWithoutSettingsPermission() + throws Exception { + mServiceContext.setPermission(Manifest.permission.LOCAL_MAC_ADDRESS, PERMISSION_DENIED); + + final TransportInfo transportInfo = mock(TransportInfo.class); + when(transportInfo.getApplicableRedactions()) + .thenReturn(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_NETWORK_SETTINGS); + final NetworkCapabilities netCap = + new NetworkCapabilities().setTransportInfo(transportInfo); + + mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + netCap, false /* includeLocationSensitiveInfoInTransportInfo */, + Process.myPid(), Process.myUid(), + mContext.getPackageName(), getAttributionTag()); + // redact both NETWORK_SETTINGS & location sensitive fields. + verify(transportInfo).makeCopy( + REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_NETWORK_SETTINGS); + } + + /** + * Test TransportInfo to verify redaction mechanism. + */ + private static class TestTransportInfo implements TransportInfo { + public final boolean locationRedacted; + public final boolean localMacAddressRedacted; + public final boolean settingsRedacted; + + TestTransportInfo() { + locationRedacted = false; + localMacAddressRedacted = false; + settingsRedacted = false; + } + + TestTransportInfo(boolean locationRedacted, boolean localMacAddressRedacted, + boolean settingsRedacted) { + this.locationRedacted = locationRedacted; + this.localMacAddressRedacted = + localMacAddressRedacted; + this.settingsRedacted = settingsRedacted; + } + + @Override + public TransportInfo makeCopy(@NetworkCapabilities.RedactionType long redactions) { + return new TestTransportInfo( + (redactions & REDACT_FOR_ACCESS_FINE_LOCATION) != 0, + (redactions & REDACT_FOR_LOCAL_MAC_ADDRESS) != 0, + (redactions & REDACT_FOR_NETWORK_SETTINGS) != 0 + ); + } + + @Override + public @NetworkCapabilities.RedactionType long getApplicableRedactions() { + return REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_LOCAL_MAC_ADDRESS + | REDACT_FOR_NETWORK_SETTINGS; + } + + @Override + public boolean equals(Object other) { + if (!(other instanceof TestTransportInfo)) return false; + TestTransportInfo that = (TestTransportInfo) other; + return that.locationRedacted == this.locationRedacted + && that.localMacAddressRedacted == this.localMacAddressRedacted + && that.settingsRedacted == this.settingsRedacted; + } + + @Override + public int hashCode() { + return Objects.hash(locationRedacted, localMacAddressRedacted, settingsRedacted); + } + } + + private void verifyNetworkCallbackLocationDataInclusionUsingTransportInfoAndOwnerUidInNetCaps( + @NonNull TestNetworkCallback wifiNetworkCallback, int actualOwnerUid, + @NonNull TransportInfo actualTransportInfo, int expectedOwnerUid, + @NonNull TransportInfo expectedTransportInfo) throws Exception { + when(mPackageManager.getTargetSdkVersion(anyString())).thenReturn(Build.VERSION_CODES.S); + final NetworkCapabilities ncTemplate = + new NetworkCapabilities() + .addTransportType(TRANSPORT_WIFI) + .setOwnerUid(actualOwnerUid); + + final NetworkRequest wifiRequest = new NetworkRequest.Builder() + .addTransportType(TRANSPORT_WIFI).build(); + mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback); + + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, new LinkProperties(), + ncTemplate); + mWiFiNetworkAgent.connect(false); + + wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + + // Send network capabilities update with TransportInfo to trigger capabilities changed + // callback. + mWiFiNetworkAgent.setNetworkCapabilities( + ncTemplate.setTransportInfo(actualTransportInfo), true); + + wifiNetworkCallback.expectCapabilitiesThat(mWiFiNetworkAgent, + nc -> Objects.equals(expectedOwnerUid, nc.getOwnerUid()) + && Objects.equals(expectedTransportInfo, nc.getTransportInfo())); + + } + + @Test + public void testVerifyLocationDataIsNotIncludedWhenInclFlagNotSet() throws Exception { + final TestNetworkCallback wifiNetworkCallack = new TestNetworkCallback(); + final int ownerUid = Process.myUid(); + final TransportInfo transportInfo = new TestTransportInfo(); + // Even though the test uid holds privileged permissions, mask location fields since + // the callback did not explicitly opt-in to get location data. + final TransportInfo sanitizedTransportInfo = new TestTransportInfo( + true, /* locationRedacted */ + true, /* localMacAddressRedacted */ + true /* settingsRedacted */ + ); + // Should not expect location data since the callback does not set the flag for including + // location data. + verifyNetworkCallbackLocationDataInclusionUsingTransportInfoAndOwnerUidInNetCaps( + wifiNetworkCallack, ownerUid, transportInfo, INVALID_UID, sanitizedTransportInfo); + } + private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType) throws Exception { final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE);