Do not apply background rules for core uids

Core uids are exempt from firewalls by the underlying stack, so they
will always be allowed network.
Similarly, apps without the INTERNET permission cannot access network
regardless of firewall rules.

Currently, the code is fragmented in applying rules to these uids.
To make debugging and code maintenance easier, we want to be consistent
by never setting any rules for such uids.

Once the feature is enabled and tested, upstream code paths for all
firewall rules can be simplified to use the same check.

Flag: com.android.server.net.never_apply_rules_to_core_uids

Test: atest FrameworksServicesTests:NetworkPolicyManagerServiceTest

BYPASS_INCLUSIVE_LANGUAGE_REASON=Existing methods

Bug: 356956588
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:da8a8cb6ea581986e7f0ba2c8c3462b2d66492fc)
Merged-In: Ibe50b806a0632d09772e7e2e8deea6d2fefdc946
Change-Id: Ibe50b806a0632d09772e7e2e8deea6d2fefdc946

3 files changed, 121 insertions, 28 deletions

diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
index 7a311bdb49b5..7d2915ba48a2 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -530,6 +530,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
      */
     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.
     final Object mUidRulesFirstLock = 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 */
     @GuardedBy("mUidRulesFirstLock")
-    private final SparseBooleanArray mInternetPermissionMap = new SparseBooleanArray();
+    @VisibleForTesting
+    final SparseBooleanArray mInternetPermissionMap = new SparseBooleanArray();
 
     /**
      * Map of uid -> UidStateCallbackInfo objects holding the data received from
@@ -1038,6 +1045,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
             mUseMeteredFirewallChains = Flags.useMeteredFirewallChains();
             mUseDifferentDelaysForBackgroundChain = Flags.useDifferentDelaysForBackgroundChain();
+            mNeverApplyRulesToCoreUids = Flags.neverApplyRulesToCoreUids();
 
             synchronized (mUidRulesFirstLock) {
                 synchronized (mNetworkPoliciesSecondLock) {
@@ -4088,6 +4096,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                         + mUseMeteredFirewallChains);
                 fout.println(Flags.FLAG_USE_DIFFERENT_DELAYS_FOR_BACKGROUND_CHAIN + ": "
                         + mUseDifferentDelaysForBackgroundChain);
+                fout.println(Flags.FLAG_NEVER_APPLY_RULES_TO_CORE_UIDS + ": "
+                        + mNeverApplyRulesToCoreUids);
 
                 fout.println();
                 fout.println("mRestrictBackgroundLowPowerMode: " + mRestrictBackgroundLowPowerMode);
@@ -4878,6 +4888,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 int[] idleUids = mUsageStats.getIdleUidsForUser(user.id);
                 for (int uid : idleUids) {
                     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
                         // doesn't have network access anyway, so it is a waste to mess
                         // with it here.
@@ -5180,6 +5196,11 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
     @GuardedBy("mUidRulesFirstLock")
     private boolean isUidValidForAllowlistRulesUL(int uid) {
+        return isUidValidForRulesUL(uid);
+    }
+
+    @GuardedBy("mUidRulesFirstLock")
+    private boolean isUidValidForRulesUL(int 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
      * here to netd.  It will clean up dead rules and make sure the target chain only contains rules
      * specified here.
      */
+    @GuardedBy("mUidRulesFirstLock")
     private void setUidFirewallRulesUL(int chain, SparseIntArray uidRules) {
-        addSdkSandboxUidsIfNeeded(uidRules);
         try {
             int size = uidRules.size();
-            int[] uids = new int[size];
-            int[] rules = new int[size];
+            final IntArray uids = new IntArray(size);
+            final IntArray rules = new IntArray(size);
             for(int index = size - 1; index >= 0; --index) {
-                uids[index] = uidRules.keyAt(index);
-                rules[index] = uidRules.valueAt(index);
+                final int uid = uidRules.keyAt(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) {
             Log.wtf(TAG, "problem setting firewall uid rules", e);
         } catch (RemoteException e) {
@@ -6241,6 +6254,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
      */
     @GuardedBy("mUidRulesFirstLock")
     private void setUidFirewallRuleUL(int chain, int uid, int rule) {
+        if (mNeverApplyRulesToCoreUids && !isUidValidForRulesUL(uid)) {
+            return;
+        }
         if (Trace.isTagEnabled(Trace.TRACE_TAG_NETWORK)) {
             Trace.traceBegin(Trace.TRACE_TAG_NETWORK,
                     "setUidFirewallRuleUL: " + chain + "/" + uid + "/" + rule);
@@ -6249,8 +6265,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             if (chain == FIREWALL_CHAIN_STANDBY) {
                 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 {
                 mNetworkManager.setFirewallUidRule(chain, uid, rule);
@@ -6295,6 +6309,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
      * Resets all firewall rules associated with an 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 {
             mNetworkManager.setFirewallUidRule(FIREWALL_CHAIN_DOZABLE, uid,
                     FIREWALL_RULE_DEFAULT);
diff --git a/services/core/java/com/android/server/net/flags.aconfig b/services/core/java/com/android/server/net/flags.aconfig
index 586baf022897..7f04e665567e 100644
--- a/services/core/java/com/android/server/net/flags.aconfig
+++ b/services/core/java/com/android/server/net/flags.aconfig
@@ -27,3 +27,13 @@ flag {
       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
+    }
+}
diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
index 3d6884925098..dddab657be14 100644
--- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
@@ -108,6 +108,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.CALLS_REAL_METHODS;
@@ -165,6 +166,7 @@ import android.os.PowerExemptionManager;
 import android.os.PowerManager;
 import android.os.PowerManagerInternal;
 import android.os.PowerSaveState;
+import android.os.Process;
 import android.os.RemoteException;
 import android.os.SimpleClock;
 import android.os.SystemClock;
@@ -197,6 +199,7 @@ import androidx.test.filters.FlakyTest;
 import androidx.test.filters.MediumTest;
 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.FutureIntent;
 import com.android.internal.util.test.FsUtil;
@@ -2310,6 +2313,70 @@ public class NetworkPolicyManagerServiceTest {
         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) {
         final NetworkPolicyManager.UidState uidState = mService.getUidStateForTest(uid);
         if (uidState == null) {