From 5a42a4132fe57b344c68065d079d7abc29a6418d Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Tue, 16 Jun 2020 19:10:02 +0800 Subject: [PATCH 1/2] 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/jni/android_net_NetUtils.cpp | 8 +++ .../java/android/net/NetworkUtilsTest.java | 60 +++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/core/java/android/net/NetworkUtils.java b/core/java/android/net/NetworkUtils.java index 779f7bc91e..0b92b95128 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/jni/android_net_NetUtils.cpp b/core/jni/android_net_NetUtils.cpp index 03b9793ccb..d4805acb06 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 7748288aeb..3158cc8637 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(); + } } From 51221efa456559189a1b226865221a33e5787b1a Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Tue, 16 Jun 2020 08:13:28 +0800 Subject: [PATCH 2/2] Move DnsPacket to libs net This class might be used by some mainline modules. Bug: 151052811 Test: atest DnsPacketTest Test: atest DnsResolverTest Change-Id: I8841d91456952ded5efbf8ea221289aecc7746ad --- core/java/android/net/DnsResolver.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/core/java/android/net/DnsResolver.java b/core/java/android/net/DnsResolver.java index 0b1a84534e..3f7660f570 100644 --- a/core/java/android/net/DnsResolver.java +++ b/core/java/android/net/DnsResolver.java @@ -38,6 +38,8 @@ import android.os.MessageQueue; import android.system.ErrnoException; import android.util.Log; +import com.android.net.module.util.DnsPacket; + import java.io.FileDescriptor; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -97,7 +99,7 @@ public final class DnsResolver { @interface DnsError {} /** * Indicates that there was an error parsing the response the query. - * The cause of this error is available via getCause() and is a ParseException. + * The cause of this error is available via getCause() and is a {@link ParseException}. */ public static final int ERROR_PARSE = 0; /** @@ -290,8 +292,15 @@ public final class DnsResolver { } try { mAllAnswers.addAll(new DnsAddressAnswer(answer).getAddresses()); - } catch (ParseException e) { - mDnsException = new DnsException(ERROR_PARSE, e); + } catch (DnsPacket.ParseException e) { + // Convert the com.android.net.module.util.DnsPacket.ParseException to an + // android.net.ParseException. This is the type that was used in Q and is implied + // by the public documentation of ERROR_PARSE. + // + // DnsPacket cannot throw android.net.ParseException directly because it's @hide. + ParseException pe = new ParseException(e.reason, e.getCause()); + pe.setStackTrace(e.getStackTrace()); + mDnsException = new DnsException(ERROR_PARSE, pe); } maybeReportAnswer(); }