summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJosh Gao <jmgao@google.com>2020-06-04 17:58:48 -0700
committerLuca Stefani <luca.stefani.ge1@gmail.com>2020-06-09 12:52:27 +0200
commit350500d60d8808867e531c1769b179f3db423d87 (patch)
treedadb75e59ca4af882f67c02f951b923b91a30688
parent9f8e60656e3240a3ade46ab5f63f76ab842a4ec7 (diff)
downloadsystem_core-350500d60d8808867e531c1769b179f3db423d87.tar.gz
system_core-350500d60d8808867e531c1769b179f3db423d87.tar.bz2
system_core-350500d60d8808867e531c1769b179f3db423d87.zip
adbd: remove ifdefs guarding root/secure.
The same adbd module prebuilt will get used for both user and userdebug builds in the post-APEX world, so we can't guard functionality with product variable ifdefs anymore. The code that was previously compiled out runs before we drop root, so the increased attack surface essentially consists of an attacker having control over system properties, and that likely implies that we're doomed already (either they have filesystem control, or they have code execution in init). Bug: http://b/158156979 Test: treehugger Change-Id: Ia70d3140189e5212beb813ff719355e30ca5fa04
-rw-r--r--adb/Android.bp11
-rw-r--r--adb/daemon/main.cpp33
-rw-r--r--adb/daemon/restart_service.cpp2
3 files changed, 9 insertions, 37 deletions
diff --git a/adb/Android.bp b/adb/Android.bp
index 0d7e9ff55..4fc0303d3 100644
--- a/adb/Android.bp
+++ b/adb/Android.bp
@@ -25,7 +25,6 @@ cc_defaults {
"-Wthread-safety",
"-Wvla",
"-DADB_HOST=1", // overridden by adbd_defaults
- "-DALLOW_ADBD_ROOT=0", // overridden by adbd_defaults
],
cpp_std: "experimental",
@@ -77,16 +76,6 @@ cc_defaults {
defaults: ["adb_defaults"],
cflags: ["-UADB_HOST", "-DADB_HOST=0"],
- product_variables: {
- debuggable: {
- cflags: [
- "-UALLOW_ADBD_ROOT",
- "-DALLOW_ADBD_ROOT=1",
- "-DALLOW_ADBD_DISABLE_VERITY",
- "-DALLOW_ADBD_NO_AUTH",
- ],
- },
- },
}
cc_defaults {
diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp
index 1f2fc88c4..8521b5fbe 100644
--- a/adb/daemon/main.cpp
+++ b/adb/daemon/main.cpp
@@ -58,23 +58,7 @@
#if defined(__ANDROID__)
static const char* root_seclabel = nullptr;
-static inline bool is_device_unlocked() {
- return "orange" == android::base::GetProperty("ro.boot.verifiedbootstate", "");
-}
-
-static bool should_drop_capabilities_bounding_set() {
- if (ALLOW_ADBD_ROOT || is_device_unlocked()) {
- if (__android_log_is_debuggable()) {
- return false;
- }
- }
- return true;
-}
-
static bool should_drop_privileges() {
- // "adb root" not allowed, always drop privileges.
- if (!ALLOW_ADBD_ROOT && !is_device_unlocked()) return true;
-
// 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.
@@ -128,7 +112,7 @@ 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()) {
- const bool should_drop_caps = should_drop_capabilities_bounding_set();
+ const bool should_drop_caps = !__android_log_is_debuggable();
if (should_drop_caps) {
minijail_use_caps(jail.get(), CAP_TO_MASK(CAP_SETUID) | CAP_TO_MASK(CAP_SETGID));
@@ -205,16 +189,15 @@ int adbd_main(int server_port) {
// descriptor will always be open.
adbd_cloexec_auth_socket();
-#if defined(ALLOW_ADBD_NO_AUTH)
- // If ro.adb.secure is unset, default to no authentication required.
- auth_required = android::base::GetBoolProperty("ro.adb.secure", false);
+#if defined(__ANDROID__)
+ // If we're on userdebug/eng or the device is unlocked, permit no-authentication.
+ bool device_unlocked = "orange" == android::base::GetProperty("ro.boot.verifiedbootstate", "");
+ if (__android_log_is_debuggable() || device_unlocked) {
+ auth_required = android::base::GetBoolProperty("ro.adb.secure", false);
#if defined(__ANDROID_RECOVERY__)
- auth_required = auth_required &&
- android::base::GetBoolProperty("ro.adb.secure.recovery", true);
+ auth_required = auth_required &&
+ android::base::GetBoolProperty("ro.adb.secure.recovery", true);
#endif
-#elif defined(__ANDROID__)
- if (is_device_unlocked()) { // allows no authentication when the device is unlocked.
- auth_required = android::base::GetBoolProperty("ro.adb.secure", false);
}
#endif
diff --git a/adb/daemon/restart_service.cpp b/adb/daemon/restart_service.cpp
index 0334cfc97..b955f00b4 100644
--- a/adb/daemon/restart_service.cpp
+++ b/adb/daemon/restart_service.cpp
@@ -58,7 +58,7 @@ void restart_root_service(unique_fd fd) {
#if defined(__ANDROID__) && !defined(__ANDROID_RECOVERY__)
bool enabled = false;
- if (auto status = service->getEnabled(&enabled); !status.isOk() || !ALLOW_ADBD_ROOT) {
+ if (auto status = service->getEnabled(&enabled); !status.isOk()) {
#endif
if (!__android_log_is_debuggable()) {
WriteFdExactly(fd.get(), "adbd cannot run as root in production builds\n");