commit e4528e1bc495c44f61c15b0457c0e3701cdf8ccc
author Luis Hector Chavez <lhchavez@google.com> Wed Apr 04 15:59:59 2018 -0700
committer Luis Hector Chavez <lhchavez@google.com> Thu Apr 05 15:34:05 2018 -0700
tree 1790a656af19b3f1fd85bc95eea90c25d9b7e60e
parent f5e901005c69ef9f5d70e1888fd659e2b31ba465

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

daemon/main.cpp

diff --git a/daemon/main.cpp b/daemon/main.cpp
index 527a264..232d9c5 100644
--- a/daemon/main.cpp
+++ b/daemon/main.cpp
@@ -24,6 +24,7 @@
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/capability.h>
 #include <sys/prctl.h>
 
 #include <memory>
@@ -50,13 +51,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() {
@@ -117,13 +118,37 @@
     // 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<std::remove_pointer<cap_t>::type, std::function<void(cap_t)>>;
+        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.