Android: SecureStorage Adding a value after calling RemoveAll() will not persist the value

[phillippschmedt]

Description

When I call Microsoft.Maui.Storage.SecureStorage.Default.RemoveAll() and then add a value, the value will disappear when restarting the app. Adding a value after restart will then again persist it properly.

Steps to Reproduce

Start Android App
Call Microsoft.Maui.Storage.SecureStorage.Default.RemoveAll();
Call: Microsoft.Maui.Storage.SecureStorage.Default.SetAsync(key, value)
Call: Microsoft.Maui.Storage.SecureStorage.Default.GetAsync(key) -> Returns the value
Restart App
Call: Microsoft.Maui.Storage.SecureStorage.Default.GetAsync(key) -> Value is gone
Call: Microsoft.Maui.Storage.SecureStorage.Default.SetAsync(key, value)
Call: Microsoft.Maui.Storage.SecureStorage.Default.GetAsync(key) -> Returns the value
Restart App:
Call: Microsoft.Maui.Storage.SecureStorage.Default.GetAsync(key) -> Returns the value

My conclusion is, that calling RemoveAll() will somehow break the SecureStorage until next app start

Link to public reproduction project repository

https://github.com/phillippschmedt/SecureStorageIssue (see MainPage.xaml.cs, use debugger to inspect)

Version with bug

8.0.6

Is this a regression from previous behavior?

Yes, this used to work in Xamarin.Forms

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

Tested on Android 13

Did you find any workaround?

Not yet

Relevant log output

No response

[phillippschmedt]

@jsuarezruiz I noticed you worked on a SecureStorage issue on Android before. Is there any chance you can have a look into this? Thank you!

[ghost]

Hi @phillippschmedt. We have added the "s/needs-repro" label to this issue, which indicates that we require steps and sample code to reproduce the issue before we can take further action. Please try to create a minimal sample project/solution or code samples which reproduce the issue, ideally as a GitHub repo that we can clone. See more details about creating repros here: https://github.com/dotnet/maui/blob/main/.github/repro.md

This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[PureWeen]

@phillippschmedt when restarting the app are you restarting through the debugger or just starting the app back up from the icon?

[phillippschmedt]

@PureWeen It happens in both cases.

[phillippschmedt]

@PureWeen I added a repo for easier reproduction.

[Redth]

I haven't been able to reproduce this myself. Is there a particular API level or device you are seeing this on?

[ghost]

Hi @phillippschmedt. We have added the "s/needs-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[phillippschmedt]

I haven't been able to reproduce this myself. Is there a particular API level or device you are seeing this on?

I can reproduce it on multiple android devices. But the easiest for you might be that I can reproduce it on a Pixel 5 API 30 Emulator. See my example project I provided for easier reproduction. #20292 seems to report the same issue too.

See the debugger on the first click, you can see that the "readNewKey" value is successfully written and fetched.

Now after restarting the app and trying to read the value again, you see the "readOldKey" is empty which is wrong since we wrote to it on line 19 on the first launch.

[Redth]

OK, thanks for the update - i was able to use API 30 simulator and your repro, and see the same behaviour locally, I'll take a closer look at the PR :)

[Redth]

I think we should actually be disposing and recreating the ISharedPreferences instance that GetEncryptedSharedPreferences() returns after each operation and not caching it in between.

[Redth]

A bit more info after digging further for future me and others:

In Xamarin.Essentials, we switched at one point from Commit to Apply: xamarin/Essentials#637 We did this as Commit basically synchronously flushes out the changes to disk and returns once this is done. This is 'safer' in the sense of ensuring clearing or updating a value persists, however there was some concern from users that it was too slow/blocking and instead we should use Apply since that updates the in memory state of the preferences immediately and returns.

What I think we are seeing here is in some cases, clearing out the editor gets persisted, but the change immediately after is not always, and if you force close the app, it may never be (looking in the source code for shared preferences on android, it seems like the flushing may only happen in activity pause - maybe this isn't getting called).

If we look at our preferences implementation, we don't cache the preferences instance like we do in secure storage (which was added in a PR https://github.com/dotnet/maui/pull/13940/files#diff-5a84fca1553652d053c48c47be8906cb4b7c450d25e77ec3b191eddcd4289b62R77-R81 ) and we don't seem to have the issue there.

We have a few options I think to 'fix' this...

1. We can stop caching the preferences provider - android caches this under the hood anyway, so there isn't a lot saved by doing this ourselves. This seems to fix the issue, but I'm still a bit unsure exactly why it does.
2. We could switch to using Commit() instead of apply for setting values and clearing values - this should in theory cause the state to be written more predictably, though it's not clear if this would still also require us to stop caching the preferences provider instance.

[phillippschmedt]

@Redth Thanks a lot for the quick solution. I can confirm that the problem does not exist in the nightly builds anymore.

[axylophon]

Is there a workaround other then waiting for the new version?

[stoff99]

@Redth is there any timeline when this fix will be rolled out?

[axylophon]

The .NET 8 update is currently blocked on our side because if this issue 😥

[phillippschmedt]

@stoff99 @axylophon I can’t help you with the release date but we are using the nightly builds to work around this.

[kasemov]

Will there be a solution to the problem in the upcoming updates, or must we resort to other methods?

[febinDonz]

Had same issue in Maui 8.0.7. Solved by adding pro guard settings.
Create a new file named proguard_maui.cfg in platform\Android.
set Build Action = ProguardConfiguration

Add below lines of code
-dontobfuscate
-keep class com.google.crypto.tink.proto.** { ; }
-keep class com.google.crypto.tink.proto.* { ; }
-keep class androidx.security.crypto.* { *; }