summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Eric Biggers <ebiggers@google.com> 2023-09-06 17:07:54 +0000
committer Eric Biggers <ebiggers@google.com> 2023-09-08 04:02:53 +0000
commitc677227767d6c500c4a17c2008b195c57c77a40f (patch)
tree149ae259b2ab039b6d51ae0311c5e5b408e5dca3
parent3787368695d233e8ff2a2be3e805b5ff4545b384 (diff)
Fix refreshDeviceLockedForUser() to use correct trust state
TrustManagerService#refreshDeviceLockedForUser() incorrectly considers the device to be unlocked by a trust agent whenever a trust agent has granted trust. This ignores the conditions that TrustManagerService#updateTrust() has for recognizing trust grants. This code used to be correct, but it became incorrect in Android 10 when trust agents were made to extend unlock rather than actively unlock. The correct state is sent to Keyguard, while the incorrect state is sent to Keystore. This would cause UnlockedDeviceRequired keys to sometimes be usable when the device is locked, though since Android 12 this bug is hidden by other bugs with UnlockedDeviceRequired keys that make them unusable in many cases. However, these bugs are planned to be fixed. Therefore, fix this bug by making refreshDeviceLockedForUser() use mUserTrustState, which holds the user's authoritative trust state. Bug: 296464083 Bug: 298249081 Flag: 296464083 Test: adb shell device_config put hardware_backed_security android.security.fix_unlocked_device_required_keys true atest TrustTests adb shell device_config put hardware_backed_security android.security.fix_unlocked_device_required_keys false atest TrustTests Change-Id: I0880685c23ebe71a799671fa611fafb42642fa83
-rw-r--r--services/core/java/com/android/server/trust/TrustManagerService.java7
-rw-r--r--tests/TrustTests/Android.bp1
-rw-r--r--tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt52
-rw-r--r--tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt7
4 files changed, 66 insertions, 1 deletions
diff --git a/services/core/java/com/android/server/trust/TrustManagerService.java b/services/core/java/com/android/server/trust/TrustManagerService.java
index 15b63ec9356e..a3f3ff5f79f0 100644
--- a/services/core/java/com/android/server/trust/TrustManagerService.java
+++ b/services/core/java/com/android/server/trust/TrustManagerService.java
@@ -870,7 +870,12 @@ public class TrustManagerService extends SystemService {
continue;
}
- boolean trusted = aggregateIsTrusted(id);
+ final boolean trusted;
+ if (android.security.Flags.fixUnlockedDeviceRequiredKeys()) {
+ trusted = getUserTrustStateInner(id) == TrustState.TRUSTED;
+ } else {
+ trusted = aggregateIsTrusted(id);
+ }
boolean showingKeyguard = true;
boolean biometricAuthenticated = false;
boolean currentUserIsUnlocked = false;
diff --git a/tests/TrustTests/Android.bp b/tests/TrustTests/Android.bp
index a1b888aef934..c216bced81f0 100644
--- a/tests/TrustTests/Android.bp
+++ b/tests/TrustTests/Android.bp
@@ -25,6 +25,7 @@ android_test {
"androidx.test.rules",
"androidx.test.ext.junit",
"androidx.test.uiautomator_uiautomator",
+ "flag-junit",
"mockito-target-minus-junit4",
"servicestests-utils",
"truth-prebuilt",
diff --git a/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt b/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt
index a7b8c4faba45..1dfd5c06167b 100644
--- a/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt
+++ b/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt
@@ -16,6 +16,10 @@
package android.trust.test
+import android.content.pm.PackageManager
+import android.platform.test.annotations.RequiresFlagsDisabled
+import android.platform.test.annotations.RequiresFlagsEnabled
+import android.platform.test.flag.junit.DeviceFlagsValueProvider
import android.service.trust.GrantTrustResult
import android.trust.BaseTrustAgentService
import android.trust.TrustTestActivity
@@ -27,6 +31,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation
import androidx.test.uiautomator.UiDevice
import com.android.server.testutils.mock
+import org.junit.Assume.assumeFalse
import org.junit.Before
import org.junit.Rule
import org.junit.Test
@@ -45,6 +50,7 @@ class GrantAndRevokeTrustTest {
private val activityScenarioRule = ActivityScenarioRule(TrustTestActivity::class.java)
private val lockStateTrackingRule = LockStateTrackingRule()
private val trustAgentRule = TrustAgentRule<GrantAndRevokeTrustAgent>()
+ private val packageManager = getInstrumentation().getTargetContext().getPackageManager()
@get:Rule
val rule: RuleChain = RuleChain
@@ -52,6 +58,7 @@ class GrantAndRevokeTrustTest {
.around(ScreenLockRule())
.around(lockStateTrackingRule)
.around(trustAgentRule)
+ .around(DeviceFlagsValueProvider.createCheckFlagsRule())
@Before
fun manageTrust() {
@@ -86,6 +93,51 @@ class GrantAndRevokeTrustTest {
}
@Test
+ @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS)
+ fun grantCannotActivelyUnlockDevice() {
+ // On automotive, trust agents can actively unlock the device.
+ assumeFalse(packageManager.hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE))
+
+ // Lock the device.
+ uiDevice.sleep()
+ lockStateTrackingRule.assertLocked()
+
+ // Grant trust.
+ trustAgentRule.agent.grantTrust(GRANT_MESSAGE, 10000, 0) {}
+
+ // The grant should not have unlocked the device. Wait a bit so that
+ // TrustManagerService probably will have finished processing the grant.
+ await()
+ lockStateTrackingRule.assertLocked()
+
+ // Turn the screen on and off to cause TrustManagerService to refresh
+ // its deviceLocked state. Then verify the state is still locked. This
+ // part failed before the fix for b/296464083.
+ uiDevice.wakeUp()
+ uiDevice.sleep()
+ await()
+ lockStateTrackingRule.assertLocked()
+ }
+
+ @Test
+ @RequiresFlagsDisabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS)
+ fun grantCouldCauseWrongDeviceLockedStateDueToBug() {
+ // On automotive, trust agents can actively unlock the device.
+ assumeFalse(packageManager.hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE))
+
+ // Verify that b/296464083 exists. That is, when the device is locked
+ // and a trust agent grants trust, the deviceLocked state incorrectly
+ // becomes false even though the device correctly remains locked.
+ uiDevice.sleep()
+ lockStateTrackingRule.assertLocked()
+ trustAgentRule.agent.grantTrust(GRANT_MESSAGE, 10000, 0) {}
+ uiDevice.wakeUp()
+ uiDevice.sleep()
+ await()
+ lockStateTrackingRule.assertUnlockedButNotReally()
+ }
+
+ @Test
fun grantDoesNotCallBack() {
val callback = mock<(GrantTrustResult) -> Unit>()
trustAgentRule.agent.grantTrust(GRANT_MESSAGE, 0, 0, callback)
diff --git a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
index afcd1d3194f8..583cfc781bd0 100644
--- a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
+++ b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
@@ -64,6 +64,13 @@ class LockStateTrackingRule : TestRule {
wait("not trusted") { trustState.trusted == false }
}
+ // TODO(b/299298338) remove this when removing FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS
+ fun assertUnlockedButNotReally() {
+ wait("device unlocked") { !keyguardManager.isDeviceLocked }
+ wait("not trusted") { trustState.trusted == false }
+ wait("keyguard locked") { windowManager.isKeyguardLocked }
+ }
+
fun assertUnlockedAndTrusted() {
wait("device unlocked") { !keyguardManager.isDeviceLocked }
wait("trusted") { trustState.trusted == true }