From d81932b8367ef3aa8f9e8ea5ff83b956d8eb1e12 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 23 Mar 2021 21:01:07 +0900 Subject: [PATCH] Re-implement NetworkUtils#queryUserAccess. Currently, queryUserAccess talks to netd via FwmarkServer. Doing this from the module would require exposing queryUserAccess as an NDK API or reimplementing FwmarkClient. Because queryUserAccess really only uses information that comes from ConnectivityService/PermissionMonitor anyway, just use that information without calling to net. Test: atest HostsideVpnTests Bug: 171540887 Change-Id: If855de1ea3e1fd2ed30f2795d9b4acfcf969a2dc --- framework/jni/android_net_NetworkUtils.cpp | 6 --- framework/src/android/net/NetworkUtils.java | 5 ++- .../android/server/ConnectivityService.java | 42 +++++++++++++++++-- .../connectivity/PermissionMonitor.java | 7 ++++ .../server/ConnectivityServiceTest.java | 2 +- 5 files changed, 51 insertions(+), 11 deletions(-) diff --git a/framework/jni/android_net_NetworkUtils.cpp b/framework/jni/android_net_NetworkUtils.cpp index c7c0beee5b..fd4d9db23d 100644 --- a/framework/jni/android_net_NetworkUtils.cpp +++ b/framework/jni/android_net_NetworkUtils.cpp @@ -123,11 +123,6 @@ static jint android_net_utils_bindSocketToNetwork(JNIEnv *env, jobject thiz, job return setNetworkForSocket(netId, AFileDescriptor_getFD(env, javaFd)); } -static jboolean android_net_utils_queryUserAccess(JNIEnv *env, jobject thiz, jint uid, jint netId) -{ - return (jboolean) !queryUserAccess(uid, netId); -} - static bool checkLenAndCopy(JNIEnv* env, const jbyteArray& addr, int len, void* dst) { if (env->GetArrayLength(addr) != len) { @@ -267,7 +262,6 @@ static const JNINativeMethod gNetworkUtilMethods[] = { { "getBoundNetworkHandleForProcess", "()J", (void*) android_net_utils_getBoundNetworkHandleForProcess }, { "bindProcessToNetworkForHostResolution", "(I)Z", (void*) android_net_utils_bindProcessToNetworkForHostResolution }, { "bindSocketToNetwork", "(Ljava/io/FileDescriptor;I)I", (void*) android_net_utils_bindSocketToNetwork }, - { "queryUserAccess", "(II)Z", (void*)android_net_utils_queryUserAccess }, { "attachDropAllBPFFilter", "(Ljava/io/FileDescriptor;)V", (void*) android_net_utils_attachDropAllBPFFilter }, { "detachBPFFilter", "(Ljava/io/FileDescriptor;)V", (void*) android_net_utils_detachBPFFilter }, { "getTcpRepairWindow", "(Ljava/io/FileDescriptor;)Landroid/net/TcpRepairWindow;", (void*) android_net_utils_getTcpRepairWindow }, diff --git a/framework/src/android/net/NetworkUtils.java b/framework/src/android/net/NetworkUtils.java index 16ae55f8c1..f52485933c 100644 --- a/framework/src/android/net/NetworkUtils.java +++ b/framework/src/android/net/NetworkUtils.java @@ -103,7 +103,10 @@ public class NetworkUtils { * Determine if {@code uid} can access network designated by {@code netId}. * @return {@code true} if {@code uid} can access network, {@code false} otherwise. */ - public native static boolean queryUserAccess(int uid, int netId); + public static boolean queryUserAccess(int uid, int netId) { + // TODO (b/183485986): remove this method + return false; + } /** * DNS resolver series jni method. diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index fbba189e1d..9b29001661 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1145,8 +1145,8 @@ public class ConnectivityService extends IConnectivityManager.Stub /** * @see NetworkUtils#queryUserAccess(int, int) */ - public boolean queryUserAccess(int uid, int netId) { - return NetworkUtils.queryUserAccess(uid, netId); + public boolean queryUserAccess(int uid, Network network, ConnectivityService cs) { + return cs.queryUserAccess(uid, network); } /** @@ -4839,6 +4839,42 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.networkMonitor().forceReevaluation(uid); } + // TODO: call into netd. + private boolean queryUserAccess(int uid, Network network) { + final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network); + if (nai == null) return false; + + // Any UID can use its default network. + if (nai == getDefaultNetworkForUid(uid)) return true; + + // Privileged apps can use any network. + if (mPermissionMonitor.hasRestrictedNetworksPermission(uid)) { + return true; + } + + // An unprivileged UID can use a VPN iff the VPN applies to it. + if (nai.isVPN()) { + return nai.networkCapabilities.appliesToUid(uid); + } + + // An unprivileged UID can bypass the VPN that applies to it only if it can protect its + // sockets, i.e., if it is the owner. + final NetworkAgentInfo vpn = getVpnForUid(uid); + if (vpn != null && !vpn.networkAgentConfig.allowBypass + && uid != vpn.networkCapabilities.getOwnerUid()) { + return false; + } + + // The UID's permission must be at least sufficient for the network. Since the restricted + // permission was already checked above, that just leaves background networks. + if (!nai.networkCapabilities.hasCapability(NET_CAPABILITY_FOREGROUND)) { + return mPermissionMonitor.hasUseBackgroundNetworksPermission(uid); + } + + // Unrestricted network. Anyone gets to use it. + return true; + } + /** * Returns information about the proxy a certain network is using. If given a null network, it * it will return the proxy for the bound network for the caller app or the default proxy if @@ -4859,7 +4895,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return null; } return getLinkPropertiesProxyInfo(activeNetwork); - } else if (mDeps.queryUserAccess(mDeps.getCallingUid(), network.getNetId())) { + } else if (mDeps.queryUserAccess(mDeps.getCallingUid(), network, this)) { // Don't call getLinkProperties() as it requires ACCESS_NETWORK_STATE permission, which // caller may not have. return getLinkPropertiesProxyInfo(network); diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java index 488677ac1b..37116797d8 100644 --- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java +++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java @@ -271,6 +271,13 @@ public class PermissionMonitor { return mApps.containsKey(uid); } + /** + * Returns whether the given uid has permission to use restricted networks. + */ + public synchronized boolean hasRestrictedNetworksPermission(int uid) { + return Boolean.TRUE.equals(mApps.get(uid)); + } + private void update(Set users, Map apps, boolean add) { List network = new ArrayList<>(); List system = new ArrayList<>(); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index db1334ab3f..153d7758e2 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -1556,7 +1556,7 @@ public class ConnectivityServiceTest { doReturn(mNetworkStack).when(deps).getNetworkStack(); doReturn(mSystemProperties).when(deps).getSystemProperties(); doReturn(mock(ProxyTracker.class)).when(deps).makeProxyTracker(any(), any()); - doReturn(true).when(deps).queryUserAccess(anyInt(), anyInt()); + doReturn(true).when(deps).queryUserAccess(anyInt(), any(), any()); doAnswer(inv -> { mPolicyTracker = new WrappedMultinetworkPolicyTracker( inv.getArgument(0), inv.getArgument(1), inv.getArgument(2));