From 092af055252afd667f89328ee4502d14b9c97cf7 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 Merged-In: If855de1ea3e1fd2ed30f2795d9b4acfcf969a2dc Change-Id: If855de1ea3e1fd2ed30f2795d9b4acfcf969a2dc --- core/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/core/jni/android_net_NetworkUtils.cpp b/core/jni/android_net_NetworkUtils.cpp index a781a37769..1cee8955a7 100644 --- a/core/jni/android_net_NetworkUtils.cpp +++ b/core/jni/android_net_NetworkUtils.cpp @@ -102,11 +102,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) { @@ -246,7 +241,6 @@ static const JNINativeMethod gNetworkUtilMethods[] = { { "getBoundNetworkForProcess", "()I", (void*) android_net_utils_getBoundNetworkForProcess }, { "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 c0f262815b..da3350ef62 100644 --- a/framework/src/android/net/NetworkUtils.java +++ b/framework/src/android/net/NetworkUtils.java @@ -92,7 +92,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 d254d7eccc..2fc750968e 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1143,8 +1143,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); } /** @@ -4738,6 +4738,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 @@ -4758,7 +4794,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 f489c9866e..2a44add5f7 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -1553,7 +1553,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));