Fixed connectivity state in some restricted network scenarios.
NetworkPolicyManagerService (NMPS) keeps an internal list of uid
rules (mUidRules) for network restrictions, and when these rules
changes it needs to notify external listeners (such as
ConnectivityService / CS).
Prior to Android N, both Data Saver mode (the feature previously known
as "Restrict Baground Data") and Battery Save mode used the same set of
firewall rules to implement their restrictions: when Battery Saver mode
NPMS would mark all networks as metered and set the proper firewall
rules externally.
Recently, these 2 modes were split in 2 distinct firewall rules and
NMPS.updateRuleForRestrictBackgroundLocked() was changed to update
the mUidRules logic based on the Data Saver firewall (since the Battery
Saver firewall changes are handled externally, on
updateRuleForRestrictPowerLocked()). As such, CS was not notified when
the power-related changes were made, which would cause apps to get a
state of CONNECTED / CONNECTED when querying its active connection.
Another scenario that is not properly handled is when a UID whitelisted
for Data Saver is brought back to foreground: although the proper
firewall rules are set, CS is not notified, and the apps state would be
DISCONNECTED / BLOCKED.
This CL introduces many changes that fix this issue:
- Fixed updateRuleForRestrictBackgroundLocked() to invoke
onUidRulesChanged() when the Battery Saver status changed.
- Fixed updateRuleForRestrictBackgroundLocked() to invoke
onUidRulesChanged() when an app whitelisted for Data Saver is brought
back to the foreground.
- Added a new API (onRestrictPowerChanged() and getRestrictPower())
to notify external services about Battery Saver mode changes.
- Fixed CS logic to properly handle the Battery Saver changes.
Externally to this change, the CTS tests were also improved to verify
the apps get the proper connection state; they can be verified running:
cts-tradefed run commandAndExit cts -m CtsHostsideNetworkTests \
-t com.android.cts.net.HostsideRestrictBackgroundNetworkTests
BUG: 28521946
Change-Id: I8eaccd39968eb4b8c6b34f462fbc541e5daf55f1
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 3508701..8763b93 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -30,8 +30,9 @@
import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED;
import static android.net.NetworkPolicyManager.RULE_ALLOW_ALL;
import static android.net.NetworkPolicyManager.RULE_ALLOW_METERED;
+import static android.net.NetworkPolicyManager.MASK_METERED_NETWORKS;
+import static android.net.NetworkPolicyManager.MASK_ALL_NETWORKS;
import static android.net.NetworkPolicyManager.RULE_NONE;
-import static android.net.NetworkPolicyManager.RULE_REJECT_ALL;
import static android.net.NetworkPolicyManager.RULE_REJECT_METERED;
import static android.net.NetworkPolicyManager.RULE_TEMPORARY_ALLOW_METERED;
import static android.net.NetworkPolicyManager.uidRulesToString;
@@ -217,6 +218,9 @@
/** Flag indicating if background data is restricted. */
@GuardedBy("mRulesLock")
private boolean mRestrictBackground;
+ /** Flag indicating if background data is restricted due to battery savings. */
+ @GuardedBy("mRulesLock")
+ private boolean mRestrictPower;
final private Context mContext;
private int mNetworkPreference;
@@ -665,9 +669,10 @@
try {
mPolicyManager.setConnectivityListener(mPolicyListener);
mRestrictBackground = mPolicyManager.getRestrictBackground();
+ mRestrictPower = mPolicyManager.getRestrictPower();
} catch (RemoteException e) {
// ouch, no rules updates means some processes may never get network
- loge("unable to register INetworkPolicyListener" + e.toString());
+ loge("unable to register INetworkPolicyListener" + e);
}
final PowerManager powerManager = (PowerManager) context.getSystemService(
@@ -919,21 +924,33 @@
uidRules = mUidRules.get(uid, RULE_NONE);
}
- switch (uidRules) {
- case RULE_ALLOW_ALL:
- case RULE_ALLOW_METERED:
- case RULE_TEMPORARY_ALLOW_METERED:
- return false;
- case RULE_REJECT_METERED:
- return networkMetered;
- case RULE_REJECT_ALL:
- return true;
- case RULE_NONE:
- default:
- // When background data is restricted device-wide, the default
- // behavior for apps should be like RULE_REJECT_METERED
- return mRestrictBackground ? networkMetered : false;
+ boolean allowed = true;
+ // Check Data Saver Mode first...
+ if (networkMetered) {
+ if ((uidRules & RULE_REJECT_METERED) != 0) {
+ if (LOGD_RULES) Log.d(TAG, "uid " + uid + " is blacklisted");
+ // Explicitly blacklisted.
+ allowed = false;
+ } else {
+ allowed = !mRestrictBackground
+ || (uidRules & RULE_ALLOW_METERED) != 0
+ || (uidRules & RULE_TEMPORARY_ALLOW_METERED) != 0;
+ if (LOGD_RULES) Log.d(TAG, "allowed status for uid " + uid + " when"
+ + " mRestrictBackground=" + mRestrictBackground
+ + ", whitelisted=" + ((uidRules & RULE_ALLOW_METERED) != 0)
+ + ", tempWhitelist= + ((uidRules & RULE_TEMPORARY_ALLOW_METERED) != 0)"
+ + ": " + allowed);
+ }
}
+ // ...then Battery Saver Mode.
+ if (allowed && mRestrictPower) {
+ allowed = (uidRules & RULE_ALLOW_ALL) != 0;
+ if (LOGD_RULES) Log.d(TAG, "allowed status for uid " + uid + " when"
+ + " mRestrictPower=" + mRestrictPower
+ + ", whitelisted=" + ((uidRules & RULE_ALLOW_ALL) != 0)
+ + ": " + allowed);
+ }
+ return !allowed;
}
private void maybeLogBlockedNetworkInfo(NetworkInfo ni, int uid) {
@@ -1380,7 +1397,7 @@
synchronized (mRulesLock) {
// skip update when we've already applied rules
- final int oldRules = mUidRules.get(uid, RULE_ALLOW_ALL);
+ final int oldRules = mUidRules.get(uid, RULE_NONE);
if (oldRules == uidRules) return;
mUidRules.put(uid, uidRules);
@@ -1422,6 +1439,18 @@
}
@Override
+ public void onRestrictPowerChanged(boolean restrictPower) {
+ // caller is NPMS, since we only register with them
+ if (LOGD_RULES) {
+ log("onRestrictPowerChanged(restrictPower=" + restrictPower + ")");
+ }
+
+ synchronized (mRulesLock) {
+ mRestrictPower = restrictPower;
+ }
+ }
+
+ @Override
public void onRestrictBackgroundWhitelistChanged(int uid, boolean whitelisted) {
if (LOGD_RULES) {
// caller is NPMS, since we only register with them
@@ -1862,6 +1891,10 @@
pw.println(mRestrictBackground);
pw.println();
+ pw.print("Restrict power: ");
+ pw.println(mRestrictPower);
+ pw.println();
+
pw.println("Status for known UIDs:");
pw.increaseIndent();
final int size = mUidRules.size();
@@ -3998,9 +4031,9 @@
synchronized(mRulesLock) {
uidRules = mUidRules.get(uid, RULE_ALLOW_ALL);
}
- if (uidRules != RULE_ALLOW_ALL) {
+ if ((uidRules & RULE_ALLOW_ALL) == 0) {
// we could silently fail or we can filter the available nets to only give
- // them those they have access to. Chose the more useful
+ // them those they have access to. Chose the more useful option.
networkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED);
}
}
diff --git a/services/core/java/com/android/server/job/controllers/ConnectivityController.java b/services/core/java/com/android/server/job/controllers/ConnectivityController.java
index 9fd22686..e0179dc 100644
--- a/services/core/java/com/android/server/job/controllers/ConnectivityController.java
+++ b/services/core/java/com/android/server/job/controllers/ConnectivityController.java
@@ -170,6 +170,11 @@
}
@Override
+ public void onRestrictPowerChanged(boolean restrictPower) {
+ updateTrackedJobs(-1);
+ }
+
+ @Override
public void onRestrictBackgroundChanged(boolean restrictBackground) {
updateTrackedJobs(-1);
}
diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
index abd7ff4..344ba17 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -48,10 +48,12 @@
import static android.net.NetworkPolicyManager.FIREWALL_RULE_DENY;
import static android.net.NetworkPolicyManager.POLICY_NONE;
import static android.net.NetworkPolicyManager.POLICY_REJECT_METERED_BACKGROUND;
+import static android.net.NetworkPolicyManager.RULE_ALLOW_ALL;
import static android.net.NetworkPolicyManager.RULE_ALLOW_METERED;
import static android.net.NetworkPolicyManager.MASK_METERED_NETWORKS;
import static android.net.NetworkPolicyManager.MASK_ALL_NETWORKS;
import static android.net.NetworkPolicyManager.RULE_NONE;
+import static android.net.NetworkPolicyManager.RULE_REJECT_ALL;
import static android.net.NetworkPolicyManager.RULE_REJECT_METERED;
import static android.net.NetworkPolicyManager.RULE_TEMPORARY_ALLOW_METERED;
import static android.net.NetworkPolicyManager.computeLastCycleBoundary;
@@ -264,6 +266,7 @@
private static final int MSG_RESTRICT_BACKGROUND_WHITELIST_CHANGED = 9;
private static final int MSG_UPDATE_INTERFACE_QUOTA = 10;
private static final int MSG_REMOVE_INTERFACE_QUOTA = 11;
+ private static final int MSG_RESTRICT_POWER_CHANGED = 12;
private final Context mContext;
private final IActivityManager mActivityManager;
@@ -554,9 +557,19 @@
updateRulesForGlobalChangeLocked(true);
}
}
+ mHandler.obtainMessage(MSG_RESTRICT_POWER_CHANGED,
+ enabled ? 1 : 0, 0).sendToTarget();
}
});
+ final boolean oldRestrictPower = mRestrictPower;
mRestrictPower = mPowerManagerInternal.getLowPowerModeEnabled();
+ if (mRestrictPower != oldRestrictPower) {
+ // Some early services may have read the default value,
+ // so notify them that it's changed
+ mHandler.obtainMessage(MSG_RESTRICT_POWER_CHANGED,
+ mRestrictPower ? 1 : 0, 0).sendToTarget();
+ }
+
mSystemReady = true;
// read policy from disk
@@ -2133,6 +2146,15 @@
}
@Override
+ public boolean getRestrictPower() {
+ mContext.enforceCallingOrSelfPermission(MANAGE_NETWORK_POLICY, TAG);
+
+ synchronized (mRulesLock) {
+ return mRestrictPower;
+ }
+ }
+
+ @Override
public void setDeviceIdleMode(boolean enabled) {
mContext.enforceCallingOrSelfPermission(MANAGE_NETWORK_POLICY, TAG);
@@ -2795,6 +2817,10 @@
* have permission to use the internet.
*
* <p>The {@link #mUidRules} map is used to define the transtion of states of an UID.
+ *
+ * <p>This method also updates the {@link #mUidRules} with the power-related status for the uid
+ * and send the proper {@value #MSG_RULES_CHANGED} notification, although it does not change
+ * the power-related firewall rules per se.
*/
private void updateRuleForRestrictBackgroundLocked(int uid) {
updateRuleForRestrictBackgroundLocked(uid, false);
@@ -2817,30 +2843,47 @@
// Data Saver status.
final boolean isDsBlacklisted = (uidPolicy & POLICY_REJECT_METERED_BACKGROUND) != 0;
final boolean isDsWhitelisted = mRestrictBackgroundWhitelistUids.get(uid);
- int newDsRule = RULE_NONE;
final int oldDsRule = oldUidRules & MASK_METERED_NETWORKS;
+ int newDsRule = RULE_NONE;
+
+ // Battery Saver status.
+ final boolean isBsWhitelisted = isWhitelistedBatterySaverLocked(uid);
+ final int oldBsRule = oldUidRules & MASK_ALL_NETWORKS;
+ int newBsRule = RULE_NONE;
// First step: define the new rule based on user restrictions and foreground state.
if (isForeground) {
+ // Data Saver rules
if (isDsBlacklisted || (mRestrictBackground && !isDsWhitelisted)) {
newDsRule = RULE_TEMPORARY_ALLOW_METERED;
- }
- } else {
- if (isDsBlacklisted) {
- newDsRule = RULE_REJECT_METERED;
} else if (isDsWhitelisted) {
newDsRule = RULE_ALLOW_METERED;
}
+ // Battery Saver rules
+ if (mRestrictPower) {
+ newBsRule = RULE_ALLOW_ALL;
+ }
+ } else {
+ // Data Saver rules
+ if (isDsBlacklisted) {
+ newDsRule = RULE_REJECT_METERED;
+ } else if (mRestrictBackground && isDsWhitelisted) {
+ newDsRule = RULE_ALLOW_METERED;
+ }
+ // Battery Saver rules
+ if (mRestrictPower) {
+ newBsRule = isBsWhitelisted ? RULE_ALLOW_ALL : RULE_REJECT_ALL;
+ }
}
-
- final int newUidRules = newDsRule;
+ final int newUidRules = newDsRule | newBsRule;
if (LOGV) {
Log.v(TAG, "updateRuleForRestrictBackgroundLocked(" + uid + "):"
+ " isForeground=" +isForeground + ", isBlacklisted: " + isDsBlacklisted
+ ", isDsWhitelisted: " + isDsWhitelisted
- + ", newUidRule: " + uidRulesToString(newUidRules)
- + ", oldUidRule: " + uidRulesToString(oldUidRules));
+ + ", isBsWhitelisted: " + isBsWhitelisted
+ + ", newUidRules: " + uidRulesToString(newUidRules)
+ + ", oldUidRules: " + uidRulesToString(oldUidRules));
}
if (newUidRules == RULE_NONE) {
@@ -2849,8 +2892,14 @@
mUidRules.put(uid, newUidRules);
}
+ boolean changed = false;
+
// Second step: apply bw changes based on change of state.
+
+ // Apply Data Saver rules.
if (newDsRule != oldDsRule) {
+ changed = true;
+
if ((newDsRule & RULE_TEMPORARY_ALLOW_METERED) != 0) {
// Temporarily whitelist foreground app, removing from blacklist if necessary
// (since bw_penalty_box prevails over bw_happy_box).
@@ -2895,11 +2944,32 @@
+ ": foreground=" + isForeground
+ ", whitelisted=" + isDsWhitelisted
+ ", blacklisted=" + isDsBlacklisted
- + ", newRules=" + uidRulesToString(newUidRules)
- + ", oldRules=" + uidRulesToString(oldUidRules));
+ + ", newRule=" + uidRulesToString(newUidRules)
+ + ", oldRule=" + uidRulesToString(oldUidRules));
}
+ }
- // dispatch changed rule to existing listeners
+ // Apply Battery Saver rules.
+ // NOTE: the firewall rules are changed outside this method, but it's still necessary to
+ // send the MSG_RULES_CHANGED so ConnectivityService can update its internal status.
+ if (newBsRule != oldBsRule) {
+ changed = true;
+ if (newBsRule == RULE_NONE || (newBsRule & RULE_ALLOW_ALL) != 0) {
+ if (LOGV) Log.v(TAG, "Allowing non-metered access for UID " + uid);
+ } else if ((newBsRule & RULE_REJECT_ALL) != 0) {
+ if (LOGV) Log.v(TAG, "Rejecting non-metered access for UID " + uid);
+ } else {
+ // All scenarios should have been covered above
+ Log.wtf(TAG, "Unexpected change of non-metered UID state for " + uid
+ + ": foreground=" + isForeground
+ + ", whitelisted=" + isBsWhitelisted
+ + ", newRule=" + uidRulesToString(newUidRules)
+ + ", oldRule=" + uidRulesToString(oldUidRules));
+ }
+ }
+
+ // Final step: dispatch changed rule to existing listeners
+ if (changed) {
mHandler.obtainMessage(MSG_RULES_CHANGED, uid, newUidRules).sendToTarget();
}
}
@@ -2966,6 +3036,16 @@
}
}
+ private void dispatchRestrictPowerChanged(INetworkPolicyListener listener,
+ boolean restrictPower) {
+ if (listener != null) {
+ try {
+ listener.onRestrictPowerChanged(restrictPower);
+ } catch (RemoteException ignored) {
+ }
+ }
+ }
+
private Handler.Callback mHandlerCallback = new Handler.Callback() {
@Override
public boolean handleMessage(Message msg) {
@@ -3013,6 +3093,11 @@
}
return true;
}
+ case MSG_RESTRICT_POWER_CHANGED: {
+ final boolean restrictPower = msg.arg1 != 0;
+ dispatchRestrictPowerChanged(mConnectivityListener, restrictPower);
+ return true;
+ }
case MSG_RESTRICT_BACKGROUND_CHANGED: {
final boolean restrictBackground = msg.arg1 != 0;
dispatchRestrictBackgroundChanged(mConnectivityListener, restrictBackground);