diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index 4a99d290f3..c19a906ecd 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/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/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 39a990cf5d..ac97fe18c8 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); diff --git a/tests/common/java/android/net/NetworkCapabilitiesTest.java b/tests/common/java/android/net/NetworkCapabilitiesTest.java index b178bad712..d74b802c87 100644 --- a/tests/common/java/android/net/NetworkCapabilitiesTest.java +++ b/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/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index ab50798206..221cc9b26a 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/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