sameerkapps / SecureStorage

119 stars 32 forks source link

Use the Android Keystore provider #37

Open mrts opened 6 years ago

mrts commented 6 years ago

Currently the Android keystore implementation is essentially not secure as there is no secure way to manage keys inside the app. Using the hardware serial number as the key provides no protection against knowledgeable attackers who root the phone. They are able to use the serial number themselves to decrypt the secrets file and get full access to stored secrets. Embedding the key in the app binary and using a key obfuscation system like Dotfuscator makes key retrieval a little harder, but a knowledgeable and persistent attacker is still able to reverse-engineer the obfuscation.

Since API level 18, Android provides the Android Keystore provider. Using the Android Keystore provider lets an individual app store its own credentials that only the app itself can access. This provides a way for apps to manage credentials that are usable only by itself while providing the same security benefits that the KeyChain API provides for system-wide credentials. This method requires no user interaction to select the credentials and assures that key material never enters the application process. When an application performs cryptographic operations using an Android Keystore key, behind the scenes plaintext, ciphertext, and messages to be signed or verified are fed to a system process which carries out the cryptographic operations.

You don't need to provide a password since the OS will derive it from your lock screen PIN and other variables itself and the secrets remain secure even if the adversary has rooted the phone.

sameerkapps commented 6 years ago

Good suggestion. Will look into it.

mrts commented 6 years ago

Thanks!

mrts commented 6 years ago

Here is some inspiration from Cordova secure storage plug-in:

https://github.com/Crypho/cordova-plugin-secure-storage/blob/master/src/android/SecureStorage.java

sameerkapps commented 6 years ago

Done. Please check the latest package 2.5.0. It has some breaking changes. Also, the source code has been updated. Please let me know, how it goes and close the issue.

mrts commented 6 years ago

Excellent, many thanks! I will test and get back to you with results. It may take some time unfortunately...

large commented 6 years ago

@sameerkapps I upgraded in an existing project and it worked fine in debug mode. Tried a release and it crashed, Devicelog said something about a base64 string in the class that failed. Is this a common issue?

sameerkapps commented 6 years ago

I ran the sample app in release mode for all iOS+Android+UWP. Could not repro the issue. Can you send a sample please?

large commented 6 years ago

@sameerkapps I investigated it further and I believe the error was found.

When you have a variable that does not exists in the storage, it returns (null) right, but it fails when you provide a default value for a not-stored object: string sSecure = CrossSecureStorage.Current.GetValue("Secure"); //Secure == (null) string sSecure = CrossSecureStorage.Current.GetValue("Secure", "False"); //Exception thrown: System.FormatException: Invalid length for a Base-64 char array or string.

Reason it did work in debug-mode was and #release block that was triggered only on release build.

mrts commented 6 years ago

There's one thing though, quoting cordova-plugin-secure-storage README:

Users must have a secure screen-lock set.

The plugin will only work correctly if the user has sufficiently secure settings on the lock screen. If not, the plugin will fail to initialize and the failure callback will be called on init(). This is because in order to use the Android Credential Storage and create RSA keys the device needs to be somewhat secure.

In case of failure to initialize, the app developer should inform the user about the security requirements of her app and initialize again after the user has changed the screen-lock settings. To facilitate this, we provide secureDevice which will bring up the screen-lock settings and will call the success or failure callbacks depending on whether the user locked the screen appropriately.

Does the current implementation work and what is the security guarantee if the user does not have secure screen lock set?

MalmoIt commented 6 years ago

@large Experiencing the same issue, doing a null check as a workaround solved the issue for now.

large commented 5 years ago

@sameerkapps did you figure out the error on https://github.com/sameerkapps/SecureStorage/issues/37#issuecomment-392606489 ?