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

fix(weaver): usage of weak PRNG #2765

Closed
jagpreetsinghsasan opened this issue Oct 11, 2023 · 1 comment · Fixed by #2907, #2953 or #3324
Closed

fix(weaver): usage of weak PRNG #2765

jagpreetsinghsasan opened this issue Oct 11, 2023 · 1 comment · Fixed by #2907, #2953 or #3324
Assignees
Labels
good-first-issue Good for newcomers good-first-issue-300-advanced good-first-issue-400-expert Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label. P4 Priority 4: Low Security Related to existing or potential security vulnerabilities Weaver Tasks related to the future of Cactus & Weaver together.

Comments

@jagpreetsinghsasan
Copy link
Contributor

Description

Static source code assessment has picked up a potential vulnerability regarding use of weak pseudo RNG. Use of secure pseudo random number generators rather than using basic random methods can fix the above vulnerability

The report from which the above information was summarized

Risk Rating: Low
Category: Security Misconfiguration

Description

The application uses a weak method of generating pseudo-random values, such that other numbers could be determined from a relatively small sample size. Since the pseudo-random number generator used is designed for statistically uniform distribution of values, it is approximately deterministic. Thus, after collecting a few generated values, it would be possible for an attacker to calculate past or future values. These values are then used as parameters in a cryptographic function. If an attacker were to obtain the original value from the weak PRNG, they may be able to predict or crack values produced by the cryptographic function.

Impact

Depending on what this random value is used for, an attacker with knowledge of the PRNG used would be able to predict the next values generated, or previously generated values, based on sources often used to derive certain randomness; however, while they may seem random, large statistical samples would demonstrate that they are insufficiently random, producing a much smaller space of possible "random" values than a truly random sample would. In the context of cryptography, this could enable an attacker to derive or guess the value, and thus crack the cryptographic mechanism or its outputs.

Remediation Recommendation

• Always use a cryptographically secure pseudo-random number generator, instead of basic random methods, particularly when dealing with a security context such as cryptographic functions
• Use the cryptorandom generator that is built-in to your language or platform, and ensure it is securely seeded. Do not seed the generator with a weak, non-random seed. (In most cases, the default is securely random).
• Ensure you use a long enough random value, thus making brute-force attacks unfeasible.

Affected files (path - line number)

weaver/core/network/fabric-interop-cc/contracts/interop/certificate_utils.go - 304
weaver/sdks/corda/src/main/kotlin/org/hyperledger/cacti/weaver/sdk/corda/HashFunctions.kt - 46
weaver/sdks/fabric/interoperation-node-sdk/src/eciesCrypto.js - 177

Snapshot of the sourcecode at the time of scan

image image image

Source: APP PE Hyperledger Cacti v2.0.0 - Static Application Assessment Report.odt

cc: @takeutak @izuru0 @outSH @petermetz

@jagpreetsinghsasan jagpreetsinghsasan added good-first-issue Good for newcomers Security Related to existing or potential security vulnerabilities Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label. good-first-issue-300-advanced good-first-issue-400-expert P4 Priority 4: Low Weaver Tasks related to the future of Cactus & Weaver together. labels Oct 11, 2023
KaganCanSit added a commit to KaganCanSit/cacti that referenced this issue Nov 25, 2023
The length of the AES algorithm was changed by taking random values from more reliable sources.
KaganCanSit added a commit to KaganCanSit/cacti that referenced this issue Nov 25, 2023
Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
KaganCanSit added a commit to KaganCanSit/cacti that referenced this issue Nov 26, 2023
Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
KaganCanSit referenced this issue in KaganCanSit/cacti Nov 27, 2023
Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
KaganCanSit referenced this issue in KaganCanSit/cacti Nov 27, 2023
Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
KaganCanSit added a commit to KaganCanSit/cacti that referenced this issue Dec 2, 2023
The Logic Behind the Problem
When RNG (Random Number Generator) values are not received through a hardware TRNG, seed values apply a certain pattern. (It takes a seed value such as a mathematical formula or time.) In response to this situation, there are various secure random classes to increase security.

Solution
Changes have been made to get random values using safe randomness instead of mathematical randomness. This increases the complexity of the pattern, making it difficult to discover even if data is listened to for long periods of time.

The changes that have been made;
- In the certificate_utils.go file, the random value was taken from the math class (mrand math/rand) and used. By taking this random value from the secure random class, we obtain a more reliable random value. I added HmacGenerate and generateSecureRandomKey functions for readability and ease of use. If you want to generate a key again, the generateSecureRandomKey function, which uses secure random, can be used.

- In HashFunctions.kt, kotlin.random.Random class has been replaced with the more reliable java.security.SecureRandom class.

- The reason for the change in eciesCrypto.js is that the length of aes-128-ctr is not considered reliable by various standards. For this reason, I preferred the more reliable 256 length.

Fixes hyperledger-cacti#2765
-------
**Pull Request Requirements**
- [☑] Rebased onto  branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.

- [☑] Have git sign off at the end of commit message to avoid being marked red. You can add  flag when using On branch main
Your branch is up to date with 'origin/main'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	.vs/

nothing added to commit but untracked files present (use "git add" to track) command. You may refer to this [link](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) for more information.
- [  ] Follow the Commit Linting specification. You may refer to this [link](https://www.conventionalcommits.org/en/v1.0.0-beta.4/#specification) for more information.

**Character Limit**
- [☑] Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
- [ ] Commit Message per line must not exceed 80 characters (including spaces and special characters).

**A Must Read for Beginners**
For rebasing and squashing, here's a [must read guide](https://github.com/servo/servo/wiki/Beginner's-guide-to-rebasing-and-squashing) for beginners.
KaganCanSit added a commit to KaganCanSit/cacti that referenced this issue Dec 2, 2023
The Logic Behind the Problem
When RNG (Random Number Generator) values are not received through a hardware TRNG, seed values apply a certain pattern. (It takes a seed value such as a mathematical formula or time.) In response to this situation, there are various secure random classes to increase security.

Solution
Changes have been made to get random values using safe randomness instead of mathematical randomness. This increases the complexity of the pattern, making it difficult to discover even if data is listened to for long periods of time.

The changes that have been made;
- In the certificate_utils.go file, the random value was taken from the math class (mrand math/rand) and used. By taking this random value from the secure random class, we obtain a more reliable random value. I added HmacGenerate and generateSecureRandomKey functions for readability and ease of use. If you want to generate a key again, the generateSecureRandomKey function, which uses secure random, can be used.

- In HashFunctions.kt, kotlin.random.Random class has been replaced with the more reliable java.security.SecureRandom class.

- The reason for the change in eciesCrypto.js is that the length of aes-128-ctr is not considered reliable by various standards. For this reason, I preferred the more reliable 256 length.

Fixes hyperledger-cacti#2765
KaganCanSit added a commit to KaganCanSit/cacti that referenced this issue Dec 2, 2023
The Logic Behind the Problem
When RNG (Random Number Generator) values are not received through a hardware TRNG, seed values apply a certain pattern. (It takes a seed value such as a mathematical formula or time.) In response to this situation, there are various secure random classes to increase security.

Solution
Changes have been made to get random values using safe randomness instead of mathematical randomness. This increases the complexity of the pattern, making it difficult to discover even if data is listened to for long periods of time.

The changes that have been made;
- In the certificate_utils.go file, the random value was taken from the math class (mrand math/rand) and used. By taking this random value from the secure random class, we obtain a more reliable random value. I added HmacGenerate and generateSecureRandomKey functions for readability and ease of use. If you want to generate a key again, the generateSecureRandomKey function, which uses secure random, can be used.

- In HashFunctions.kt, kotlin.random.Random class has been replaced with the more reliable java.security.SecureRandom class.

- The reason for the change in eciesCrypto.js is that the length of aes-128-ctr is not considered reliable by various standards. For this reason, I preferred the more reliable 256 length.

Fixes hyperledger-cacti#2765

Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
KaganCanSit added a commit to KaganCanSit/cacti that referenced this issue Dec 4, 2023
The Logic Behind the Problem
When RNG (Random Number Generator) values are not received through a hardware TRNG, seed values apply a certain pattern. (It takes a seed value such as a mathematical formula or time.) In response to this situation, there are various secure random classes to increase security.

Solution
Changes have been made to get random values using safe randomness instead of mathematical randomness. This increases the complexity of the pattern, making it difficult to discover even if data is listened to for long periods of time.

The changes that have been made;
- In the certificate_utils.go file, the random value was taken from the math class (mrand math/rand) and used. By taking this random value from the secure random class, we obtain a more reliable random value. I added HmacGenerate and generateSecureRandomKey functions for readability and ease of use. If you want to generate a key again, the generateSecureRandomKey function, which uses secure random, can be used.

- In HashFunctions.kt, kotlin.random.Random class has been replaced with the more reliable java.security.SecureRandom class.

- The reason for the change in eciesCrypto.js is that the length of aes-128-ctr is not considered reliable by various standards. For this reason, I preferred the more reliable 256 length.

Fixes hyperledger-cacti#2765

Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
petermetz pushed a commit that referenced this issue Dec 5, 2023
The Logic Behind the Problem
When RNG (Random Number Generator) values are not received through a hardware TRNG, seed values apply a certain pattern. (It takes a seed value such as a mathematical formula or time.) In response to this situation, there are various secure random classes to increase security.

Solution
Changes have been made to get random values using safe randomness instead of mathematical randomness. This increases the complexity of the pattern, making it difficult to discover even if data is listened to for long periods of time.

The changes that have been made;
- In the certificate_utils.go file, the random value was taken from the math class (mrand math/rand) and used. By taking this random value from the secure random class, we obtain a more reliable random value. I added HmacGenerate and generateSecureRandomKey functions for readability and ease of use. If you want to generate a key again, the generateSecureRandomKey function, which uses secure random, can be used.

- In HashFunctions.kt, kotlin.random.Random class has been replaced with the more reliable java.security.SecureRandom class.

- The reason for the change in eciesCrypto.js is that the length of aes-128-ctr is not considered reliable by various standards. For this reason, I preferred the more reliable 256 length.

Fixes #2765

Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
@VRamakrishna
Copy link
Contributor

The PR used for the fix had an error as indicated in the comments starting from https://github.com/hyperledger/cacti/pull/2907#issuecomment-1855746888. Reopening this issue to create another PR for a more complete fix.

@VRamakrishna VRamakrishna reopened this Dec 18, 2023
@VRamakrishna VRamakrishna self-assigned this Dec 18, 2023
sandeepnRES pushed a commit to sandeepnRES/cacti that referenced this issue Dec 21, 2023
The Logic Behind the Problem
When RNG (Random Number Generator) values are not received through a hardware TRNG, seed values apply a certain pattern. (It takes a seed value such as a mathematical formula or time.) In response to this situation, there are various secure random classes to increase security.

Solution
Changes have been made to get random values using safe randomness instead of mathematical randomness. This increases the complexity of the pattern, making it difficult to discover even if data is listened to for long periods of time.

The changes that have been made;
- In the certificate_utils.go file, the random value was taken from the math class (mrand math/rand) and used. By taking this random value from the secure random class, we obtain a more reliable random value. I added HmacGenerate and generateSecureRandomKey functions for readability and ease of use. If you want to generate a key again, the generateSecureRandomKey function, which uses secure random, can be used.

- In HashFunctions.kt, kotlin.random.Random class has been replaced with the more reliable java.security.SecureRandom class.

- The reason for the change in eciesCrypto.js is that the length of aes-128-ctr is not considered reliable by various standards. For this reason, I preferred the more reliable 256 length.

Fixes hyperledger-cacti#2765

Signed-off-by: Kağan Can Şit <kagancansit@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Good for newcomers good-first-issue-300-advanced good-first-issue-400-expert Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label. P4 Priority 4: Low Security Related to existing or potential security vulnerabilities Weaver Tasks related to the future of Cactus & Weaver together.
Projects
None yet
2 participants