Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Android] SecureStorage: Rework logic to delete shared prefs when key is corrupt #23850

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

Redth
Copy link
Member

@Redth Redth commented Jul 26, 2024

Sometimes encrypted shared preferences can become unusable on android when backed up data gets migrated between different devices (and possibly in other scenarios).

We tried to work around this by catching one particular exception and calling PlatformRemoveAll() to try and delete the shared prefs so we could create a new set, however this logic was flawed since the error occurs when getting an instance of the encrypted shared preferences, which the PlatformRemoveAll attempts to do itself (so it would fail to get the thing to remove the thing).

This changes up the logic a bit and directly clears the shared preference that the encrypted one is stored in, without first trying to get an instance of the corrupt shared prefs.

It then also tries to directly create a new instance afterwards, and return that, in an attempt to make this failsafe / reset operation transparent to the original call to get or set a secure storage key/value.

Description of Change

Issues Fixed

Fixes #18230
Fixes #22094

@Redth Redth requested a review from a team as a code owner July 26, 2024 13:17
@Redth Redth requested review from Eilon and rmarinho July 26, 2024 13:17
@samhouts samhouts added the area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info label Jul 31, 2024
@Tviljan
Copy link

Tviljan commented Aug 21, 2024

Any ETA for this fix or should I just fix it in my code first while waiting for library update? It looks like android update can break the secure storage

@Redth
Copy link
Member Author

Redth commented Aug 22, 2024

/rebase

Sometimes encrypted shared preferences can become unusable on android when backed up data gets migrated between different devices (and possibly in other scenarios).

We tried to work around this by catching one particular exception and calling PlatformRemoveAll() to try and delete the shared prefs so we could create a new set, however this logic was flawed since the error occurs when getting an instance of the encrypted shared preferences, which the PlatformRemoveAll attempts to do itself (so it would fail to get the thing to remove the thing).

This changes up the logic a bit and directly clears the shared preference that the encrypted one is stored in, without first trying to get an instance of the corrupt shared prefs.

It then also tries to directly create a new instance afterwards, and return that, in an attempt to make this failsafe / reset operation transparent to the original call to get or set a secure storage key/value.
@github-actions github-actions bot force-pushed the dev/redth/fix-android-securestorage-corrupt-key branch from 65d3f94 to ceed3a0 Compare August 22, 2024 18:01
@Redth Redth requested a review from mattleibow August 22, 2024 18:02
@Redth
Copy link
Member Author

Redth commented Aug 22, 2024

It's worth pointing this out again here too:

#18230 (comment)

@Redth Redth requested a review from mattleibow August 22, 2024 20:14
@mattleibow
Copy link
Member

Failing tests unrelated, retrying but this can probably be merged.

@mattleibow mattleibow merged commit c6600c6 into main Aug 30, 2024
93 of 97 checks passed
@mattleibow mattleibow deleted the dev/redth/fix-android-securestorage-corrupt-key branch August 30, 2024 18:14
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Sep 5, 2024
@samhouts samhouts added fixed-in-9.0.0-rc.2.24503.2 fixed-in-8.0.90 and removed fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Oct 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.90 fixed-in-9.0.0-rc.2.24503.2
Projects
None yet
4 participants