From 06ea4b0cf14e8d06b80e898eb6452100f4e495d0 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Fri, 6 May 2016 14:53:50 -0700 Subject: [PATCH] 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 --- .../android/server/ConnectivityService.java | 71 ++++++++++++++----- 1 file changed, 52 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 3508701193..8763b93f31 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_NOT_RESTRICTED; 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 @@ public class ConnectivityService extends IConnectivityManager.Stub /** 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 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 @@ public class ConnectivityService extends IConnectivityManager.Stub 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); @@ -1421,6 +1438,18 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + @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) { @@ -1862,6 +1891,10 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 @@ public class ConnectivityService extends IConnectivityManager.Stub 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); } }