Merge "Pass caller information in cancelBugreport." am: 21d36ec4f5

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1552882

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: Iec3425e57af20906ba0459ba96b06fccedfb8954
This commit is contained in:
Treehugger Robot
2021-01-18 10:30:54 +00:00
committed by Automerger Merge Worker
3 changed files with 138 additions and 12 deletions

View File

@@ -26,7 +26,6 @@ import android.annotation.SystemApi;
import android.annotation.SystemService;
import android.app.ActivityManager;
import android.content.Context;
import android.os.Handler;
import android.util.Log;
import android.widget.Toast;
@@ -189,13 +188,18 @@ public final class BugreportManager {
}
}
/*
* Cancels a currently running bugreport.
/**
* Cancels the currently running bugreport.
*
* <p>Apps are only able to cancel their own bugreports. App A cannot cancel a bugreport started
* by app B.
*
* @throws SecurityException if trying to cancel another app's bugreport in progress
*/
@RequiresPermission(android.Manifest.permission.DUMP)
public void cancelBugreport() {
try {
mBinder.cancelBugreport();
mBinder.cancelBugreport(-1 /* callingUid */, mContext.getOpPackageName());
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}

View File

@@ -23,7 +23,10 @@ import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import android.Manifest;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.os.BugreportManager;
import android.os.BugreportManager.BugreportCallback;
import android.os.BugreportParams;
@@ -31,7 +34,9 @@ import android.os.FileUtils;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.ParcelFileDescriptor;
import android.os.Process;
import android.os.StrictMode;
import android.text.TextUtils;
import android.util.Log;
import androidx.annotation.NonNull;
@@ -53,10 +58,11 @@ import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import java.io.File;
import java.io.IOException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
/**
* Tests for BugreportManager API.
*/
@@ -67,8 +73,16 @@ public class BugreportManagerTest {
private static final String TAG = "BugreportManagerTest";
private static final long BUGREPORT_TIMEOUT_MS = TimeUnit.MINUTES.toMillis(10);
private static final long DUMPSTATE_STARTUP_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10);
private static final long UIAUTOMATOR_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10);
// Sent by Shell when its bugreport finishes (contains final bugreport/screenshot file name
// associated with the bugreport).
private static final String INTENT_BUGREPORT_FINISHED =
"com.android.internal.intent.action.BUGREPORT_FINISHED";
private static final String EXTRA_BUGREPORT = "android.intent.extra.BUGREPORT";
private static final String EXTRA_SCREENSHOT = "android.intent.extra.SCREENSHOT";
private Handler mHandler;
private Executor mExecutor;
private BugreportManager mBrm;
@@ -211,6 +225,48 @@ public class BugreportManagerTest {
assertFdsAreClosed(mBugreportFd, mScreenshotFd);
}
@Test
public void cancelBugreport_noReportStarted() throws Exception {
// Without the native DumpstateService running, we don't get a SecurityException.
mBrm.cancelBugreport();
}
@LargeTest
@Test
public void cancelBugreport_fromDifferentUid() throws Exception {
assertThat(Process.myUid()).isNotEqualTo(Process.SHELL_UID);
// Start a bugreport through ActivityManager's shell command - this starts a BR from the
// shell UID rather than our own.
BugreportBroadcastReceiver br = new BugreportBroadcastReceiver();
InstrumentationRegistry.getContext()
.registerReceiver(br, new IntentFilter(INTENT_BUGREPORT_FINISHED));
UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
.executeShellCommand("am bug-report");
// The command triggers the report through a broadcast, so wait until dumpstate actually
// starts up, which may take a bit.
waitTillDumpstateRunningOrTimeout();
try {
mBrm.cancelBugreport();
fail("Expected cancelBugreport to throw SecurityException when report started by "
+ "different UID");
} catch (SecurityException expected) {
} finally {
// Do this in the finally block so that even if this test case fails, we don't break
// other test cases unexpectedly due to the still-running shell report.
try {
// The shell's BR is still running and should complete successfully.
br.waitForBugreportFinished();
} finally {
// The latch may fail for a number of reasons but we still need to unregister the
// BroadcastReceiver.
InstrumentationRegistry.getContext().unregisterReceiver(br);
}
}
}
@Test
public void insufficientPermissions_throwsException() throws Exception {
dropPermissions();
@@ -346,6 +402,28 @@ public class BugreportManagerTest {
.adoptShellPermissionIdentity(Manifest.permission.DUMP);
}
private static boolean isDumpstateRunning() {
String[] output;
try {
output =
UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
.executeShellCommand("ps -A -o NAME | grep dumpstate")
.trim()
.split("\n");
} catch (IOException e) {
Log.w(TAG, "Failed to check if dumpstate is running", e);
return false;
}
for (String line : output) {
// Check for an exact match since there may be other things that contain "dumpstate" as
// a substring (e.g. the dumpstate HAL).
if (TextUtils.equals("dumpstate", line)) {
return true;
}
}
return false;
}
private static void assertFdIsClosed(ParcelFileDescriptor pfd) {
try {
int fd = pfd.getFd();
@@ -364,18 +442,25 @@ public class BugreportManagerTest {
return System.currentTimeMillis();
}
private static boolean shouldTimeout(long startTimeMs) {
return now() - startTimeMs >= BUGREPORT_TIMEOUT_MS;
private static void waitTillDumpstateRunningOrTimeout() throws Exception {
long startTimeMs = now();
while (!isDumpstateRunning()) {
Thread.sleep(500 /* .5s */);
if (now() - startTimeMs >= DUMPSTATE_STARTUP_TIMEOUT_MS) {
break;
}
Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for dumpstate to start");
}
}
private static void waitTillDoneOrTimeout(BugreportCallbackImpl callback) throws Exception {
long startTimeMs = now();
while (!callback.isDone()) {
Thread.sleep(1000 /* 1s */);
if (shouldTimeout(startTimeMs)) {
if (now() - startTimeMs >= BUGREPORT_TIMEOUT_MS) {
break;
}
Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms");
Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for bugreport to finish");
}
}
@@ -450,6 +535,36 @@ public class BugreportManagerTest {
assertTrue(device.wait(Until.gone(consentTitleObj), UIAUTOMATOR_TIMEOUT_MS));
}
private class BugreportBroadcastReceiver extends BroadcastReceiver {
Intent mBugreportFinishedIntent = null;
final CountDownLatch mLatch;
BugreportBroadcastReceiver() {
mLatch = new CountDownLatch(1);
}
@Override
public void onReceive(Context context, Intent intent) {
setBugreportFinishedIntent(intent);
mLatch.countDown();
}
private void setBugreportFinishedIntent(Intent intent) {
mBugreportFinishedIntent = intent;
}
public Intent getBugreportFinishedIntent() {
return mBugreportFinishedIntent;
}
public void waitForBugreportFinished() throws Exception {
if (!mLatch.await(BUGREPORT_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
throw new Exception("Failed to receive BUGREPORT_FINISHED in "
+ BUGREPORT_TIMEOUT_MS + " ms.");
}
}
}
/**
* A rule to change strict mode vm policy temporarily till test method finished.
*

View File

@@ -94,9 +94,12 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
@Override
@RequiresPermission(android.Manifest.permission.DUMP)
public void cancelBugreport() {
public void cancelBugreport(int callingUidUnused, String callingPackage) {
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP,
"cancelBugreport");
int callingUid = Binder.getCallingUid();
mAppOps.checkPackage(callingUid, callingPackage);
synchronized (mLock) {
IDumpstate ds = getDumpstateBinderServiceLocked();
if (ds == null) {
@@ -104,7 +107,11 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
return;
}
try {
ds.cancelBugreport();
// Note: this may throw SecurityException back out to the caller if they aren't
// allowed to cancel the report, in which case we should NOT be setting ctl.stop,
// since that would unintentionally kill some other app's bugreport, which we
// specifically disallow.
ds.cancelBugreport(callingUid, callingPackage);
} catch (RemoteException e) {
Slog.e(TAG, "RemoteException in cancelBugreport", e);
}
@@ -182,7 +189,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
// lifecycle correctly. If we don't subsequent callers will get
// BUGREPORT_ERROR_ANOTHER_REPORT_IN_PROGRESS error.
// Note that listener will be notified by the death recipient below.
cancelBugreport();
cancelBugreport(callingUid, callingPackage);
}
}