Prepare setresuid()/setresgid() seccomp filter in AppZygote.
The application zygote can run untrusted user code; since it also
has the capability to change the uid/gid of the process, we need
to ensure that any changes to the uid and/or gid stay within the
range that we have allocated for this application zygote.
For application zygotes, we install the app_zygote seccomp
filter instead of the regular app filter; the only difference
between this filter and the app one is that it allows
setuid/setgid calls.
To further limit this, pass down the allocated UID range to the
Zygote itself, which in turn installs an additional seccomp
filter that restricts setuid/setgid calls to this range.
The actual calls into seccomp are commented out until the seccomp
changes are merged; to avoid catastrophe, this will leave the
regular app filter for the app_zygote, which is more restrictive
and doesn't allow setuid at all.
Bug: 111434506
Test: atest CtsSeccompHostTestCases passes
Change-Id: I112419629f5ee4774ccbf77e2b1cfa5ddcf77e73
diff --git a/core/java/android/os/AppZygote.java b/core/java/android/os/AppZygote.java
index 40cbaf7..950f381 100644
--- a/core/java/android/os/AppZygote.java
+++ b/core/java/android/os/AppZygote.java
@@ -34,8 +34,15 @@
public class AppZygote {
private static final String LOG_TAG = "AppZygote";
+ // UID of the Zygote itself
private final int mZygoteUid;
+ // First UID/GID of the range the AppZygote can setuid()/setgid() to
+ private final int mZygoteUidGidMin;
+
+ // Last UID/GID of the range the AppZygote can setuid()/setgid() to
+ private final int mZygoteUidGidMax;
+
private final Object mLock = new Object();
/**
@@ -47,9 +54,11 @@
private final ApplicationInfo mAppInfo;
- public AppZygote(ApplicationInfo appInfo, int zygoteUid) {
+ public AppZygote(ApplicationInfo appInfo, int zygoteUid, int uidGidMin, int uidGidMax) {
mAppInfo = appInfo;
mZygoteUid = zygoteUid;
+ mZygoteUidGidMin = uidGidMin;
+ mZygoteUidGidMax = uidGidMax;
}
/**
@@ -104,7 +113,9 @@
"app_zygote", // seInfo
abi, // abi
abi, // acceptedAbiList
- null); // instructionSet
+ null, // instructionSet
+ mZygoteUidGidMin,
+ mZygoteUidGidMax);
ZygoteProcess.waitForConnectionToZygote(mZygote.getPrimarySocketAddress());
// preload application code in the zygote
diff --git a/core/java/android/os/ZygoteProcess.java b/core/java/android/os/ZygoteProcess.java
index 251c5ee..99a85ea 100644
--- a/core/java/android/os/ZygoteProcess.java
+++ b/core/java/android/os/ZygoteProcess.java
@@ -809,6 +809,8 @@
* may be different from <code>abi</code> in case the children
* spawned from this Zygote only communicate using ABI-safe methods.
* @param instructionSet null-ok the instruction set to use.
+ * @param uidRangeStart The first UID in the range the child zygote may setuid()/setgid() to
+ * @param uidRangeEnd The last UID in the range the child zygote may setuid()/setgid() to
*/
public ChildZygoteProcess startChildZygote(final String processClass,
final String niceName,
@@ -817,13 +819,17 @@
String seInfo,
String abi,
String acceptedAbiList,
- String instructionSet) {
+ String instructionSet,
+ int uidRangeStart,
+ int uidRangeEnd) {
// Create an unguessable address in the global abstract namespace.
final LocalSocketAddress serverAddress = new LocalSocketAddress(
processClass + "/" + UUID.randomUUID().toString());
final String[] extraArgs = {Zygote.CHILD_ZYGOTE_SOCKET_NAME_ARG + serverAddress.getName(),
- Zygote.CHILD_ZYGOTE_ABI_LIST_ARG + acceptedAbiList};
+ Zygote.CHILD_ZYGOTE_ABI_LIST_ARG + acceptedAbiList,
+ Zygote.CHILD_ZYGOTE_UID_RANGE_START + uidRangeStart,
+ Zygote.CHILD_ZYGOTE_UID_RANGE_END + uidRangeEnd};
Process.ProcessStartResult result;
try {
diff --git a/core/java/android/webkit/WebViewZygote.java b/core/java/android/webkit/WebViewZygote.java
index 9f7aa6a..29b3b3c 100644
--- a/core/java/android/webkit/WebViewZygote.java
+++ b/core/java/android/webkit/WebViewZygote.java
@@ -160,7 +160,9 @@
"webview_zygote", // seInfo
sPackage.applicationInfo.primaryCpuAbi, // abi
TextUtils.join(",", Build.SUPPORTED_ABIS),
- null); // instructionSet
+ null, // instructionSet
+ Process.FIRST_ISOLATED_UID,
+ Process.LAST_ISOLATED_UID);
// All the work below is usually done by LoadedApk, but the zygote can't talk to
// PackageManager or construct a LoadedApk since it's single-threaded pre-fork, so
diff --git a/core/java/com/android/internal/os/ChildZygoteInit.java b/core/java/com/android/internal/os/ChildZygoteInit.java
index f90cd02..a052a3b 100644
--- a/core/java/com/android/internal/os/ChildZygoteInit.java
+++ b/core/java/com/android/internal/os/ChildZygoteInit.java
@@ -15,6 +15,7 @@
*/
package com.android.internal.os;
+import android.os.Process;
import android.system.ErrnoException;
import android.system.Os;
import android.system.OsConstants;
@@ -49,6 +50,22 @@
return null;
}
+ static int parseIntFromArg(String[] argv, String desiredArg) {
+ int value = -1;
+ for (String arg : argv) {
+ if (arg.startsWith(desiredArg)) {
+ String valueStr = arg.substring(arg.indexOf('=') + 1);
+ try {
+ value = Integer.parseInt(valueStr);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("Invalid int argument: "
+ + valueStr, e);
+ }
+ }
+ }
+ return value;
+ }
+
/**
* Starts a ZygoteServer and listens for requests
*
@@ -72,6 +89,27 @@
throw new RuntimeException("Failed to set PR_SET_NO_NEW_PRIVS", ex);
}
+ int uidGidMin = parseIntFromArg(args, Zygote.CHILD_ZYGOTE_UID_RANGE_START);
+ int uidGidMax = parseIntFromArg(args, Zygote.CHILD_ZYGOTE_UID_RANGE_END);
+ if (uidGidMin == -1 || uidGidMax == -1) {
+ throw new RuntimeException("Couldn't parse UID range start/end");
+ }
+ if (uidGidMin > uidGidMax) {
+ throw new RuntimeException("Passed in UID range is invalid, min > max.");
+ }
+
+ // Verify the UIDs are in the isolated UID range, as that's the only thing that we should
+ // be forking right now
+ if (!Process.isIsolated(uidGidMin) || !Process.isIsolated(uidGidMax)) {
+ throw new RuntimeException("Passed in UID range does not map to isolated processes.");
+ }
+
+ /**
+ * Install a seccomp filter that ensure this Zygote can only setuid()/setgid()
+ * to the passed in range.
+ */
+ Zygote.nativeInstallSeccompUidGidFilter(uidGidMin, uidGidMax);
+
final Runnable caller;
try {
server.registerServerSocketAtAbstractName(socketName);
diff --git a/core/java/com/android/internal/os/Zygote.java b/core/java/com/android/internal/os/Zygote.java
index d720c68..f5746ca 100644
--- a/core/java/com/android/internal/os/Zygote.java
+++ b/core/java/com/android/internal/os/Zygote.java
@@ -104,6 +104,20 @@
*/
public static final String CHILD_ZYGOTE_ABI_LIST_ARG = "--abi-list=";
+ /**
+ * An extraArg passed when a zygote process is forking a child-zygote, specifying the
+ * start of the UID range the children of the Zygote may setuid()/setgid() to. This
+ * will be enforced with a seccomp filter.
+ */
+ public static final String CHILD_ZYGOTE_UID_RANGE_START = "--uid-range-start=";
+
+ /**
+ * An extraArg passed when a zygote process is forking a child-zygote, specifying the
+ * end of the UID range the children of the Zygote may setuid()/setgid() to. This
+ * will be enforced with a seccomp filter.
+ */
+ public static final String CHILD_ZYGOTE_UID_RANGE_END = "--uid-range-end=";
+
private Zygote() {}
/** Called for some security initialization before any fork. */
@@ -222,6 +236,13 @@
native protected static void nativeAllowFileAcrossFork(String path);
/**
+ * Installs a seccomp filter that limits setresuid()/setresgid() to the passed-in range
+ * @param uidGidMin The smallest allowed uid/gid
+ * @param uidGidMax The largest allowed uid/gid
+ */
+ native protected static void nativeInstallSeccompUidGidFilter(int uidGidMin, int uidGidMax);
+
+ /**
* Zygote unmount storage space on initializing.
* This method is called once.
*/
diff --git a/core/jni/com_android_internal_os_Zygote.cpp b/core/jni/com_android_internal_os_Zygote.cpp
index a8e1427..0ccbd2f 100644
--- a/core/jni/com_android_internal_os_Zygote.cpp
+++ b/core/jni/com_android_internal_os_Zygote.cpp
@@ -303,7 +303,7 @@
mallopt(M_DECAY_TIME, 1);
}
-static void SetUpSeccompFilter(uid_t uid) {
+static void SetUpSeccompFilter(uid_t uid, bool is_child_zygote) {
if (!g_is_security_enforced) {
ALOGI("seccomp disabled by setenforce 0");
return;
@@ -311,7 +311,14 @@
// Apply system or app filter based on uid.
if (uid >= AID_APP_START) {
- set_app_seccomp_filter();
+ if (is_child_zygote) {
+ // set_app_zygote_seccomp_filter();
+ // TODO(b/111434506) install the filter; for now, install the app filter
+ // which is more restrictive.
+ set_app_seccomp_filter();
+ } else {
+ set_app_seccomp_filter();
+ }
} else {
set_system_seccomp_filter();
}
@@ -996,7 +1003,7 @@
// alternative is to call prctl(PR_SET_NO_NEW_PRIVS, 1) afterward, but that
// breaks SELinux domain transition (see b/71859146). As the result,
// privileged syscalls used below still need to be accessible in app process.
- SetUpSeccompFilter(uid);
+ SetUpSeccompFilter(uid, is_child_zygote);
if (setresuid(uid, uid, uid) == -1) {
fail_fn(CREATE_ERROR("setresuid(%d) failed: %s", uid, strerror(errno)));
@@ -1295,6 +1302,23 @@
UnmountTree("/storage");
}
+static void com_android_internal_os_Zygote_nativeInstallSeccompUidGidFilter(
+ JNIEnv* env, jclass, jint uidGidMin, jint uidGidMax) {
+ if (!g_is_security_enforced) {
+ ALOGI("seccomp disabled by setenforce 0");
+ return;
+ }
+
+ // TODO(b/111434506) install the filter
+
+ /*
+ bool installed = install_setuidgid_seccomp_filter(uidGidMin, uidGidMax);
+ if (!installed) {
+ RuntimeAbort(env, __LINE__, "Could not install setuid/setgid seccomp filter.");
+ }
+ */
+}
+
static const JNINativeMethod gMethods[] = {
{ "nativeSecurityInit", "()V",
(void *) com_android_internal_os_Zygote_nativeSecurityInit },
@@ -1308,7 +1332,9 @@
{ "nativeUnmountStorageOnInit", "()V",
(void *) com_android_internal_os_Zygote_nativeUnmountStorageOnInit },
{ "nativePreApplicationInit", "()V",
- (void *) com_android_internal_os_Zygote_nativePreApplicationInit }
+ (void *) com_android_internal_os_Zygote_nativePreApplicationInit },
+ { "nativeInstallSeccompUidGidFilter", "(II)V",
+ (void *) com_android_internal_os_Zygote_nativeInstallSeccompUidGidFilter }
};
int register_com_android_internal_os_Zygote(JNIEnv* env) {
diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java
index 5bc8845..c1be387 100644
--- a/services/core/java/com/android/server/am/ProcessList.java
+++ b/services/core/java/com/android/server/am/ProcessList.java
@@ -1678,12 +1678,14 @@
AppZygote appZygote = mAppZygotes.get(app.info.processName, app.info.uid);
final ArrayList<ProcessRecord> zygoteProcessList;
if (appZygote == null) {
- final int userId = UserHandle.getUserId(app.info.uid);
final IsolatedUidRange uidRange =
mAppIsolatedUidRangeAllocator.getIsolatedUidRangeLocked(app.info);
- // Allocate an isolated UID out of this range for the Zygote itself
- final int zygoteIsolatedUid = uidRange.allocateIsolatedUidLocked(userId);
- appZygote = new AppZygote(app.info, zygoteIsolatedUid);
+ final int userId = UserHandle.getUserId(app.info.uid);
+ // Create the app-zygote and provide it with the UID-range it's allowed
+ // to setresuid/setresgid to.
+ final int firstUid = UserHandle.getUid(userId, uidRange.mFirstUid);
+ final int lastUid = UserHandle.getUid(userId, uidRange.mLastUid);
+ appZygote = new AppZygote(app.info, app.info.uid, firstUid, lastUid);
mAppZygotes.put(app.info.processName, app.info.uid, appZygote);
zygoteProcessList = new ArrayList<ProcessRecord>();
mAppZygoteProcesses.put(appZygote, zygoteProcessList);