Merge "Do not apply background rules for core uids" into main (96b61aec) · Commits · e / os / android_frameworks_base

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

+44 −28

Original line number	Diff line number	Diff line
		@@ -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);

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

+10 −0

Original line number	Diff line number	Diff line
		@@ -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
		}
		}

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

+67 −0

Original line number	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.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) {