Key management updates in "Testing cryptography", "Testing data storage" for iOS and Android #1884
Conversation
* Update "Protecting Keys in Memory" adding more information of keys clean up and key wrapping techniques. * Clarify IV usage, add note about popular mistake of copy-pasting example code with hardcoded IVs from OSS crypto libs. * Mention Blake3 * Add @veorq cryptocoding guidelines as reference for zeroing memory. These changes were discussed with @sushi2k and @julepka in a separate doc.
|
markdown check is failing, but due to links in other places |
|
the Great BLAKE creator @veorq told me that BLAKE should be written as BLAKE not Blake. I obey. |
There was a problem hiding this comment.
hi @vixentael, thanks a lot for the nice PR. Please take a look at the change suggestions and the comments. There sre mainly 2 points where a little bit research would be great. It's very nice that you brought this up since the guide really needs to give clearer recommendations on this. I hope the reference links I've included help you come up with something. Could you please enhance the section with that? Thanks a lot in advance!
Co-authored-by: cpholguera <perezholguera@gmail.com>
|
Short update: half review has already happened. Hopefully I can send you some feedback this week. We need to ensure that we properly address these topics, we might have to relocate and merge some info with other parts of the guide. I'll let you know as soon as I have something new. btw. thanks for the link to the key wrapping, that's what I was looking for, I couldn't recall the technique myself and I'd like to ensure that we keep using standard wording/naming, etc. |
|
thank you @cpholguera! |
There was a problem hiding this comment.
Hi @vixentael, the review took some time since you're touching very interesting points here. I was thinking that this give us the chance to improve the guide even more, so I wanted to carefully craft the suggestions. I hope you like them and can help us out with a little refactoring / update of some sections. We should keep this chapter "general" and move the specifics to the Android/iOS chapters.
I'd understand if something is not completely clear, please feel free to discuss here or in Slack :)
Thanks a lot again for taking care of this!
| - Make sure that all cryptographic actions and the keys itself remain in the Trusted Execution Environment (e.g. use [Android Keystore](https://developer.android.com/training/articles/keystore.html "Android keystore system")) or [Secure Enclave](https://developer.apple.com/documentation/security/certificate_key_and_trust_services/keys/storing_keys_in_the_secure_enclave "Storing Keys in the Secure Enclave") (e.g. use the Keychain and when you sign, use ECDHE). | ||
| - If keys are necessary which are outside of the TEE / SE, make sure keys are protected using multi-layered approach. For example, data encryption key (DEK) can be encrypted with key encryption key (KEK) stored in Keychain / Keystore, thus DEK is stored encrypted. This key wrapping technique requires reading KEK first and then decrypting DEK before usage. An illustration of this approach is [EncryptedSharedPreferences from androidx.security.crypto package](https://developer.android.com/jetpack/androidx/releases/security "androidx.security.crypto API reference"). Another approach of protecting keys is obfuscation: obfuscate DEK and de-obfuscate it before usage. |
There was a problem hiding this comment.
| - Make sure that all cryptographic actions and the keys itself remain in the Trusted Execution Environment (e.g. use [Android Keystore](https://developer.android.com/training/articles/keystore.html "Android keystore system")) or [Secure Enclave](https://developer.apple.com/documentation/security/certificate_key_and_trust_services/keys/storing_keys_in_the_secure_enclave "Storing Keys in the Secure Enclave") (e.g. use the Keychain and when you sign, use ECDHE). | |
| - If keys are necessary which are outside of the TEE / SE, make sure keys are protected using multi-layered approach. For example, data encryption key (DEK) can be encrypted with key encryption key (KEK) stored in Keychain / Keystore, thus DEK is stored encrypted. This key wrapping technique requires reading KEK first and then decrypting DEK before usage. An illustration of this approach is [EncryptedSharedPreferences from androidx.security.crypto package](https://developer.android.com/jetpack/androidx/releases/security "androidx.security.crypto API reference"). Another approach of protecting keys is obfuscation: obfuscate DEK and de-obfuscate it before usage. | |
| - Make sure that all cryptographic actions and the keys itself remain in the Trusted Execution Environment (e.g. use [Android Keystore](https://developer.android.com/training/articles/keystore.html "Android keystore system")) or [Secure Enclave](https://developer.apple.com/documentation/security/certificate_key_and_trust_services/keys/storing_keys_in_the_secure_enclave "Storing Keys in the Secure Enclave") (e.g. use the Keychain). Refer to the [Android Data Storage](0x05d-Testing-Data-Storage.md#storing-a-key---example) and [iOS Data Storage](0x06d-Testing-Data-Storage.md#the-keychain) chapters for more information. |
This point is addressing one special case of the previous point for Android, I'd suggest to remove it since the previous point covers it. The current paragraph should be reduced and the specifics moved/merged to their corresp. chapters.
You can use the info from the deleted paragraph to update Android Data Storage - Storing a key section. We need a nice overview / clear recommendation there (e.g. most secure if it never leaves the KeyStore, 2nd most secure if using EncryptedPreferences, etc.). You could take the chance to introduce the official term "Envelope encryption" from the site you've sent me which refers to Google.
Maybe it could be a nice idea to include another example, since this is also good to know for other kind of apps such as the "React Native"-based ones (and goes in-line with our statement/recommendation here):
- https://reactnative.dev/docs/security
- https://github.com/emeraldsanto/react-native-encrypted-storage/blob/master/android/src/main/java/com/emeraldsanto/encryptedstorage/RNEncryptedStorageModule.java
Unfortunately I could not find more examples for other frameworks, only this lib for Flutter , but it has a custom implementation.
There was a problem hiding this comment.
I'm realizing now that the location of the mentioned information is not yet consistent. The iOS chapter should have an introductory section regarding iOS Data Storage (reflecting the Android one), instead of inside the test case.
This refactoring should be eventually made. If you want please go ahead and move/consolidate that info (as part of a new PR).
| - Make sure that keys live in memory for the shortest time possible. Ideally, keys should be read/allocated right before usage and clean up after usage or in case of error. Avoid storing keys in constants. | ||
| - Consider zeroing out and nullify keys after successful cryptographic operation and in case of error. Unfortunately, there are certain limitations of realibly cleaning up secret data in languages with garbage collector (Java) and immutable `String` (Swift, Objective-C, Kotlin). [Java Cryptography Architecture Reference Guide](https://docs.oracle.com/en/java/javase/16/security/java-cryptography-architecture-jca-reference-guide.html#GUID-C9F76AFB-6B20-45A7-B84F-96756C8A94B4 "Java Cryptography Architecture (JCA) Reference Guide") suggests using `char[]` instead of `String` for storing sensitive data, and nullify array after usage. [Apple Secure Coding Guide](https://developer.apple.com/library/archive/documentation/Security/Conceptual/SecureCodingGuide/SecurityDevelopmentChecklists/SecurityDevelopmentChecklists.html "Security Development Checklists") also suggests zeroing passwords after usage, but provides no recommended ways of doing this. Consider using `NSMutableData` for storing secrets on Swift/Objective-C and use [`resetBytes(in:)` method](https://developer.apple.com/documentation/foundation/nsmutabledata/1415526-resetbytes "NSMutableData resetBytes(in:) API reference") for zeroing. Also, see [Clean memory of secret data](https://github.com/veorq/cryptocoding#clean-memory-of-secret-data/ "The Cryptocoding Guidelines by @veorq: Clean memory of secret data") for reference. |
There was a problem hiding this comment.
| - Make sure that keys live in memory for the shortest time possible. Ideally, keys should be read/allocated right before usage and clean up after usage or in case of error. Avoid storing keys in constants. | |
| - Consider zeroing out and nullify keys after successful cryptographic operation and in case of error. Unfortunately, there are certain limitations of realibly cleaning up secret data in languages with garbage collector (Java) and immutable `String` (Swift, Objective-C, Kotlin). [Java Cryptography Architecture Reference Guide](https://docs.oracle.com/en/java/javase/16/security/java-cryptography-architecture-jca-reference-guide.html#GUID-C9F76AFB-6B20-45A7-B84F-96756C8A94B4 "Java Cryptography Architecture (JCA) Reference Guide") suggests using `char[]` instead of `String` for storing sensitive data, and nullify array after usage. [Apple Secure Coding Guide](https://developer.apple.com/library/archive/documentation/Security/Conceptual/SecureCodingGuide/SecurityDevelopmentChecklists/SecurityDevelopmentChecklists.html "Security Development Checklists") also suggests zeroing passwords after usage, but provides no recommended ways of doing this. Consider using `NSMutableData` for storing secrets on Swift/Objective-C and use [`resetBytes(in:)` method](https://developer.apple.com/documentation/foundation/nsmutabledata/1415526-resetbytes "NSMutableData resetBytes(in:) API reference") for zeroing. Also, see [Clean memory of secret data](https://github.com/veorq/cryptocoding#clean-memory-of-secret-data/ "The Cryptocoding Guidelines by @veorq: Clean memory of secret data") for reference. | |
| - Make sure that keys live in memory for the shortest time possible and consider zeroing out and nullifying keys after successful cryptographic operations, or in case of error. For more detailed information refer to sections [Testing Memory for Sensitive Data](0x05d-Testing-Data-Storage.md#checking-memory-for-sensitive-data-mstg-storage-10) and [Checking Memory for Sensitive Data](0x05d-Testing-Data-Storage.md#checking-memory-for-sensitive-data-mstg-storage-10) for Android and iOS respectively. |
To keep it simple and general (since we're in the general guide here), I suggest to reduce these two points to the suggestion and use the removed information to update these sections:
- https://github.com/OWASP/owasp-mstg/blob/master/Document/0x05d-Testing-Data-Storage.md#checking-memory-for-sensitive-data-mstg-storage-10
- https://github.com/OWASP/owasp-mstg/blob/master/Document/0x06d-Testing-Data-Storage.md#testing-memory-for-sensitive-data-mstg-storage-10
Also consider renaming the section from "Checking Memory for Sensitive Data" to "Testing Memory for Sensitive Data" for consistency.
vixentael
changed the title
|
@cpholguera thank you for the review! I totally support the idea of having general details in general guide and moving specific details into iOS/Android guides. Things I've done: 0x04g-Testing-Cryptography
0x05d-Testing-Data-Storage (Android)
0x06d-Testing-Data-Storage (iOS)
Things I haven't done:
|
|
@cpholguera I mean no rush, but what do you think about the latest update? I'd like to get it sorted and to move to updating iOS crypto part next. |
|
Hi, we've been very busy with the MASVS release and now on leave until June. We'll go back to this one then. Thanks for the patience! |
|
@cpholguera no mean to rush, but Apple has introduced AES.KeyWrap API on the latest WWDC :) (No docs now, but it's similar to https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/wrapKey) It means that after merging this PR, I'll have even more things-to-write about in a new chapter in iOS Testing guide :) |
|
Sounds very nice, is there a video for that? Thanks for your patience, we hope we can get back to the PRs soon. There's a lot going on right now. We'll announce something soon, I'm sure you'll like it :) |
|
Hi @vixentael, just to let you know: we did not forget about this PR. As you know we're currently performing a big refactor of the MASVS and your content in this PR is helping us a lot currently since we're taking a look at V2 specifically :) Thanks for your patience! |
|
hi @cpholguera thank you for the update! I keep an eye on discussions in MASVS, but I'm also available in OWASP slack, so if any questions – i'd be happy to help in shaping crypto requirements. |
There was a problem hiding this comment.
Thanks a lot @vixentael and sorry about the long wait. I hope you're doing well.
I'm proposing changes I got from a review I started doing some time ago. Nothing for you to change for now. @TheDauntless will help with the review by first reviewing my review (if that makes sense :) ).
However, feel free to comment if you want to.
| In order to securely store symmetric keys on devices running on Android 5.1 (API level 22) or lower, we need to generate a public/private key pair. We encrypt the symmetric key using the public key and store the private key in the `AndroidKeyStore`. The encrypted symmetric key can encoded using base64 and stored in the `SharedPreferences`. Whenever we need the symmetric key, the application retrieves the private key from the `AndroidKeyStore` and decrypts the symmetric key. | ||
|
|
||
| Envelope encryption, or key wrapping, is a similar approach that uses symmetric encryption to encapsulate key material. Data encryption keys (DEK) can be encrypted with key encryption keys (KEK) which is securely stored. Encrypted DEK can be stored in `SharedPreferences` or written in files. When required, application reads KEK, then decrypts DEK. Refer to [OWASP Cryptographic Storage Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#encrypting-stored-keys "OWASP Cryptographic Storage Cheat Sheet: Encrypting Stored Keys") to learn more about encrypting cryptographic keys. | ||
|
|
||
| Also, as the illustration of this approach, refer to the [EncryptedSharedPreferences from androidx.security.crypto package](https://developer.android.com/jetpack/androidx/releases/security "androidx.security.crypto API reference"). |
There was a problem hiding this comment.
Examples in Corona apps:
Co-authored-by: Jeroen Beckers <me.githbub@dauntless.be>
There was a problem hiding this comment.
@vixentael this is awesome content, we've applied some fixes and will merge. We hope to have your support for the upcoming MSTG refactoring!
|
Closing. Now replaced by #2127 keeping the original commits. |
I slightly refreshed crypto chapter:
These changes were discussed with @sushi2k and @julepka in a separate doc.
Thank you for submitting a Pull Request to the Mobile Security Testing Guide. Please make sure that: