- Add millmore@.
Note that this applies to B&R code across all git projects because other
backup-related OWNERS files refer to this one (bug 159055442).
Test: Treehugger
Change-Id: Ic5d2f521ebd7731e47495ae43e7e0d45144d6e69
http://ag/12885739 introduced a enforceCallingPermission(BACKUP) check
but callers of this API do not hold that permission. This CL fixes
this by changing the check to enforceCallingOrSelfPermission(BACKUP),
and clearing the binder identity in DevicePolicyManagerService, which
makes the system server process the owner of that call.
Bug: 158482162
Bug: 172466964
Test: atest com.android.cts.devicepolicy.{Device,Profile}OwnerTest#testBackupServiceEnabling
Change-Id: I11d863229c4d62a058aaf37446a694b9c73ae5b8
Merged-In: I11d863229c4d62a058aaf37446a694b9c73ae5b8
(cherry picked from commit 630dec9eb4)
Base CL ag/12885739 introduced unconditional enforcement of the BACKUP
permission for callers of BackupManagerService.isBackupServiceActive()
in the service, but dropped the enforcement on the app process side
(BackupManager).
This CL makes the behavior change conditional on a compat ChangeId.
Bug: 158482162
Test: Manually checked that an app similar to the code sample from
http://b/158482162#comment1 can reproduce the behavior.
This is true both before the base CL and after this CL, when
the app targets an old SDK version (26).
Test: Checked that both (a) before this CL, (b) after this CL where
the change is manually enabled for the app via the below commands,
the app runs into a SecurityException instead:
$ adb shell am compat enable 158482162 com.example.tester
$ adb shell dumpsys platform_compat | grep 158482162
ChangeId(158482162; name=IS_BACKUP_SERVICE_ACTIVE_ENFORCE_PERMISSION_IN_SERVICE; enableSinceTargetSdk=31; packageOverrides={com.example.tester=true})
Change-Id: I58e5d2a0b438296137fd76720636c8fdce740ded
Merged-In: I58e5d2a0b438296137fd76720636c8fdce740ded
(cherry picked from commit 7671f0d95c)
BackupManager runs in the client process, whereas BackupManagerService
runs in the system server process. Therefore, apps permissions need should
be enforced on the service side.
Bug: 158482162
Test: Manually checked that a sample app encounters SecurityException
after but not before this CL, when running code similar to the
sample in http://b/158482162#comment1 to figure out whether
isBackupServiceActive() for its own uid.
Change-Id: I59693819542a80a065a9c88373393b0ba0dbef65
Merged-In: I59693819542a80a065a9c88373393b0ba0dbef65
(cherry picked from commit b2ab2c414f)
Strictly speaking other owners for SettingsProvider/test/...
should exist but there aren't any right now, so it's not
worth keeping a per-file rule since I don't think that
those can be combined with include.
Bug: 159055442
Test: Treehugger
Change-Id: I38c9c227e16c91f72ee0fc761670f82160e46ae2
With the goal of reducing log spam, print only a summary 'Found stale
backup journal' messages instead of logging within the inner loop.
Previously, over 12k messages could be printed at a time from this
function.
Before this CL:
- a backup was scheduled for each packageName from each stale
journal
- one (or two, if MORE_DEBUG) message was logged for each
packageName in each journal file.
After this CL:
- packageNames are de-duplicated before scheduling backups or logging
(it's not clear to me whether duplicate packageNames previously
occurred, in practice).
- one message is logged for the number (if > 0) of stale journals.
- one message is logged for the number (including their names, if
MORE_DEBUG) of packages.
Bug: 161940947
Test: fewer 'Found state backup journal' messages printed
Merged-In: Ia1343e4cea31feb1eba9da561d20736eb5df0a14
Change-Id: Ia1343e4cea31feb1eba9da561d20736eb5df0a14
With the goal of reducing log spam, print only a summary 'Found stale
backup journal' messages instead of logging within the inner loop.
Previously, over 12k messages could be printed at a time from this
function.
Before this CL:
- a backup was scheduled for each packageName from each stale
journal
- one (or two, if MORE_DEBUG) message was logged for each
packageName in each journal file.
After this CL:
- packageNames are de-duplicated before scheduling backups or logging
(it's not clear to me whether duplicate packageNames previously
occurred, in practice).
- one message is logged for the number (if > 0) of stale journals.
- one message is logged for the number (including their names, if
MORE_DEBUG) of packages.
Bug: 161940947
Test: fewer 'Found state backup journal' messages printed
Merged-In: Ia1343e4cea31feb1eba9da561d20736eb5df0a14
Change-Id: Ia1343e4cea31feb1eba9da561d20736eb5df0a14
(cherry picked from commit 112e3c2d04)
Since Android 10, backupPm() includes sendDataToTransport(), which was
not previously the case. This means that error handling logic that
deletes the backup state file (causing initialize_device() on the next
attempt, which deletes any existing backup) will now also be triggered
upon errors during sendDataToTransport(), which wasn't previously
(Android <= 9) the case.
This has the potential of making an existing temporary outage much
worse:
1. A few devices might run into temporary issues, e.g. a B&R server
returning HTTP 503 Service Unavailable (treated as a
TransientHttpStatusException instanceof NetworkException, which
is mapped to TRANSPORT_ERROR during handleTransportStatus(),
which results in a TaskException with stateCompromised==false
but which backupPm() wraps in another TaskException that forces
stateCompromised=true).
2. On their next backup attempt, those devices throw away any
existing backup and start from scratch (initialize_device()),
increasing the load on the server.
3. This leads to a positive-feedback loop where more devices than
before run into HTTP 503 Service Unavailable.
4. As a result, masses of devices delete their backups and then
hammer the B&R server with attempts to upload new backups.
5. Backups are unavailable to any users who would otherwise rely
on them during this outage.
To improve on this dangerous situation, this CL changes the code to
force stateCompromised=true only for TaskExceptions thrown
specifically during extractPmAgentData(), and (as before) for all
AgentExceptions.
Note that the code is still quite brittle. It still seems like we
are probably forcing stateCompromised=true in too many situations,
but it's hard to say so this CL is being conservative about the
changes. Changing back to the old behavior could be done through
a local change around KeyValueBackupTask.java:676; a future CL may
do this to have a safety hatch in case we want to cherry-pick this
CL into an upcoming Android release late in the release cycle.
[1] https://android.googlesource.com/platform/frameworks/base/+/refs/heads/pie-dev/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java#1035
[2] https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java#1040
[3] https://source.corp.google.com/piper///depot/google3/java/com/google/android/gmscore/integ/modules/backup/transport/src/com/google/android/gms/backup/transport/GmsBackupTransport.java;l=770;rcl=281845876
Bug: 144030477
Test: Checked that the following passes after this CL, but
testRunTask_whenTransportReturnsErrorForPm_updatesFilesAndCleansUp()
fails if I revert the state of KeyValueBackupTask.java to before this CL:
ROBOTEST_FILTER=KeyValueBackupTaskTest make \
RunBackupFrameworksServicesRoboTests
Change-Id: I6c622c55fbd804ec0a12e0bea7ade1308f7a3877
(cherry picked from commit 819ed81faa)
Bug: 159648065
Test: N/A
Change the setting key to be 'backup_skip_user_facing_packages'. See the
bug for context.
Change-Id: I315141a509976821c7db311544a5c0f4e6fd1917
Previously, we attempt to pass ownership of the fd owned by a
ParcelFileDescriptor to a FileInputStream, which explodes when we try to
close it.
Bug: http://b/156867945
Bug: http://b/159264419
Test: treehugger
Change-Id: I9d5124658beb50f0a08499ed09e652037cb9ae66
(cherry picked from commit 3e29e4a39c)
https://r.android.com/950439 and http://ag/7949925 already attempted
this, but a whitespace difference remained.
This CL copies ('cp') the internal file to AOSP master; the Merged-In line
refers to the internal CL that added the extra empty line, which should
lead to automerger skipping this CL when merging into branches that have
the other CL.
Bug: 129861520
Bug: 159055442
Test: Treehugger
Change-Id: I3f83e223a1c7d812f74515008e4c6005ebc3dbe2
Merged-In: I42c27dfc0f3f792c46fe60e0f6807e51a2c3f21e
Previously, we attempt to pass ownership of the fd owned by a
ParcelFileDescriptor to a FileInputStream, which explodes when we try to
close it.
Bug: http://b/156867945
Test: treehugger
Change-Id: I9d5124658beb50f0a08499ed09e652037cb9ae66
Over the years we've had several obscure bugs related to how SDK level
comparisons are performed, specifically during the window of time
where we've started distributing the "frankenbuild" to developers.
Consider the case where a framework developer shipping release "R"
wants to only grant a specific behavior to modern apps; they could
write this in two different ways:
1. if (targetSdkVersion > Build.VERSION_CODES.Q) {
2. if (targetSdkVersion >= Build.VERSION_CODES.R) {
The safer of these two options is (2), which will ensure that
developers only get the behavior when *both* the app and the
platform concur on the specific SDK level having shipped.
Consider the breakage that would happen with option (1) if we
started shipping APKs that are based on the final R SDK, but are
then installed on earlier preview releases which still consider R
to be CUR_DEVELOPMENT; they'd risk crashing due to behaviors that
were never part of the official R SDK.
Bug: 64412239
Test: ./build/soong/soong_ui.bash --make-mode services RUN_ERROR_PRONE=true
Exempt-From-Owner-Approval: trivial blueprint changes
Change-Id: Ia20181f8602451ac9a719ea488d148e160708592
Add the user ID so it's easier to see which user log messages refer to.
There's a bit of churn around conditionals in this to ensure that the
code complies with the style guidelines.
Fixes: 148376687
Test: make RunBackupFrameworksServicesRoboTests
Change-Id: I3ca92d21492fae4b89cb73fb39db1a490c796f5d
This reverts commit 27c64a3bed.
Reason for revert: The GMSCore code to handle the flag will be part of DP2 and so we can remove the flag ahead of the cut.
GMSCore CL Status; https://cl-status.corp.google.com/#/summary/gmscore_prod/291378558
GMSCore Calendar; http://go/gms-schedule
Bug: 147481066
Test: m -j RunBackupFrameworksServicesRoboTests
Change-Id: I4159e064e739c6f366063c7fadd7cca40a7f07d9
Adding a log statement at debug level so when the key blocking system
is tested end-to-end there's an easy indicator of a key being blocked.
Bug: 149548272
Test: No code which affects functionality to test
Change-Id: I9de8a7d47ffb75520d1adcba620ec13345c1c4ae
Bug: 145126096
Test: atest KeyValueRestoreExclusionHostSideTest
atest PerformUnifiedRestoreHostSideTest
atest UserBackupPreferencesTest
Currently PerformUnifiedRestoreTask gets the list of blocked restore
keys at construction. However, at that point the list might not be fully
constructed yet. We should get the keys through the getter avaialble in
UserBMS when we need them.
Change-Id: I62ad34138ba7a893e66d6af05d2e242c9c964a44
The implementations of IBackupTransport are whitelisted therefore in the short-term,
its OK to allow these calls and avoid the warning logs which can be distracting when
looking at logs to investigate bugs and are not of any real value since we are already
aware of this.
Bug: 148783926
Test: atest -v CtsBackupTestCases
Test: atest -v CtsBackupHostTestCases
Change-Id: I13e2a638891d0369310bc2c665fa772306a28199
state.
Also add debug logging when reading or writing backup enabled state to
be able to better investigate bugs.
Bug: 148587496
Bug: 147352819
Test: atest -v RunBackupFrameworksServicesRoboTests
Test: atest -v $(find frameworks/base/services/tests/servicestests/src/com/android/server/backup -name '\''*Test.java'\'')'
Change-Id: I3c9b158ce57558daa5437cebe6aa0a0c924692fc
Bug: 145126096
Test: atest KeyValueRestoreExclusionHostSideTest
atest PerformUnifiedRestoreTaskTest
In a KV restore after getting data from the transport, we save it into a
stage file. Then we go through the keys and do filtering: skip the keys
that should be excluded and extract the widget data into a separate
file. The rest of the data is wirtten into the file where the app's
backup agent will read it from.
However, this process is skipped for 'android' package. It was done as
an optimization before the ability to exclude keys from restore was
introduced: as 'android' backup data doesn't contain any widget info.
However, now we need to process 'android' package as well because it can
contain keys to be excluded.
Change-Id: I612f8cc9c6903c9bd257762360dadb81ed12d106
This is so that we can add a CTS test which asserts through logcat
inspection that backup and restore operations acquire and release
wakelock in the expected manner.
Change-Id: I1a73bd674c22ad7f0f37aba100aee819abe9f4d2
This will allow us to turn off the new functionality while the
transport implementations are updated.
Bug: 147481066
Test: m -j RunBackupFrameworksServicesRoboTests
Change-Id: I8c0019ff80d94dd8d94299a7b03b78e3081f2b8e
In order to allow transports to know when a K/V backup would have
been performed, but was not due to no data changes being reported,
we need to introduce some code to find the K/V backup participants,
remove any which are backed up in this sweep or are currently failing
to back up due to an error, and then inform the transport of the
remaing set.
This CL introduces the code to do that.
We use a state file to determine if a package backed up without an
error on the last run, if the backup fails we remove the file for
that package. When all packages with changed data have been backed
up we get a list of all packages which have the file set (i.e. were
successful), remove the set of packages backed up on this run, and
inform the transport the rest had no data changes.
Bug:147481066
Test: m -j RunBackupFrameworksServicesRoboTests
Change-Id: I5c9f94c925096faf7b65307c0be1a7aba48c1cfb
Currently, the logic for backupnow is split between UserBackupManagerService,
RunBackupReceiver and BackupHandler. This makes it very confusing as
the pre checks are split over three classes and there's no obvious
reason why it is like that.
On the other hand, requestBackup is split nicely into checks in
UserBackupManagerService and starting the work from BackupHandler.
This change removes RunBackupReceiver for backupNow and splits the logic
only between UserBackupManager and BackupHandler - in the direction of
making it consistent with requestBackup by moving the checks to
UseBackupManagerService. (except the check for isBackupRunning which
needs to be in the same synchronised block as setBackupRunning).
Also, this CL moves the wakelock acquire from RunBackupReceiver (removed in this CL)
to BackupHandler. Previously, RunBackupReceiver would acquire the wakelock and then
send MSG_RUN_BACKUP. BackupHandler would then have to release that wakelock when it finds
it cant run the backup because there's no transport.
Now, the acquire is in BackupHandler after checking the transport. Therefore we dont need
that release (when there's no transport) anymore
A side benefit is we get rid of an extra hop to the broadcast receiver
so potentially backups should be scheduled (albeit very slightly)
faster.
Test: atest -v RunBackupFrameworksServicesRoboTests
Test: atest -v $(find frameworks/base/services/tests/servicestests/src/com/android/server/backup -name '\''*Test.java'\'')
Test: atest -v CtsBackupTestCases
Test: atest -v CtsBackupHostTestCases
Bug: 147741497
Change-Id: I40051540c7e8531ef05076eab7ccc5b44b0c08d2
The only two actions within the block are sending an intent and
canceling a job none of which have anything to do with the queuelock
Test: atest -v RunBackupFrameworksServicesRoboTests
Test: atest -v CtsBackupTestCases
Test: atest -v CtsBackupHostTestCases
Bug: 136738613
Change-Id: Ieedb73edf6cd40792232b48a26613b497c87ba02
previously marked as @Ignore.
The test was bad because it set 11 as the ancestral serial number for
the system user UserBackupManagerService but asserted that 11 corresponds
to user 1 - which is not the system user. This CL fixes the assert.
Also added corresponding test for non system user.
Fixes: 147012496
Test: atest com.android.server.backup.BackupManagerServiceTest
Change-Id: Iab736885264aa4befc644678e5fe66d602ed00e3
Currently backup dumpsys in bugreport only contains information
for system user making it hard to debug backup bugs in non-system
users. Therefore, add dumpsys information for all users running backup.
For system user, keep the dumpsys format the same for compatibility with
cts and gts tests. For non-system users, add a prefix "User <userid>:"
to all dumpsys headers.
The changes in Android.bp and AndroidManifest.xml are to support mocking
of the static method DumpUtils.checkDumpAndUsageStatsPermission in the
test testDump_systemUserFirst
Bug: 143867387
Test: atest com.android.server.backup.BackupManagerServiceTest
Test: atest com.android.server.backup.UserBackupManagerServiceTest
Test: adb shell pm create-user test1 -> say this gives 11
adb shell am start-user 11
adb shell bmgr --user 11 activate true
adb shell dumpsys backup users -> "Backup Manager is running for users: 0 11"
adb shell dumpsys backup -> contains both
"Backup Manager is enabled / setup complete / not pending init" and
"User 11:Backup Manager is disabled / not setup complete / not pending init"
adb shell pm remove-user 11
Test: "adb bugreport" on device with secondary user as created above and check that result
contains dumpsys for both system and secondary users.
Test: atest -v CtsBackupTestCases
Test: atest -v CtsBackupHostTestCases
Change-Id: Ib94c168f8e89b0ba8f398152ea744fe3d626efc4
Bug: 144431410
Test: 1. atest BackupHandlerTest
2. Manual (with and without the fix):
1) Locally create a host-side CTS test that extends
BaseMultiUserBackupHostSideTest
2) Modify the test so that it creates a user, starts backup
init and removes the user
3) Add log in BackupHandler to indicate when an exception is
suppressed.
3) If the fix is applied, verify the crash doesn't happen and
the log message from 3) is present. If the fix isn't applied
verify that the crash happens.
After backup service for a user is stopped, leftover work on the
corresponding BackupHandler can throw exceptions. If uncaught, they
can crash the system process. Catch all uncaught BackupHandler
exceptions after the backup service has entered the stopping state, to
allow any leftover work to finish harmlessly.
Change-Id: I8c233ad0e117ec0ae65599a762d87f15f8a3cec2
