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

Commit 3ce96dbe authored by Kholoud Mohamed's avatar Kholoud Mohamed
Browse files

Skip user consent for subsequent reports 

Bug: 340439309
Test: atest android.bugreport.cts_root.BugreportManagerTest
Change-Id: Icb167a05052bd18fa410bc37f843a5706008f3e5
parent c80cb5ed

core/java/android/app/admin/flags/flags.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -333,3 +333,13 @@ flag { 
      purpose: PURPOSE_BUGFIX

    }

}



flag {

  name: "onboarding_consentless_bugreports"

  namespace: "enterprise"

  description: "Allow subsequent bugreports to skip user consent within a time frame"

  bug: "340439309"

  metadata {

    purpose: PURPOSE_BUGFIX

  }

}

core/java/android/os/BugreportManager.java

+3 −1
Original line number Diff line number Diff line
@@ -252,7 +252,8 @@ public final class BugreportManager { 
                    params.getMode(),

                    params.getFlags(),

                    dsListener,

                    isScreenshotRequested);

                    isScreenshotRequested,

                    /* skipUserConsent = */ false);

        } catch (RemoteException e) {

            throw e.rethrowFromSystemServer();

        } catch (FileNotFoundException e) {
@@ -313,6 +314,7 @@ public final class BugreportManager { 
                    bugreportFd.getFileDescriptor(),

                    bugreportFile,

                    /* keepBugreportOnRetrieval = */ false,

                    /* skipUserConsent = */ false,

                    dsListener);

        } catch (RemoteException e) {

            throw e.rethrowFromSystemServer();

packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java

+3 −3
Original line number Diff line number Diff line
@@ -200,7 +200,7 @@ public class BugreportReceiverTest { 
            mBugreportFd = ParcelFileDescriptor.dup(invocation.getArgument(2));

            return null;

        }).when(mMockIDumpstate).startBugreport(anyInt(), any(), any(), any(), anyInt(), anyInt(),

                any(), anyBoolean());

                any(), anyBoolean(), anyBoolean());



        setWarningState(mContext, STATE_HIDE);
@@ -543,7 +543,7 @@ public class BugreportReceiverTest { 
        getInstrumentation().waitForIdleSync();



        verify(mMockIDumpstate, times(1)).startBugreport(anyInt(), any(), any(), any(),

                anyInt(), anyInt(), any(), anyBoolean());

                anyInt(), anyInt(), any(), anyBoolean(), anyBoolean());

        sendBugreportFinished();

    }
@@ -608,7 +608,7 @@ public class BugreportReceiverTest { 
        ArgumentCaptor<IDumpstateListener> listenerCap = ArgumentCaptor.forClass(

                IDumpstateListener.class);

        verify(mMockIDumpstate, timeout(TIMEOUT)).startBugreport(anyInt(), any(), any(), any(),

                anyInt(), anyInt(), listenerCap.capture(), anyBoolean());

                anyInt(), anyInt(), listenerCap.capture(), anyBoolean(), anyBoolean());

        mIDumpstateListener = listenerCap.getValue();

        assertNotNull("Dumpstate listener should not be null", mIDumpstateListener);

        mIDumpstateListener.onProgress(0);

services/core/java/com/android/server/os/BugreportManagerServiceImpl.java

+100 −12
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.server.os;



import static android.app.admin.flags.Flags.onboardingBugreportV2Enabled;

import static android.app.admin.flags.Flags.onboardingConsentlessBugreports;



import android.Manifest;

import android.annotation.NonNull;
@@ -31,6 +32,7 @@ import android.content.pm.UserInfo; 
import android.os.Binder;

import android.os.BugreportManager.BugreportCallback;

import android.os.BugreportParams;

import android.os.Build;

import android.os.Environment;

import android.os.IDumpstate;

import android.os.IDumpstateListener;
@@ -69,12 +71,14 @@ import java.io.FileOutputStream; 
import java.io.IOException;

import java.io.InputStream;

import java.io.PrintWriter;

import java.util.HashMap;

import java.util.HashSet;

import java.util.List;

import java.util.Map;

import java.util.Objects;

import java.util.OptionalInt;

import java.util.Set;

import java.util.concurrent.TimeUnit;



/**

 * Implementation of the service that provides a privileged API to capture and consume bugreports.
@@ -98,6 +102,9 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
    private static final String BUGREPORT_SERVICE = "bugreportd";

    private static final long DEFAULT_BUGREPORT_SERVICE_TIMEOUT_MILLIS = 30 * 1000;



    private static final long DEFAULT_BUGREPORT_CONSENTLESS_GRACE_PERIOD_MILLIS =

            TimeUnit.MINUTES.toMillis(2);



    private final Object mLock = new Object();

    private final Injector mInjector;

    private final Context mContext;
@@ -132,6 +139,10 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
        private ArrayMap<Pair<Integer, String>, ArraySet<String>> mBugreportFiles =

                new ArrayMap<>();



        // Map of <CallerPackage, Pair<TimestampOfLastConsent, skipConsentForFullReport>>

        @GuardedBy("mLock")

        private Map<String, Pair<Long, Boolean>> mConsentGranted = new HashMap<>();



        @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE)

        @GuardedBy("mLock")

        final Set<String> mBugreportFilesToPersist = new HashSet<>();
@@ -238,6 +249,64 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
            }

        }



        /**

         * Logs an entry with a timestamp of a consent being granted by the user to the calling

         * {@code packageName}.

         */

        void logConsentGrantedForCaller(

                String packageName, boolean consentGranted, boolean isDeferredReport) {

            if (!onboardingConsentlessBugreports() || !Build.IS_DEBUGGABLE) {

                return;

            }

            synchronized (mLock) {

                // Adds an entry with the timestamp of the consent being granted by the user, and

                // whether the consent can be skipped for a full bugreport, because a single

                // consent can be used for multiple deferred reports but only one full report.

                if (consentGranted) {

                    mConsentGranted.put(packageName, new Pair<>(

                            System.currentTimeMillis(),

                            isDeferredReport));

                } else if (!isDeferredReport) {

                    if (!mConsentGranted.containsKey(packageName)) {

                        Slog.e(TAG, "Previous consent from package: " + packageName + " should"

                                + "have been logged.");

                        return;

                    }

                    mConsentGranted.put(packageName, new Pair<>(

                            mConsentGranted.get(packageName).first,

                            /* second = */ false

                    ));

                }

            }

        }



        /**

         * Returns {@code true} if user consent be skippeb because a previous consens has been

         * granted to the caller within the allowed time period.

         */

        boolean canSkipConsentScreen(String packageName, boolean isFullReport) {

            if (!onboardingConsentlessBugreports() || !Build.IS_DEBUGGABLE) {

                return false;

            }

            synchronized (mLock) {

                if (!mConsentGranted.containsKey(packageName)) {

                    return false;

                }

                long currentTime = System.currentTimeMillis();

                long consentGrantedTime = mConsentGranted.get(packageName).first;

                if (consentGrantedTime + DEFAULT_BUGREPORT_CONSENTLESS_GRACE_PERIOD_MILLIS

                        < currentTime) {

                    mConsentGranted.remove(packageName);

                    return false;

                }

                boolean skipConsentForFullReport = mConsentGranted.get(packageName).second;

                if (isFullReport && !skipConsentForFullReport) {

                    return false;

                }

                return true;

            }

        }



        private void addBugreportMapping(Pair<Integer, String> caller, String bugreportFile) {

            synchronized (mLock) {

                if (!mBugreportFiles.containsKey(caller)) {
@@ -418,7 +487,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
    public void startBugreport(int callingUidUnused, String callingPackage,

            FileDescriptor bugreportFd, FileDescriptor screenshotFd,

            int bugreportMode, int bugreportFlags, IDumpstateListener listener,

            boolean isScreenshotRequested) {

            boolean isScreenshotRequested, boolean skipUserConsentUnused) {

        Objects.requireNonNull(callingPackage);

        Objects.requireNonNull(bugreportFd);

        Objects.requireNonNull(listener);
@@ -509,7 +578,8 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
    @RequiresPermission(value = Manifest.permission.DUMP, conditional = true)

    public void retrieveBugreport(int callingUidUnused, String callingPackage, int userId,

            FileDescriptor bugreportFd, String bugreportFile,

            boolean keepBugreportOnRetrievalUnused, IDumpstateListener listener) {

            boolean keepBugreportOnRetrievalUnused, boolean skipUserConsentUnused,

            IDumpstateListener listener) {

        int callingUid = Binder.getCallingUid();

        enforcePermission(callingPackage, callingUid, false);
@@ -540,9 +610,13 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
                return;

            }



            boolean skipUserConsent = mBugreportFileManager.canSkipConsentScreen(

                    callingPackage, /* isFullReport = */ false);



            // Wrap the listener so we can intercept binder events directly.

            DumpstateListener myListener = new DumpstateListener(listener, ds,

                    new Pair<>(callingUid, callingPackage), /* reportFinishedFile= */ true);

                    new Pair<>(callingUid, callingPackage), /* reportFinishedFile= */ true,

                    !skipUserConsent, /* isDeferredReport = */ true);



            boolean keepBugreportOnRetrieval = false;

            if (onboardingBugreportV2Enabled()) {
@@ -553,7 +627,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
            setCurrentDumpstateListenerLocked(myListener);

            try {

                ds.retrieveBugreport(callingUid, callingPackage, userId, bugreportFd,

                        bugreportFile, keepBugreportOnRetrieval, myListener);

                        bugreportFile, keepBugreportOnRetrieval, skipUserConsent, myListener);

            } catch (RemoteException e) {

                Slog.e(TAG, "RemoteException in retrieveBugreport", e);

            }
@@ -754,7 +828,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
            }

        }



        boolean reportFinishedFile =

        boolean isDeferredConsentReport =

                (bugreportFlags & BugreportParams.BUGREPORT_FLAG_DEFER_CONSENT) != 0;



        boolean keepBugreportOnRetrieval =
@@ -766,14 +840,17 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
            reportError(listener, IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR);

            return;

        }



        boolean skipUserConsent = mBugreportFileManager.canSkipConsentScreen(

                callingPackage, !isDeferredConsentReport);

        DumpstateListener myListener = new DumpstateListener(listener, ds,

                new Pair<>(callingUid, callingPackage), reportFinishedFile,

                keepBugreportOnRetrieval);

                new Pair<>(callingUid, callingPackage),

                /* reportFinishedFile = */ isDeferredConsentReport, keepBugreportOnRetrieval,

                !isDeferredConsentReport && !skipUserConsent,

                isDeferredConsentReport);

        setCurrentDumpstateListenerLocked(myListener);

        try {

            ds.startBugreport(callingUid, callingPackage, bugreportFd, screenshotFd, bugreportMode,

                    bugreportFlags, myListener, isScreenshotRequested);

                    bugreportFlags, myListener, isScreenshotRequested, skipUserConsent);

        } catch (RemoteException e) {

            // dumpstate service is already started now. We need to kill it to manage the

            // lifecycle correctly. If we don't subsequent callers will get
@@ -930,14 +1007,21 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
        private boolean mDone;

        private boolean mKeepBugreportOnRetrieval;



        private boolean mConsentGranted;



        private boolean mIsDeferredReport;



        DumpstateListener(IDumpstateListener listener, IDumpstate ds,

                Pair<Integer, String> caller, boolean reportFinishedFile) {

            this(listener, ds, caller, reportFinishedFile, /* keepBugreportOnRetrieval= */ false);

                Pair<Integer, String> caller, boolean reportFinishedFile,

                boolean consentGranted, boolean isDeferredReport) {

            this(listener, ds, caller, reportFinishedFile, /* keepBugreportOnRetrieval= */ false,

                    consentGranted, isDeferredReport);

        }



        DumpstateListener(IDumpstateListener listener, IDumpstate ds,

                Pair<Integer, String> caller, boolean reportFinishedFile,

                boolean keepBugreportOnRetrieval) {

                boolean keepBugreportOnRetrieval, boolean consentGranted,

                boolean isDeferredReport) {

            if (DEBUG) {

                Slogf.d(TAG, "Starting DumpstateListener(id=%d) for caller %s", mId, caller);

            }
@@ -946,6 +1030,8 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
            mCaller = caller;

            mReportFinishedFile = reportFinishedFile;

            mKeepBugreportOnRetrieval = keepBugreportOnRetrieval;

            mConsentGranted = consentGranted;

            mIsDeferredReport = isDeferredReport;

            try {

                mDs.asBinder().linkToDeath(this, 0);

            } catch (RemoteException e) {
@@ -985,6 +1071,8 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { 
            } else if (DEBUG) {

                Slog.d(TAG, "Not reporting finished file");

            }

            mBugreportFileManager.logConsentGrantedForCaller(

                    mCaller.second, mConsentGranted, mIsDeferredReport);

            mListener.onFinished(bugreportFile);

        }

services/tests/servicestests/src/com/android/server/os/BugreportManagerServiceImplTest.java

+9 −5
Original line number Diff line number Diff line
@@ -186,7 +186,8 @@ public class BugreportManagerServiceImplTest { 
                new FileDescriptor(), /* screenshotFd= */ null,

                BugreportParams.BUGREPORT_MODE_FULL,

                /* flags= */ 0, new Listener(new CountDownLatch(1)),

                /* isScreenshotRequested= */ false);

                /* isScreenshotRequested= */ false,

                /* skipUserConsentUnused = */ false);



        assertThat(mInjector.isBugreportStarted()).isTrue();

    }
@@ -202,7 +203,8 @@ public class BugreportManagerServiceImplTest { 
                new FileDescriptor(), /* screenshotFd= */ null,

                BugreportParams.BUGREPORT_MODE_FULL,

                /* flags= */ 0, new Listener(new CountDownLatch(1)),

                /* isScreenshotRequested= */ false);

                /* isScreenshotRequested= */ false,

                /* skipUserConsentUnused = */ false);



        assertThat(mInjector.isBugreportStarted()).isTrue();

    }
@@ -216,7 +218,8 @@ public class BugreportManagerServiceImplTest { 
                        new FileDescriptor(), /* screenshotFd= */ null,

                        BugreportParams.BUGREPORT_MODE_FULL,

                        /* flags= */ 0, new Listener(new CountDownLatch(1)),

                        /* isScreenshotRequested= */ false));

                        /* isScreenshotRequested= */ false,

                        /* skipUserConsentUnused = */ false));



        assertThat(thrown.getMessage()).contains("not an admin user");

    }
@@ -232,7 +235,8 @@ public class BugreportManagerServiceImplTest { 
                        new FileDescriptor(), /* screenshotFd= */ null,

                        BugreportParams.BUGREPORT_MODE_REMOTE,

                        /* flags= */ 0, new Listener(new CountDownLatch(1)),

                        /* isScreenshotRequested= */ false));

                        /* isScreenshotRequested= */ false,

                        /* skipUserConsentUnused = */ false));



        assertThat(thrown.getMessage()).contains("not affiliated to the device owner");

    }
@@ -243,7 +247,7 @@ public class BugreportManagerServiceImplTest { 
        Listener listener = new Listener(latch);

        mService.retrieveBugreport(Binder.getCallingUid(), mContext.getPackageName(),

                mContext.getUserId(), new FileDescriptor(), mBugreportFile,

                /* keepOnRetrieval= */ false, listener);

                /* keepOnRetrieval= */ false, /* skipUserConsent = */ false, listener);

        assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue();

        assertThat(listener.getErrorCode()).isEqualTo(

                BugreportCallback.BUGREPORT_ERROR_NO_BUGREPORT_TO_RETRIEVE);