adb: Use Minijail for privilege dropping.
By using Minijail we avoid writing the same priv-dropping code over
and over again. This also enables future hardening opportunities.
We're already using Minijail for priv-dropping on Brillo.
Minijail is unit- and integration-tested on the
Chrome OS CI infrastructure (pulling from the same repo, see
https://wmatrix.googleplex.com/security?tests=security_Minijail0).
Once CI comes to Android, it will be continuously tested there as well.
Both 'adb root' and 'adb unroot' still work.
Bug: 26099611
Change-Id: I52c456ea272d27bd6fbc71200f4fdd928a592158
diff --git a/Android.mk b/Android.mk
index fe3c9cc..cb6da62 100644
--- a/Android.mk
+++ b/Android.mk
@@ -346,6 +346,9 @@
libsquashfs_utils \
libcutils \
libbase \
- libcrypto_static
+ libcrypto_static \
+ libminijail \
+ libminijail_generated \
+ libcap
include $(BUILD_EXECUTABLE)
diff --git a/daemon/main.cpp b/daemon/main.cpp
index 53117d6..78db69d 100644
--- a/daemon/main.cpp
+++ b/daemon/main.cpp
@@ -25,8 +25,12 @@
#include <getopt.h>
#include <sys/prctl.h>
+#include <memory>
+
#include <android-base/logging.h>
#include <android-base/stringprintf.h>
+#include <libminijail.h>
+
#include "cutils/properties.h"
#include "private/android_filesystem_config.h"
#include "selinux/android.h"
@@ -86,12 +90,12 @@
bool adb_root = (strcmp(value, "1") == 0);
bool adb_unroot = (strcmp(value, "0") == 0);
- // ...except "adb root" lets you keep privileges in a debuggable build.
+ // ... except "adb root" lets you keep privileges in a debuggable build.
if (ro_debuggable && adb_root) {
drop = false;
}
- // ...and "adb unroot" lets you explicitly drop privileges.
+ // ... and "adb unroot" lets you explicitly drop privileges.
if (adb_unroot) {
drop = true;
}
@@ -103,6 +107,9 @@
}
static void drop_privileges(int server_port) {
+ std::unique_ptr<minijail, void (*)(minijail*)> jail(minijail_new(),
+ &minijail_destroy);
+
// Add extra groups:
// AID_ADB to access the USB driver
// AID_LOG to read system logs (adb logcat)
@@ -116,26 +123,29 @@
gid_t groups[] = {AID_ADB, AID_LOG, AID_INPUT,
AID_INET, AID_NET_BT, AID_NET_BT_ADMIN,
AID_SDCARD_R, AID_SDCARD_RW, AID_NET_BW_STATS,
- AID_READPROC };
- if (setgroups(sizeof(groups) / sizeof(groups[0]), groups) != 0) {
- PLOG(FATAL) << "Could not set supplemental groups";
+ AID_READPROC};
+ if (minijail_set_supplementary_gids(
+ jail.get(),
+ sizeof(groups) / sizeof(groups[0]),
+ groups) != 0) {
+ LOG(FATAL) << "Could not configure supplementary groups";
}
- /* don't listen on a port (default 5037) if running in secure mode */
- /* don't run as root if we are running in secure mode */
+ // 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();
- /* then switch user and group to "shell" */
- if (setgid(AID_SHELL) != 0) {
- PLOG(FATAL) << "Could not setgid";
- }
- if (setuid(AID_SHELL) != 0) {
- PLOG(FATAL) << "Could not setuid";
- }
+ 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());
D("Local port disabled");
} else {
+ // minijail_enter() will abort if any priv-dropping step fails.
+ minijail_enter(jail.get());
+
if (root_seclabel != nullptr) {
if (selinux_android_setcon(root_seclabel) < 0) {
LOG(FATAL) << "Could not set SELinux context";
@@ -147,7 +157,7 @@
if (install_listener(local_name, "*smartsocket*", nullptr, 0,
&error)) {
LOG(FATAL) << "Could not install *smartsocket* listener: "
- << error;
+ << error;
}
}
}