Skip to content

Commit

Permalink
fix(weaver): usage of weak PRNG issue
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
KaganCanSit authored and petermetz committed Dec 5, 2023
1 parent f6c3541 commit fa17b52
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"hash"
"math/big"
mrand "math/rand"
"time"

"golang.org/x/crypto/ed25519"
Expand Down Expand Up @@ -207,7 +206,7 @@ func ecdsaVerify(verKey *ecdsa.PublicKey, msgHash, signature []byte) error {
return nil
}

//Validate Ed25519 signature
// Validate Ed25519 signature
func verifyEd25519Signature(pubKey []byte, hashedMessage []byte, signature []byte) error {

result := ed25519.Verify(pubKey, hashedMessage, signature)
Expand Down Expand Up @@ -297,12 +296,31 @@ func encryptWithEd25519PublicKey(message []byte, pubKey []byte) ([]byte, error)
return []byte(""), nil
}

func generateSecureRandomKey(length int) ([]byte, error) {
key := make([]byte, length)
_, err := rand.Read(key)
if err != nil {
return nil, err
}
return key, nil
}

func generateHMAC(data, key []byte) ([]byte, error) {
hmacHash := hmac.New(sha256.New, key)
_, err := hmacHash.Write(data)
if err != nil {
return nil, err
}
return hmacHash.Sum(nil), nil
}

func generateConfidentialInteropPayloadAndHash(message []byte, cert string) ([]byte, error) {
// Generate a 16-byte random key for the HMAC
hashKey := make([]byte, 16)
for i := 0; i < 16 ; i++ {
hashKey[i] = byte(mrand.Intn(255))
hashKey, err := generateSecureRandomKey(16)
if err != nil {
return []byte(""), err
}

confidentialPayloadContents := common.ConfidentialPayloadContents{
Payload: message,
Random: hashKey,
Expand All @@ -311,22 +329,26 @@ func generateConfidentialInteropPayloadAndHash(message []byte, cert string) ([]b
if err != nil {
return []byte(""), err
}

x509Cert, err := parseCert(cert)
if err != nil {
return []byte(""), err
}

encryptedPayload, err := encryptWithCert(confidentialPayloadContentsBytes, x509Cert)
if err != nil {
return []byte(""), err
}

payloadHMAC := hmac.New(sha256.New, hashKey)
payloadHMAC.Write(message)
payloadHMACBytes := payloadHMAC.Sum(nil)
payloadHMAC, err := generateHMAC(message, hashKey)
if err != nil {
return []byte(""), err
}

confidentialPayload := common.ConfidentialPayload{
EncryptedPayload: encryptedPayload,
HashType: common.ConfidentialPayload_HMAC,
Hash: payloadHMACBytes,
Hash: payloadHMAC,
}
confidentialPayloadBytes, err := proto.Marshal(&confidentialPayload)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ package org.hyperledger.cacti.weaver.sdk.corda;
import java.util.Base64
import net.corda.core.utilities.OpaqueBytes
import net.corda.core.crypto.sha256
import kotlin.random.Random
import java.security.SecureRandom
import org.hyperledger.cacti.weaver.protos.common.asset_locks.AssetLocks.HashMechanism
import org.hyperledger.cacti.weaver.imodule.corda.states.sha512

Expand Down Expand Up @@ -42,8 +42,9 @@ class HashFunctions {

override fun generateRandomPreimage(length: Int)
{
val bytes = ByteArray(length)
Random.nextBytes(bytes)
val secureRandom = SecureRandom.getInstanceStrong();
val bytes = ByteArray(length);
secureRandom.nextBytes(bytes);
this.setPreimage(Base64.getEncoder().encodeToString(bytes));
}
override fun setPreimage(preImage: String) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ function eciesEncryptMessage(recipientPublicKey, msg, options) {
const hKm = bitsToBytes(hmacKeyHash.finalize());

const iv = crypto.randomBytes(IVLength);
const cipher = crypto.createCipheriv("aes-128-ctr", Buffer.from(aesKey), iv);
const cipher = crypto.createCipheriv("aes-256-ctr", Buffer.from(aesKey), iv);

This comment has been minimized.

Copy link
@VRamakrishna

VRamakrishna Dec 14, 2023

Contributor

@petermetz This change by itself is incorrect and causing the unit test to fail.

@KaganCanSit Just changing the algorithm without updating other parts of this code is causing the entire function to fail.

Can you please work on upgrading the entire code in this file to be consistent with the above change? There's a unit test you need to run to verify that the code works. Use npm run test for that.

This comment has been minimized.

Copy link
@VRamakrishna

VRamakrishna Dec 14, 2023

Contributor

This comment has been minimized.

Copy link
@petermetz

petermetz Dec 14, 2023

Contributor

@VRamakrishna The origin story for the issue can be found here: https://github.com/hyperledger/cacti/issues/2765 sorry about the tests getting messed up!

This comment has been minimized.

Copy link
@KaganCanSit

KaganCanSit Dec 17, 2023

Author Contributor

Hello @VRamakrishna and @petermetz,
I have now made various changes to the function. However, I am experiencing various problems with npm dependencies for both Windows and Linux. A little tip for editing dependencies might be helpful.

This comment has been minimized.

Copy link
@petermetz

petermetz Dec 21, 2023

Contributor

@KaganCanSit Might not matter anymore (because the issue has been resolved) but I wanted to dig into your issue anyway just to make sure it doesn't hit next time.
What is your OS that you are working with?
Does everything work OK if you manually add a dependency to the package.json file and then run yarn install ? (do not run npm install because we use yarn)

This comment has been minimized.

Copy link
@KaganCanSit

KaganCanSit Mar 23, 2024

Author Contributor

I don't know how I missed this, sorry. I can't remember, but I think the issue here is because I'm using npm, not yarn.

This comment has been minimized.

Copy link
@petermetz

petermetz May 20, 2024

Contributor

@KaganCanSit Alright, no worries! It's hard to keep track of GitHub notifications sometimes! :-)
If you happen to gather more details later feel free to open an issue in the tracker and we can continue the conversation there!

const encryptedBytes = cipher.update(msg);
const EM = Buffer.concat([iv, encryptedBytes]);
const D = hmac(hKm, EM, options);
Expand Down

0 comments on commit fa17b52

Please sign in to comment.