From 3533c1672086ec6a3a09d8389f64bf25419ba722 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 10 May 2021 00:49:14 +0900 Subject: [PATCH 1/3] Do not automatically redact TransportInfo objects. Currently, NetworkCapabilities always redacts the TransportInfo objects it contains whenever a defensive copy is made. This makes it impossible to make a defensive copy on a TransportInfo parcelled from another process without redacting it. Stop redacting by default; instead rely on ConnectivityService explicitly calling NetworkCapabilities' redacting constructor when it returns a NetworkCapabilities object to an app via a callback or synchronous call. This is currently done by - createWithLocationInfoSanitizedIfNecessaryWhenParceled, which is called from callCallbackForRequest, getNetworkCapabilities, and getDefaultNetworkCapabilitiesForUser. - getNetworkCapabilitiesWithoutUids, which is used when sending ConnectivityDiagnosticsManager callbacks. In this method, unconditionally redact all information, which is what the code did previously due to the default redaction setting for empty NetworkCapabilities objects being REDACT_ALL. Bug: 183938194 Test: atest NetworkCapabilitiesTest Test: atest FrameworksNetTests CtsNetTestCases HostsideVpnTests Change-Id: I3108ee94cb0930958e071ba678c3554525b0db82 --- .../src/android/net/NetworkCapabilities.java | 22 +---- .../android/net/NetworkCapabilitiesTest.java | 2 +- .../server/ConnectivityServiceTest.java | 91 +++++++++++++++---- .../android/server/ConnectivityService.java | 3 +- 4 files changed, 82 insertions(+), 36 deletions(-) diff --git a/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java b/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java index 4a99d290f38c1..c19a906ecd70e 100644 --- a/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java +++ b/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java @@ -139,19 +139,13 @@ public final class NetworkCapabilities implements Parcelable { */ private String mRequestorPackageName; - /** - * Indicates what fields should be redacted from this instance. - */ - private final @RedactionType long mRedactions; - public NetworkCapabilities() { - mRedactions = REDACT_ALL; clearAll(); mNetworkCapabilities = DEFAULT_CAPABILITIES; } public NetworkCapabilities(NetworkCapabilities nc) { - this(nc, REDACT_ALL); + this(nc, REDACT_NONE); } /** @@ -163,10 +157,12 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ public NetworkCapabilities(@Nullable NetworkCapabilities nc, @RedactionType long redactions) { - mRedactions = redactions; if (nc != null) { set(nc); } + if (mTransportInfo != null) { + mTransportInfo = nc.mTransportInfo.makeCopy(redactions); + } } /** @@ -175,14 +171,6 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ public void clearAll() { - // 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 mRedactions is set"); - } mNetworkCapabilities = mTransportTypes = mForbiddenNetworkCapabilities = 0; mLinkUpBandwidthKbps = mLinkDownBandwidthKbps = LINK_BANDWIDTH_UNSPECIFIED; mNetworkSpecifier = null; @@ -211,7 +199,7 @@ public final class NetworkCapabilities implements Parcelable { mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps; mNetworkSpecifier = nc.mNetworkSpecifier; if (nc.getTransportInfo() != null) { - setTransportInfo(nc.getTransportInfo().makeCopy(mRedactions)); + setTransportInfo(nc.getTransportInfo()); } else { setTransportInfo(null); } diff --git a/packages/Connectivity/tests/common/java/android/net/NetworkCapabilitiesTest.java b/packages/Connectivity/tests/common/java/android/net/NetworkCapabilitiesTest.java index b178bad7123d8..d74b802c87291 100644 --- a/packages/Connectivity/tests/common/java/android/net/NetworkCapabilitiesTest.java +++ b/packages/Connectivity/tests/common/java/android/net/NetworkCapabilitiesTest.java @@ -340,7 +340,7 @@ public class NetworkCapabilitiesTest { private void testParcelSane(NetworkCapabilities cap) { if (isAtLeastS()) { - assertParcelSane(cap, 17); + assertParcelSane(cap, 16); } else if (isAtLeastR()) { assertParcelSane(cap, 15); } else { diff --git a/packages/Connectivity/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/packages/Connectivity/tests/unit/java/com/android/server/ConnectivityServiceTest.java index ab5079820611d..221cc9b26ae46 100644 --- a/packages/Connectivity/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/packages/Connectivity/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -19,6 +19,7 @@ package com.android.server; import static android.Manifest.permission.CHANGE_NETWORK_STATE; import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS; import static android.Manifest.permission.DUMP; +import static android.Manifest.permission.LOCAL_MAC_ADDRESS; import static android.Manifest.permission.NETWORK_FACTORY; import static android.Manifest.permission.NETWORK_SETTINGS; import static android.app.PendingIntent.FLAG_IMMUTABLE; @@ -9407,9 +9408,9 @@ public class ConnectivityServiceTest { @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 + locationRedacted | (redactions & REDACT_FOR_ACCESS_FINE_LOCATION) != 0, + localMacAddressRedacted | (redactions & REDACT_FOR_LOCAL_MAC_ADDRESS) != 0, + settingsRedacted | (redactions & REDACT_FOR_NETWORK_SETTINGS) != 0 ); } @@ -9432,8 +9433,26 @@ public class ConnectivityServiceTest { public int hashCode() { return Objects.hash(locationRedacted, localMacAddressRedacted, settingsRedacted); } + + @Override + public String toString() { + return String.format( + "TestTransportInfo{locationRedacted=%s macRedacted=%s settingsRedacted=%s}", + locationRedacted, localMacAddressRedacted, settingsRedacted); + } } + private TestTransportInfo getTestTransportInfo(NetworkCapabilities nc) { + return (TestTransportInfo) nc.getTransportInfo(); + } + + private TestTransportInfo getTestTransportInfo(TestNetworkAgentWrapper n) { + final NetworkCapabilities nc = mCm.getNetworkCapabilities(n.getNetwork()); + assertNotNull(nc); + return getTestTransportInfo(nc); + } + + private void verifyNetworkCallbackLocationDataInclusionUsingTransportInfoAndOwnerUidInNetCaps( @NonNull TestNetworkCallback wifiNetworkCallback, int actualOwnerUid, @NonNull TransportInfo actualTransportInfo, int expectedOwnerUid, @@ -9462,7 +9481,6 @@ public class ConnectivityServiceTest { wifiNetworkCallback.expectCapabilitiesThat(mWiFiNetworkAgent, nc -> Objects.equals(expectedOwnerUid, nc.getOwnerUid()) && Objects.equals(expectedTransportInfo, nc.getTransportInfo())); - } @Test @@ -9483,6 +9501,40 @@ public class ConnectivityServiceTest { wifiNetworkCallack, ownerUid, transportInfo, INVALID_UID, sanitizedTransportInfo); } + @Test + public void testTransportInfoRedactionInSynchronousCalls() throws Exception { + final NetworkCapabilities ncTemplate = new NetworkCapabilities() + .addTransportType(TRANSPORT_WIFI) + .setTransportInfo(new TestTransportInfo()); + + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, new LinkProperties(), + ncTemplate); + mWiFiNetworkAgent.connect(true /* validated; waits for callback */); + + // NETWORK_SETTINGS redaction is controlled by the NETWORK_SETTINGS permission + assertTrue(getTestTransportInfo(mWiFiNetworkAgent).settingsRedacted); + withPermission(NETWORK_SETTINGS, () -> { + assertFalse(getTestTransportInfo(mWiFiNetworkAgent).settingsRedacted); + }); + assertTrue(getTestTransportInfo(mWiFiNetworkAgent).settingsRedacted); + + // LOCAL_MAC_ADDRESS redaction is controlled by the LOCAL_MAC_ADDRESS permission + assertTrue(getTestTransportInfo(mWiFiNetworkAgent).localMacAddressRedacted); + withPermission(LOCAL_MAC_ADDRESS, () -> { + assertFalse(getTestTransportInfo(mWiFiNetworkAgent).localMacAddressRedacted); + }); + assertTrue(getTestTransportInfo(mWiFiNetworkAgent).localMacAddressRedacted); + + // Synchronous getNetworkCapabilities calls never return unredacted location-sensitive + // information. + assertTrue(getTestTransportInfo(mWiFiNetworkAgent).locationRedacted); + setupLocationPermissions(Build.VERSION_CODES.S, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + assertTrue(getTestTransportInfo(mWiFiNetworkAgent).locationRedacted); + denyAllLocationPrivilegedPermissions(); + assertTrue(getTestTransportInfo(mWiFiNetworkAgent).locationRedacted); + } + private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType) throws Exception { final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); @@ -9840,12 +9892,27 @@ public class ConnectivityServiceTest { // Connect the cell agent verify that it notifies TestNetworkCallback that it is available final TestNetworkCallback callback = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(callback); - mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + + final NetworkCapabilities ncTemplate = new NetworkCapabilities() + .addTransportType(TRANSPORT_CELLULAR) + .setTransportInfo(new TestTransportInfo()); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, new LinkProperties(), + ncTemplate); mCellNetworkAgent.connect(true); callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); callback.assertNoCallback(); } + private boolean areConnDiagCapsRedacted(NetworkCapabilities nc) { + TestTransportInfo ti = (TestTransportInfo) nc.getTransportInfo(); + return nc.getUids() == null + && nc.getAdministratorUids().length == 0 + && nc.getOwnerUid() == Process.INVALID_UID + && getTestTransportInfo(nc).locationRedacted + && getTestTransportInfo(nc).localMacAddressRedacted + && getTestTransportInfo(nc).settingsRedacted; + } + @Test public void testConnectivityDiagnosticsCallbackOnConnectivityReportAvailable() throws Exception { @@ -9856,12 +9923,7 @@ public class ConnectivityServiceTest { // Verify onConnectivityReport fired verify(mConnectivityDiagnosticsCallback).onConnectivityReportAvailable( - argThat(report -> { - final NetworkCapabilities nc = report.getNetworkCapabilities(); - return nc.getUids() == null - && nc.getAdministratorUids().length == 0 - && nc.getOwnerUid() == Process.INVALID_UID; - })); + argThat(report -> areConnDiagCapsRedacted(report.getNetworkCapabilities()))); } @Test @@ -9877,12 +9939,7 @@ public class ConnectivityServiceTest { // Verify onDataStallSuspected fired verify(mConnectivityDiagnosticsCallback).onDataStallSuspected( - argThat(report -> { - final NetworkCapabilities nc = report.getNetworkCapabilities(); - return nc.getUids() == null - && nc.getAdministratorUids().length == 0 - && nc.getOwnerUid() == Process.INVALID_UID; - })); + argThat(report -> areConnDiagCapsRedacted(report.getNetworkCapabilities()))); } @Test diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 39a990cf5d730..ac97fe18c8c31 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -9041,7 +9041,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } private NetworkCapabilities getNetworkCapabilitiesWithoutUids(@NonNull NetworkCapabilities nc) { - final NetworkCapabilities sanitized = new NetworkCapabilities(nc); + final NetworkCapabilities sanitized = new NetworkCapabilities(nc, + NetworkCapabilities.REDACT_ALL); sanitized.setUids(null); sanitized.setAdministratorUids(new int[0]); sanitized.setOwnerUid(Process.INVALID_UID); From 1ad18702f15ad4d1cc2dfb466569fa39893487c2 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 10 May 2021 10:19:11 +0900 Subject: [PATCH 2/3] Immediately redact VcnTransportInfo. Redaction of NetworkCapabilities is changing from redacting at parcel time to redacting immediately when makeCopy() is called. Update VcnTransportInfo redaction accordingly. Bug: 183938194 Test: atest VcnTransportInfoTest Change-Id: I0c9406f426b66fd36b47d11799955def531c16ba --- .../android/net/vcn/VcnTransportInfo.java | 76 +++---------------- .../android/net/vcn/VcnTransportInfoTest.java | 57 ++++++-------- 2 files changed, 35 insertions(+), 98 deletions(-) diff --git a/core/java/android/net/vcn/VcnTransportInfo.java b/core/java/android/net/vcn/VcnTransportInfo.java index 0e9ccf144c2ea..1f1818420b561 100644 --- a/core/java/android/net/vcn/VcnTransportInfo.java +++ b/core/java/android/net/vcn/VcnTransportInfo.java @@ -16,23 +16,17 @@ package android.net.vcn; -import static android.net.NetworkCapabilities.REDACT_ALL; -import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; +import static android.net.NetworkCapabilities.REDACT_NONE; import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; -import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIVATE; - import android.annotation.NonNull; import android.annotation.Nullable; -import android.net.NetworkCapabilities; import android.net.TransportInfo; import android.net.wifi.WifiInfo; import android.os.Parcel; import android.os.Parcelable; import android.telephony.SubscriptionManager; -import com.android.internal.annotations.VisibleForTesting; - import java.util.Objects; /** @@ -55,32 +49,17 @@ public class VcnTransportInfo implements TransportInfo, Parcelable { @Nullable private final WifiInfo mWifiInfo; private final int mSubId; - /** - * The redaction scheme to use when parcelling. - * - *

The TransportInfo/NetworkCapabilities redaction mechanisms rely on redaction being - * performed at parcelling time. This means that the redaction scheme must be stored for later - * use. - * - *

Since the redaction scheme itself is not parcelled, this field is listed as a transient. - * - *

Defaults to REDACT_ALL when constructed using public constructors, or creating from - * parcels. - */ - private final transient long mRedactions; - public VcnTransportInfo(@NonNull WifiInfo wifiInfo) { - this(wifiInfo, INVALID_SUBSCRIPTION_ID, REDACT_ALL); + this(wifiInfo, INVALID_SUBSCRIPTION_ID); } public VcnTransportInfo(int subId) { - this(null /* wifiInfo */, subId, REDACT_ALL); + this(null /* wifiInfo */, subId); } - private VcnTransportInfo(@Nullable WifiInfo wifiInfo, int subId, long redactions) { + private VcnTransportInfo(@Nullable WifiInfo wifiInfo, int subId) { mWifiInfo = wifiInfo; mSubId = subId; - mRedactions = redactions; } /** @@ -102,25 +81,14 @@ public class VcnTransportInfo implements TransportInfo, Parcelable { * SubscriptionManager#INVALID_SUBSCRIPTION_ID}. * * @return the Subscription ID if a cellular underlying Network is present, else {@link - * android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID}. + * android.telephony.SubscriptionManager#INVALID_SUBSCRIPTION_ID}. */ public int getSubId() { return mSubId; } - /** - * Gets the redaction scheme - * - * @hide - */ - @VisibleForTesting(visibility = PRIVATE) - public long getRedaction() { - return mRedactions; - } - @Override public int hashCode() { - // mRedactions not hashed, as it is a transient, for control of parcelling return Objects.hash(mWifiInfo, mSubId); } @@ -128,8 +96,6 @@ public class VcnTransportInfo implements TransportInfo, Parcelable { public boolean equals(Object o) { if (!(o instanceof VcnTransportInfo)) return false; final VcnTransportInfo that = (VcnTransportInfo) o; - - // mRedactions not compared, as it is a transient, for control of parcelling return Objects.equals(mWifiInfo, that.mWifiInfo) && mSubId == that.mSubId; } @@ -143,31 +109,19 @@ public class VcnTransportInfo implements TransportInfo, Parcelable { @NonNull public TransportInfo makeCopy(long redactions) { return new VcnTransportInfo( - mWifiInfo == null ? null : mWifiInfo.makeCopy(redactions), mSubId, redactions); + (mWifiInfo == null) ? null : mWifiInfo.makeCopy(redactions), mSubId); } @Override public long getApplicableRedactions() { - long redactions = REDACT_FOR_NETWORK_SETTINGS; - - // Add additional wifi redactions if necessary - if (mWifiInfo != null) { - redactions |= mWifiInfo.getApplicableRedactions(); - } - - return redactions; - } - - private boolean shouldParcelNetworkSettingsFields() { - return (mRedactions & NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS) == 0; + return (mWifiInfo == null) ? REDACT_NONE : mWifiInfo.getApplicableRedactions(); } /** {@inheritDoc} */ @Override public void writeToParcel(@NonNull Parcel dest, int flags) { - dest.writeInt(shouldParcelNetworkSettingsFields() ? mSubId : INVALID_SUBSCRIPTION_ID); - dest.writeParcelable( - shouldParcelNetworkSettingsFields() ? (Parcelable) mWifiInfo : null, flags); + dest.writeInt(mSubId); + dest.writeParcelable(mWifiInfo, flags); } @Override @@ -181,17 +135,7 @@ public class VcnTransportInfo implements TransportInfo, Parcelable { public VcnTransportInfo createFromParcel(Parcel in) { final int subId = in.readInt(); final WifiInfo wifiInfo = in.readParcelable(null); - - // If all fields are their null values, return null TransportInfo to avoid - // leaking information about this being a VCN Network (instead of macro - // cellular, etc) - if (wifiInfo == null && subId == INVALID_SUBSCRIPTION_ID) { - return null; - } - - // Prevent further forwarding by redacting everything in future parcels from - // this VcnTransportInfo - return new VcnTransportInfo(wifiInfo, subId, REDACT_ALL); + return new VcnTransportInfo(wifiInfo, subId); } public VcnTransportInfo[] newArray(int size) { diff --git a/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java b/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java index 582275d0547d6..00a0bffa15c57 100644 --- a/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java +++ b/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java @@ -16,14 +16,17 @@ package android.net.vcn; -import static android.net.NetworkCapabilities.REDACT_ALL; +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.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; +import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiInfo; import android.os.Parcel; @@ -38,12 +41,6 @@ public class VcnTransportInfoTest { private static final VcnTransportInfo CELL_UNDERLYING_INFO = new VcnTransportInfo(SUB_ID); private static final VcnTransportInfo WIFI_UNDERLYING_INFO = new VcnTransportInfo(WIFI_INFO); - @Test - public void testRedactionDefaults() { - assertEquals(REDACT_ALL, CELL_UNDERLYING_INFO.getRedaction()); - assertEquals(REDACT_ALL, WIFI_UNDERLYING_INFO.getRedaction()); - } - @Test public void testGetWifiInfo() { assertEquals(WIFI_INFO, WIFI_UNDERLYING_INFO.getWifiInfo()); @@ -59,15 +56,15 @@ public class VcnTransportInfoTest { } @Test - public void testMakeCopySetsRedactions() { + public void testMakeCopyRedactForAccessFineLocation() { assertEquals( - REDACT_FOR_NETWORK_SETTINGS, - ((VcnTransportInfo) CELL_UNDERLYING_INFO.makeCopy(REDACT_FOR_NETWORK_SETTINGS)) - .getRedaction()); + SUB_ID, + ((VcnTransportInfo) CELL_UNDERLYING_INFO.makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION)) + .getSubId()); assertEquals( - REDACT_FOR_NETWORK_SETTINGS, - ((VcnTransportInfo) WIFI_UNDERLYING_INFO.makeCopy(REDACT_FOR_NETWORK_SETTINGS)) - .getRedaction()); + WifiConfiguration.INVALID_NETWORK_ID, + ((VcnTransportInfo) WIFI_UNDERLYING_INFO.makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION)) + .getWifiInfo().getNetworkId()); } @Test @@ -78,35 +75,31 @@ public class VcnTransportInfoTest { } @Test - public void testParcelUnparcel() { - verifyParcelingIsNull(CELL_UNDERLYING_INFO); - verifyParcelingIsNull(WIFI_UNDERLYING_INFO); - } - - private void verifyParcelingIsNull(VcnTransportInfo vcnTransportInfo) { - // Verify redacted by default - Parcel parcel = Parcel.obtain(); - vcnTransportInfo.writeToParcel(parcel, 0 /* flags */); - parcel.setDataPosition(0); - - assertNull(VcnTransportInfo.CREATOR.createFromParcel(parcel)); + public void testApplicableRedactions() { + assertEquals(REDACT_NONE, CELL_UNDERLYING_INFO.getApplicableRedactions()); + assertEquals(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_LOCAL_MAC_ADDRESS + | REDACT_FOR_NETWORK_SETTINGS, + WIFI_UNDERLYING_INFO.getApplicableRedactions()); } @Test - public void testParcelUnparcelNotRedactedForSysUi() { - verifyParcelingForSysUi(CELL_UNDERLYING_INFO); - verifyParcelingForSysUi(WIFI_UNDERLYING_INFO); + public void testParcelNotRedactedForSysUi() { + VcnTransportInfo cellRedacted = parcelForSysUi(CELL_UNDERLYING_INFO); + assertEquals(SUB_ID, cellRedacted.getSubId()); + VcnTransportInfo wifiRedacted = parcelForSysUi(WIFI_UNDERLYING_INFO); + assertEquals(NETWORK_ID, wifiRedacted.getWifiInfo().getNetworkId()); } - private void verifyParcelingForSysUi(VcnTransportInfo vcnTransportInfo) { + private VcnTransportInfo parcelForSysUi(VcnTransportInfo vcnTransportInfo) { // Allow fully unredacted; SysUI will have all the relevant permissions. - final VcnTransportInfo unRedacted = (VcnTransportInfo) vcnTransportInfo.makeCopy(0); + final VcnTransportInfo unRedacted = (VcnTransportInfo) vcnTransportInfo.makeCopy( + REDACT_NONE); final Parcel parcel = Parcel.obtain(); unRedacted.writeToParcel(parcel, 0 /* flags */); parcel.setDataPosition(0); final VcnTransportInfo unparceled = VcnTransportInfo.CREATOR.createFromParcel(parcel); assertEquals(vcnTransportInfo, unparceled); - assertEquals(REDACT_ALL, unparceled.getRedaction()); + return unparceled; } } From 34748be15064212ca5d0065d8003d37fa3b96f38 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 13 May 2021 17:20:10 +0900 Subject: [PATCH 3/3] Make VcnTransportInfoTest pass on AOSP. AOSP contains the core networking redaction changes, but not the wifi redaction changes, so the tests that depend on wifi S behaviour will not pass on AOSP. Fix testApplicableRedactions by checking that the applicable redactions are the same ones as returned by the underlying WifiInfo object. Fix testMakeCopyRedactForAccessFineLocation by testing the wifi redaction code iff Build.VERSION.SDK_INT is S or later. This is different from SdkLevel.isAtLeastS, because the latter returns true in AOSP. Bug: 183938194 Test: atest VcnTransportInfoTest Change-Id: Ia5bc896c92be19f570638f8051492e0f2d7342f9 --- .../android/net/vcn/VcnTransportInfoTest.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java b/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java index 00a0bffa15c57..abae81cf17429 100644 --- a/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java +++ b/tests/vcn/java/android/net/vcn/VcnTransportInfoTest.java @@ -17,8 +17,6 @@ package android.net.vcn; 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.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; @@ -28,6 +26,7 @@ import static org.junit.Assert.assertNull; import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiInfo; +import android.os.Build; import android.os.Parcel; import org.junit.Test; @@ -61,10 +60,14 @@ public class VcnTransportInfoTest { SUB_ID, ((VcnTransportInfo) CELL_UNDERLYING_INFO.makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION)) .getSubId()); - assertEquals( - WifiConfiguration.INVALID_NETWORK_ID, - ((VcnTransportInfo) WIFI_UNDERLYING_INFO.makeCopy(REDACT_FOR_ACCESS_FINE_LOCATION)) - .getWifiInfo().getNetworkId()); + + // TODO: remove the if statement when S pushes to AOSP. + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + assertEquals( + WifiConfiguration.INVALID_NETWORK_ID, + ((VcnTransportInfo) WIFI_UNDERLYING_INFO.makeCopy( + REDACT_FOR_ACCESS_FINE_LOCATION)).getWifiInfo().getNetworkId()); + } } @Test @@ -77,9 +80,9 @@ public class VcnTransportInfoTest { @Test public void testApplicableRedactions() { assertEquals(REDACT_NONE, CELL_UNDERLYING_INFO.getApplicableRedactions()); - assertEquals(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_LOCAL_MAC_ADDRESS - | REDACT_FOR_NETWORK_SETTINGS, - WIFI_UNDERLYING_INFO.getApplicableRedactions()); + + final long wifiRedactions = WIFI_INFO.getApplicableRedactions(); + assertEquals(wifiRedactions, WIFI_UNDERLYING_INFO.getApplicableRedactions()); } @Test