Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c6772277 authored by Eric Biggers's avatar Eric Biggers
Browse files

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
parent 37873686

services/core/java/com/android/server/trust/TrustManagerService.java

+6 −1
Original line number Diff line number Diff line
@@ -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;

tests/TrustTests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -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",

tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt

+52 −0
Original line number Diff line number Diff line
@@ -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() {
@@ -85,6 +92,51 @@ class GrantAndRevokeTrustTest { 
        lockStateTrackingRule.assertLocked()

    }



    @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>()

tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt

+7 −0
Original line number Diff line number Diff line
@@ -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 }