From 0603ec4294bc92449643608ee5f18e172d99fce0 Mon Sep 17 00:00:00 2001 From: Bowgo Tsai Date: Thu, 31 Aug 2017 06:26:47 +0000 Subject: [PATCH] Revert "adbd: lessen security constraints when the device is unlocked" This reverts commit f1d3dbc32f18d9b3604da03bc043c1b4cc3f5a35. With the following changes to move /sbin/adbd to /system/bin/adbd, we don't need this workaround anymore. https://android-review.googlesource.com/#/q/topic:move-adbd-to-system+(status:open+OR+status:merged) Bug: 63313955 Bug: 63381692 Bug: 64822208 Test: 'adb root' works in VTS for a non-A/B device (userdebug GSI + user boot.img) Change-Id: Ic1249d6abd7d6e6e7380a661df16d25447853a48 --- adb/Android.mk | 2 +- adb/daemon/main.cpp | 24 +++++++++--------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/adb/Android.mk b/adb/Android.mk index e195bec6a..b63cce03e 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -345,11 +345,11 @@ LOCAL_CFLAGS := \ -D_GNU_SOURCE \ -Wno-deprecated-declarations \ -LOCAL_CFLAGS += -DALLOW_ADBD_ROOT=$(if $(filter userdebug eng,$(TARGET_BUILD_VARIANT)),1,0) LOCAL_CFLAGS += -DALLOW_ADBD_NO_AUTH=$(if $(filter userdebug eng,$(TARGET_BUILD_VARIANT)),1,0) ifneq (,$(filter userdebug eng,$(TARGET_BUILD_VARIANT))) LOCAL_CFLAGS += -DALLOW_ADBD_DISABLE_VERITY=1 +LOCAL_CFLAGS += -DALLOW_ADBD_ROOT=1 endif LOCAL_MODULE := adbd diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp index e0629abdd..1c94298da 100644 --- a/adb/daemon/main.cpp +++ b/adb/daemon/main.cpp @@ -49,23 +49,17 @@ static const char* root_seclabel = nullptr; -static inline bool is_device_unlocked() { - return "orange" == android::base::GetProperty("ro.boot.verifiedbootstate", ""); -} - static void drop_capabilities_bounding_set_if_needed(struct minijail *j) { - if (ALLOW_ADBD_ROOT || is_device_unlocked()) { - if (__android_log_is_debuggable()) { - return; - } +#if defined(ALLOW_ADBD_ROOT) + if (__android_log_is_debuggable()) { + return; } +#endif minijail_capbset_drop(j, CAP_TO_MASK(CAP_SETUID) | CAP_TO_MASK(CAP_SETGID)); } static bool should_drop_privileges() { - // "adb root" not allowed, always drop privileges. - if (!ALLOW_ADBD_ROOT && !is_device_unlocked()) return true; - +#if defined(ALLOW_ADBD_ROOT) // The properties that affect `adb root` and `adb unroot` are ro.secure and // ro.debuggable. In this context the names don't make the expected behavior // particularly obvious. @@ -95,6 +89,9 @@ static bool should_drop_privileges() { } return drop; +#else + return true; // "adb root" not allowed, always drop privileges. +#endif // ALLOW_ADBD_ROOT } static void drop_privileges(int server_port) { @@ -161,10 +158,7 @@ int adbd_main(int server_port) { // descriptor will always be open. adbd_cloexec_auth_socket(); - // Respect ro.adb.secure in userdebug/eng builds (ALLOW_ADBD_NO_AUTH), or when the - // device is unlocked. - if ((ALLOW_ADBD_NO_AUTH || is_device_unlocked()) && - !android::base::GetBoolProperty("ro.adb.secure", false)) { + if (ALLOW_ADBD_NO_AUTH && !android::base::GetBoolProperty("ro.adb.secure", false)) { auth_required = false; }