Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3533c167 authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

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
parent 5b481564

packages/Connectivity/framework/src/android/net/NetworkCapabilities.java

+5 −17
Original line number Diff line number Diff line
@@ -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);

        }

packages/Connectivity/tests/common/java/android/net/NetworkCapabilitiesTest.java

+1 −1
Original line number Diff line number Diff line
@@ -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 {

packages/Connectivity/tests/unit/java/com/android/server/ConnectivityServiceTest.java

+74 −17
Original line number Diff line number Diff line
@@ -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<UidRange> 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

































services/core/java/com/android/server/ConnectivityService.java



+2
−1




























Original line number
Diff line number
Diff line








@@ -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);