IssueTracker

As per https://issuetracker.google.com/issues/232206670, Android 13 shows an allow access dialog when an app that has READ_LOGS permission runs logcat command for whether to grant it access or not.

The two issues raised in the above issuetracker and mentioned below were actually apparently fixed in AOSP on May 2, 2022 before the above linked issue was opened on May 18, 2022 but the latest android builds at the time didn't have the fixes and were available the month after in June. Likely that's why no one from google responded specifically for them.

• If multiple logcat commands are run, then multiple prompts would be shown for each command. Fixed in d1122015.
• A bug where an Allow %s to access allow device logs? message is being shown before the dialog is shown and is not dismissed if user clicks outside dialog or dialog times out. Fixed in 2b6a2340.

Current Design

The current design now as per following commits is detailed below.

d1122015.

Batch log access requests from same client

For multiple log access requests from the same client (same UID + package name) within a short timeframe, show only one confirmation prompt to the user.

When access has been approved/denied, further requests will automatically be approved/denied until another timeout expires, after which a new request will show a prompt again.

If the prompt is shown but the request isn't approved or denied within a certain time, the client will automatically be denied access.

6a694788

Generate per-use prompt only when the process is foreground

This change is to implement dialog when device log access request comes from foreground.

For which apps the allow access dialog will be shown?

The allow access dialog will only be shown for the top foreground apps that have ActivityManager.PROCESS_STATE_TOP and access will be automatically be denied for apps running in background, even if the apps have a foreground service. Note that an app can get PROCESS_STATE_TOP from background by showing an invisible dialog before running the logcat command if they have draw over apps permission.

Why was this behaviour changed even though it was initially allowed for foreground service apps to request access as per b4f8ed9b? An app that has a foreground service is still considered visible to the user. Even other privileged permissions like location, camera and microphone are allowed as per https://developer.android.com/guide/components/foreground-services#bg-access-restrictions for apps that have foreground services.

What are the timeouts for allow access dialog?

There are two timeouts.

• The LogcatManagerService sets a timeout of PENDING_CONFIRMATION_TIMEOUT_MILLIS when it receives a new request from an app and it starts LogAccessDialogActivity to show the allow access dialog. The timeout value is 400s/6.6min for production builds and 70s for debug builds. If user does not accept or deny the access in the dialog or LogAccessDialogActivity fails to send a response, access is automatically denied when timeout expires. Any new requests from the same app before dialog is clicked/dismissed or timeout expires are added to a pending requests queue and are collectively given approved/denied response and only a single dialog is shown for all of them.

• The LogAccessDialogActivity also sets its own timeout to DIALOG_TIME_OUT when showing the allow access dialog. The timeout value is 300s/5min for production builds and 60s for debug builds. This timeout will expire before LogcatManagerService one does and will usually take precedence. When timeout expires, dialog will be dismissed and access denied for all pending requests.

What are the timeouts for approvals and denials?

The LogcatManagerService sets a timeout of STATUS_EXPIRATION_TIMEOUT_MILLIS when it approves or denies access. The timeout value is 60s for all builds.

What this means is that once an app is approved or denied access either via dialog buttons, dismissal or the two timeouts, for the next 60s, the same response will be given for all new requests from the same app. So if access for denied, then the app, even despite being on the top, will not be able to show allow access dialog again for the next 60s to get access. If it was allowed access, it will be able to start new logcat processes for the next 60s and will automatically be allowed access.

Bugs

There are a few bugs in the current design.

Dialog does not deny access on home button press until timeout passes

The 2b6a2340 added following

Handles cancel/dismiss of the dialog

When the dialog is canceled (eg. pressing back button or tapping outside the dialog) decline the log access request. When the dialog is dismissed finish the activity hosting the dialog.

But it does not consider home button. Since LogAccessDialogActivity has excludeFromRecents=true, user won't be able to get back to dialog if home button is pressed. The activity does show in recents menu if recents button is pressed, although it would be better to just consider it dismissed.

When home button is pressed, LogAccessDialogActivity.onDestroy() is not called to dismiss the dialog and neither is dialog's OnDismissListener to finish the activity. Only onStop() is normally called as per activity lifecycle, which is not overridden. This will also happen if two apps having PROCESS_STATE_TOP like in split screen (or previously/in-future-hopefully having PROCESS_STATE_FOREGROUND_SERVICE) triggering a new dialog which closes previous.

What this result in is app having to wait for 300s/5min for the DIALOG_TIME_OUT to expire before getting denied access for currently running logcat processes (which will stay hung) and then another 60s for the app to be able to show the dialog again to get access.

Dialog is not being dismissed when activity destroyed for configuration change

The dialog is not being dismissed before activity is destroyed for configuration change like rotation, a change deliberately made in bd9cf8f9, which then raises the WindowLeaked exception. The dialog will be recreated again anyways since activity will be recreated again with a call to onCreate(), so no point in not dismissing the dialog. I assume the change was made because finish() would be called by OnDismissListener and dialog wouldn't show again after rotation. That should be fixable by setting OnDismissListener to null if configuration change is being made or update OnDismissListener to not finish the activity for that case.

 E/WindowManager: android.view.WindowLeaked: Activity com.android.server.logcat.LogAccessDialogActivity has leaked window DecorView@e47f0a2[LogAccessDialogActivity] that was originally added here
        at android.view.ViewRootImpl.<init>(ViewRootImpl.java:900)
        at android.view.ViewRootImpl.<init>(ViewRootImpl.java:884)
        at android.view.WindowManagerGlobal.addView(WindowManagerGlobal.java:386)
        at android.view.WindowManagerImpl.addView(WindowManagerImpl.java:148)
        at android.app.Dialog.show(Dialog.java:352)
        at com.android.server.logcat.LogAccessDialogActivity.onCreate(LogAccessDialogActivity.java:99)
        at android.app.Activity.performCreate(Activity.java:8290)
        at android.app.Activity.performCreate(Activity.java:8269)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1384)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3657)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3813)
        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:101)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2308)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at com.android.server.SystemServer.run(SystemServer.java:964)
        at com.android.server.SystemServer.main(SystemServer.java:649)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:914)

Solutions

To solve the above two bugs, access should be denied in onStop() if user hasn't clicked and dialog dismissed as well instead of in onDestroy(). Moreover, MSG_DISMISS_DIALOG handler messages should ideally be removed when user clicks the dialog, which aren't currently being removed anywhere. The dialog should always be dismissed as well, even for configuration changes.

private boolean mHasClicked;

protected void onCreate(Bundle savedInstanceState) {
    ...
    mHasClicked = false;
}

public void onClick(View view) {
    switch (view.getId()) {
        case R.id.log_access_dialog_allow_button:
            mHasClicked = true;
            mHandler.removeMessages(MSG_DISMISS_DIALOG);
            mLogcatManagerInternal.approveAccessForClient(mUid, mPackageName);
            finish();
            break;
        case R.id.log_access_dialog_deny_button:
            mHasClicked = true;
            mHandler.removeMessages(MSG_DISMISS_DIALOG);
            declineLogAccess();
            finish();
            break;
    }
}

protected void onStop() {
    if (mAlert != null && mAlert.isShowing()) {
        if (isChangingConfigurations())
            mAlert.setOnDismissListener(null);
        mAlert.dismiss();
    }
    mAlert = null;

    if (!isChangingConfigurations()) {
        if (!mHasClicked)
            declineLogAccess();

        finish();
    }
}

Handler messages in LogcatManagerService are wrongly implemented

The MSG_PENDING_TIMEOUT messages are not removed in LogcatManagerService.scheduleStatusExpiry() by calling mHandler.removeMessages(<what>, client).

The reason is that the Handler.removeMessages() calls remove messages based on p.obj == object instead of equals() check, i.e both client objects must have the same instance/reference and not just the same values. Someone else also got similar issues for long values.

The LogcatManagerService.getClientForRequest() is responsibly for creating a new LogAccessClient object when called by onLogAccessRequested() (and also onLogAccessFinished()). This client object is used by processNewLogAccessRequest() to send the initial MSG_PENDING_TIMEOUT message.

void onLogAccessRequested(LogAccessRequest request) {
    final LogAccessClient client = getClientForRequest(request);
    ...
    switch (logAccessStatus.mStatus) {
        case STATUS_NEW_REQUEST:
            logAccessStatus.mPendingRequests.add(request);
            processNewLogAccessRequest(client);
            break;
    }
    ...
}

private void processNewLogAccessRequest(LogAccessClient client) {
    ...
    mHandler.sendMessageAtTime(mHandler.obtainMessage(MSG_PENDING_TIMEOUT, client),
            mClock.get() + PENDING_CONFIRMATION_TIMEOUT_MILLIS);
}


private LogAccessClient getClientForRequest(LogAccessRequest request) {
    final String packageName = getPackageName(request);
    if (packageName == null) {
        return null;
    }

    return new LogAccessClient(request.mUid, packageName);
}

When LogAccessDialogActivity calls LogcatManagerServiceInternal.approveAccessForClient() or LogcatManagerServiceInternal.declineAccessForClient(), a new client object is created, which is then sent via MSG_APPROVE_LOG_ACCESS and MSG_DECLINE_LOG_ACCESS and then received by LogAccessRequestHandler.handleMessage() and then ultimately received by scheduleStatusExpiry(). Since this client object does not have the same instance as the one used to send the initial MSG_PENDING_TIMEOUT message, the initial message never gets removed and is always sent. This doesn't always cause an ill effect because onPendingTimeoutExpired() also checks logAccessStatus.mStatus == STATUS_PENDING before declining access, but it does cause a denial in background in some cases as noted below. It would be easier to check issues by adding log messages in handleMessage() and related calls.

• Run logcat command and immediately approve access and note time, since this is when MSG_PENDING_TIMEOUT is initially sent.
• After 60s of noted time on development builds, MSG_LOG_ACCESS_STATUS_EXPIRED will be received.
• Run logcat command again and dialog should show again, but don't approve or deny and just wait.
• After 70s of noted time, MSG_PENDING_TIMEOUT will be delivered and it will deny access for all pending requests due to call of onAccessDeclinedForClient().
• Allowing access in dialog won't do anything now, since all pending requests would already have been processed and denied access.

final class LogcatManagerServiceInternal {
    public void approveAccessForClient(int uid, @NonNull String packageName) {
        final LogAccessClient client = new LogAccessClient(uid, packageName);
        if (DEBUG) {
            Slog.d(TAG, "Approving log access for client: " + client);
        }
        final Message msg = mHandler.obtainMessage(MSG_APPROVE_LOG_ACCESS, client);
        mHandler.sendMessageAtTime(msg, mClock.get());
    }

    public void declineAccessForClient(int uid, @NonNull String packageName) {
        final LogAccessClient client = new LogAccessClient(uid, packageName);
        if (DEBUG) {
            Slog.d(TAG, "Declining log access for client: " + client);
        }
        final Message msg = mHandler.obtainMessage(MSG_DECLINE_LOG_ACCESS, client);
        mHandler.sendMessageAtTime(msg, mClock.get());
    }
}

private static class LogAccessRequestHandler extends Handler {

    @Override
    public void handleMessage(Message msg) {
        switch (msg.what) {
            case MSG_APPROVE_LOG_ACCESS: {
                LogAccessClient client = (LogAccessClient) msg.obj;
                mService.onAccessApprovedForClient(client);
                break;
            }
            case MSG_DECLINE_LOG_ACCESS: {
                LogAccessClient client = (LogAccessClient) msg.obj;
                mService.onAccessDeclinedForClient(client);
                break;
            }
        }
    }
}

void onAccessApprovedForClient(LogAccessClient client) {
    scheduleStatusExpiry(client);
    ...
}

void onAccessDeclinedForClient(LogAccessClient client) {
    scheduleStatusExpiry(client);
    ...
}

private void scheduleStatusExpiry(LogAccessClient client) {
    mHandler.removeMessages(MSG_PENDING_TIMEOUT, client);
    mHandler.removeMessages(MSG_LOG_ACCESS_STATUS_EXPIRED, client);
    mHandler.sendMessageAtTime(mHandler.obtainMessage(MSG_LOG_ACCESS_STATUS_EXPIRED, client),
            mClock.get() + STATUS_EXPIRATION_TIMEOUT_MILLIS);
}

Solutions

To solve the above bug, one way could be to use an ArrayMap<String, LogAccessClient> mLogAccessClients to store LogAccessClient instances with the key uid + ":" + packageName. A new instance should only be created in onLogAccessRequested() if one does not exist with a call to getExistingOrNewClientForRequest(). All other methods should call getExistingClient*() to get existing client from the ArrayMap instead of generating a new one. The client can then be removed from mLogAccessClients in onAccessStatusExpired() along with its status from mLogAccessStatus. If access is allowed till next reboot, mLogAccessClients and mLogAccessStatus would grow to the size of all apps allowed access, but it would normally only be a few, so shouldn't be a problem.

private final ArrayMap<String, LogAccessClient> mLogAccessClients = new ArrayMap<>();

final class LogcatManagerServiceInternal {
    public void approveAccessForClient(int uid, @NonNull String packageName) {
        final LogAccessClient client = getExistingClient(uid, packageName);
        if (client == null) {
            return;
        }
        ...
    }

    public void declineAccessForClient(int uid, @NonNull String packageName) {
        final LogAccessClient client = getExistingClient(uid, packageName);
        if (client == null) {
            return;
        }
        ...
    }
}

void onLogAccessRequested(LogAccessRequest request) {
    final LogAccessClient client = getExistingOrNewClientForRequest(request);
    if (client == null) {
        declineRequest(request);
        return;
    }
    ...
}

void onLogAccessFinished(LogAccessRequest request) {
    final LogAccessClient client = getExistingClientForRequest(request);
    if (client == null) {
        return;
    }
    ...
}

void onAccessStatusExpired(LogAccessClient client) {
    if (client == null) {
        return;
    }

    if (DEBUG) {
        Slog.d(TAG, "Log access status expired for " + client);
    }

    mLogAccessStatus.remove(client);
    mLogAccessClients.remove(client.mUid + ":" + client.mPackageName);
}

@Nullable
private LogAccessClient getExistingOrNewClientForRequest(LogAccessRequest request) {
    final String packageName = getPackageName(request);
    if (packageName == null) {
        return null;
    }

    LogAccessClient client = getExistingClient(request.mUid, packageName);
    if (client == null) {
        // Create and store new instance if one does not exist
        client = new LogAccessClient(request.mUid, packageName);
        mLogAccessClients.put(uid + ":" + packageName, client);
    }

    return client;
}

@Nullable
private LogAccessClient getExistingClientForRequest(LogAccessRequest request) {
    final String packageName = getPackageName(request);
    if (packageName == null) {
        return null;
    }

    return getExistingClient(request.mUid, packageName);
}


@Nullable
private LogAccessClient getExistingClient(int uid, String packageName) {
    return mLogAccessClients.get(uid + ":" + packageName);
}

Discussion

With the currently implemented "design decisions" and usability issues, an app not being able to request access from background even with a foreground service, being denied access automatically if dialog was accidentally denied/dismissed for the next 60s (or currently many minutes due to bug), having to request access whenever app process is killed and restarted instead of once per boot, like due to OOM or OEM aggressive killing which would specially be a concern on low memory devices... All these things will just push users to give apps access to adb on every reboot or root access for logcat commands to function with ease, which is obviously much worse from a security point of view since those accesses have much higher privileges. With apps like shizuku, only one app needs to be granted wireless adb access at boot and it can then give access to others. The tasker app in its recent beta v6.1.3-beta has already added support to use adb or root for logcat commands because of issues with current design.

I can support the decision that a dialog should be shown before granting access, but users should definitely have an option to grant access once per boot, even if its not enabled by default. There could be secure setting for the STATUS_EXPIRATION_TIMEOUT_MILLIS for allowed access timeout. An app with just the READ_LOGS permission won't be able to modify the setting and a user can manually set a value of -1 to allow access till next boot or positive value for some defined timeout other than 60s. Can also add a button in the dialog itself that will allow access till next reboot per app, which would obviously be better since user will get to decide which app gets till next boot access and which one only for a specified seconds.

There should ideally be a setting for the STATUS_EXPIRATION_TIMEOUT_MILLIS for denied access timeout, so that users who don't want to wait 60s before being shown a dialog can do so.

Xposed Module

To solve the design issues and the bugs, I have created the XLogcatManager xposed module for rooted users that can be run with Magisk and LSPosed that hooks into LogcatManagerService and LogAccessDialogActivity methods. This will also allow rooted users to not have to grant adb or root access to apps that shouldn't require it just to read logs.

Current features are listed below. Further configuration options can be added in future when I get time.

• Allows access till next reboot to the app if users selects allow button in the allow access dialog instead of just for the next 60s.
• No 60s timeout to reshow dialog if access was (accidentally) denied by user.
• The dialog will also show for apps with foreground service instead of just top apps.
• Fixes the above mentioned bugs based on the solutions provided, but implementation is slightly different in some cases due to method hooking limitations. The main classes that do the hooking are XLogcatManagerService.java and XLogAccessDialogActivity.java.

Device Info

Software

OS_VERSION: 5.15.41-android13-8-00205-gf1bf82c3dacd-ab8747247
SDK_INT: 33
RELEASE: 13
ID: TPB4.220624.004
DISPLAY: sdk_gphone64_x86_64-userdebug 13 TPB4.220624.004 8808248 dev-keys
INCREMENTAL: 8808248
SECURITY_PATCH: 2022-07-05
IS_DEBUGGABLE: 1
IS_EMULATOR: 1
IS_TREBLE_ENABLED: true
TYPE: userdebug
TAGS: dev-keys

Hardware

MANUFACTURER: Google
BRAND: google
MODEL: sdk_gphone64_x86_64
PRODUCT: sdk_gphone64_x86_64
BOARD: goldfish_x86_64
HARDWARE: ranchu
DEVICE: emu64xa
SUPPORTED_ABIS: x86_64, arm64-v8a