From 42a4bb5730266f80585e67262c73505d0bfffbf8 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 7 Nov 2013 17:21:03 -0800 Subject: [PATCH] Remove incorrect (and unused) capget code. Note that if you revert this change, the code it removes is incorrect, and doesn't handle the top 32 bits of capabilities, one of which we're already using: CAP_BLOCK_SUSPEND. Bug: 11508244 Change-Id: Ice1f51334bce4941c6d24d6016450a2ebcf92886 --- .../android/internal/os/ZygoteConnection.java | 71 ++----------------- .../com/android/internal/os/ZygoteInit.java | 9 --- .../com_android_internal_os_ZygoteInit.cpp | 25 ------- 3 files changed, 5 insertions(+), 100 deletions(-) diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java index 3381959667bfe..4f3b5b3370a68 100644 --- a/core/java/com/android/internal/os/ZygoteConnection.java +++ b/core/java/com/android/internal/os/ZygoteConnection.java @@ -197,10 +197,14 @@ class ZygoteConnection { try { parsedArgs = new Arguments(args); + if (parsedArgs.permittedCapabilities != 0 || parsedArgs.effectiveCapabilities != 0) { + throw new ZygoteSecurityException("Client may not specify capabilities: " + + "permitted=0x" + Long.toHexString(parsedArgs.permittedCapabilities) + + ", effective=0x" + Long.toHexString(parsedArgs.effectiveCapabilities)); + } applyUidSecurityPolicy(parsedArgs, peer, peerSecurityContext); applyRlimitSecurityPolicy(parsedArgs, peer, peerSecurityContext); - applyCapabilitiesSecurityPolicy(parsedArgs, peer, peerSecurityContext); applyInvokeWithSecurityPolicy(parsedArgs, peer, peerSecurityContext); applyseInfoSecurityPolicy(parsedArgs, peer, peerSecurityContext); @@ -702,71 +706,6 @@ class ZygoteConnection { } } - /** - * Applies zygote security policy per bug #1042973. A root peer may - * spawn an instance with any capabilities. All other uids may spawn - * instances with any of the capabilities in the peer's permitted set - * but no more. - * - * @param args non-null; zygote spawner arguments - * @param peer non-null; peer credentials - * @throws ZygoteSecurityException - */ - private static void applyCapabilitiesSecurityPolicy( - Arguments args, Credentials peer, String peerSecurityContext) - throws ZygoteSecurityException { - - if (args.permittedCapabilities == 0 - && args.effectiveCapabilities == 0) { - // nothing to check - return; - } - - boolean allowed = SELinux.checkSELinuxAccess(peerSecurityContext, - peerSecurityContext, - "zygote", - "specifycapabilities"); - if (!allowed) { - throw new ZygoteSecurityException( - "Peer may not specify capabilities"); - } - - if (peer.getUid() == 0) { - // root may specify anything - return; - } - - long permittedCaps; - - try { - permittedCaps = ZygoteInit.capgetPermitted(peer.getPid()); - } catch (IOException ex) { - throw new ZygoteSecurityException( - "Error retrieving peer's capabilities."); - } - - /* - * Ensure that the client did not specify an effective set larger - * than the permitted set. The kernel will enforce this too, but we - * do it here to make the following check easier. - */ - if (((~args.permittedCapabilities) & args.effectiveCapabilities) != 0) { - throw new ZygoteSecurityException( - "Effective capabilities cannot be superset of " - + " permitted capabilities" ); - } - - /* - * Ensure that the new permitted (and thus the new effective) set is - * a subset of the peer process's permitted set - */ - - if (((~permittedCaps) & args.permittedCapabilities) != 0) { - throw new ZygoteSecurityException( - "Peer specified unpermitted capabilities" ); - } - } - /** * Applies zygote security policy. * Based on the credentials of the process issuing a zygote command: diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java index 4c82ce80a080e..e2715f1a51446 100644 --- a/core/java/com/android/internal/os/ZygoteInit.java +++ b/core/java/com/android/internal/os/ZygoteInit.java @@ -709,15 +709,6 @@ public class ZygoteInit { static native void setCloseOnExec(FileDescriptor fd, boolean flag) throws IOException; - /** - * Retrieves the permitted capability set from another process. - * - * @param pid >=0 process ID or 0 for this process - * @throws IOException on error - */ - static native long capgetPermitted(int pid) - throws IOException; - /** * Invokes select() on the provider array of file descriptors (selecting * for readability only). Array elements of null are ignored. diff --git a/core/jni/com_android_internal_os_ZygoteInit.cpp b/core/jni/com_android_internal_os_ZygoteInit.cpp index 44452f0010909..2233ee3e012b2 100644 --- a/core/jni/com_android_internal_os_ZygoteInit.cpp +++ b/core/jni/com_android_internal_os_ZygoteInit.cpp @@ -159,29 +159,6 @@ static void com_android_internal_os_ZygoteInit_setCloseOnExec (JNIEnv *env, } } -static jlong com_android_internal_os_ZygoteInit_capgetPermitted (JNIEnv *env, - jobject clazz, jint pid) -{ - struct __user_cap_header_struct capheader; - struct __user_cap_data_struct capdata; - int err; - - memset (&capheader, 0, sizeof(capheader)); - memset (&capdata, 0, sizeof(capdata)); - - capheader.version = _LINUX_CAPABILITY_VERSION; - capheader.pid = pid; - - err = capget (&capheader, &capdata); - - if (err < 0) { - jniThrowIOException(env, errno); - return 0; - } - - return (jlong) capdata.permitted; -} - static jint com_android_internal_os_ZygoteInit_selectReadable ( JNIEnv *env, jobject clazz, jobjectArray fds) { @@ -274,8 +251,6 @@ static JNINativeMethod gMethods[] = { (void *) com_android_internal_os_ZygoteInit_reopenStdio}, { "setCloseOnExec", "(Ljava/io/FileDescriptor;Z)V", (void *) com_android_internal_os_ZygoteInit_setCloseOnExec}, - { "capgetPermitted", "(I)J", - (void *) com_android_internal_os_ZygoteInit_capgetPermitted }, { "selectReadable", "([Ljava/io/FileDescriptor;)I", (void *) com_android_internal_os_ZygoteInit_selectReadable }, { "createFileDescriptor", "(I)Ljava/io/FileDescriptor;",