In remote bugreport collection, Shell sends REMOTE_BUGREPORT_DISPATCH to
DevicePolicyManagerService which in turn notifies Device Owners that a
bug report is ready for collection. There existed a threat where a
malicous user could spoof the REMOTE_BUGREPORT_DISPATCH broadcast via
ADB to send a crafted bugreport to the Device Owner. Securing
REMOTE_BUGREPORT_DISPATCH is not as easy as it appears: putting a
permission on REMOTE_BUGREPORT_DISPATCH does not work since both the
legitimate sender and the malicious user are UID_SHELL. Instead, we
introduces a nonce which was sent from DPMS to Shell when bugreport is
triggered, and DPM will only accept REMOTE_BUGREPORT_DISPATCH when
a matching nonce is seen.
Ignore-AOSP-First: security fix
Bug: 171495100
Test: atest DeviceOwnerTest#testRemoteBugreportWithTwoUsers
Test: atest DeviceOwnerTest#testAdminActionBookkeeping
Test: atest BugreportManagerTest
Change-Id: I7649b4f22b74647d152d76bb46d5ca70bfa3617d
In ag/13068228, we amended the bug report functionality for Wear to
include a share intent (ACTION_SEND_MULTIPLE) which would be sent once a
bug report is complete. However, this intent is not required as Wear
sends the bug report directly to WCS via TriggerBugReportService in
ClockworkShell.
TriggerBugReportService listens for the bug report finished action
"com.android.internal.intent.action.BUGREPORT_FINISHED" and sends the
bugreport files to WCS. A chooser activity is not launched on Wear.
This issue surfaced due to a silent FileNotFoundException in the logs.
This occurs because the bug report files are consumed and deleted by
TriggerBugReportService in the standard Wear bug repot flow.
By the time this share intent is read, the files no longer exist,
leading to the exception.
Most of the flow is illustrated in the following screenshot:
https://screenshot.googleplex.com/8kjMkirzvPyFHg7
The changes here does two things:
- remove code that introduced the share intent
- remove redundant code after above change
- add missing break statement
BUG: 208822859
Change-Id: I38d1ee961caf2eead105acb9e84101eef45df9b0
In rvc-wear-dev, bugreports were not shared to phone until user tapped
on the bugreport-complete notification. Now, we are sending bugreports
to phones immediately after bugreport is taken. We still retain the
bugreport-complete notification. The bugreport-complete notification
will have an action button that leads the user to a bugreport-warning
screen as long as the user has not opted out from seeing the warning.
If the user chooses not to see that message again (which they can do
in the warning screen), there will not be any action button on the
bugreport-complete notification, and the user can just swipe to
delete it. Refer to the look of the notification when no warning message
is to be displayed (https://screenshot.googleplex.com/JStqotLwCaTXeL9)
and when a warning message is to be displayed (https://screenshot.googleplex.com/AdGKWFqbky8Ad8s)
Note: this change does NOT affect the functionality of the
BugreportProgressService for any other device except Wear devices.
Bug: 163083307
Test: the described feature tested with salmon running rvc-wear-dev
Change-Id: If6890ef8cada60e454c9232d402bbdb8a1b0315e
Since the existing notification style sets maxLines to 2, the title was
being truncated and the percentage was thus not visible to the user.
As recommended by UX, we are updating the string so that it is no longer
truncated and the percentage text is visible.
Screenshots:
Before - https://screenshot.googleplex.com/9U6RKuMcnECQ3pV
After - https://screenshot.googleplex.com/8U9NCJHhR5qkRCD
BUG: 198747050
Change-Id: I2b94cc65e2b9c3af860dd35532e1060814f6a68e
(cherry picked from commit 262e835016e340e5fd7b5eb00b7ac6579ca127ae)
System Traces collected with the native System Tracing App (Traceur) are
now added under a separate directory (systraces) for Wear bugreports.
Note that this change is functionally no-op for non-Wear form-factors (changes only one log's wordings for non-Wear form-factors).
Bug: 183239853
Test: manual
Change-Id: Id6b9aa0d38a0d465b763ec86fb1192875379431c
(cherry picked from commit 7111510c691ccf20baf372dab5cea2ac30ae0c23)
Shell app exception resulted from two missing fields in BugreportInfo
when it's parceling. Application ran into the duplicate logic to send
notification and start foreground service then caused the exception.
Bug: 176624074
Test: Request a bugreport; kill the com.android.shell; \
tap the share notification
Change-Id: Ia80f5e77f5486addf9ea7822f566c3fd7d75f42a
There's a case that BugreportProgressService is invoked twice quickly,
and both services create the same bugreport file name. The later one
may delete current running bugreport file in its clean function,
when it detects another bugreport is running.
Bug: 174314124
Bug: 175287931
Test: atest BugreportReceiverTest
Change-Id: I5e1802c5912f4414f1ad3b8bdaf7c7420332b9d6
Merged-In: I5e1802c5912f4414f1ad3b8bdaf7c7420332b9d6
Calls onError function when bugreport is finished and file is empty.
Bug: 174314124
Bug: 175287931
Test: atest BugreportReceiverTest
Change-Id: I4542568fd2d2ad1c75c7c3b223accca4995938a3
Merged-In: I4542568fd2d2ad1c75c7c3b223accca4995938a3
- Implements a local binder and #onBind in the service to return the
service instance.
- Mocks BugreportManager in the service to avoid service interacting
with the dumpstate.
- Fixes dialog name field did not disable after service is finished.
- Fixes screenshot did not remove if the name is empty.
- Extends screenshot delay timeout in tests.
- Fine tune the UiBot to fit new bugreport notification.
- Removes obsolete cases in the tests.
Bug: 143130523
Bug: 175287931
Test: atest BugreportReceiverTest
Change-Id: Iae89206da1d08a10891503869bbbf1ce18d4e31f
Merged-In: Iae89206da1d08a10891503869bbbf1ce18d4e31f
Only alert bugreport notification at the begining and end.
Bug: 146135200
Bug: 175287931
Test: Manually changing the notification settings to default
from silent.
Change-Id: Ie955266ad8a7a27a9dc74743e276b5e9c7a2d6fb
Merged-In: Ie955266ad8a7a27a9dc74743e276b5e9c7a2d6fb
There's a case that BugreportProgressService is invoked twice quickly,
and both services create the same bugreport file name. The later one
may delete current running bugreport file in its clean function,
when it detects another bugreport is running.
Bug: 174314124
Test: atest BugreportReceiverTest
Change-Id: I5e1802c5912f4414f1ad3b8bdaf7c7420332b9d6
Calls onError function when bugreport is finished and file is empty.
Bug: 174314124
Test: atest BugreportReceiverTest
Change-Id: I4542568fd2d2ad1c75c7c3b223accca4995938a3
Explicitly set FLAG_IMMUTABLE for all PendingIntents
in BugreportProgressService.
Test: Builds
Bug: 170165227
Bug: 171830604
Change-Id: Iae1489b9c8fa323448e60615e99ad96aee1cd1d7
Some careful spot-checking has revealed common cases where it's
reasonable to blanket apply the new FLAG_IMMUTABLE option to newly
created PendingIntents. Specifically these situations:
-- Simple notification content clicks are immutable; there's no need
to communicate customized data back to the creator
-- Simple notification action clicks are immutable; there's no need
to communicate customized data back to the creator
-- Broadcast intents sent by AlarmManager are immutable; the system
dispatches them without customization.
Bug: 170165227, 170424283, 170425388, 170425877
Bug: 169791183, 170771965, 170226088, 170224928, 170767530
Test: none
Change-Id: I5ed68710d2ccad4635a30fd91136a9e6ad76a01d
There's a lot of confusing logic where 90 and 270 rotation values need
to be flipped to ensure the screenshot is taken the correct orientation.
There's also confusion what useIdentityTransform means, especially if a
non 0 rotation value is sent.
The cases screenshot cares about is the following:
1. Take screenshot in current display orientation
2. Take screenshot with 0 rotation so the caller can handle rotating the
screenshot themselves.
With these two cases in mind, remove the rotation value passed in for
screenshots. If useIdentityTransform is true, it will rotate the
screenshot so it's in the 0 orientation. If useIdentityTransform is
false, it will use the current display rotation.
This simplifies the caller logic since they no longer have to find the
current display rotation to ensure the screenshot is taken in the
current rotation. The callers can just request the screenshot with
useIdentityTransform set to false.
Test: adb shell screencap
Test: Power + volume screenshot
Test: Screen rotation
Fixes: 135942984
Change-Id: I3435ee8b5dac05e910ec1e695f398c5dcdcff9e9
- Implements a local binder and #onBind in the service to return the
service instance.
- Mocks BugreportManager in the service to avoid service interacting
with the dumpstate.
- Fixes dialog name field did not disable after service is finished.
- Fixes screenshot did not remove if the name is empty.
- Extends screenshot delay timeout in tests.
- Fine tune the UiBot to fit new bugreport notification.
- Removes obsolete cases in the tests.
Bug: 143130523
Test: atest BugreportReceiverTest
Change-Id: Iae89206da1d08a10891503869bbbf1ce18d4e31f
Only alert bugreport notification at the begining and end.
Bug: 146135200
Test: Manually changing the notification settings to default
from silent.
Change-Id: Ie955266ad8a7a27a9dc74743e276b5e9c7a2d6fb
We want to take a screenshot immediately, not after dumpsys finished.
This also gives the user some feedback that we took a screenshot when
they were paying attention, whereas the notification might go under
the radar if posted the 30-90 seconds later it will be shown
otherwise.
Also gives us the progress notification in the Android TV notification
tray, which helps the user keep track of what is going on and how long
it is expected to take.
Previously either no feedback was shown at all on key press, or a simple
Toast was shown on selecting the bugreport option from the TV Settings
app.
Test: Press DPAD_CENTER + BACK, observe screenshot and progress.
Fix: 151865303
Change-Id: I78753697a7867042d28364a6b6d61fdb96f8bc96
This reverts commit 6b1c4a9e09.
Bug: 149226068
Bug: 149307007
Bug: 149780604
Test: $ lunch aosp_crosshatch-userdebug; m
# flash and boot on a crosshatch
# Settings -> Apps & notifications -> Default apps -> Home apps
# Only Quickstep home is there; no Shell home any more
$ lunch csi_arm64-userdebug; m
# flash the resulted system.img on a crosshatch and boot
$ atest DeviceHealthTests # all passed
Change-Id: I927b7dae779da25589f2198e0480c0f2a788a7e4
Merged-In: I927b7dae779da25589f2198e0480c0f2a788a7e4
(cherry picked from commit 539298c1b9)
This reverts commit 6b1c4a9e09.
Bug: 149226068
Bug: 149307007
Bug: 149780604
Test: $ lunch aosp_crosshatch-userdebug; m
# flash and boot on a crosshatch
# Settings -> Apps & notifications -> Default apps -> Home apps
# Only Quickstep home is there; no Shell home any more
$ lunch csi_arm64-userdebug; m
# flash the resulted system.img on a crosshatch and boot
$ atest DeviceHealthTests # all passed
Change-Id: I927b7dae779da25589f2198e0480c0f2a788a7e4
Bug: 152839953
Test: Take another bugreport, while one is in progress.
Without this change: bugreport files for the second bugreport can be
seen in /bugreports
With this change: no file for the second bugreport can be seen. This
is the expected behavior.
Change-Id: I45c1f66f3a52ba9a31413cf9748bc5504eaa65d0
* Make Atomic variables final so it can never be re-assigned to a null
object and hence avoid NPE.
* Set the value of the variables instead of re-assigning the object.
Test: Manual
Bug: 152479372
Change-Id: I58279d589d6247c53155755223fd8863f1049257
Remove an old hack that was put in place as bugreport ID was updated
a bit later in the bugreport generation phase.
This change removes the race condition around triggering bugreports
almost instantly.
Bug: 152292912
Test: Trigger consecutive bugreports from ActivityManager WAI. (Fails
without this change)
Change-Id: Ib08bc2c7d8ae765894998e5bfa0d9b6f3efa088b
Note: bugreport Cancel is not the same as bugreport onError. Files are not
deleted onError, as it still has some information before the bugreport
crashed/stopped. Bugreport Cancel is triggered by the user, hence clean
up all the files.
Bug: 146994281
Test: Cancel interactive bugreport from UI, deletes the current
bugreport files
Change-Id: I2606e491cbaa20eaa60980e98af56a194bae2280
This is essential for bugreports that do not send progress updates.
Bug: 152343458
Test: Take and share full bugreport
Change-Id: Id634afd82a92c02d1cf96ffd719fff8670fb8709
shareTitle and shareDescription is passed by the caller of the
bugreport. Show these in the finished bugreport notification.
Pre-set shareTitle takes precedence over user modified title.
shareDescription and description are not related/dependant on each other
in any way.
Bug: 150333444
Test: Manual (by passing EXTRA_TITLE and EXTRA_DESCRIPTION from
ActivityManagerService when trigerring a bugreport)
Change-Id: I2bfd080aeee677cdc8d0af339d7ad4a29451c3e0
* Make fields (progress, lastUpdate and finished) atomic to make them
thread safe and also reduce getters/setters.
* Make lastProgress a private field in BugreportInfo.
* Move deleteScreenshots to BugreportInfo class.
* Make fields with lock protected getters/setters as private.
* Make fields that should not be changed as final.
Bug: 147033613
Test: manual
Merged-In: I8f0fb4865c1b7c5d62bebca3e250eee59b4e71f4
Change-Id: I8f0fb4865c1b7c5d62bebca3e250eee59b4e71f4
(cherry picked from commit fe7d1ab0cb)
- Let screenshot fd be null if Shell doesn't want to take default screenshot when bug report is triggered.
BUG:149525300
Test: Flash and test interactive/full bugreports generated using Shell, and Shell flow does not break during tests
Change-Id: I50908af63d74ed66156354ec0b5a237f7131061a
Merged-In: I50908af63d74ed66156354ec0b5a237f7131061a
Make methods that read/write mBugreportInfos lock protected and add
"Locked" suffix to the method name. BugreportInfo objects are read/write
in different functions, to keep the code clean, add lock protected
getter/setter methods.
Bug: 142217059
Test: Takes interactive/full bugreports as expected
Change-Id: Iaadd6c9dce5009e40dd015e3b62bcd6e36966e00
Handle Exceptions around file creation/open fd more gracefully inside
BugreportInfo class. This is communicated to the caller of BugreportInfo
by returning null for getFd() methods.
Test: builds
Change-Id: Ie241dce0810645e04b2492dc8b1bcbc3e9f8f664
