Merge "Skip user consent for subsequent reports" into main (b050e836) · Commits · e / os / android_frameworks_base

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

+10 −0

Original line number	Diff line number	Diff line
		@@ -346,3 +346,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);