From 2685c77c34c29f86b5a7053fdb98e4f146e1b33b Mon Sep 17 00:00:00 2001 From: Luis Hector Chavez Date: Wed, 4 Apr 2018 15:59:59 -0700 Subject: [PATCH] adb: Correctly drop caps when ambient capabilities are used This change splits the capability-dropping step of adbd into two. This is more robust when ambient capabilities are being used, since minijail cannot currently handle that case. Bug: 77146512 Test: grep Cap /proc/`pidof adbd`/status CapInh: 0000000000000000 CapPrm: 0000000000000000 CapEff: 0000000000000000 CapBnd: 00000000000000c0 CapAmb: 0000000000000000 Change-Id: I0476a8d80f7a2497600196932542045f3bc87537 Merged-In: I0476a8d80f7a2497600196932542045f3bc87537 (cherry picked from commit daacf4f6f3aba6e0584103820100da7a7ea81123) --- adb/daemon/main.cpp | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp index 3c2758276..5adeb4446 100644 --- a/adb/daemon/main.cpp +++ b/adb/daemon/main.cpp @@ -19,10 +19,11 @@ #include "sysdeps.h" #include +#include #include #include #include -#include +#include #include #include @@ -49,13 +50,13 @@ static const char* root_seclabel = nullptr; -static void drop_capabilities_bounding_set_if_needed(struct minijail *j) { +static bool should_drop_capabilities_bounding_set() { #if defined(ALLOW_ADBD_ROOT) if (__android_log_is_debuggable()) { - return; + return false; } #endif - minijail_capbset_drop(j, CAP_TO_MASK(CAP_SETUID) | CAP_TO_MASK(CAP_SETGID)); + return true; } static bool should_drop_privileges() { @@ -116,13 +117,37 @@ static void drop_privileges(int server_port) { // Don't listen on a port (default 5037) if running in secure mode. // Don't run as root if running in secure mode. if (should_drop_privileges()) { - drop_capabilities_bounding_set_if_needed(jail.get()); + const bool should_drop_caps = should_drop_capabilities_bounding_set(); + + if (should_drop_caps) { + minijail_use_caps(jail.get(), CAP_TO_MASK(CAP_SETUID) | CAP_TO_MASK(CAP_SETGID)); + } minijail_change_gid(jail.get(), AID_SHELL); minijail_change_uid(jail.get(), AID_SHELL); // minijail_enter() will abort if any priv-dropping step fails. minijail_enter(jail.get()); + // Whenever ambient capabilities are being used, minijail cannot + // simultaneously drop the bounding capability set to just + // CAP_SETUID|CAP_SETGID while clearing the inheritable, effective, + // and permitted sets. So we need to do that in two steps. + using ScopedCaps = + std::unique_ptr::type, std::function>; + ScopedCaps caps(cap_get_proc(), &cap_free); + if (cap_clear_flag(caps.get(), CAP_INHERITABLE) == -1) { + PLOG(FATAL) << "cap_clear_flag(INHERITABLE) failed"; + } + if (cap_clear_flag(caps.get(), CAP_EFFECTIVE) == -1) { + PLOG(FATAL) << "cap_clear_flag(PEMITTED) failed"; + } + if (cap_clear_flag(caps.get(), CAP_PERMITTED) == -1) { + PLOG(FATAL) << "cap_clear_flag(PEMITTED) failed"; + } + if (cap_set_proc(caps.get()) != 0) { + PLOG(FATAL) << "cap_set_proc() failed"; + } + D("Local port disabled"); } else { // minijail_enter() will abort if any priv-dropping step fails.