search

OWASP / owasp-mastg

The Mobile Application Security Testing Guide (MASTG) is a comprehensive manual for mobile app security testing and reverse engineering. It describes the technical processes for verifying the controls listed in the OWASP Mobile Application Security Verification Standard (MASVS).
https://mas.owasp.org/
Creative Commons Attribution Share Alike 4.0 International
11.66k stars 2.3k forks source link

Encrypted Shared Preferences not covered in MSTG-STORAGE-1 and MSTG-STORAGE-2 #1662

Open galapogos opened 4 years ago

galapogos commented 4 years ago

MSTG-STORAGE-1 and MSTG-STORAGE-2 lists proper usage of Shared Preferences, but doesn't provide guidelines for the new Encrypted Shared Preferences, which automatically encrypt the contents of the XML file.

cpholguera commented 4 years ago

Hi @galapogos , thanks for pointing that out! If you have experience using that feature, would you mind opening a PR including what's missing? That'd be a great help for us, we'll gladly review it :)

galapogos commented 4 years ago

I actually don't have experience using this, however I recently encountered an app using this while doing an assessment of it, and wanted to refer to MSTG on testing it, but didn't find anything, hence the issue. However, I did note that I could simply hook on the API call and get the decrypted data.

A-AFTAHI commented 4 years ago

I think that testing encrypted that is similar either they're stored in Shared Preferences or elsewhere. don't you think? Perhaps adding a section specifically for Shared_Prefs would be Good.

cpholguera commented 4 years ago

On a first step we should add a reference on usage of encrypted shared preferences in the current section. After that we could think if we need a separate section or a better sub-sections structure, I'm not sure if the current static and dynamic analysis sections even match.

cpholguera commented 4 years ago

I think we should update the section accordingly:

https://developer.android.com/topic/security/data

Do we have already something about using Jetpack and EncryptedFiles?

lwierzbicki commented 4 years ago

@cpholguera I think I can work on it, as mentioned in issues #947 and #949.

cpholguera commented 4 years ago

That'd be perfect, thank you so much!

jadeboer commented 4 years ago

The mechanism being proposed by the new security lib is super simple to use, but the lib itself is an alpha.

However, I would suggest the MSTG recommend its use when it is generally released. I would not recommend to any dev teams that they make use of alpha libraries in general.

https://developer.android.com/topic/security/data#edit-shared-preferences

lwierzbicki commented 4 years ago

@jadeboer thanks for being thorough :) Lib will make things easier in the future. But I've seen implementations which use the same pattern - master key is stored in Keystore (or at the end it was advised to be stored in Keystore ;) ) and it is used to encrypt keys and values (which are encoded with base64 and saved in shared preferences - app container).

jadeboer commented 4 years ago

Yep, the pattern is useful and this new security lib will also simplify consistent implementation of the pattern for developers. I just wanted to caution against recommending use of an alpha lib =)

galapogos commented 4 years ago

Actually I'm not sure if EncryptedSharedPreferences is even a good idea. It seems to only provide data at rest protection, but is susceptible to runtime hooking in order to either dump out the decrypted key/value pair, or the data encryption key of the key/value pair. Seems to give a false sense of security in this sense.

jadeboer commented 4 years ago

Hey @galapogos , I think there is still value here.

In my experience, teams will implement their own idea of something to call secured shared preferences, with varying tactics, code quality and resultant security outcomes. This new library's capability will provide consistent implementation, make use of the android key store for master keys, make use of enums for configuration options, etc - which are useful in my opinion.

galapogos commented 4 years ago

True, but I think we need to list out the caveats in the MSTG once it's included.

sushi2k commented 4 years ago

Actually I'm not sure if EncryptedSharedPreferences is even a good idea. It seems to only provide data at rest protection, but is susceptible to runtime hooking in order to either dump out the decrypted key/value pair, or the data encryption key of the key/value pair. Seems to give a false sense of security in this sense.

I agree with @jadeboer. It streamlines the effort to encrypt data and abstracts it so it's easier to use. @galapogos You are right it provides additional protection at rest and an attacker will always be able to hock into apps (it's always just a matter of time and skills of the attacker) to get the decrypted data in this case. Nevertheless it's a best practice and should be used once out of the alpha stage.

mkaraoz commented 4 years ago

Fyi, AndroidX Security library is now a release candidate.

https://mvnrepository.com/artifact/androidx.security/security-crypto/1.0.0-rc01