[MASWE-0009] Add Weak Cryptographic Key Generation (by appknox) #2622

sk3l10x1ng · 2024-05-22T03:08:49Z

…y-generation]

sushi2k

Thanks you @sk3l10x1ng for the PR! Have a look at the comments. The main change that is needed is to not mention AES-128 as insecure algorithm. You could use AES-64 instead as example. Let me know if any questions

sushi2k · 2024-06-20T17:13:43Z

.../2-weak-crypto-key-generation/weak-key-generation/android-weak-key-generation/demo-1/demo.md

+
+Review each of the reported instances.
+
+- Line 2 has inialized the RSA key size of 1024 bits.


Suggested change

- Line 2 has inialized the RSA key size of 1024 bits.

- Line 2 has initialized the RSA key size of 1024 bits.

sushi2k · 2024-06-20T17:25:49Z

risks/MASVS-CRYPTO/2-weak-crypto-key-generation/weak-key-generation/risk.md

+
+## Overview
+
+The key size, also known as the key length, is dependent on the number of bits in which the message is stored. Encryption algorithms that utilize insufficient key sizes are vulnerable to attacks, while longer keys typically entail more intricate encryption.


Generating short keys is one thing, but the generation of keys should also be based on libraries or API's of the OS with accurate entropy and should have a true random-number generator. This is missing at the moment, could you add a paragraph about this? We should also add to use the available mechanisms on the OS, for example using the KeyStore on Android or Apple CryptoKit on iOS.

I don't understand the first sentence, why is the key length dependent on the message size? For example for AES, the key sizes are 128, 192, and 256 bits. The message will then be divided into blocks according to the key length to encrypt it.

You can also reference to the android documentation about PRNG's: https://developer.android.com/privacy-and-security/risks/weak-prng

sushi2k · 2024-06-20T17:27:41Z

risks/MASVS-CRYPTO/2-weak-crypto-key-generation/weak-key-generation/risk.md

+## Impact
+
+- **Risk of Brute-Force Attacks**:
+With a shorter key length, the number of possible combinations decreases, increasing the likelihood of an attacker successfully cracking the encryption through brute force method.


Not only the length, but also if the PRNG was having a predictable input.

sushi2k · 2024-06-21T04:24:38Z

risks/MASVS-CRYPTO/2-weak-crypto-key-generation/weak-key-generation/risk.md

+With a shorter key length, the number of possible combinations decreases, increasing the likelihood of an attacker successfully cracking the encryption through brute force method.
+
+- **Loss of  Confidentiality**:
+Encryption is utilized to safeguard the confidentiality of data, allowing only authorized users to access it. Weak encryption keys can enable attackers to obtain encrypted information and exploit it effortlessly.


Suggested change

Encryption is utilized to safeguard the confidentiality of data, allowing only authorized users to access it. Weak encryption keys can enable attackers to obtain encrypted information and exploit it effortlessly.

Encryption is utilized to safeguard the confidentiality of data, allowing only authorized users to access it. Weak encryption keys can enable attackers to obtain encrypted information and exploit it.

Let's stay objective, therefore removed "effortlessly".

sushi2k · 2024-06-21T04:32:54Z

risks/MASVS-CRYPTO/2-weak-crypto-key-generation/weak-key-generation/risk.md

+The libraries used by the application, the algorithms used and cryptographic schemes used are outdated.
+
+- **Use of Insecure Algorithms**:
+The use of insecure algorithms (e.g using the 1024-bit RSA key, 128-bit AES key, 160-bit ECDSA key) in an application poses significant security risks to data.


128-bit AES is not insecure but a recommended algorithm for encryption.

Suggested change

The use of insecure algorithms (e.g using the 1024-bit RSA key, 128-bit AES key, 160-bit ECDSA key) in an application poses significant security risks to data.

The use of deprecated algorithms (e.g using the 1024-bit RSA key or 160-bit ECDSA key) in an application poses significant security risks to data.

A 3rd point should be the usage of weak PRNGs

sushi2k · 2024-06-21T06:45:52Z

...ASVS-CRYPTO/2-weak-crypto-key-generation/weak-key-generation/ios-weak-key-generation/test.md

+
+## Overview
+
+The application appears to utilize a symmetric algorithm known as `AES-128` with a weak key configuration, making it susceptible to multiple vulnerabilities and potential security breaches.


It can be insecure if the CBC mode is used for AES and if the IV is empty or weak when using the AES key for encryption as you can then try to brute force, but AES-128 is a strong encryption cipher.

See also https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-57pt1r5.pdf

https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TG02102/BSI-TR-02102-1.pdf;jsessionid=139E7E1917DFDEAF23598BDC2CE60B01.1_cid341?__blob=publicationFile&v=9

sushi2k · 2024-06-22T04:34:45Z

risks/MASVS-CRYPTO/2-weak-crypto-key-generation/weak-key-generation/risk.md

+## Modes of Introduction
+
+- **Third Party Libraries**:
+The libraries used by the application, the algorithms used and cryptographic schemes used are outdated.


Suggested change

The libraries used by the application, the algorithms used and cryptographic schemes used are outdated.

The libraries, algorithms and cryptographic schemes used by the application are out of date.

sushi2k · 2024-06-22T04:58:26Z

...n/weak-key-generation/android-weak-key-generation/rules/mastg-weak-crypto-key-generation.yml

+              $S = new ECGenParameterSpec("secp224r1");
+              ...
+              $K.initialize($S);
+          - pattern: |


Please remove this pattern

sushi2k · 2024-06-22T05:00:27Z

...n/weak-key-generation/android-weak-key-generation/rules/mastg-weak-crypto-key-generation.yml

@@ -0,0 +1,49 @@
+rules:


Let's add the source, if it wasn't done by us from scratch.

Suggested change

rules:

# original source: https://github.com/MobSF/mobsfscan/blob/main/mobsfscan/rules/semgrep/crypto/weak_key_size.yaml

rules:

sushi2k · 2024-06-22T05:03:22Z

...ak-crypto-key-generation/weak-key-generation/android-weak-key-generation/demo-1/output.sarif

@@ -0,0 +1,135 @@
+{


Can you please remove the SARIF file for now? We can add later to keep the PR cleaner.

sk3l10x1ng · 2024-07-04T12:26:50Z

Thanks you @sk3l10x1ng for the PR! Have a look at the comments. The main change that is needed is to not mention AES-128 as insecure algorithm. You could use AES-64 instead as example. Let me know if any questions

@sushi2k

We have created the test case on AES-128 bit based on the following examples provided in the issue [MASWE-0009] Weak Cryptographic Key Generation #2573 .
Can we use DES algorithm for the test case for the ios . Where AES-64 is not possible because the AES contain key sizes 128, 192, 256

sushi2k · 2024-07-28T05:32:43Z

Hi @sk3l10x1ng. You could use 3DES, as this is algorithm is deprecated, instead of AES-128. You are right AES-128 is listed as example in the issue, but this is not correct.
Can you please also go through my comments and follow-up on them? Regarding the test cases and weaknesses we have a new structure which can be found here: https://docs.google.com/document/d/1EMsVdfrDBAu0gmjWAUEs60q-fWaOmDB5oecY9d9pOlg/edit?usp=sharing. Can you please update the content to this new format? Let me know if any questions.

Thanks for your support.

sk3l10x1ng · 2024-07-30T11:39:05Z

@cpholguera @sushi2k . updated according to new file structure and requested changed done in #2849

Add Risk and Test - Weak Cryptographic Key Generation [weak-crypto-ke…

d3f3708

…y-generation]

cpholguera requested a review from sushi2k June 7, 2024 07:36

sushi2k requested changes Jun 22, 2024

View reviewed changes

cpholguera changed the title ~~Add Risk and Test - Weak Cryptographic Key Generation [weak-crypto-key-generation] (by appknox)~~ [MASWE-0009] Add Weak Cryptographic Key Generation (by appknox) Jul 10, 2024

cpholguera added the MASWE label Jul 10, 2024

sk3l10x1ng mentioned this pull request Jul 30, 2024

[MASWE-0009] Add Weak Cryptographic Key Generation (by appknox) #2849

Merged

sk3l10x1ng closed this Jul 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MASWE-0009] Add Weak Cryptographic Key Generation (by appknox) #2622

[MASWE-0009] Add Weak Cryptographic Key Generation (by appknox) #2622

sk3l10x1ng commented May 22, 2024 •

edited by cpholguera

Loading

sushi2k left a comment

sushi2k Jun 20, 2024

sushi2k Jun 20, 2024

sushi2k Jun 21, 2024

sushi2k Jun 20, 2024

sushi2k Jun 21, 2024

sushi2k Jun 21, 2024

sushi2k Jun 21, 2024

sushi2k Jun 21, 2024

sushi2k Jun 22, 2024

sushi2k Jun 22, 2024

sushi2k Jun 22, 2024

sushi2k Jun 22, 2024

sk3l10x1ng commented Jul 4, 2024 •

edited

Loading

sushi2k commented Jul 28, 2024

sk3l10x1ng commented Jul 30, 2024 •

edited

Loading


		Review each of the reported instances.

		- Line 2 has inialized the RSA key size of 1024 bits.

	- Line 2 has inialized the RSA key size of 1024 bits.
	- Line 2 has initialized the RSA key size of 1024 bits.


		## Overview

		The key size, also known as the key length, is dependent on the number of bits in which the message is stored. Encryption algorithms that utilize insufficient key sizes are vulnerable to attacks, while longer keys typically entail more intricate encryption.

	Encryption is utilized to safeguard the confidentiality of data, allowing only authorized users to access it. Weak encryption keys can enable attackers to obtain encrypted information and exploit it effortlessly.
	Encryption is utilized to safeguard the confidentiality of data, allowing only authorized users to access it. Weak encryption keys can enable attackers to obtain encrypted information and exploit it.

	The use of insecure algorithms (e.g using the 1024-bit RSA key, 128-bit AES key, 160-bit ECDSA key) in an application poses significant security risks to data.
	The use of deprecated algorithms (e.g using the 1024-bit RSA key or 160-bit ECDSA key) in an application poses significant security risks to data.


		## Overview

		The application appears to utilize a symmetric algorithm known as `AES-128` with a weak key configuration, making it susceptible to multiple vulnerabilities and potential security breaches.

	The libraries used by the application, the algorithms used and cryptographic schemes used are outdated.
	The libraries, algorithms and cryptographic schemes used by the application are out of date.

	rules:
	# original source: https://github.com/MobSF/mobsfscan/blob/main/mobsfscan/rules/semgrep/crypto/weak_key_size.yaml
	rules:

[MASWE-0009] Add Weak Cryptographic Key Generation (by appknox) #2622

[MASWE-0009] Add Weak Cryptographic Key Generation (by appknox) #2622

Conversation

sk3l10x1ng commented May 22, 2024 • edited by cpholguera Loading

sushi2k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sk3l10x1ng commented Jul 4, 2024 • edited Loading

sushi2k commented Jul 28, 2024

sk3l10x1ng commented Jul 30, 2024 • edited Loading

sk3l10x1ng commented May 22, 2024 •

edited by cpholguera

Loading

sk3l10x1ng commented Jul 4, 2024 •

edited

Loading

sk3l10x1ng commented Jul 30, 2024 •

edited

Loading