add visibility check before resuming top activity from user switch #164
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ANR stack traces file contains stack traces of all app's threads and of all threads of relevant or possibly relevant system processes, such as system_server.
Access to these files is controlled by their SELinux policy. They are labeled as anr_data_file. Enforcing additional read restrictions for ANR stack traces files through Unix permissions prevented LogViewer app from accessing them, since it doesn't run as the highly privileged UID 1000 (android.uid.system) which owns these files.
Adds a "Show details" item to crash and ANR (app not responding) dialogs which opens the LogViewer app.
…ystem_server side Don't require reboot or settings re-set for always on and lockdown to take effect on first vpn connection. The requirement for reboot, re-set at settings has been caused by a permission not granted or declared by VpnDialogs, which caused the unexpected behavior. Prevent this by checking the permission of local process instead.
In the general case, ContentProvider authorities can't be renamed because they aren't required to be based on the package name. Chromium always forms ContentProvider authorities by prefixing them with its package name, and relies on this invariant in code. When its package is renamed by original-package handling code, statements like String authority = context.getPackageName() + CONSTANT become invalid. Add a special-case for Vanadium to fix this.
This resolves the NPE crash when updating an application under the new package name that initially had the original package as package name.
This is needed for properly verifying updates of system packages.
versionCode of many system packages, including privileged ones, is set to the current SDK version and is thus not incremented during non-major OS upgrades. This allowed to downgrade them to the older version that had the same versionCode.
Change-Id: I5ccc4d61e52ac11ef33f44618d0e610089885b87 Squashed with: Author: Daniel Micay <[email protected]> Date: Wed Mar 15 06:32:20 2023 -0400 simplify removal of SUPL IMSI / phone number This is not required for SUPL to work and the comment about Google is unnecessary.
Adds a global data structure that is accessible by privileged installers and allows them to avoid installing the same package at the same time.
Applies to device PIN, SIM PIN and SIM PUK input screens.
This setting disables animations in keyguard PIN input UI.
This allows apps that have minor dependency on GmsCore (such as Pixel Camera) to work without having GmsCore installed.
Depends on commit: "don't crash apps that depend on missing Gservices provider"
- isolate EuiccGoogle from all non-system package via AppsFilter, which stops it from sending data to Google through GmsCore. EuiccGoogle doesn't send data to Google directly - keep EuiccGoogle disabled by default, but do not disable it after each reboot - remove a misleading "device information will be sent to Google" message that appears before eSIM download
Google's LPA that is shipped on GrapheneOS handles requesting the Camera permission at runtime, which allows the user to give it a one-time grant.
Android Auto requires this broadcast to be sent to complete its initialization.
Build.getSerial() is protected with the very broad READ_PRIVILEGED_PHONE_STATE permission. Android Auto needs Build.getSerial() in some cases, but doesn't need most of the other privileges that READ_PRIVILEGED_PHONE_STATE grants.
This was not directly exploitable due to there being 2 layers of update package signature verification and downgrade protection, but the first layer of protection should work properly to avoid a vulnerability in the 2nd layer being exploited.
Always load ApplicationInfo object needed for RemoteViews Contexts directly from PackageManager. The key used is the package name. Previously this object was read from the RemoteViews bundle, which was provided by the Widget providing app, and this object could not be relied on to have accurate data fields. Bug: 376028556 Flag: EXEMPT Security Fix Test: atest CtsWidgetTestCases:RemoteViewsActivityTest#testApplicationInfo (cherry picked from commit 352fb48) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f2251f1222e59b68d083a016bcc07d7c96980aab) Merged-In: Ie263b51fd2c2bdbf9d622533bb3f77d9f3f7181e Change-Id: Ie263b51fd2c2bdbf9d622533bb3f77d9f3f7181e
This fixes the following error on duplicate permissions across
HardeningTestApp, potentially caused by aapt2 peculiarity:
```
RUNNER ERROR: com.android.tradefed.targetprep.TargetSetupError[APK_INSTALLATION_FAILED|520001|DEPENDENCY_ISSUE]: Failed to install app.grapheneos.hardeningtest.sdk_27 with [${TMP_DIR}/HardeningTestAppSdk27.apk] on $SERIAL_NUMBER. Reason: 'INSTALL_FAILED_DUPLICATE_PERMISSION: Package app.grapheneos.hardeningtest.sdk_27 attempting to redeclare permission app.grapheneos.hardeningtest.DYNAMIC_RECEIVER_NOT_EXPORTED_PERMISSION already owned by app.grapheneos.hardeningtest.preinstalled' [$SERIAL_NUMBER $TARGET_PRODUCT:$TARGET_PRODUCT $BUILD_NUMBER]
at com.android.tradefed.targetprep.TestAppInstallSetup.installSinglePackage(TestAppInstallSetup.java:595)
at com.android.tradefed.targetprep.TestAppInstallSetup.installer(TestAppInstallSetup.java:562)
at com.android.tradefed.targetprep.TestAppInstallSetup.setUp(TestAppInstallSetup.java:429)
at com.android.tradefed.testtype.suite.ModuleDefinition.runPreparerSetup(ModuleDefinition.java:1048)
at com.android.tradefed.testtype.suite.ModuleDefinition.runTargetPreparation(ModuleDefinition.java:1517)
at com.android.tradefed.testtype.suite.ModuleDefinition.runPreparation(ModuleDefinition.java:1007)
at com.android.tradefed.testtype.suite.ModuleDefinition.run(ModuleDefinition.java:515)
at com.android.tradefed.testtype.suite.ITestSuite.runSingleModule(ITestSuite.java:1277)
at com.android.tradefed.testtype.suite.ITestSuite.run(ITestSuite.java:1044)
at com.android.tradefed.invoker.InvocationExecution.runTest(InvocationExecution.java:1457)
at com.android.tradefed.invoker.InvocationExecution.runTests(InvocationExecution.java:1232)
at com.android.tradefed.invoker.TestInvocation.prepareAndRun(TestInvocation.java:636)
at com.android.tradefed.invoker.TestInvocation.performInvocation(TestInvocation.java:291)
at com.android.tradefed.invoker.TestInvocation.invoke(TestInvocation.java:1429)
at com.android.tradefed.command.CommandScheduler$InvocationThread.run(CommandScheduler.java:692)
```
Modify the codepath for resuming top activities after a user switch so the activity that was left open when the user is locked isn't unconditionally resumed after switching away and switching back to the user. The keyguard visibility is checked so that apps that explicitly opt into showing an activity when locked are still shown when switching back. Test: atest FrameworksServicesTests:UserControllerTest WmTests:ActivityTaskManagerServiceTests WmTests:RootWindowContainerTests Test: manual
| if (!mTaskSupervisor.readyToResume()) { | ||
| return false; | ||
| } | ||
|
|
||
| boolean result = false; | ||
| if (targetRootTask != null && (targetRootTask.isTopRootTaskInDisplayArea() | ||
| if (!isAfterUserForegrounded && targetRootTask != null && | ||
| (targetRootTask.isTopRootTaskInDisplayArea() |
There was a problem hiding this comment.
Although this method is currently always called with targetRootTask == null when isAfterUserForegrounded == true, should this condition also include shouldResumeForUserForeground(targetRootTask) similar to the two other checks below?
There was a problem hiding this comment.
It looks like a non-null targetRootTask is being used by startActivityInner inside of ActivityStarter, so it might be called very often:
I wanted to modify the least amount of code paths to fix the underlying issue of top activities being resumed on user switch. I think for something like general hardening of Activities to try to prevent them from ever being resumed with keyguard active, it should be in a separate PR (requires more testing).
thestinger
force-pushed
the
15-qpr2
branch
10 times, most recently
from
3ec1472 to
ccaf1e6
Compare
thestinger
force-pushed
the
15-qpr2
branch
3 times, most recently
from
44f6627 to
c0c2721
Compare
thestinger
force-pushed
the
15-qpr2
branch
4 times, most recently
from
fe7b0c1 to
0c8ac15
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes GrapheneOS/os-issue-tracker#5031
Modify the codepath for resuming top activities after a user switch so the activity that was left open when the user is locked isn't unconditionally resumed after switching away and switching back to the user. The keyguard visibility is checked so that apps that explicitly opt into showing an activity when locked are still shown when switching back.
Test:
atest FrameworksServicesTests:UserControllerTest WmTests:ActivityTaskManagerServiceTests WmTests:RootWindowContainerTestsTest: manual