From 3897df2e36d4d015304b398c545c1232ee35a1c9 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Fri, 12 Apr 2019 09:05:40 +0000 Subject: [PATCH] Revert "Block incoming non-VPN packets to apps under fully-routed VPN" This reverts commit fd8f96d71925cc80f5052365af2e5150cc0ec3ca. This change does not have any topic: not reverting the other 2 commits in the original topic. Reason for revert: broke FrameworksNetTests presubmit: b/130397860 Change-Id: Iff41d9fe97fafea44680c8d67d1ce19277548cc0 --- core/java/android/net/NetworkCapabilities.java | 5 - core/java/android/net/UidRange.java | 27 --- .../com/android/server/ConnectivityService.java | 113 +---------- .../server/connectivity/PermissionMonitor.java | 212 ++------------------- .../java/com/android/server/connectivity/Vpn.java | 7 +- tests/net/java/android/net/RouteInfoTest.java | 149 +++++++-------- .../android/server/ConnectivityServiceTest.java | 200 +------------------ .../server/connectivity/PermissionMonitorTest.java | 155 +++------------ 8 files changed, 119 insertions(+), 749 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index dfd70898dbd9..99375f842b4c 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -822,11 +822,6 @@ public final class NetworkCapabilities implements Parcelable { mEstablishingVpnAppUid = uid; } - /** @hide */ - public int getEstablishingVpnAppUid() { - return mEstablishingVpnAppUid; - } - /** * Value indicating that link bandwidth is unspecified. * @hide diff --git a/core/java/android/net/UidRange.java b/core/java/android/net/UidRange.java index a1ac96037136..fa0eeb9e0e49 100644 --- a/core/java/android/net/UidRange.java +++ b/core/java/android/net/UidRange.java @@ -21,8 +21,6 @@ import static android.os.UserHandle.PER_USER_RANGE; import android.os.Parcel; import android.os.Parcelable; -import java.util.Collection; - /** * An inclusive range of UIDs. * @@ -44,16 +42,10 @@ public final class UidRange implements Parcelable { return new UidRange(userId * PER_USER_RANGE, (userId + 1) * PER_USER_RANGE - 1); } - /** Returns the smallest user Id which is contained in this UidRange */ public int getStartUser() { return start / PER_USER_RANGE; } - /** Returns the largest user Id which is contained in this UidRange */ - public int getEndUser() { - return stop / PER_USER_RANGE; - } - public boolean contains(int uid) { return start <= uid && uid <= stop; } @@ -125,23 +117,4 @@ public final class UidRange implements Parcelable { return new UidRange[size]; } }; - - /** - * Returns whether any of the UidRange in the collection contains the specified uid - * - * @param ranges The collection of UidRange to check - * @param uid the uid in question - * @return {@code true} if the uid is contained within the ranges, {@code false} otherwise - * - * @see UidRange#contains(int) - */ - public static boolean containsUid(Collection ranges, int uid) { - if (ranges == null) return false; - for (UidRange range : ranges) { - if (range.contains(uid)) { - return true; - } - } - return false; - } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index c1aff757d474..524548f6233d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -47,7 +47,6 @@ import static android.system.OsConstants.IPPROTO_UDP; import static com.android.internal.util.Preconditions.checkNotNull; -import android.annotation.NonNull; import android.annotation.Nullable; import android.app.BroadcastOptions; import android.app.NotificationManager; @@ -277,8 +276,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private Tethering mTethering; - @VisibleForTesting - protected final PermissionMonitor mPermissionMonitor; + private final PermissionMonitor mPermissionMonitor; private KeyStore mKeyStore; @@ -831,13 +829,13 @@ public class ConnectivityService extends IConnectivityManager.Stub public ConnectivityService(Context context, INetworkManagementService netManager, INetworkStatsService statsService, INetworkPolicyManager policyManager) { this(context, netManager, statsService, policyManager, - getDnsResolver(), new IpConnectivityLog(), NetdService.getInstance()); + getDnsResolver(), new IpConnectivityLog()); } @VisibleForTesting protected ConnectivityService(Context context, INetworkManagementService netManager, INetworkStatsService statsService, INetworkPolicyManager policyManager, - IDnsResolver dnsresolver, IpConnectivityLog logger, INetd netd) { + IDnsResolver dnsresolver, IpConnectivityLog logger) { if (DBG) log("ConnectivityService starting up"); mSystemProperties = getSystemProperties(); @@ -877,7 +875,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mDnsResolver = checkNotNull(dnsresolver, "missing IDnsResolver"); mProxyTracker = makeProxyTracker(); - mNetd = netd; + mNetd = NetdService.getInstance(); mKeyStore = KeyStore.getInstance(); mTelephonyManager = (TelephonyManager) mContext.getSystemService(Context.TELEPHONY_SERVICE); @@ -963,7 +961,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mTethering = makeTethering(); - mPermissionMonitor = new PermissionMonitor(mContext, mNetd); + mPermissionMonitor = new PermissionMonitor(mContext, mNMS, mNetd); // Set up the listener for user state for creating user VPNs. // Should run on mHandler to avoid any races. @@ -2443,13 +2441,6 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.println("NetworkStackClient logs:"); pw.increaseIndent(); NetworkStackClient.getInstance().dump(pw); - pw.decreaseIndent(); - - pw.println(); - pw.println("Permission Monitor:"); - pw.increaseIndent(); - mPermissionMonitor.dump(pw); - pw.decreaseIndent(); } private void dumpNetworks(IndentingPrintWriter pw) { @@ -5474,11 +5465,6 @@ public class ConnectivityService extends IConnectivityManager.Stub networkAgent.clatd.fixupLinkProperties(oldLp, newLp); updateInterfaces(newLp, oldLp, netId, networkAgent.networkCapabilities); - - // update filtering rules, need to happen after the interface update so netd knows about the - // new interface (the interface name -> index map becomes initialized) - updateVpnFiltering(newLp, oldLp, networkAgent); - updateMtu(newLp, oldLp); // TODO - figure out what to do for clat // for (LinkProperties lp : newLp.getStackedLinks()) { @@ -5644,37 +5630,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void updateVpnFiltering(LinkProperties newLp, LinkProperties oldLp, - NetworkAgentInfo nai) { - final String oldIface = oldLp != null ? oldLp.getInterfaceName() : null; - final String newIface = newLp != null ? newLp.getInterfaceName() : null; - final boolean wasFiltering = requiresVpnIsolation(nai, nai.networkCapabilities, oldLp); - final boolean needsFiltering = requiresVpnIsolation(nai, nai.networkCapabilities, newLp); - - if (!wasFiltering && !needsFiltering) { - // Nothing to do. - return; - } - - if (Objects.equals(oldIface, newIface) && (wasFiltering == needsFiltering)) { - // Nothing changed. - return; - } - - final Set ranges = nai.networkCapabilities.getUids(); - final int vpnAppUid = nai.networkCapabilities.getEstablishingVpnAppUid(); - // TODO: this create a window of opportunity for apps to receive traffic between the time - // when the old rules are removed and the time when new rules are added. To fix this, - // make eBPF support two whitelisted interfaces so here new rules can be added before the - // old rules are being removed. - if (wasFiltering) { - mPermissionMonitor.onVpnUidRangesRemoved(oldIface, ranges, vpnAppUid); - } - if (needsFiltering) { - mPermissionMonitor.onVpnUidRangesAdded(newIface, ranges, vpnAppUid); - } - } - private int getNetworkPermission(NetworkCapabilities nc) { if (!nc.hasCapability(NET_CAPABILITY_NOT_RESTRICTED)) { return INetd.PERMISSION_SYSTEM; @@ -5817,34 +5772,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - /** - * Returns whether VPN isolation (ingress interface filtering) should be applied on the given - * network. - * - * Ingress interface filtering enforces that all apps under the given network can only receive - * packets from the network's interface (and loopback). This is important for VPNs because - * apps that cannot bypass a fully-routed VPN shouldn't be able to receive packets from any - * non-VPN interfaces. - * - * As a result, this method should return true iff - * 1. the network is an app VPN (not legacy VPN) - * 2. the VPN does not allow bypass - * 3. the VPN is fully-routed - * 4. the VPN interface is non-null - * - * @See INetd#firewallAddUidInterfaceRules - * @See INetd#firewallRemoveUidInterfaceRules - */ - private boolean requiresVpnIsolation(@NonNull NetworkAgentInfo nai, NetworkCapabilities nc, - LinkProperties lp) { - if (nc == null || lp == null) return false; - return nai.isVPN() - && !nai.networkMisc.allowBypass - && nc.getEstablishingVpnAppUid() != Process.SYSTEM_UID - && lp.getInterfaceName() != null - && (lp.hasIPv4DefaultRoute() || lp.hasIPv6DefaultRoute()); - } - private void updateUids(NetworkAgentInfo nai, NetworkCapabilities prevNc, NetworkCapabilities newNc) { Set prevRanges = null == prevNc ? null : prevNc.getUids(); @@ -5857,12 +5784,6 @@ public class ConnectivityService extends IConnectivityManager.Stub newRanges.removeAll(prevRangesCopy); try { - // When updating the VPN uid routing rules, add the new range first then remove the old - // range. If old range were removed first, there would be a window between the old - // range being removed and the new range being added, during which UIDs contained - // in both ranges are not subject to any VPN routing rules. Adding new range before - // removing old range works because, unlike the filtering rules below, it's possible to - // add duplicate UID routing rules. if (!newRanges.isEmpty()) { final UidRange[] addedRangesArray = new UidRange[newRanges.size()]; newRanges.toArray(addedRangesArray); @@ -5873,31 +5794,9 @@ public class ConnectivityService extends IConnectivityManager.Stub prevRanges.toArray(removedRangesArray); mNMS.removeVpnUidRanges(nai.network.netId, removedRangesArray); } - final boolean wasFiltering = requiresVpnIsolation(nai, prevNc, nai.linkProperties); - final boolean shouldFilter = requiresVpnIsolation(nai, newNc, nai.linkProperties); - final String iface = nai.linkProperties.getInterfaceName(); - // For VPN uid interface filtering, old ranges need to be removed before new ranges can - // be added, due to the range being expanded and stored as invidiual UIDs. For example - // the UIDs might be updated from [0, 99999] to ([0, 10012], [10014, 99999]) which means - // prevRanges = [0, 99999] while newRanges = [0, 10012], [10014, 99999]. If prevRanges - // were added first and then newRanges got removed later, there would be only one uid - // 10013 left. A consequence of removing old ranges before adding new ranges is that - // there is now a window of opportunity when the UIDs are not subject to any filtering. - // Note that this is in contrast with the (more robust) update of VPN routing rules - // above, where the addition of new ranges happens before the removal of old ranges. - // TODO Fix this window by computing an accurate diff on Set, so the old range - // to be removed will never overlap with the new range to be added. - if (wasFiltering && !prevRanges.isEmpty()) { - mPermissionMonitor.onVpnUidRangesRemoved(iface, prevRanges, - prevNc.getEstablishingVpnAppUid()); - } - if (shouldFilter && !newRanges.isEmpty()) { - mPermissionMonitor.onVpnUidRangesAdded(iface, newRanges, - newNc.getEstablishingVpnAppUid()); - } } catch (Exception e) { // Never crash! - loge("Exception in updateUids: ", e); + loge("Exception in updateUids: " + e); } } diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java index f8582cd7928f..b6946023e870 100644 --- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java +++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java @@ -37,27 +37,22 @@ import android.content.pm.PackageManager.NameNotFoundException; import android.content.pm.PackageManagerInternal; import android.content.pm.UserInfo; import android.net.INetd; -import android.net.UidRange; import android.os.Build; +import android.os.INetworkManagementService; import android.os.RemoteException; -import android.os.ServiceSpecificException; import android.os.UserHandle; import android.os.UserManager; -import android.system.OsConstants; import android.util.ArraySet; import android.util.Log; import android.util.SparseArray; import android.util.SparseIntArray; -import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; -import com.android.internal.util.IndentingPrintWriter; import com.android.server.LocalServices; import com.android.server.SystemConfig; import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -65,7 +60,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; - /** * A utility class to inform Netd of UID permisisons. * Does a mass update at boot and then monitors for app install/remove. @@ -79,29 +73,18 @@ public class PermissionMonitor { protected static final Boolean NETWORK = Boolean.FALSE; private static final int VERSION_Q = Build.VERSION_CODES.Q; + private final Context mContext; private final PackageManager mPackageManager; private final UserManager mUserManager; + private final INetworkManagementService mNMS; private final INetd mNetd; // Values are User IDs. - @GuardedBy("this") private final Set mUsers = new HashSet<>(); - // Keys are app uids. Values are true for SYSTEM permission and false for NETWORK permission. - @GuardedBy("this") + // Keys are App IDs. Values are true for SYSTEM permission and false for NETWORK permission. private final Map mApps = new HashMap<>(); - // Keys are active non-bypassable and fully-routed VPN's interface name, Values are uid ranges - // for apps under the VPN - @GuardedBy("this") - private final Map> mVpnUidRanges = new HashMap<>(); - - // A set of appIds for apps across all users on the device. We track appIds instead of uids - // directly to reduce its size and also eliminate the need to update this set when user is - // added/removed. - @GuardedBy("this") - private final Set mAllApps = new HashSet<>(); - private class PackageListObserver implements PackageManagerInternal.PackageListObserver { private int getPermissionForUid(int uid) { @@ -135,10 +118,12 @@ public class PermissionMonitor { } } - public PermissionMonitor(Context context, INetd netd) { + public PermissionMonitor(Context context, INetworkManagementService nms, INetd netdService) { + mContext = context; mPackageManager = context.getPackageManager(); - mUserManager = (UserManager) context.getSystemService(Context.USER_SERVICE); - mNetd = netd; + mUserManager = UserManager.get(context); + mNMS = nms; + mNetd = netdService; } // Intended to be called only once at startup, after the system is ready. Installs a broadcast @@ -166,7 +151,6 @@ public class PermissionMonitor { if (uid < 0) { continue; } - mAllApps.add(UserHandle.getAppId(uid)); boolean isNetwork = hasNetworkPermission(app); boolean hasRestrictedPermission = hasRestrictedNetworkPermission(app); @@ -286,11 +270,10 @@ public class PermissionMonitor { } } - private int[] toIntArray(Collection list) { + private int[] toIntArray(List list) { int[] array = new int[list.size()]; - int i = 0; - for (Integer item : list) { - array[i++] = item; + for (int i = 0; i < list.size(); i++) { + array[i] = list.get(i); } return array; } @@ -306,11 +289,11 @@ public class PermissionMonitor { } try { if (add) { - mNetd.networkSetPermissionForUser(INetd.PERMISSION_NETWORK, toIntArray(network)); - mNetd.networkSetPermissionForUser(INetd.PERMISSION_SYSTEM, toIntArray(system)); + mNMS.setPermission("NETWORK", toIntArray(network)); + mNMS.setPermission("SYSTEM", toIntArray(system)); } else { - mNetd.networkClearPermissionForUser(toIntArray(network)); - mNetd.networkClearPermissionForUser(toIntArray(system)); + mNMS.clearPermission(toIntArray(network)); + mNMS.clearPermission(toIntArray(system)); } } catch (RemoteException e) { loge("Exception when updating permissions: " + e); @@ -393,19 +376,6 @@ public class PermissionMonitor { apps.put(uid, permission); update(mUsers, apps, true); } - - // If the newly-installed package falls within some VPN's uid range, update Netd with it. - // This needs to happen after the mApps update above, since removeBypassingUids() depends - // on mApps to check if the package can bypass VPN. - for (Map.Entry> vpn : mVpnUidRanges.entrySet()) { - if (UidRange.containsUid(vpn.getValue(), uid)) { - final Set changedUids = new HashSet<>(); - changedUids.add(uid); - removeBypassingUids(changedUids, /* vpnAppUid */ -1); - updateVpnUids(vpn.getKey(), changedUids, true); - } - } - mAllApps.add(UserHandle.getAppId(uid)); } /** @@ -416,23 +386,8 @@ public class PermissionMonitor { * @hide */ public synchronized void onPackageRemoved(int uid) { - // If the newly-removed package falls within some VPN's uid range, update Netd with it. - // This needs to happen before the mApps update below, since removeBypassingUids() depends - // on mApps to check if the package can bypass VPN. - for (Map.Entry> vpn : mVpnUidRanges.entrySet()) { - if (UidRange.containsUid(vpn.getValue(), uid)) { - final Set changedUids = new HashSet<>(); - changedUids.add(uid); - removeBypassingUids(changedUids, /* vpnAppUid */ -1); - updateVpnUids(vpn.getKey(), changedUids, false); - } - } - // If the package has been removed from all users on the device, clear it form mAllApps. - if (mPackageManager.getNameForUid(uid) == null) { - mAllApps.remove(UserHandle.getAppId(uid)); - } - Map apps = new HashMap<>(); + Boolean permission = null; String[] packages = mPackageManager.getPackagesForUid(uid); if (packages != null && packages.length > 0) { @@ -488,121 +443,6 @@ public class PermissionMonitor { } /** - * Called when a new set of UID ranges are added to an active VPN network - * - * @param iface The active VPN network's interface name - * @param rangesToAdd The new UID ranges to be added to the network - * @param vpnAppUid The uid of the VPN app - */ - public synchronized void onVpnUidRangesAdded(@NonNull String iface, Set rangesToAdd, - int vpnAppUid) { - // Calculate the list of new app uids under the VPN due to the new UID ranges and update - // Netd about them. Because mAllApps only contains appIds instead of uids, the result might - // be an overestimation if an app is not installed on the user on which the VPN is running, - // but that's safe. - final Set changedUids = intersectUids(rangesToAdd, mAllApps); - removeBypassingUids(changedUids, vpnAppUid); - updateVpnUids(iface, changedUids, true); - if (mVpnUidRanges.containsKey(iface)) { - mVpnUidRanges.get(iface).addAll(rangesToAdd); - } else { - mVpnUidRanges.put(iface, new HashSet(rangesToAdd)); - } - } - - /** - * Called when a set of UID ranges are removed from an active VPN network - * - * @param iface The VPN network's interface name - * @param rangesToRemove Existing UID ranges to be removed from the VPN network - * @param vpnAppUid The uid of the VPN app - */ - public synchronized void onVpnUidRangesRemoved(@NonNull String iface, - Set rangesToRemove, int vpnAppUid) { - // Calculate the list of app uids that are no longer under the VPN due to the removed UID - // ranges and update Netd about them. - final Set changedUids = intersectUids(rangesToRemove, mAllApps); - removeBypassingUids(changedUids, vpnAppUid); - updateVpnUids(iface, changedUids, false); - Set existingRanges = mVpnUidRanges.getOrDefault(iface, null); - if (existingRanges == null) { - loge("Attempt to remove unknown vpn uid Range iface = " + iface); - return; - } - existingRanges.removeAll(rangesToRemove); - if (existingRanges.size() == 0) { - mVpnUidRanges.remove(iface); - } - } - - /** - * Compute the intersection of a set of UidRanges and appIds. Returns a set of uids - * that satisfies: - * 1. falls into one of the UidRange - * 2. matches one of the appIds - */ - private Set intersectUids(Set ranges, Set appIds) { - Set result = new HashSet<>(); - for (UidRange range : ranges) { - for (int userId = range.getStartUser(); userId <= range.getEndUser(); userId++) { - for (int appId : appIds) { - final int uid = UserHandle.getUid(userId, appId); - if (range.contains(uid)) { - result.add(uid); - } - } - } - } - return result; - } - - /** - * Remove all apps which can elect to bypass the VPN from the list of uids - * - * An app can elect to bypass the VPN if it hold SYSTEM permission, or if its the active VPN - * app itself. - * - * @param uids The list of uids to operate on - * @param vpnAppUid The uid of the VPN app - */ - private void removeBypassingUids(Set uids, int vpnAppUid) { - uids.remove(vpnAppUid); - uids.removeIf(uid -> mApps.getOrDefault(uid, NETWORK) == SYSTEM); - } - - /** - * Update netd about the list of uids that are under an active VPN connection which they cannot - * bypass. - * - * This is to instruct netd to set up appropriate filtering rules for these uids, such that they - * can only receive ingress packets from the VPN's tunnel interface (and loopback). - * - * @param iface the interface name of the active VPN connection - * @param add {@code true} if the uids are to be added to the interface, {@code false} if they - * are to be removed from the interface. - */ - private void updateVpnUids(String iface, Set uids, boolean add) { - if (uids.size() == 0) { - return; - } - try { - if (add) { - mNetd.firewallAddUidInterfaceRules(iface, toIntArray(uids)); - } else { - mNetd.firewallRemoveUidInterfaceRules(toIntArray(uids)); - } - } catch (ServiceSpecificException e) { - // Silently ignore exception when device does not support eBPF, otherwise just log - // the exception and do not crash - if (e.errorCode != OsConstants.EOPNOTSUPP) { - loge("Exception when updating permissions: ", e); - } - } catch (RemoteException e) { - loge("Exception when updating permissions: ", e); - } - } - - /** * Called by PackageListObserver when a package is installed/uninstalled. Send the updated * permission information to netd. * @@ -688,24 +528,6 @@ public class PermissionMonitor { } } - /** Should only be used by unit tests */ - @VisibleForTesting - public Set getVpnUidRanges(String iface) { - return mVpnUidRanges.get(iface); - } - - /** Dump info to dumpsys */ - public void dump(IndentingPrintWriter pw) { - pw.println("Interface filtering rules:"); - pw.increaseIndent(); - for (Map.Entry> vpn : mVpnUidRanges.entrySet()) { - pw.println("Interface: " + vpn.getKey()); - pw.println("UIDs: " + vpn.getValue().toString()); - pw.println(); - } - pw.decreaseIndent(); - } - private static void log(String s) { if (DBG) { Log.d(TAG, s); diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 0271d3bcb57c..8005dda3faf9 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -1604,7 +1604,12 @@ public class Vpn { if (mNetworkInfo.isConnected()) { return !appliesToUid(uid); } else { - return UidRange.containsUid(mBlockedUsers, uid); + for (UidRange uidRange : mBlockedUsers) { + if (uidRange.contains(uid)) { + return true; + } + } + return false; } } diff --git a/tests/net/java/android/net/RouteInfoTest.java b/tests/net/java/android/net/RouteInfoTest.java index 2edbd403b5b5..831fefd283db 100644 --- a/tests/net/java/android/net/RouteInfoTest.java +++ b/tests/net/java/android/net/RouteInfoTest.java @@ -16,16 +16,15 @@ package android.net; -import static android.net.RouteInfo.RTN_UNREACHABLE; +import java.lang.reflect.Method; +import java.net.InetAddress; +import android.net.IpPrefix; +import android.net.RouteInfo; import android.os.Parcel; -import android.test.suitebuilder.annotation.SmallTest; import junit.framework.TestCase; - -import java.net.Inet4Address; -import java.net.Inet6Address; -import java.net.InetAddress; +import android.test.suitebuilder.annotation.SmallTest; public class RouteInfoTest extends TestCase { @@ -153,85 +152,67 @@ public class RouteInfoTest extends TestCase { } public void testHostAndDefaultRoutes() { - RouteInfo r; - - r = new RouteInfo(Prefix("0.0.0.0/0"), Address("0.0.0.0"), "wlan0"); - assertFalse(r.isHostRoute()); - assertTrue(r.isDefaultRoute()); - assertTrue(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("::/0"), Address("::"), "wlan0"); - assertFalse(r.isHostRoute()); - assertTrue(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertTrue(r.isIPv6Default()); + RouteInfo r; - r = new RouteInfo(Prefix("192.0.2.0/24"), null, "wlan0"); - assertFalse(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("2001:db8::/48"), null, "wlan0"); - assertFalse(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("192.0.2.0/32"), Address("0.0.0.0"), "wlan0"); - assertTrue(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("2001:db8::/128"), Address("::"), "wlan0"); - assertTrue(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("192.0.2.0/32"), null, "wlan0"); - assertTrue(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("2001:db8::/128"), null, "wlan0"); - assertTrue(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("::/128"), Address("fe80::"), "wlan0"); - assertTrue(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("0.0.0.0/32"), Address("192.0.2.1"), "wlan0"); - assertTrue(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(Prefix("0.0.0.0/32"), Address("192.0.2.1"), "wlan0"); - assertTrue(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), RTN_UNREACHABLE); - assertFalse(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); - - r = new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE); - assertFalse(r.isHostRoute()); - assertFalse(r.isDefaultRoute()); - assertFalse(r.isIPv4Default()); - assertFalse(r.isIPv6Default()); + r = new RouteInfo(Prefix("0.0.0.0/0"), Address("0.0.0.0"), "wlan0"); + assertFalse(r.isHostRoute()); + assertTrue(r.isDefaultRoute()); + assertTrue(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("::/0"), Address("::"), "wlan0"); + assertFalse(r.isHostRoute()); + assertTrue(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertTrue(r.isIPv6Default()); + + r = new RouteInfo(Prefix("192.0.2.0/24"), null, "wlan0"); + assertFalse(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("2001:db8::/48"), null, "wlan0"); + assertFalse(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("192.0.2.0/32"), Address("0.0.0.0"), "wlan0"); + assertTrue(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("2001:db8::/128"), Address("::"), "wlan0"); + assertTrue(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("192.0.2.0/32"), null, "wlan0"); + assertTrue(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("2001:db8::/128"), null, "wlan0"); + assertTrue(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("::/128"), Address("fe80::"), "wlan0"); + assertTrue(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); + + r = new RouteInfo(Prefix("0.0.0.0/32"), Address("192.0.2.1"), "wlan0"); + assertTrue(r.isHostRoute()); + assertFalse(r.isDefaultRoute()); + assertFalse(r.isIPv4Default()); + assertFalse(r.isIPv6Default()); } public void testTruncation() { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index ee9d78537fe4..c2fc0b311757 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -16,8 +16,6 @@ package com.android.server; -import static android.content.pm.PackageManager.GET_PERMISSIONS; -import static android.content.pm.PackageManager.MATCH_ANY_USER; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.NETID_UNSET; import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OFF; @@ -62,13 +60,11 @@ 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.RouteInfo.RTN_UNREACHABLE; import static com.android.internal.util.TestUtils.waitForIdleHandler; import static com.android.internal.util.TestUtils.waitForIdleLooper; import static com.android.internal.util.TestUtils.waitForIdleSerialExecutor; -import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -76,14 +72,12 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.eq; -import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; @@ -103,10 +97,6 @@ import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.content.pm.ApplicationInfo; -import android.content.pm.PackageInfo; -import android.content.pm.PackageManager; -import android.content.pm.UserInfo; import android.content.res.Resources; import android.net.ConnectivityManager; import android.net.ConnectivityManager.NetworkCallback; @@ -161,7 +151,6 @@ import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; import android.os.UserHandle; -import android.os.UserManager; import android.provider.Settings; import android.system.Os; import android.test.mock.MockContentResolver; @@ -197,7 +186,6 @@ import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.Spy; @@ -206,7 +194,6 @@ import org.mockito.stubbing.Answer; import java.io.IOException; import java.net.DatagramSocket; import java.net.Inet4Address; -import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Socket; @@ -274,8 +261,6 @@ public class ConnectivityServiceTest { @Mock IDnsResolver mMockDnsResolver; @Mock INetd mMockNetd; @Mock NetworkStackClient mNetworkStack; - @Mock PackageManager mPackageManager; - @Mock UserManager mUserManager; private ArgumentCaptor mStringArrayCaptor = ArgumentCaptor.forClass(String[].class); @@ -346,7 +331,6 @@ public class ConnectivityServiceTest { if (Context.CONNECTIVITY_SERVICE.equals(name)) return mCm; if (Context.NOTIFICATION_SERVICE.equals(name)) return mock(NotificationManager.class); if (Context.NETWORK_STACK_SERVICE.equals(name)) return mNetworkStack; - if (Context.USER_SERVICE.equals(name)) return mUserManager; return super.getSystemService(name); } @@ -359,11 +343,6 @@ public class ConnectivityServiceTest { public Resources getResources() { return mResources; } - - @Override - public PackageManager getPackageManager() { - return mPackageManager; - } } public void waitForIdle(int timeoutMsAsInt) { @@ -1078,7 +1057,7 @@ public class ConnectivityServiceTest { public WrappedConnectivityService(Context context, INetworkManagementService netManager, INetworkStatsService statsService, INetworkPolicyManager policyManager, IpConnectivityLog log, INetd netd, IDnsResolver dnsResolver) { - super(context, netManager, statsService, policyManager, dnsResolver, log, netd); + super(context, netManager, statsService, policyManager, dnsResolver, log); mNetd = netd; mLingerDelayMs = TEST_LINGER_DELAY_MS; } @@ -1217,11 +1196,6 @@ public class ConnectivityServiceTest { fail("ConditionVariable was blocked for more than " + TIMEOUT_MS + "ms"); } - private static final int VPN_USER = 0; - private static final int APP1_UID = UserHandle.getUid(VPN_USER, 10100); - private static final int APP2_UID = UserHandle.getUid(VPN_USER, 10101); - private static final int VPN_UID = UserHandle.getUid(VPN_USER, 10043); - @Before public void setUp() throws Exception { mContext = InstrumentationRegistry.getContext(); @@ -1229,17 +1203,6 @@ public class ConnectivityServiceTest { MockitoAnnotations.initMocks(this); when(mMetricsService.defaultNetworkMetrics()).thenReturn(mDefaultNetworkMetrics); - when(mUserManager.getUsers(eq(true))).thenReturn( - Arrays.asList(new UserInfo[] { - new UserInfo(VPN_USER, "", 0), - })); - when(mPackageManager.getInstalledPackages(eq(GET_PERMISSIONS | MATCH_ANY_USER))).thenReturn( - Arrays.asList(new PackageInfo[] { - buildPackageInfo(/* SYSTEM */ false, APP1_UID), - buildPackageInfo(/* SYSTEM */ false, APP2_UID), - buildPackageInfo(/* SYSTEM */ false, VPN_UID) - })); - // InstrumentationTestRunner prepares a looper, but AndroidJUnitRunner does not. // http://b/25897652 . if (Looper.myLooper() == null) { @@ -6166,165 +6129,4 @@ public class ConnectivityServiceTest { assertEquals(testProxyInfo, mService.getProxyForNetwork(mWiFiNetworkAgent.getNetwork())); assertEquals(testProxyInfo, mService.getProxyForNetwork(null)); } - - @Test - public void testFullyRoutedVpnResultsInInterfaceFilteringRules() throws Exception { - LinkProperties lp = new LinkProperties(); - lp.setInterfaceName("tun0"); - lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); - // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(UidRange.createForUser(VPN_USER)); - final MockNetworkAgent vpnNetworkAgent = establishVpn(lp, VPN_UID, vpnRange); - - // Connected VPN should have interface rules set up. There are two expected invocations, - // one during VPN uid update, one during VPN LinkProperties update - ArgumentCaptor uidCaptor = ArgumentCaptor.forClass(int[].class); - verify(mMockNetd, times(2)).firewallAddUidInterfaceRules(eq("tun0"), uidCaptor.capture()); - assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID); - assertContainsExactly(uidCaptor.getAllValues().get(1), APP1_UID, APP2_UID); - assertTrue(mService.mPermissionMonitor.getVpnUidRanges("tun0").equals(vpnRange)); - - vpnNetworkAgent.disconnect(); - waitForIdle(); - - // Disconnected VPN should have interface rules removed - verify(mMockNetd).firewallRemoveUidInterfaceRules(uidCaptor.capture()); - assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); - assertNull(mService.mPermissionMonitor.getVpnUidRanges("tun0")); - } - - @Test - public void testLegacyVpnDoesNotResultInInterfaceFilteringRule() throws Exception { - LinkProperties lp = new LinkProperties(); - lp.setInterfaceName("tun0"); - lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); - // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(UidRange.createForUser(VPN_USER)); - final MockNetworkAgent vpnNetworkAgent = establishVpn(lp, Process.SYSTEM_UID, vpnRange); - - // Legacy VPN should not have interface rules set up - verify(mMockNetd, never()).firewallAddUidInterfaceRules(any(), any()); - } - - public void testLocalIpv4OnlyVpnDoesNotResultInInterfaceFilteringRule() - throws Exception { - LinkProperties lp = new LinkProperties(); - lp.setInterfaceName("tun0"); - lp.addRoute(new RouteInfo(new IpPrefix("192.0.2.0/24"), null, "tun0")); - lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE)); - // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(UidRange.createForUser(VPN_USER)); - final MockNetworkAgent vpnNetworkAgent = establishVpn(lp, Process.SYSTEM_UID, vpnRange); - - // IPv6 unreachable route should not be misinterpreted as a default route - verify(mMockNetd, never()).firewallAddUidInterfaceRules(any(), any()); - } - - @Test - public void testVpnHandoverChangesInterfaceFilteringRule() throws Exception { - LinkProperties lp = new LinkProperties(); - lp.setInterfaceName("tun0"); - lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); - // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(UidRange.createForUser(VPN_USER)); - final MockNetworkAgent vpnNetworkAgent = establishVpn(lp, VPN_UID, vpnRange); - - // Connected VPN should have interface rules set up. There are two expected invocations, - // one during VPN uid update, one during VPN LinkProperties update - ArgumentCaptor uidCaptor = ArgumentCaptor.forClass(int[].class); - verify(mMockNetd, times(2)).firewallAddUidInterfaceRules(eq("tun0"), uidCaptor.capture()); - assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID); - assertContainsExactly(uidCaptor.getAllValues().get(1), APP1_UID, APP2_UID); - - reset(mMockNetd); - InOrder inOrder = inOrder(mMockNetd); - lp.setInterfaceName("tun1"); - vpnNetworkAgent.sendLinkProperties(lp); - waitForIdle(); - // VPN handover (switch to a new interface) should result in rules being updated (old rules - // removed first, then new rules added) - inOrder.verify(mMockNetd).firewallRemoveUidInterfaceRules(uidCaptor.capture()); - assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); - inOrder.verify(mMockNetd).firewallAddUidInterfaceRules(eq("tun1"), uidCaptor.capture()); - assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); - - reset(mMockNetd); - lp = new LinkProperties(); - lp.setInterfaceName("tun1"); - lp.addRoute(new RouteInfo(new IpPrefix("192.0.2.0/24"), null, "tun1")); - vpnNetworkAgent.sendLinkProperties(lp); - waitForIdle(); - // VPN not routing everything should no longer have interface filtering rules - verify(mMockNetd).firewallRemoveUidInterfaceRules(uidCaptor.capture()); - assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); - - reset(mMockNetd); - lp = new LinkProperties(); - lp.setInterfaceName("tun1"); - lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); - vpnNetworkAgent.sendLinkProperties(lp); - waitForIdle(); - // Back to routing all IPv6 traffic should have filtering rules - verify(mMockNetd).firewallAddUidInterfaceRules(eq("tun1"), uidCaptor.capture()); - assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); - } - - @Test - public void testUidUpdateChangesInterfaceFilteringRule() throws Exception { - LinkProperties lp = new LinkProperties(); - lp.setInterfaceName("tun0"); - lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); - // The uid range needs to cover the test app so the network is visible to it. - final UidRange vpnRange = UidRange.createForUser(VPN_USER); - final MockNetworkAgent vpnNetworkAgent = establishVpn(lp, VPN_UID, - Collections.singleton(vpnRange)); - - reset(mMockNetd); - InOrder inOrder = inOrder(mMockNetd); - - // Update to new range which is old range minus APP1, i.e. only APP2 - final Set newRanges = new HashSet<>(Arrays.asList( - new UidRange(vpnRange.start, APP1_UID - 1), - new UidRange(APP1_UID + 1, vpnRange.stop))); - vpnNetworkAgent.setUids(newRanges); - waitForIdle(); - - ArgumentCaptor uidCaptor = ArgumentCaptor.forClass(int[].class); - // Verify old rules are removed before new rules are added - inOrder.verify(mMockNetd).firewallRemoveUidInterfaceRules(uidCaptor.capture()); - assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); - inOrder.verify(mMockNetd).firewallAddUidInterfaceRules(eq("tun0"), uidCaptor.capture()); - assertContainsExactly(uidCaptor.getValue(), APP2_UID); - } - - - private MockNetworkAgent establishVpn(LinkProperties lp, int establishingUid, - Set vpnRange) { - final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN, lp); - vpnNetworkAgent.getNetworkCapabilities().setEstablishingVpnAppUid(establishingUid); - mMockVpn.setNetworkAgent(vpnNetworkAgent); - mMockVpn.connect(); - mMockVpn.setUids(vpnRange); - vpnNetworkAgent.connect(true); - waitForIdle(); - return vpnNetworkAgent; - } - - private void assertContainsExactly(int[] actual, int... expected) { - int[] sortedActual = Arrays.copyOf(actual, actual.length); - int[] sortedExpected = Arrays.copyOf(expected, expected.length); - Arrays.sort(sortedActual); - Arrays.sort(sortedExpected); - assertArrayEquals(sortedExpected, sortedActual); - } - - private static PackageInfo buildPackageInfo(boolean hasSystemPermission, int uid) { - final PackageInfo packageInfo = new PackageInfo(); - packageInfo.requestedPermissions = new String[0]; - packageInfo.applicationInfo = new ApplicationInfo(); - packageInfo.applicationInfo.privateFlags = 0; - packageInfo.applicationInfo.uid = UserHandle.getUid(UserHandle.USER_SYSTEM, - UserHandle.getAppId(uid)); - return packageInfo; - } } diff --git a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java index 62a471896579..106cd1fba869 100644 --- a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java +++ b/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java @@ -28,7 +28,6 @@ import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_PRODUCT; import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_VENDOR; import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED; import static android.content.pm.PackageManager.GET_PERMISSIONS; -import static android.content.pm.PackageManager.MATCH_ANY_USER; import static android.os.Process.SYSTEM_UID; import static com.android.server.connectivity.PermissionMonitor.NETWORK; @@ -37,16 +36,13 @@ import static com.android.server.connectivity.PermissionMonitor.SYSTEM; import static junit.framework.Assert.fail; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -57,12 +53,10 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageList; import android.content.pm.PackageManager; import android.content.pm.PackageManagerInternal; -import android.content.pm.UserInfo; import android.net.INetd; -import android.net.UidRange; import android.os.Build; +import android.os.INetworkManagementService; import android.os.UserHandle; -import android.os.UserManager; import android.util.SparseIntArray; import androidx.test.filters.SmallTest; @@ -79,12 +73,7 @@ import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; -import java.util.Set; - @RunWith(AndroidJUnit4.class) @SmallTest @@ -95,12 +84,10 @@ public class PermissionMonitorTest { private static final int MOCK_UID2 = 10086; private static final int SYSTEM_UID1 = 1000; private static final int SYSTEM_UID2 = 1008; - private static final int VPN_UID = 10002; private static final String MOCK_PACKAGE1 = "appName1"; private static final String MOCK_PACKAGE2 = "appName2"; private static final String SYSTEM_PACKAGE1 = "sysName1"; private static final String SYSTEM_PACKAGE2 = "sysName2"; - private static final String VPN_PACKAGE = "vpnApp"; private static final String PARTITION_SYSTEM = "system"; private static final String PARTITION_OEM = "oem"; private static final String PARTITION_PRODUCT = "product"; @@ -110,9 +97,9 @@ public class PermissionMonitorTest { @Mock private Context mContext; @Mock private PackageManager mPackageManager; + @Mock private INetworkManagementService mNMS; @Mock private INetd mNetdService; @Mock private PackageManagerInternal mMockPmi; - @Mock private UserManager mUserManager; private PackageManagerInternal.PackageListObserver mObserver; private PermissionMonitor mPermissionMonitor; @@ -121,14 +108,7 @@ public class PermissionMonitorTest { public void setUp() throws Exception { MockitoAnnotations.initMocks(this); when(mContext.getPackageManager()).thenReturn(mPackageManager); - when(mContext.getSystemService(eq(Context.USER_SERVICE))).thenReturn(mUserManager); - when(mUserManager.getUsers(eq(true))).thenReturn( - Arrays.asList(new UserInfo[] { - new UserInfo(MOCK_USER1, "", 0), - new UserInfo(MOCK_USER2, "", 0), - })); - - mPermissionMonitor = spy(new PermissionMonitor(mContext, mNetdService)); + mPermissionMonitor = spy(new PermissionMonitor(mContext, mNMS, mNetdService)); LocalServices.removeServiceForTest(PackageManagerInternal.class); LocalServices.addService(PackageManagerInternal.class, mMockPmi); @@ -154,7 +134,7 @@ public class PermissionMonitorTest { return mPermissionMonitor.hasUseBackgroundNetworksPermission(uid); } - private static PackageInfo packageInfoWithPermissions(String[] permissions, String partition) { + private PackageInfo packageInfoWithPermissions(String[] permissions, String partition) { int[] requestedPermissionsFlags = new int[permissions.length]; for (int i = 0; i < permissions.length; i++) { requestedPermissionsFlags[i] = REQUESTED_PERMISSION_GRANTED; @@ -163,7 +143,7 @@ public class PermissionMonitorTest { requestedPermissionsFlags); } - private static PackageInfo packageInfoWithPermissions(String[] permissions, String partition, + private PackageInfo packageInfoWithPermissions(String[] permissions, String partition, int[] requestedPermissionsFlags) { final PackageInfo packageInfo = new PackageInfo(); packageInfo.requestedPermissions = permissions; @@ -185,18 +165,6 @@ public class PermissionMonitorTest { return packageInfo; } - private static PackageInfo buildPackageInfo(boolean hasSystemPermission, int uid, int userId) { - final PackageInfo pkgInfo; - if (hasSystemPermission) { - pkgInfo = packageInfoWithPermissions(new String[] {CHANGE_NETWORK_STATE, NETWORK_STACK}, - PARTITION_SYSTEM); - } else { - pkgInfo = packageInfoWithPermissions(new String[] {}, ""); - } - pkgInfo.applicationInfo.uid = UserHandle.getUid(userId, UserHandle.getAppId(uid)); - return pkgInfo; - } - @Test public void testHasPermission() { PackageInfo app = packageInfoWithPermissions(new String[] {}, PARTITION_SYSTEM); @@ -277,14 +245,14 @@ public class PermissionMonitorTest { assertFalse(hasBgPermission(PARTITION_VENDOR, VERSION_Q, MOCK_UID1, CHANGE_WIFI_STATE)); } - private class NetdMonitor { + private class NMSMonitor { private final HashMap mApps = new HashMap<>(); - NetdMonitor(INetd mockNetd) throws Exception { + NMSMonitor(INetworkManagementService mockNMS) throws Exception { // Add hook to verify and track result of setPermission. doAnswer((InvocationOnMock invocation) -> { final Object[] args = invocation.getArguments(); - final Boolean isSystem = args[0].equals(INetd.PERMISSION_SYSTEM); + final Boolean isSystem = args[0].equals("SYSTEM"); for (final int uid : (int[]) args[1]) { // TODO: Currently, permission monitor will send duplicate commands for each uid // corresponding to each user. Need to fix that and uncomment below test. @@ -294,7 +262,7 @@ public class PermissionMonitorTest { mApps.put(uid, isSystem); } return null; - }).when(mockNetd).networkSetPermissionForUser(anyInt(), any(int[].class)); + }).when(mockNMS).setPermission(anyString(), any(int[].class)); // Add hook to verify and track result of clearPermission. doAnswer((InvocationOnMock invocation) -> { @@ -308,7 +276,7 @@ public class PermissionMonitorTest { mApps.remove(uid); } return null; - }).when(mockNetd).networkClearPermissionForUser(any(int[].class)); + }).when(mockNMS).clearPermission(any(int[].class)); } public void expectPermission(Boolean permission, int[] users, int[] apps) { @@ -339,7 +307,7 @@ public class PermissionMonitorTest { @Test public void testUserAndPackageAddRemove() throws Exception { - final NetdMonitor mNetdMonitor = new NetdMonitor(mNetdService); + final NMSMonitor mNMSMonitor = new NMSMonitor(mNMS); // MOCK_UID1: MOCK_PACKAGE1 only has network permission. // SYSTEM_UID: SYSTEM_PACKAGE1 has system permission. @@ -355,123 +323,48 @@ public class PermissionMonitorTest { // Add SYSTEM_PACKAGE2, expect only have network permission. mPermissionMonitor.onUserAdded(MOCK_USER1); addPackageForUsers(new int[]{MOCK_USER1}, SYSTEM_PACKAGE2, SYSTEM_UID); - mNetdMonitor.expectPermission(NETWORK, new int[]{MOCK_USER1}, new int[]{SYSTEM_UID}); + mNMSMonitor.expectPermission(NETWORK, new int[]{MOCK_USER1}, new int[]{SYSTEM_UID}); // Add SYSTEM_PACKAGE1, expect permission escalate. addPackageForUsers(new int[]{MOCK_USER1}, SYSTEM_PACKAGE1, SYSTEM_UID); - mNetdMonitor.expectPermission(SYSTEM, new int[]{MOCK_USER1}, new int[]{SYSTEM_UID}); + mNMSMonitor.expectPermission(SYSTEM, new int[]{MOCK_USER1}, new int[]{SYSTEM_UID}); mPermissionMonitor.onUserAdded(MOCK_USER2); - mNetdMonitor.expectPermission(SYSTEM, new int[]{MOCK_USER1, MOCK_USER2}, + mNMSMonitor.expectPermission(SYSTEM, new int[]{MOCK_USER1, MOCK_USER2}, new int[]{SYSTEM_UID}); addPackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_UID1); - mNetdMonitor.expectPermission(SYSTEM, new int[]{MOCK_USER1, MOCK_USER2}, + mNMSMonitor.expectPermission(SYSTEM, new int[]{MOCK_USER1, MOCK_USER2}, new int[]{SYSTEM_UID}); - mNetdMonitor.expectPermission(NETWORK, new int[]{MOCK_USER1, MOCK_USER2}, + mNMSMonitor.expectPermission(NETWORK, new int[]{MOCK_USER1, MOCK_USER2}, new int[]{MOCK_UID1}); // Remove MOCK_UID1, expect no permission left for all user. mPermissionMonitor.onPackageRemoved(MOCK_UID1); removePackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, MOCK_UID1); - mNetdMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2}, new int[]{MOCK_UID1}); + mNMSMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2}, new int[]{MOCK_UID1}); // Remove SYSTEM_PACKAGE1, expect permission downgrade. when(mPackageManager.getPackagesForUid(anyInt())).thenReturn(new String[]{SYSTEM_PACKAGE2}); removePackageForUsers(new int[]{MOCK_USER1, MOCK_USER2}, SYSTEM_UID); - mNetdMonitor.expectPermission(NETWORK, new int[]{MOCK_USER1, MOCK_USER2}, + mNMSMonitor.expectPermission(NETWORK, new int[]{MOCK_USER1, MOCK_USER2}, new int[]{SYSTEM_UID}); mPermissionMonitor.onUserRemoved(MOCK_USER1); - mNetdMonitor.expectPermission(NETWORK, new int[]{MOCK_USER2}, new int[]{SYSTEM_UID}); + mNMSMonitor.expectPermission(NETWORK, new int[]{MOCK_USER2}, new int[]{SYSTEM_UID}); // Remove all packages, expect no permission left. when(mPackageManager.getPackagesForUid(anyInt())).thenReturn(new String[]{}); removePackageForUsers(new int[]{MOCK_USER2}, SYSTEM_UID); - mNetdMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2}, + mNMSMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2}, new int[]{SYSTEM_UID, MOCK_UID1}); // Remove last user, expect no redundant clearPermission is invoked. mPermissionMonitor.onUserRemoved(MOCK_USER2); - mNetdMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2}, + mNMSMonitor.expectNoPermission(new int[]{MOCK_USER1, MOCK_USER2}, new int[]{SYSTEM_UID, MOCK_UID1}); } - @Test - public void testUidFilteringDuringVpnConnectDisconnectAndUidUpdates() throws Exception { - when(mPackageManager.getInstalledPackages(eq(GET_PERMISSIONS | MATCH_ANY_USER))).thenReturn( - Arrays.asList(new PackageInfo[] { - buildPackageInfo(/* SYSTEM */ true, SYSTEM_UID1, MOCK_USER1), - buildPackageInfo(/* SYSTEM */ false, MOCK_UID1, MOCK_USER1), - buildPackageInfo(/* SYSTEM */ false, MOCK_UID2, MOCK_USER1), - buildPackageInfo(/* SYSTEM */ false, VPN_UID, MOCK_USER1) - })); - when(mPackageManager.getPackageInfo(eq(MOCK_PACKAGE1), eq(GET_PERMISSIONS))).thenReturn( - buildPackageInfo(false, MOCK_UID1, MOCK_USER1)); - mPermissionMonitor.startMonitoring(); - // Every app on user 0 except MOCK_UID2 are under VPN. - final Set vpnRange1 = new HashSet<>(Arrays.asList(new UidRange[] { - new UidRange(0, MOCK_UID2 - 1), - new UidRange(MOCK_UID2 + 1, UserHandle.PER_USER_RANGE - 1)})); - final Set vpnRange2 = Collections.singleton(new UidRange(MOCK_UID2, MOCK_UID2)); - - // When VPN is connected, expect a rule to be set up for user app MOCK_UID1 - mPermissionMonitor.onVpnUidRangesAdded("tun0", vpnRange1, VPN_UID); - verify(mNetdService).firewallAddUidInterfaceRules(eq("tun0"), - aryEq(new int[] {MOCK_UID1})); - - reset(mNetdService); - - // When MOCK_UID1 package is uninstalled and reinstalled, expect Netd to be updated - mPermissionMonitor.onPackageRemoved(UserHandle.getUid(MOCK_USER1, MOCK_UID1)); - verify(mNetdService).firewallRemoveUidInterfaceRules(aryEq(new int[] {MOCK_UID1})); - mPermissionMonitor.onPackageAdded(MOCK_PACKAGE1, UserHandle.getUid(MOCK_USER1, MOCK_UID1)); - verify(mNetdService).firewallAddUidInterfaceRules(eq("tun0"), - aryEq(new int[] {MOCK_UID1})); - - reset(mNetdService); - - // During VPN uid update (vpnRange1 -> vpnRange2), ConnectivityService first deletes the - // old UID rules then adds the new ones. Expect netd to be updated - mPermissionMonitor.onVpnUidRangesRemoved("tun0", vpnRange1, VPN_UID); - verify(mNetdService).firewallRemoveUidInterfaceRules(aryEq(new int[] {MOCK_UID1})); - mPermissionMonitor.onVpnUidRangesAdded("tun0", vpnRange2, VPN_UID); - verify(mNetdService).firewallAddUidInterfaceRules(eq("tun0"), - aryEq(new int[] {MOCK_UID2})); - - reset(mNetdService); - - // When VPN is disconnected, expect rules to be torn down - mPermissionMonitor.onVpnUidRangesRemoved("tun0", vpnRange2, VPN_UID); - verify(mNetdService).firewallRemoveUidInterfaceRules(aryEq(new int[] {MOCK_UID2})); - assertNull(mPermissionMonitor.getVpnUidRanges("tun0")); - } - - @Test - public void testUidFilteringDuringPackageInstallAndUninstall() throws Exception { - when(mPackageManager.getInstalledPackages(eq(GET_PERMISSIONS | MATCH_ANY_USER))).thenReturn( - Arrays.asList(new PackageInfo[] { - buildPackageInfo(true, SYSTEM_UID1, MOCK_USER1), - buildPackageInfo(false, VPN_UID, MOCK_USER1) - })); - when(mPackageManager.getPackageInfo(eq(MOCK_PACKAGE1), eq(GET_PERMISSIONS))).thenReturn( - buildPackageInfo(false, MOCK_UID1, MOCK_USER1)); - - mPermissionMonitor.startMonitoring(); - final Set vpnRange = Collections.singleton(UidRange.createForUser(MOCK_USER1)); - mPermissionMonitor.onVpnUidRangesAdded("tun0", vpnRange, VPN_UID); - - // Newly-installed package should have uid rules added - mPermissionMonitor.onPackageAdded(MOCK_PACKAGE1, UserHandle.getUid(MOCK_USER1, MOCK_UID1)); - verify(mNetdService).firewallAddUidInterfaceRules(eq("tun0"), - aryEq(new int[] {MOCK_UID1})); - - // Removed package should have its uid rules removed - mPermissionMonitor.onPackageRemoved(UserHandle.getUid(MOCK_USER1, MOCK_UID1)); - verify(mNetdService).firewallRemoveUidInterfaceRules(aryEq(new int[] {MOCK_UID1})); - } - - // Normal package add/remove operations will trigger multiple intent for uids corresponding to // each user. To simulate generic package operations, the onPackageAdded/Removed will need to be // called multiple times with the uid corresponding to each user. -- 2.11.0