From d076857c4f6c15a70f1186c4214592837f46c57d Mon Sep 17 00:00:00 2001
From: Josh Gao <jmgao@google.com>
Date: Thu, 4 Jun 2020 17:58:48 -0700
Subject: [PATCH] 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
---
 adb/Android.bp      | 11 -----------
 adb/daemon/main.cpp | 31 +++++--------------------------
 2 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/adb/Android.bp b/adb/Android.bp
index 9db151d13..8b7238682 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
         "-DANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION=1",
     ],
     cpp_std: "experimental",
@@ -81,16 +80,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 db8f07ba4..eb28668b9 100644
--- a/adb/daemon/main.cpp
+++ b/adb/daemon/main.cpp
@@ -62,23 +62,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.
@@ -132,7 +116,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));
@@ -218,15 +202,10 @@ int adbd_main(int server_port) {
     // descriptor will always be open.
     adbd_cloexec_auth_socket();
 
-#if defined(__ANDROID_RECOVERY__)
-    if (is_device_unlocked() || __android_log_is_debuggable()) {
-        auth_required = false;
-    }
-#elif 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);
-#elif defined(__ANDROID__)
-    if (is_device_unlocked()) {  // allows no authentication when the device is unlocked.
+#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);
     }
 #endif