From 6f214e8ebee4f1665de3f2b586a22e7399482b3c Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Tue, 16 Jun 2020 19:10:02 +0800 Subject: [PATCH] Disable sockets and DNS if process lacks INTERNET permission. This is a Client-only solution. - Add to NetdClient a per-process std::atomic_boolean similar to netIdForProcess and netIdForResolv. - The boolean says whether the process should be allowed Internet connectivity. - Add an @hide method to NetUtils.java to set the boolean; call it from the initialization code of the new process just after forking from zygote. - Make netdClientSocket and dnsOpenProxy check the boolean. If the boolean is false, return EPERM from socket calls. Bug: 150028556 Test: atest NetworkUtilsTest Test: atest CtsAppSecurityHostTestCases:UseProcessTest Change-Id: If002280fbad493dfc2db3d9d505c0257d49a9056 Exempt-From-Owner-Approval: OWNERS already approved identical patchset 5 --- core/java/android/net/NetworkUtils.java | 8 +++ core/java/android/os/Process.java | 7 +++ core/java/com/android/internal/os/Zygote.java | 13 ++++ core/jni/android_net_NetUtils.cpp | 8 +++ .../java/android/net/NetworkUtilsTest.java | 60 +++++++++++++++++++ 5 files changed, 96 insertions(+) diff --git a/core/java/android/net/NetworkUtils.java b/core/java/android/net/NetworkUtils.java index 779f7bc91e8f4..0b92b95128d34 100644 --- a/core/java/android/net/NetworkUtils.java +++ b/core/java/android/net/NetworkUtils.java @@ -154,6 +154,14 @@ public class NetworkUtils { */ public static native Network getDnsNetwork() throws ErrnoException; + /** + * Allow/Disallow creating AF_INET/AF_INET6 sockets and DNS lookups for current process. + * + * @param allowNetworking whether to allow or disallow creating AF_INET/AF_INET6 sockets + * and DNS lookups. + */ + public static native void setAllowNetworkingForProcess(boolean allowNetworking); + /** * Get the tcp repair window associated with the {@code fd}. * diff --git a/core/java/android/os/Process.java b/core/java/android/os/Process.java index 5d2c9d18c00c2..a4077fbee892e 100644 --- a/core/java/android/os/Process.java +++ b/core/java/android/os/Process.java @@ -228,6 +228,13 @@ public class Process { */ public static final int EXT_OBB_RW_GID = 1079; + /** + * GID that corresponds to the INTERNET permission. + * Must match the value of AID_INET. + * @hide + */ + public static final int INET_GID = 3003; + /** {@hide} */ public static final int NOBODY_UID = 9999; diff --git a/core/java/com/android/internal/os/Zygote.java b/core/java/com/android/internal/os/Zygote.java index 505a05eb9c234..a7d9827855a21 100644 --- a/core/java/com/android/internal/os/Zygote.java +++ b/core/java/com/android/internal/os/Zygote.java @@ -24,6 +24,7 @@ import android.content.pm.ApplicationInfo; import android.net.Credentials; import android.net.LocalServerSocket; import android.net.LocalSocket; +import android.net.NetworkUtils; import android.os.FactoryTest; import android.os.IVold; import android.os.Process; @@ -286,6 +287,13 @@ public final class Zygote { private Zygote() {} + private static boolean containsInetGid(int[] gids) { + for (int i = 0; i < gids.length; i++) { + if (gids[i] == android.os.Process.INET_GID) return true; + } + return false; + } + /** * Forks a new VM instance. The current VM must have been started * with the -Xzygote flag. NOTE: new instance keeps all @@ -341,6 +349,11 @@ public final class Zygote { if (pid == 0) { // Note that this event ends at the end of handleChildProc, Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "PostFork"); + + // If no GIDs were specified, don't make any permissions changes based on groups. + if (gids != null && gids.length > 0) { + NetworkUtils.setAllowNetworkingForProcess(containsInetGid(gids)); + } } // Set the Java Language thread priority to the default value for new apps. diff --git a/core/jni/android_net_NetUtils.cpp b/core/jni/android_net_NetUtils.cpp index 03b9793ccba8b..d4805acb06d08 100644 --- a/core/jni/android_net_NetUtils.cpp +++ b/core/jni/android_net_NetUtils.cpp @@ -226,6 +226,11 @@ static jobject android_net_utils_getDnsNetwork(JNIEnv *env, jobject thiz) { class_Network, ctor, dnsNetId & ~NETID_USE_LOCAL_NAMESERVERS, privateDnsBypass); } +static void android_net_utils_setAllowNetworkingForProcess(JNIEnv *env, jobject thiz, + jboolean hasConnectivity) { + setAllowNetworkingForProcess(hasConnectivity == JNI_TRUE); +} + static jobject android_net_utils_getTcpRepairWindow(JNIEnv *env, jobject thiz, jobject javaFd) { if (javaFd == NULL) { jniThrowNullPointerException(env, NULL); @@ -266,6 +271,7 @@ static jobject android_net_utils_getTcpRepairWindow(JNIEnv *env, jobject thiz, j /* * JNI registration. */ +// clang-format off static const JNINativeMethod gNetworkUtilMethods[] = { /* name, signature, funcPtr */ { "bindProcessToNetwork", "(I)Z", (void*) android_net_utils_bindProcessToNetwork }, @@ -282,7 +288,9 @@ static const JNINativeMethod gNetworkUtilMethods[] = { { "resNetworkResult", "(Ljava/io/FileDescriptor;)Landroid/net/DnsResolver$DnsResponse;", (void*) android_net_utils_resNetworkResult }, { "resNetworkCancel", "(Ljava/io/FileDescriptor;)V", (void*) android_net_utils_resNetworkCancel }, { "getDnsNetwork", "()Landroid/net/Network;", (void*) android_net_utils_getDnsNetwork }, + { "setAllowNetworkingForProcess", "(Z)V", (void *)android_net_utils_setAllowNetworkingForProcess }, }; +// clang-format on int register_android_net_NetworkUtils(JNIEnv* env) { diff --git a/tests/net/java/android/net/NetworkUtilsTest.java b/tests/net/java/android/net/NetworkUtilsTest.java index 7748288aeb050..3158cc8637e4a 100644 --- a/tests/net/java/android/net/NetworkUtilsTest.java +++ b/tests/net/java/android/net/NetworkUtilsTest.java @@ -16,10 +16,24 @@ package android.net; +import static android.system.OsConstants.AF_INET; +import static android.system.OsConstants.AF_INET6; +import static android.system.OsConstants.AF_UNIX; +import static android.system.OsConstants.EPERM; +import static android.system.OsConstants.SOCK_DGRAM; +import static android.system.OsConstants.SOCK_STREAM; + import static junit.framework.Assert.assertEquals; +import static org.junit.Assert.fail; + +import android.system.ErrnoException; +import android.system.Os; + import androidx.test.runner.AndroidJUnit4; +import libcore.io.IoUtils; + import org.junit.Test; import org.junit.runner.RunWith; @@ -125,4 +139,50 @@ public class NetworkUtilsTest { assertEquals(BigInteger.valueOf(7l - 4 + 4 + 16 + 65536), NetworkUtils.routedIPv6AddressCount(set)); } + + private static void expectSocketSuccess(String msg, int domain, int type) { + try { + IoUtils.closeQuietly(Os.socket(domain, type, 0)); + } catch (ErrnoException e) { + fail(msg + e.getMessage()); + } + } + + private static void expectSocketPemissionError(String msg, int domain, int type) { + try { + IoUtils.closeQuietly(Os.socket(domain, type, 0)); + fail(msg); + } catch (ErrnoException e) { + assertEquals(msg, e.errno, EPERM); + } + } + + private static void expectHasNetworking() { + expectSocketSuccess("Creating a UNIX socket should not have thrown ErrnoException", + AF_UNIX, SOCK_STREAM); + expectSocketSuccess("Creating a AF_INET socket shouldn't have thrown ErrnoException", + AF_INET, SOCK_DGRAM); + expectSocketSuccess("Creating a AF_INET6 socket shouldn't have thrown ErrnoException", + AF_INET6, SOCK_DGRAM); + } + + private static void expectNoNetworking() { + expectSocketSuccess("Creating a UNIX socket should not have thrown ErrnoException", + AF_UNIX, SOCK_STREAM); + expectSocketPemissionError( + "Creating a AF_INET socket should have thrown ErrnoException(EPERM)", + AF_INET, SOCK_DGRAM); + expectSocketPemissionError( + "Creating a AF_INET6 socket should have thrown ErrnoException(EPERM)", + AF_INET6, SOCK_DGRAM); + } + + @Test + public void testSetAllowNetworkingForProcess() { + expectHasNetworking(); + NetworkUtils.setAllowNetworkingForProcess(false); + expectNoNetworking(); + NetworkUtils.setAllowNetworkingForProcess(true); + expectHasNetworking(); + } }