Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 938ab4fa authored by Hugo Benichi's avatar Hugo Benichi
Browse files

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
parent 670e750b

core/java/android/net/INetworkPolicyManager.aidl

+0 −3
Original line number Diff line number Diff line
@@ -38,9 +38,6 @@ interface INetworkPolicyManager { 


    boolean isUidForeground(int uid);



    /** Higher priority listener before general event dispatch */

    void setConnectivityListener(INetworkPolicyListener listener);



    void registerListener(INetworkPolicyListener listener);

    void unregisterListener(INetworkPolicyListener listener);

services/core/java/com/android/server/ConnectivityService.java

+30 −143
Original line number Diff line number Diff line
@@ -29,13 +29,6 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; 
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.Credentials; 
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.IndentingPrintWriter; 
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.Tethering; 
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 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    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 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    private INetworkManagementService mNetd;

    private INetworkStatsService mStatsService;

    private INetworkPolicyManager mPolicyManager;

    private NetworkPolicyManagerInternal mPolicyManagerInternal;



    private String mCurrentTcpBufferSizes;
@@ -714,12 +697,15 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        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 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    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 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        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 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        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 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        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,22 +4094,18 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    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 (networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED)) {

            // Policy already enforced.

            return;

        }

            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.

        if (mPolicyManagerInternal.isUidRestrictedOnMeteredNetworks(uid)) {

            // If UID is restricted, don't allow them to bring up metered APNs.

            networkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED);

        }

    }

    }



    @Override

    public NetworkRequest pendingRequestForNetwork(NetworkCapabilities networkCapabilities,

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

+11 −0
Original line number Diff line number Diff line
@@ -27,4 +27,15 @@ public abstract class NetworkPolicyManagerInternal { 
     * Resets all policies associated with a given user.

     */

    public abstract void resetUserState(int userId);



    /**

     * @return true if the given uid is restricted from doing networking on metered networks.

     */

    public abstract boolean isUidRestrictedOnMeteredNetworks(int uid);



    /**

     * @return true if networking is blocked on the given interface for the given uid according

     * to current networking policies.

     */

    public abstract boolean isUidNetworkingBlocked(int uid, String ifname);

}

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

+70 −18
Original line number Diff line number Diff line
@@ -245,8 +245,8 @@ import java.util.concurrent.TimeUnit; 
 */

public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {

    static final String TAG = "NetworkPolicy";

    private static final boolean LOGD = false;

    private static final boolean LOGV = false;

    private static final boolean LOGD = true; // UNDO

    private static final boolean LOGV = true; // UNDO



    private static final int VERSION_INIT = 1;

    private static final int VERSION_ADDED_SNOOZE = 2;
@@ -425,9 +425,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
    @GuardedBy("mUidRulesFirstLock")

    final SparseIntArray mUidState = new SparseIntArray();



    /** Higher priority listener before general event dispatch */

    private INetworkPolicyListener mConnectivityListener;



    private final RemoteCallbackList<INetworkPolicyListener>

            mListeners = new RemoteCallbackList<>();
@@ -2240,15 +2237,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
        return changed;

    }



    @Override

    public void setConnectivityListener(INetworkPolicyListener listener) {

        mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG);

        if (mConnectivityListener != null) {

            throw new IllegalStateException("Connectivity listener already registered");

        }

        mConnectivityListener = listener;

    }



    @Override

    public void registerListener(INetworkPolicyListener listener) {

        // TODO: create permission for observing network policy
@@ -3560,7 +3548,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
                case MSG_RULES_CHANGED: {

                    final int uid = msg.arg1;

                    final int uidRules = msg.arg2;

                    dispatchUidRulesChanged(mConnectivityListener, uid, uidRules);

                    final int length = mListeners.beginBroadcast();

                    for (int i = 0; i < length; i++) {

                        final INetworkPolicyListener listener = mListeners.getBroadcastItem(i);
@@ -3571,7 +3558,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
                }

                case MSG_METERED_IFACES_CHANGED: {

                    final String[] meteredIfaces = (String[]) msg.obj;

                    dispatchMeteredIfacesChanged(mConnectivityListener, meteredIfaces);

                    final int length = mListeners.beginBroadcast();

                    for (int i = 0; i < length; i++) {

                        final INetworkPolicyListener listener = mListeners.getBroadcastItem(i);
@@ -3602,7 +3588,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
                }

                case MSG_RESTRICT_BACKGROUND_CHANGED: {

                    final boolean restrictBackground = msg.arg1 != 0;

                    dispatchRestrictBackgroundChanged(mConnectivityListener, restrictBackground);

                    final int length = mListeners.beginBroadcast();

                    for (int i = 0; i < length; i++) {

                        final INetworkPolicyListener listener = mListeners.getBroadcastItem(i);
@@ -3620,7 +3605,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
                    final int policy = msg.arg2;

                    final Boolean notifyApp = (Boolean) msg.obj;

                    // First notify internal listeners...

                    dispatchUidPoliciesChanged(mConnectivityListener, uid, policy);

                    final int length = mListeners.beginBroadcast();

                    for (int i = 0; i < length; i++) {

                        final INetworkPolicyListener listener = mListeners.getBroadcastItem(i);
@@ -4053,6 +4037,74 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { 
                }

            }

        }



        /**

         * @return true if the given uid is restricted from doing networking on metered networks.

         */

        @Override

        public boolean isUidRestrictedOnMeteredNetworks(int uid) {

            final int uidRules;

            final boolean isBackgroundRestricted;

            synchronized (mUidRulesFirstLock) {

                uidRules = mUidRules.get(uid, RULE_ALLOW_ALL);

                isBackgroundRestricted = mRestrictBackground;

            }

            return isBackgroundRestricted

                    && !hasRule(uidRules, RULE_ALLOW_METERED)

                    && !hasRule(uidRules, RULE_TEMPORARY_ALLOW_METERED);

        }



        /**

         * @return true if networking is blocked on the given interface for the given uid according

         * to current networking policies.

         */

        @Override

        public boolean isUidNetworkingBlocked(int uid, String ifname) {

            final int uidRules;

            final boolean isBackgroundRestricted;

            final boolean isNetworkMetered;

            synchronized (mUidRulesFirstLock) {

                uidRules = mUidRules.get(uid, RULE_NONE);

                isBackgroundRestricted = mRestrictBackground;

                synchronized (mNetworkPoliciesSecondLock) {

                    isNetworkMetered = mMeteredIfaces.contains(ifname);

                }

            }

            if (hasRule(uidRules, RULE_REJECT_ALL)) {

                if (LOGV) logUidStatus(uid, "blocked by power restrictions");

                return true;

            }

            if (!isNetworkMetered) {

                if (LOGV) logUidStatus(uid, "allowed on unmetered network");

                return false;

            }

            if (hasRule(uidRules, RULE_REJECT_METERED)) {

                if (LOGV) logUidStatus(uid, "blacklisted on metered network");

                return true;

            }

            if (hasRule(uidRules, RULE_ALLOW_METERED)) {

                if (LOGV) logUidStatus(uid, "whitelisted on metered network");

                return false;

            }

            if (hasRule(uidRules, RULE_TEMPORARY_ALLOW_METERED)) {

                if (LOGV) logUidStatus(uid, "temporary whitelisted on metered network");

                return false;

            }

            if (isBackgroundRestricted) {

                if (LOGV) logUidStatus(uid, "blocked when background is restricted");

                return true;

            }

            if (LOGV) logUidStatus(uid, "allowed by default");

            return false;

        }

    }



    private static boolean hasRule(int uidRules, int rule) {

        return (uidRules & rule) != 0;

    }



    private static void logUidStatus(int uid, String descr) {

        Slog.d(TAG, String.format("uid %d is %s", uid, descr));

    }



    /**

tests/net/java/com/android/server/ConnectivityServiceTest.java

+4 −0
Original line number Diff line number Diff line
@@ -80,6 +80,7 @@ import com.android.server.connectivity.NetworkAgentInfo; 
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 @@ public class ConnectivityServiceTest extends AndroidTestCase { 
        }



        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),