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

Commit d85e3742 authored by Treehugger Robot's avatar Treehugger Robot Committed by Automerger Merge Worker
Browse files

Merge "Do not apply background rules for core uids" into main am: 72719079 

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/3214197



Change-Id: Idd749ce6ad6df0704f5b48d05ddcc64fad72814c
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 2008d30e 72719079

services/core/java/com/android/server/net/NetworkPolicyManagerService.java

+44 −28
Original line number Original line Diff line number Diff line
@@ -530,6 +530,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
     */

     */

    private boolean mUseDifferentDelaysForBackgroundChain;

    private boolean mUseDifferentDelaysForBackgroundChain;





    /**

     * Core uids and apps without the internet permission will not have any firewall rules applied

     * to them.

     */

    private boolean mNeverApplyRulesToCoreUids;



    // See main javadoc for instructions on how to use these locks.

    // See main javadoc for instructions on how to use these locks.

    final Object mUidRulesFirstLock = new Object();

    final Object mUidRulesFirstLock = new Object();

    final Object mNetworkPoliciesSecondLock = new Object();

    final Object mNetworkPoliciesSecondLock = new Object();
@@ -760,7 +766,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 




    /** List of apps indexed by uid and whether they have the internet permission */

    /** List of apps indexed by uid and whether they have the internet permission */

    @GuardedBy("mUidRulesFirstLock")

    @GuardedBy("mUidRulesFirstLock")

    private final SparseBooleanArray mInternetPermissionMap = new SparseBooleanArray();

    @VisibleForTesting

    final SparseBooleanArray mInternetPermissionMap = new SparseBooleanArray();





    /**

    /**

     * Map of uid -> UidStateCallbackInfo objects holding the data received from

     * Map of uid -> UidStateCallbackInfo objects holding the data received from
@@ -1038,6 +1045,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 




            mUseMeteredFirewallChains = Flags.useMeteredFirewallChains();

            mUseMeteredFirewallChains = Flags.useMeteredFirewallChains();

            mUseDifferentDelaysForBackgroundChain = Flags.useDifferentDelaysForBackgroundChain();

            mUseDifferentDelaysForBackgroundChain = Flags.useDifferentDelaysForBackgroundChain();

            mNeverApplyRulesToCoreUids = Flags.neverApplyRulesToCoreUids();





            synchronized (mUidRulesFirstLock) {

            synchronized (mUidRulesFirstLock) {

                synchronized (mNetworkPoliciesSecondLock) {

                synchronized (mNetworkPoliciesSecondLock) {
@@ -4088,6 +4096,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
                        + mUseMeteredFirewallChains);

                        + mUseMeteredFirewallChains);

                fout.println(Flags.FLAG_USE_DIFFERENT_DELAYS_FOR_BACKGROUND_CHAIN + ": "

                fout.println(Flags.FLAG_USE_DIFFERENT_DELAYS_FOR_BACKGROUND_CHAIN + ": "

                        + mUseDifferentDelaysForBackgroundChain);

                        + mUseDifferentDelaysForBackgroundChain);

                fout.println(Flags.FLAG_NEVER_APPLY_RULES_TO_CORE_UIDS + ": "

                        + mNeverApplyRulesToCoreUids);





                fout.println();

                fout.println();

                fout.println("mRestrictBackgroundLowPowerMode: " + mRestrictBackgroundLowPowerMode);

                fout.println("mRestrictBackgroundLowPowerMode: " + mRestrictBackgroundLowPowerMode);
@@ -4878,6 +4888,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
                int[] idleUids = mUsageStats.getIdleUidsForUser(user.id);

                int[] idleUids = mUsageStats.getIdleUidsForUser(user.id);

                for (int uid : idleUids) {

                for (int uid : idleUids) {

                    if (!mPowerSaveTempWhitelistAppIds.get(UserHandle.getAppId(uid), false)) {

                    if (!mPowerSaveTempWhitelistAppIds.get(UserHandle.getAppId(uid), false)) {

                        if (mNeverApplyRulesToCoreUids && !isUidValidForRulesUL(uid)) {

                            // This check is needed to keep mUidFirewallStandbyRules free of any

                            // such uids. Doing this keeps it in sync with the actual rules applied

                            // in the underlying connectivity stack.

                            continue;

                        }

                        // quick check: if this uid doesn't have INTERNET permission, it

                        // quick check: if this uid doesn't have INTERNET permission, it

                        // doesn't have network access anyway, so it is a waste to mess

                        // doesn't have network access anyway, so it is a waste to mess

                        // with it here.

                        // with it here.
@@ -5180,6 +5196,11 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 




    @GuardedBy("mUidRulesFirstLock")

    @GuardedBy("mUidRulesFirstLock")

    private boolean isUidValidForAllowlistRulesUL(int uid) {

    private boolean isUidValidForAllowlistRulesUL(int uid) {

        return isUidValidForRulesUL(uid);

    }



    @GuardedBy("mUidRulesFirstLock")

    private boolean isUidValidForRulesUL(int uid) {

        return UserHandle.isApp(uid) && hasInternetPermissionUL(uid);

        return UserHandle.isApp(uid) && hasInternetPermissionUL(uid);

    }

    }
@@ -6194,41 +6215,33 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
        }

        }

    }

    }





    private void addSdkSandboxUidsIfNeeded(SparseIntArray uidRules) {

        final int size = uidRules.size();

        final SparseIntArray sdkSandboxUids = new SparseIntArray();

        for (int index = 0; index < size; index++) {

            final int uid = uidRules.keyAt(index);

            final int rule = uidRules.valueAt(index);

            if (Process.isApplicationUid(uid)) {

                sdkSandboxUids.put(Process.toSdkSandboxUid(uid), rule);

            }

        }



        for (int index = 0; index < sdkSandboxUids.size(); index++) {

            final int uid = sdkSandboxUids.keyAt(index);

            final int rule = sdkSandboxUids.valueAt(index);

            uidRules.put(uid, rule);

        }

    }



    /**

    /**

     * Set uid rules on a particular firewall chain. This is going to synchronize the rules given

     * Set uid rules on a particular firewall chain. This is going to synchronize the rules given

     * here to netd.  It will clean up dead rules and make sure the target chain only contains rules

     * here to netd.  It will clean up dead rules and make sure the target chain only contains rules

     * specified here.

     * specified here.

     */

     */

    @GuardedBy("mUidRulesFirstLock")

    private void setUidFirewallRulesUL(int chain, SparseIntArray uidRules) {

    private void setUidFirewallRulesUL(int chain, SparseIntArray uidRules) {

        addSdkSandboxUidsIfNeeded(uidRules);

        try {

        try {

            int size = uidRules.size();

            int size = uidRules.size();

            int[] uids = new int[size];

            final IntArray uids = new IntArray(size);

            int[] rules = new int[size];

            final IntArray rules = new IntArray(size);

            for(int index = size - 1; index >= 0; --index) {

            for(int index = size - 1; index >= 0; --index) {

                uids[index] = uidRules.keyAt(index);

                final int uid = uidRules.keyAt(index);

                rules[index] = uidRules.valueAt(index);

                if (mNeverApplyRulesToCoreUids && !isUidValidForRulesUL(uid)) {

                    continue;

                }

                uids.add(uid);

                rules.add(uidRules.valueAt(index));

                if (Process.isApplicationUid(uid)) {

                    uids.add(Process.toSdkSandboxUid(uid));

                    rules.add(uidRules.valueAt(index));

                }

                }

            mNetworkManager.setFirewallUidRules(chain, uids, rules);

            }

            mLogger.firewallRulesChanged(chain, uids, rules);

            final int[] uidArray = uids.toArray();

            final int[] ruleArray = rules.toArray();

            mNetworkManager.setFirewallUidRules(chain, uidArray, ruleArray);

            mLogger.firewallRulesChanged(chain, uidArray, ruleArray);

        } catch (IllegalStateException e) {

        } catch (IllegalStateException e) {

            Log.wtf(TAG, "problem setting firewall uid rules", e);

            Log.wtf(TAG, "problem setting firewall uid rules", e);

        } catch (RemoteException e) {

        } catch (RemoteException e) {
@@ -6241,6 +6254,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
     */

     */

    @GuardedBy("mUidRulesFirstLock")

    @GuardedBy("mUidRulesFirstLock")

    private void setUidFirewallRuleUL(int chain, int uid, int rule) {

    private void setUidFirewallRuleUL(int chain, int uid, int rule) {

        if (mNeverApplyRulesToCoreUids && !isUidValidForRulesUL(uid)) {

            return;

        }

        if (Trace.isTagEnabled(Trace.TRACE_TAG_NETWORK)) {

        if (Trace.isTagEnabled(Trace.TRACE_TAG_NETWORK)) {

            Trace.traceBegin(Trace.TRACE_TAG_NETWORK,

            Trace.traceBegin(Trace.TRACE_TAG_NETWORK,

                    "setUidFirewallRuleUL: " + chain + "/" + uid + "/" + rule);

                    "setUidFirewallRuleUL: " + chain + "/" + uid + "/" + rule);
@@ -6249,8 +6265,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
            if (chain == FIREWALL_CHAIN_STANDBY) {

            if (chain == FIREWALL_CHAIN_STANDBY) {

                mUidFirewallStandbyRules.put(uid, rule);

                mUidFirewallStandbyRules.put(uid, rule);

            }

            }

            // Note that we do not need keep a separate cache of uid rules for chains that we do

            // not call #setUidFirewallRulesUL for.





            try {

            try {

                mNetworkManager.setFirewallUidRule(chain, uid, rule);

                mNetworkManager.setFirewallUidRule(chain, uid, rule);
@@ -6295,6 +6309,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
     * Resets all firewall rules associated with an UID.

     * Resets all firewall rules associated with an UID.

     */

     */

    private void resetUidFirewallRules(int uid) {

    private void resetUidFirewallRules(int uid) {

        // Resetting rules for uids with isUidValidForRulesUL = false should be OK as no rules

        // should be previously set and the downstream code will skip no-op changes.

        try {

        try {

            mNetworkManager.setFirewallUidRule(FIREWALL_CHAIN_DOZABLE, uid,

            mNetworkManager.setFirewallUidRule(FIREWALL_CHAIN_DOZABLE, uid,

                    FIREWALL_RULE_DEFAULT);

                    FIREWALL_RULE_DEFAULT);

services/core/java/com/android/server/net/flags.aconfig

+10 −0
Original line number Original line Diff line number Diff line
@@ -27,3 +27,13 @@ flag { 
      purpose: PURPOSE_BUGFIX

      purpose: PURPOSE_BUGFIX

    }

    }

}

}



flag {

    name: "never_apply_rules_to_core_uids"

    namespace: "backstage_power"

    description: "Removes all rule bookkeeping and evaluation logic for core uids and uids without the internet permission"

    bug: "356956588"

    metadata {

      purpose: PURPOSE_BUGFIX

    }

}

services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java

+67 −0
Original line number Original line Diff line number Diff line
@@ -108,6 +108,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean; 
import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyLong;

import static org.mockito.ArgumentMatchers.anyLong;

import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.argThat;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.isA;

import static org.mockito.ArgumentMatchers.isA;

import static org.mockito.Mockito.CALLS_REAL_METHODS;

import static org.mockito.Mockito.CALLS_REAL_METHODS;
@@ -165,6 +166,7 @@ import android.os.PowerExemptionManager; 
import android.os.PowerManager;

import android.os.PowerManager;

import android.os.PowerManagerInternal;

import android.os.PowerManagerInternal;

import android.os.PowerSaveState;

import android.os.PowerSaveState;

import android.os.Process;

import android.os.RemoteException;

import android.os.RemoteException;

import android.os.SimpleClock;

import android.os.SimpleClock;

import android.os.SystemClock;

import android.os.SystemClock;
@@ -197,6 +199,7 @@ import androidx.test.filters.FlakyTest; 
import androidx.test.filters.MediumTest;

import androidx.test.filters.MediumTest;

import androidx.test.runner.AndroidJUnit4;

import androidx.test.runner.AndroidJUnit4;





import com.android.internal.util.ArrayUtils;

import com.android.internal.util.test.BroadcastInterceptingContext;

import com.android.internal.util.test.BroadcastInterceptingContext;

import com.android.internal.util.test.BroadcastInterceptingContext.FutureIntent;

import com.android.internal.util.test.BroadcastInterceptingContext.FutureIntent;

import com.android.internal.util.test.FsUtil;

import com.android.internal.util.test.FsUtil;
@@ -2310,6 +2313,70 @@ public class NetworkPolicyManagerServiceTest { 
        assertTrue(mService.isUidNetworkingBlocked(UID_A, false));

        assertTrue(mService.isUidNetworkingBlocked(UID_A, false));

    }

    }





    @SuppressWarnings("GuardedBy") // For not holding mUidRulesFirstLock

    @Test

    @RequiresFlagsEnabled(Flags.FLAG_NEVER_APPLY_RULES_TO_CORE_UIDS)

    public void testRulesNeverAppliedToCoreUids() throws Exception {

        clearInvocations(mNetworkManager);



        final int coreAppId = Process.FIRST_APPLICATION_UID - 102;

        final int coreUid = UserHandle.getUid(USER_ID, coreAppId);



        // Enable all restrictions and add this core uid to all allowlists.

        mService.mDeviceIdleMode = true;

        mService.mRestrictPower = true;

        setRestrictBackground(true);

        expectHasUseRestrictedNetworksPermission(coreUid, true);

        enableRestrictedMode(true);

        final NetworkPolicyManagerInternal internal = LocalServices.getService(

                NetworkPolicyManagerInternal.class);

        internal.setLowPowerStandbyActive(true);

        internal.setLowPowerStandbyAllowlist(new int[]{coreUid});

        internal.onTempPowerSaveWhitelistChange(coreAppId, true, REASON_OTHER, "testing");



        when(mPowerExemptionManager.getAllowListedAppIds(anyBoolean()))

                .thenReturn(new int[]{coreAppId});

        mPowerAllowlistReceiver.onReceive(mServiceContext, null);



        // A normal uid would undergo a rule change from denied to allowed on all chains, but we

        // should not request any rule change for this core uid.

        verify(mNetworkManager, never()).setFirewallUidRule(anyInt(), eq(coreUid), anyInt());

        verify(mNetworkManager, never()).setFirewallUidRules(anyInt(),

                argThat(ar -> ArrayUtils.contains(ar, coreUid)), any(int[].class));

    }



    @SuppressWarnings("GuardedBy") // For not holding mUidRulesFirstLock

    @Test

    @RequiresFlagsEnabled(Flags.FLAG_NEVER_APPLY_RULES_TO_CORE_UIDS)

    public void testRulesNeverAppliedToUidsWithoutInternetPermission() throws Exception {

        clearInvocations(mNetworkManager);



        mService.mInternetPermissionMap.clear();

        expectHasInternetPermission(UID_A, false);



        // Enable all restrictions and add this uid to all allowlists.

        mService.mDeviceIdleMode = true;

        mService.mRestrictPower = true;

        setRestrictBackground(true);

        expectHasUseRestrictedNetworksPermission(UID_A, true);

        enableRestrictedMode(true);

        final NetworkPolicyManagerInternal internal = LocalServices.getService(

                NetworkPolicyManagerInternal.class);

        internal.setLowPowerStandbyActive(true);

        internal.setLowPowerStandbyAllowlist(new int[]{UID_A});

        internal.onTempPowerSaveWhitelistChange(APP_ID_A, true, REASON_OTHER, "testing");



        when(mPowerExemptionManager.getAllowListedAppIds(anyBoolean()))

                .thenReturn(new int[]{APP_ID_A});

        mPowerAllowlistReceiver.onReceive(mServiceContext, null);



        // A normal uid would undergo a rule change from denied to allowed on all chains, but we

        // should not request any rule this uid without the INTERNET permission.

        verify(mNetworkManager, never()).setFirewallUidRule(anyInt(), eq(UID_A), anyInt());

        verify(mNetworkManager, never()).setFirewallUidRules(anyInt(),

                argThat(ar -> ArrayUtils.contains(ar, UID_A)), any(int[].class));

    }



    private boolean isUidState(int uid, int procState, int procStateSeq, int capability) {

    private boolean isUidState(int uid, int procState, int procStateSeq, int capability) {

        final NetworkPolicyManager.UidState uidState = mService.getUidStateForTest(uid);

        final NetworkPolicyManager.UidState uidState = mService.getUidStateForTest(uid);

        if (uidState == null) {

        if (uidState == null) {