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

Port MASTG-TEST-0015 to v2 (by @guardsquare) #3036

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nmsa
Copy link
Collaborator

@nmsa nmsa commented Nov 5, 2024

Thank you for submitting a Pull Request to the OWASP MASTG. Please make sure that:

  • Your contribution is written in the 2nd person (e.g. you)
  • Your contribution is written in an active present form for as much as possible.
  • You have made sure that the reference section is up to date (e.g. please add sources you have used, make sure that the references to MITRE/MASVS/etc. are up to date)
  • Your contribution has proper formatted markdown and/or code
  • Any references to website have been formatted as [TEXT](URL “NAME”)
  • You verified/tested the effectiveness of your contribution (e.g.: is the code really an effective remediation? Please verify it works!)

This PR closes #2948.

Most likely needs to be broken down in some atomic tests, according to what is being checked now:
1. For asymmetric keys is the key pair being used for mutiple activities (e.g. signatures vs encryption)
2. For symmetric keys, are those being used for multiple business purposes?  (hard to automate)

Case (1.) is easy to automate, (2.) is not so much

Then, there are some other checks to be discussed:
- are all keys used according to the purpose defined during its creation? (it is relevant to KeyStore keys, which can have KeyProperties defined) <-- the APIs ensure that this is done properly, maybe can be removed
- is cryptography used according to its business purpose? <-- To be discussed if it is really different from the remaining
@nmsa nmsa changed the title Port MASTG-TEST-0015 to v2 Port MASTG-TEST-0015 to v2 (by @guardsquare) Nov 5, 2024
@cpholguera
Copy link
Collaborator

I like how you split them and I agree with the mapping to "MASWE-0012: Insecure or Wrong Usage of Cryptographic Key". So we can have 2 tests here at least:

  1. For asymmetric keys is the key pair being used for multiple activities (e.g. signatures vs encryption)
  • key purpose (can be read from the properties of the key)
  1. For symmetric keys, are those being used for multiple business purposes? (hard to automate)
  • the typical "key reuse case", right? Dangerous especially for ECB if I'm not mistaken.
  • Can we test by collecting all uses of the same key with Frida and inspecting the reversed code? e.g. The observation can contains all the uses of each key with backtraces to know the locations. In the evaluation we'd suggest to reverse engineer those places and try to determine the "business purpose".

See https://crypto.stackexchange.com/questions/33751/aes-key-reuse-and-guessing-the-key

(Ideally we'd find another source from NIST for example)

The reuse of the key is not a problem as long as you have chosen a mode of operation that is appropriate for your application (and of course, if you have used it properly). This is because the mode of operation will "pre-mix" your clear text data in a way that, even before encryption, two identical block of data you feed to the actual AES algorithm will never be identical (or at least, not more often than by random chance).

@nmsa
Copy link
Collaborator Author

nmsa commented Nov 6, 2024

Addressed your comments according to what we discussed live yesterday.

@nmsa nmsa marked this pull request as ready for review November 6, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MASTG v1->v2 MASTG-TEST-0015: Testing the Purposes of Keys (android)
2 participants