From 6d3d618c53cda6d9ac55f1e49cb932ba01309d61 Mon Sep 17 00:00:00 2001 From: Michael Groover Date: Fri, 14 Apr 2023 17:20:33 -0500 Subject: [PATCH] Disconnect existing adb clients when adb grants are revoked When revoking previously authorized adb grants, the prompt indicates access will be revoked for all computers that were previously authorized; the intent of this was to remove any grants previously authorized with the 'Always allow' option, but this could also be read to indicate existing connections are terminated as well. This commit disconnects all existing sessions by toggling the adbd service; the original behavior can be maintained for lab environments by modifying the global Setting adb_disconnect_sessions_on_revoke to 0. Bug: 202836162 Bug: 261184503 Bug: 262244393 Test: Manually verfied existing USB and Wifi sessions were terminated Change-Id: I22266053cf1f8a0452bfa22b2898b4c68eeaed1d --- core/java/android/provider/Settings.java | 8 +++++ .../android/provider/SettingsBackupTest.java | 1 + .../server/adb/AdbDebuggingManager.java | 34 +++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 3487b013fa0b7..a88178a0c36b9 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -12129,6 +12129,14 @@ public final class Settings { @Readable public static final String ADB_WIFI_ENABLED = "adb_wifi_enabled"; + /** + * Whether existing ADB sessions over both USB and Wifi should be terminated when the user + * revokes debugging authorizations. + * @hide + */ + public static final String ADB_DISCONNECT_SESSIONS_ON_REVOKE = + "adb_disconnect_sessions_on_revoke"; + /** * Whether Views are allowed to save their attribute data. * @hide diff --git a/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java b/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java index 36aa2ac744063..7c2c2d97dcee9 100644 --- a/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java +++ b/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java @@ -110,6 +110,7 @@ public class SettingsBackupTest { Settings.Global.ADB_ALLOWED_CONNECTION_TIME, Settings.Global.ADB_ENABLED, Settings.Global.ADB_WIFI_ENABLED, + Settings.Global.ADB_DISCONNECT_SESSIONS_ON_REVOKE, Settings.Global.ADD_USERS_WHEN_LOCKED, Settings.Global.AIRPLANE_MODE_ON, Settings.Global.AIRPLANE_MODE_RADIOS, diff --git a/services/core/java/com/android/server/adb/AdbDebuggingManager.java b/services/core/java/com/android/server/adb/AdbDebuggingManager.java index 62a97dce8b538..6794f750c82d7 100644 --- a/services/core/java/com/android/server/adb/AdbDebuggingManager.java +++ b/services/core/java/com/android/server/adb/AdbDebuggingManager.java @@ -16,7 +16,10 @@ package com.android.server.adb; +import static android.os.InputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS; + import static com.android.internal.util.dump.DumpUtils.writeStringIfNotNull; +import static com.android.server.adb.AdbService.ADBD; import android.annotation.NonNull; import android.annotation.Nullable; @@ -58,6 +61,7 @@ import android.os.Looper; import android.os.Message; import android.os.SystemClock; import android.os.SystemProperties; +import android.os.SystemService; import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings; @@ -99,6 +103,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -153,6 +158,10 @@ public class AdbDebuggingManager { private static final String WIFI_PERSISTENT_GUID = "persist.adb.wifi.guid"; private static final int PAIRING_CODE_LENGTH = 6; + /** + * The maximum time to wait for the adbd service to change state when toggling. + */ + private static final long ADBD_STATE_CHANGE_TIMEOUT = DEFAULT_DISPATCHING_TIMEOUT_MILLIS; private PairingThread mPairingThread = null; // A list of keys connected via wifi private final Set mWifiConnectedKeys = new HashSet<>(); @@ -949,6 +958,31 @@ public class AdbDebuggingManager { mWifiConnectedKeys.clear(); mAdbKeyStore.deleteKeyStore(); cancelJobToUpdateAdbKeyStore(); + // Disconnect all active sessions unless the user opted out through Settings. + if (Settings.Global.getInt(mContentResolver, + Settings.Global.ADB_DISCONNECT_SESSIONS_ON_REVOKE, 1) == 1) { + // If adb is currently enabled, then toggle it off and back on to disconnect + // any existing sessions. + if (mAdbUsbEnabled) { + try { + SystemService.stop(ADBD); + SystemService.waitForState(ADBD, SystemService.State.STOPPED, + ADBD_STATE_CHANGE_TIMEOUT); + SystemService.start(ADBD); + SystemService.waitForState(ADBD, SystemService.State.RUNNING, + ADBD_STATE_CHANGE_TIMEOUT); + } catch (TimeoutException e) { + Slog.e(TAG, "Timeout occurred waiting for adbd to cycle: ", e); + // TODO(b/281758086): Display a dialog to the user to warn them + // of this state and direct them to manually toggle adb. + // If adbd fails to toggle within the timeout window, set adb to + // disabled to alert the user that further action is required if + // they want to continue using adb after revoking the grants. + Settings.Global.putInt(mContentResolver, + Settings.Global.ADB_ENABLED, 0); + } + } + } break; }