summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Kholoud Mohamed <kholoudm@google.com> 2024-05-15 12:39:48 +0000
committer Kholoud Mohamed <kholoudm@google.com> 2024-05-20 13:49:44 +0000
commit3ce96dbe16087371b40f41088123f5e7f3f3812c (patch)
treee23722d971264be21109daa86a593e47361c5d42
parentc80cb5ed0ef8324a4fd94b63d1bbed57117fb946 (diff)
Skip user consent for subsequent reports
Bug: 340439309 Test: atest android.bugreport.cts_root.BugreportManagerTest Change-Id: Icb167a05052bd18fa410bc37f843a5706008f3e5
-rw-r--r--core/java/android/app/admin/flags/flags.aconfig10
-rw-r--r--core/java/android/os/BugreportManager.java4
-rw-r--r--packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java6
-rw-r--r--services/core/java/com/android/server/os/BugreportManagerServiceImpl.java112
-rw-r--r--services/tests/servicestests/src/com/android/server/os/BugreportManagerServiceImplTest.java14
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);