Move back networking policy logic into NetworkPolicyManagerService
This patch removes from ConnectivityService the logic involved in
deciding if a uid has access to networking based on networking policies.
This logic is moved into NetworkPolicyManagerService which is the source
of truth with regards to the state of networking policie, both for
existing networks and uids.
Instead ConnectivityService directly queries NetworkPolicyManagerService
in a synchronous fashion for a specific uid or a (uid, network) pair.
This eliminates the need to keep a copy of the uid policy rules inside
ConnectivityService and ensures that ConnectivityService takes
networking decisions based on the correct state of networking policies,
and therefore eliminates certain data races in ConnectivityManager API
that applications are exposed to.
Test: $ runtest frameworks-net
$ runtest -x frameworks/base/services/tests/../NetworkPolicyManagerServiceTest.java
$ runtest -c com.android.server.net.ConnOnActivityStartTest frameworks-services
Bug: 32069544, 30919851
Change-Id: Ic75d4f7a8853e6be20e51262c4b59805ec35093a
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index d02b726..0e06682 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -29,13 +29,6 @@
import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED;
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.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;
import android.annotation.Nullable;
import android.app.BroadcastOptions;
@@ -104,7 +97,6 @@
import android.security.KeyStore;
import android.telephony.TelephonyManager;
import android.text.TextUtils;
-import android.util.ArraySet;
import android.util.LocalLog;
import android.util.LocalLog.ReadOnlyLocalLog;
import android.util.Log;
@@ -129,6 +121,7 @@
import com.android.internal.util.MessageUtils;
import com.android.internal.util.WakeupMessage;
import com.android.internal.util.XmlUtils;
+import com.android.server.LocalServices;
import com.android.server.am.BatteryStatsService;
import com.android.server.connectivity.DataConnectionStats;
import com.android.server.connectivity.KeepaliveTracker;
@@ -146,6 +139,7 @@
import com.android.server.connectivity.Vpn;
import com.android.server.net.BaseNetworkObserver;
import com.android.server.net.LockdownVpnTracker;
+import com.android.server.net.NetworkPolicyManagerInternal;
import com.google.android.collect.Lists;
@@ -220,18 +214,6 @@
private boolean mLockdownEnabled;
private LockdownVpnTracker mLockdownTracker;
- /** Lock around {@link #mUidRules} and {@link #mMeteredIfaces}. */
- private Object mRulesLock = new Object();
- /** Currently active network rules by UID. */
- @GuardedBy("mRulesLock")
- private SparseIntArray mUidRules = new SparseIntArray();
- /** Set of ifaces that are costly. */
- @GuardedBy("mRulesLock")
- private ArraySet<String> mMeteredIfaces = new ArraySet<>();
- /** Flag indicating if background data is restricted. */
- @GuardedBy("mRulesLock")
- private boolean mRestrictBackground;
-
final private Context mContext;
private int mNetworkPreference;
// 0 is full bad, 100 is full good
@@ -245,6 +227,7 @@
private INetworkManagementService mNetd;
private INetworkStatsService mStatsService;
private INetworkPolicyManager mPolicyManager;
+ private NetworkPolicyManagerInternal mPolicyManagerInternal;
private String mCurrentTcpBufferSizes;
@@ -714,12 +697,15 @@
mNetd = checkNotNull(netManager, "missing INetworkManagementService");
mStatsService = checkNotNull(statsService, "missing INetworkStatsService");
mPolicyManager = checkNotNull(policyManager, "missing INetworkPolicyManager");
+ mPolicyManagerInternal = checkNotNull(
+ LocalServices.getService(NetworkPolicyManagerInternal.class),
+ "missing NetworkPolicyManagerInternal");
+
mKeyStore = KeyStore.getInstance();
mTelephonyManager = (TelephonyManager) mContext.getSystemService(Context.TELEPHONY_SERVICE);
try {
- mPolicyManager.setConnectivityListener(mPolicyListener);
- mRestrictBackground = mPolicyManager.getRestrictBackground();
+ mPolicyManager.registerListener(mPolicyListener);
} catch (RemoteException e) {
// ouch, no rules updates means some processes may never get network
loge("unable to register INetworkPolicyListener" + e);
@@ -990,51 +976,22 @@
private boolean isNetworkWithLinkPropertiesBlocked(LinkProperties lp, int uid,
boolean ignoreBlocked) {
// Networks aren't blocked when ignoring blocked status
- if (ignoreBlocked) return false;
+ if (ignoreBlocked) {
+ return false;
+ }
// Networks are never blocked for system services
- if (isSystem(uid)) return false;
-
- final boolean networkMetered;
- final int uidRules;
-
+ // TODO: consider moving this check to NetworkPolicyManagerInternal.isUidNetworkingBlocked.
+ if (isSystem(uid)) {
+ return false;
+ }
synchronized (mVpns) {
final Vpn vpn = mVpns.get(UserHandle.getUserId(uid));
if (vpn != null && vpn.isBlockingUid(uid)) {
return true;
}
}
-
final String iface = (lp == null ? "" : lp.getInterfaceName());
- synchronized (mRulesLock) {
- networkMetered = mMeteredIfaces.contains(iface);
- uidRules = mUidRules.get(uid, RULE_NONE);
- }
-
- 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 power restrictions.
- if (allowed) {
- allowed = (uidRules & RULE_REJECT_ALL) == 0;
- if (LOGD_RULES) Log.d(TAG, "allowed status for uid " + uid + " when"
- + " rule is " + uidRulesToString(uidRules) + ": " + allowed);
- }
- return !allowed;
+ return mPolicyManagerInternal.isUidNetworkingBlocked(uid, iface);
}
private void maybeLogBlockedNetworkInfo(NetworkInfo ni, int uid) {
@@ -1480,67 +1437,24 @@
return true;
}
- private INetworkPolicyListener mPolicyListener = new INetworkPolicyListener.Stub() {
+ private final INetworkPolicyListener mPolicyListener = new INetworkPolicyListener.Stub() {
@Override
public void onUidRulesChanged(int uid, int uidRules) {
- // caller is NPMS, since we only register with them
- if (LOGD_RULES) {
- log("onUidRulesChanged(uid=" + uid + ", uidRules=" + uidRules + ")");
- }
-
- synchronized (mRulesLock) {
- // skip update when we've already applied rules
- final int oldRules = mUidRules.get(uid, RULE_NONE);
- if (oldRules == uidRules) return;
-
- if (uidRules == RULE_NONE) {
- mUidRules.delete(uid);
- } else {
- mUidRules.put(uid, uidRules);
- }
- }
-
// TODO: notify UID when it has requested targeted updates
}
-
@Override
public void onMeteredIfacesChanged(String[] meteredIfaces) {
- // caller is NPMS, since we only register with them
- if (LOGD_RULES) {
- log("onMeteredIfacesChanged(ifaces=" + Arrays.toString(meteredIfaces) + ")");
- }
-
- synchronized (mRulesLock) {
- mMeteredIfaces.clear();
- for (String iface : meteredIfaces) {
- mMeteredIfaces.add(iface);
- }
- }
}
-
@Override
public void onRestrictBackgroundChanged(boolean restrictBackground) {
- // caller is NPMS, since we only register with them
- if (LOGD_RULES) {
- log("onRestrictBackgroundChanged(restrictBackground=" + restrictBackground + ")");
- }
-
- synchronized (mRulesLock) {
- mRestrictBackground = restrictBackground;
- }
-
+ // TODO: relocate this specific callback in Tethering.
if (restrictBackground) {
log("onRestrictBackgroundChanged(true): disabling tethering");
mTethering.untetherAll();
}
}
-
@Override
public void onUidPoliciesChanged(int uid, int uidPolicies) {
- // caller is NPMS, since we only register with them
- if (LOGD_RULES) {
- log("onUidRulesChanged(uid=" + uid + ", uidPolicies=" + uidPolicies + ")");
- }
}
};
@@ -1982,33 +1896,6 @@
pw.decreaseIndent();
pw.println();
- pw.println("Metered Interfaces:");
- pw.increaseIndent();
- for (String value : mMeteredIfaces) {
- pw.println(value);
- }
- pw.decreaseIndent();
- pw.println();
-
- pw.print("Restrict background: ");
- pw.println(mRestrictBackground);
- pw.println();
-
- pw.println("Status for known UIDs:");
- pw.increaseIndent();
- final int size = mUidRules.size();
- for (int i = 0; i < size; i++) {
- final int uid = mUidRules.keyAt(i);
- pw.print("UID=");
- pw.print(uid);
- final int uidRules = mUidRules.get(uid, RULE_NONE);
- pw.print(" rules=");
- pw.print(uidRulesToString(uidRules));
- pw.println();
- }
- pw.println();
- pw.decreaseIndent();
-
pw.println("Network Requests:");
pw.increaseIndent();
for (NetworkRequestInfo nri : mNetworkRequests.values()) {
@@ -3443,6 +3330,10 @@
Slog.e(TAG, s);
}
+ private static void loge(String s, Throwable t) {
+ Slog.e(TAG, s, t);
+ }
+
private static <T> T checkNotNull(T value, String message) {
if (value == null) {
throw new NullPointerException(message);
@@ -4203,20 +4094,16 @@
private void enforceMeteredApnPolicy(NetworkCapabilities networkCapabilities) {
final int uid = Binder.getCallingUid();
if (isSystem(uid)) {
+ // Exemption for system uid.
return;
}
- // if UID is restricted, don't allow them to bring up metered APNs
- if (networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED) == false) {
- final int uidRules;
- synchronized(mRulesLock) {
- uidRules = mUidRules.get(uid, RULE_ALLOW_ALL);
- }
- if (mRestrictBackground && (uidRules & RULE_ALLOW_METERED) == 0
- && (uidRules & RULE_TEMPORARY_ALLOW_METERED) == 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 option.
- networkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED);
- }
+ if (networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED)) {
+ // Policy already enforced.
+ return;
+ }
+ if (mPolicyManagerInternal.isUidRestrictedOnMeteredNetworks(uid)) {
+ // If UID is restricted, don't allow them to bring up metered APNs.
+ networkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED);
}
}
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 04443a5..c562cb9 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -80,6 +80,7 @@
import com.android.server.connectivity.NetworkMonitor;
import com.android.server.connectivity.NetworkMonitor.CaptivePortalProbeResult;
import com.android.server.net.NetworkPinner;
+import com.android.server.net.NetworkPolicyManagerInternal;
import java.net.InetAddress;
import java.util.ArrayList;
@@ -713,6 +714,9 @@
}
mServiceContext = new MockContext(getContext());
+ LocalServices.removeServiceForTest(NetworkPolicyManagerInternal.class);
+ LocalServices.addService(
+ NetworkPolicyManagerInternal.class, mock(NetworkPolicyManagerInternal.class));
mService = new WrappedConnectivityService(mServiceContext,
mock(INetworkManagementService.class),
mock(INetworkStatsService.class),