OpenCS: Fix check for direct boot

[Flamefire]

IIUC isUserUnlocked makes only sense if we need to access user encrypted data, i.e. when isDirectBootEnabled/StorageManager.isFileEncryptedNativeOrEmulated returns true.

See https://developer.android.com/privacy-and-security/direct-boot#notification

Hence the check is the wrong way round: We want to exit if we need to unlock but are not yet

@shank03 Can you verify this?

[shank03]

Very sorry for delay...

@Flamefire Any particular scenario this change fixes ?

Because as far as I remember, it was when direct boot was not enabled check user unlocked would prevent from accessing locked private app data. Otherwise, direct boot allowed private app data access using broadcast.

[Flamefire]

check user unlocked would prevent from accessing

I don't understand this, can you rephrase that part?

My understanding is:

• DirectBoot is enabled: Things start before user unlocked the phone but you cannot access the users data until unlocked.
• DirectBoot is disabled: Things start only after full unlock

The relevant part is

Credential encrypted storage, which is the default storage location and only available after the user has unlocked the device. ... By default, apps don't run during Direct Boot mode. If your app needs to take action during Direct Boot mode, you can register app components to be run during this mode.

So I'm assuming that if we need to check to check for Direct Boot we may run "during Direct Boot mode", i.e. when the user might not have have unlocked.

So why should you check for unlocked if DirectBoot is disabled? If it is disabled all storage locations should be available at all times as far as I read the above doc

[shank03]

• DirectBoot is disabled: Things start only after full unlock

Exactly, that's why we check whether user is unlocked. If direct boot is disabled and user is locked, we can't continue because storage is locked until user unlocks, else direct boot gives option to createDeviceProtectedStorageContext using which we can access data even when locked.

And also any particular scenario this change fixes ?

[Flamefire]

If direct boot is disabled and user is locked, we can't continue because storage is locked until user unlocks, else direct boot gives option to createDeviceProtectedStorageContext using which we can access data even when locked.

Ah I see. I was under the impression the whole service won't start if direct boot is disabled and the device is locked. If that isn't the case then it makes sense especially in the context that createDeviceProtectedStorageContext is used iff directboot is enabled.

So no need for that.

And also any particular scenario this change fixes?

I was doing some major refactoring in my fork with the intention to make it more readable before starting to fix/improve things. E.g. the network switcher is too fast switching back. Also using a fallback for the modem seems to make IMS registration work on carriers where it didn't before: https://github.com/Flamefire/android_device_sony_yoshino-common/commit/3366f21c7826ea3f7131b9179e50ec2e6bd622ff

I just noticed this in particular recently as I misunderstood the intention (see above). Of course you can cherry-pick my changes or I can try to rebase them on this branch but the whole thing is rather large and might be...opinionated. ;-)

[shank03]

Also using a fallback for the modem seems to make IMS registration work on carriers where it didn't before: Flamefire@3366f21

Are you sure this doesn't break the ones which were already working when defaulting back to platform respective modems ?

And with the clarity of direct boot and user locked condition, is this PR change still required ?

[Flamefire]

Are you sure this doesn't break the ones which were already working when defaulting back to platform respective modems ?

I'd need to check again when that default would actually be used. But users can still set them as they wish if that doesn't work. So far there are reports on XDA that this change improves IMS compatibility.

[shank03]

Other than that, is this PR still needed ?

[Flamefire]

No, it was a misunderstanding from my side, I meant to close it with the last comment already