diff options
author | 2024-05-15 12:39:48 +0000 | |
---|---|---|
committer | 2024-05-20 13:49:44 +0000 | |
commit | 3ce96dbe16087371b40f41088123f5e7f3f3812c (patch) | |
tree | e23722d971264be21109daa86a593e47361c5d42 | |
parent | c80cb5ed0ef8324a4fd94b63d1bbed57117fb946 (diff) |
Skip user consent for subsequent reports
Bug: 340439309
Test: atest android.bugreport.cts_root.BugreportManagerTest
Change-Id: Icb167a05052bd18fa410bc37f843a5706008f3e5
5 files changed, 125 insertions, 21 deletions
diff --git a/core/java/android/app/admin/flags/flags.aconfig b/core/java/android/app/admin/flags/flags.aconfig index 3d6ec19299cb..98289f69e1d6 100644 --- a/core/java/android/app/admin/flags/flags.aconfig +++ b/core/java/android/app/admin/flags/flags.aconfig @@ -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 + } +} diff --git a/core/java/android/os/BugreportManager.java b/core/java/android/os/BugreportManager.java index 960e84d671e7..a818df5f0a8e 100644 --- a/core/java/android/os/BugreportManager.java +++ b/core/java/android/os/BugreportManager.java @@ -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(); diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java index 4579168d2784..050a3704df1f 100644 --- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java +++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java @@ -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); diff --git a/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java b/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java index 71a7d0d2a638..f07b7106c0d4 100644 --- a/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java +++ b/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java @@ -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); } diff --git a/services/tests/servicestests/src/com/android/server/os/BugreportManagerServiceImplTest.java b/services/tests/servicestests/src/com/android/server/os/BugreportManagerServiceImplTest.java index 9862663c37b2..1db97b9ede81 100644 --- a/services/tests/servicestests/src/com/android/server/os/BugreportManagerServiceImplTest.java +++ b/services/tests/servicestests/src/com/android/server/os/BugreportManagerServiceImplTest.java @@ -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); |