Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f1d3dbc3 authored by Bowgo Tsai's avatar Bowgo Tsai
Browse files

adbd: lessen security constraints when the device is unlocked

ALLOW_ADBD_ROOT ('adb root') and ALLOW_ADBD_NO_AUTH (ro.adb.secure = 0)
are false in user build. This prevents a non-A/B device from running
Treble VTS because it requires 'adb root'. Without ALLOW_ADBD_NO_AUTH,
adb still can work if ro.adb.secure = 1. However, allowing it to be 0
is better for test automation.

The image combination in VTS is:
  - system.img (userdebug): provided by Googlg
  - boot.img (user): provided by the OEM  <-- adbd is here
  - vendor.img (user): provided by the OEM

This change allows 'adb root' and 'ro.adb.secure = 0' when the device is
unlocked in user build. No changes for userdebug/eng builds.

Note that the device must be unlocked when running VTS. Otherwise,
verified boot will prevent it from booting the system.img provided by
Google (no verity metadata).

Bug: 63313955
Bug: 63381692
Test: use the above image combination, check 'adb root' and
      'ro.adb.secure = 0' can work

Change-Id: I109d96c950e54c4fb0ac0c98b989a20593681e52
parent 420eabe3
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -350,11 +350,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
+15 −9
Original line number Diff line number Diff line
@@ -49,17 +49,23 @@

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 defined(ALLOW_ADBD_ROOT)
    if (ALLOW_ADBD_ROOT || is_device_unlocked()) {
        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() {
#if defined(ALLOW_ADBD_ROOT)
    // "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.
@@ -89,9 +95,6 @@ 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) {
@@ -158,7 +161,10 @@ int adbd_main(int server_port) {
    // descriptor will always be open.
    adbd_cloexec_auth_socket();

    if (ALLOW_ADBD_NO_AUTH && !android::base::GetBoolProperty("ro.adb.secure", false)) {
    // 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)) {
        auth_required = false;
    }