Merge changes I0880685c,I865ec19d,Id3a1e34b,I31c4f4d2,I17103be4 into main am: b2348adbb8 am: e0bb26a3fc am: 9d3ad7db0c am: f42a722278 am: c0e770107d

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

Change-Id: If6827f6023ae4d101b8774eeef715766d539779a
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

6 files changed, 98 insertions, 14 deletions

diff --git a/core/java/android/security/flags.aconfig b/core/java/android/security/flags.aconfig
index b27dac2de6af..b6c2b83f0daa 100644
--- a/core/java/android/security/flags.aconfig
+++ b/core/java/android/security/flags.aconfig
@@ -6,3 +6,10 @@ flag {
     description: "Feature flag for fs-verity API"
     bug: "285185747"
 }
+
+flag {
+    name: "fix_unlocked_device_required_keys"
+    namespace: "hardware_backed_security"
+    description: "Fix bugs in behavior of UnlockedDeviceRequired keystore keys"
+    bug: "296464083"
+}
diff --git a/services/core/java/com/android/server/trust/TrustManagerService.java b/services/core/java/com/android/server/trust/TrustManagerService.java
index 0b7f6db3987e..635e11be3a16 100644
--- a/services/core/java/com/android/server/trust/TrustManagerService.java
+++ b/services/core/java/com/android/server/trust/TrustManagerService.java
@@ -828,7 +828,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 f864fedf4e62..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() {
@@ -72,7 +79,7 @@ class GrantAndRevokeTrustTest {
         trustAgentRule.agent.grantTrust(GRANT_MESSAGE, 10000, 0) {}
         uiDevice.sleep()
 
-        lockStateTrackingRule.assertUnlocked()
+        lockStateTrackingRule.assertUnlockedAndTrusted()
     }
 
     @Test
@@ -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/TemporaryAndRenewableTrustTest.kt b/tests/TrustTests/src/android/trust/test/TemporaryAndRenewableTrustTest.kt
index ae722477a2bc..96362b8e71dc 100644
--- a/tests/TrustTests/src/android/trust/test/TemporaryAndRenewableTrustTest.kt
+++ b/tests/TrustTests/src/android/trust/test/TemporaryAndRenewableTrustTest.kt
@@ -102,7 +102,7 @@ class TemporaryAndRenewableTrustTest {
         trustAgentRule.agent.grantTrust(
             GRANT_MESSAGE, 0, FLAG_GRANT_TRUST_TEMPORARY_AND_RENEWABLE) {}
 
-        lockStateTrackingRule.assertUnlocked()
+        lockStateTrackingRule.assertUnlockedAndTrusted()
     }
 
     @Test
@@ -125,7 +125,7 @@ class TemporaryAndRenewableTrustTest {
             Log.i(TAG, "Callback received; status=${it.status}")
             result = it
         }
-        lockStateTrackingRule.assertUnlocked()
+        lockStateTrackingRule.assertUnlockedAndTrusted()
 
         wait("callback triggered") { result?.status == STATUS_UNLOCKED_BY_GRANT }
     }
diff --git a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
index c1a7bd9e1d8f..5a8f82827253 100644
--- a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
+++ b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
@@ -16,6 +16,7 @@
 
 package android.trust.test.lib
 
+import android.app.KeyguardManager
 import android.app.trust.TrustManager
 import android.content.Context
 import android.util.Log
@@ -26,18 +27,23 @@ import org.junit.runner.Description
 import org.junit.runners.model.Statement
 
 /**
- * Rule for tracking the lock state of the device based on events emitted to [TrustListener].
+ * Rule for tracking the trusted state of the device based on events emitted to
+ * [TrustListener].  Provides helper methods for verifying that the trusted
+ * state has a particular value and is consistent with (a) the keyguard "locked"
+ * (i.e. showing) value when applicable, and (b) the device locked value that is
+ * tracked by TrustManagerService and is queryable via KeyguardManager.
  */
 class LockStateTrackingRule : TestRule {
     private val context: Context = getApplicationContext()
     private val windowManager = checkNotNull(WindowManagerGlobal.getWindowManagerService())
+    private val keyguardManager = context.getSystemService(KeyguardManager::class.java) as KeyguardManager
 
-    @Volatile lateinit var lockState: LockState
+    @Volatile lateinit var trustState: TrustState
         private set
 
     override fun apply(base: Statement, description: Description) = object : Statement() {
         override fun evaluate() {
-            lockState = LockState(locked = windowManager.isKeyguardLocked)
+            trustState = TrustState()
             val trustManager = context.getSystemService(TrustManager::class.java) as TrustManager
             val listener = Listener()
 
@@ -51,12 +57,25 @@ class LockStateTrackingRule : TestRule {
     }
 
     fun assertLocked() {
-        wait("un-locked per TrustListener") { lockState.locked == true }
-        wait("keyguard lock") { windowManager.isKeyguardLocked }
+        wait("device locked") { keyguardManager.isDeviceLocked }
+        // isDeviceLocked implies isKeyguardLocked && !trusted.
+        wait("keyguard locked") { windowManager.isKeyguardLocked }
+        wait("not trusted") { trustState.trusted == false }
     }
 
-    fun assertUnlocked() {
-        wait("locked per TrustListener") { lockState.locked == 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 }
+        // Can't check for !isKeyguardLocked here, since isKeyguardLocked
+        // returns true in the case where the keyguard is dismissible with
+        // swipe, which is considered "device unlocked"!
     }
 
     inner class Listener : TestTrustListener() {
@@ -68,12 +87,12 @@ class LockStateTrackingRule : TestRule {
             trustGrantedMessages: MutableList<String>
         ) {
             Log.d(TAG, "Device became trusted=$enabled")
-            lockState = lockState.copy(locked = !enabled)
+            trustState = trustState.copy(trusted=enabled)
         }
     }
 
-    data class LockState(
-        val locked: Boolean? = null
+    data class TrustState(
+        val trusted: Boolean? = null
     )
 
     companion object {