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

Issues reported by Security Scan App on Java stack #241

Open
pradeipk opened this issue Dec 16, 2024 · 4 comments
Open

Issues reported by Security Scan App on Java stack #241

pradeipk opened this issue Dec 16, 2024 · 4 comments

Comments

@pradeipk
Copy link
Contributor

Below Issues were reported by Security Scan App on the Java Stack

<style> </style>
file type cwe_id cwe_title cwe_url description severity language step_name
src/main/java/org/opcfoundation/ua/utils/CertificateUtils.java Weak Hash Strength CWE-328 CWE-328: Use of Weak Hash https://cwe.mitre.org/data/definitions/328.html 'getInstance' method of 'java.security.MessageDigest' uses a non-recommended hash algorithm. Low Java* Weak Hash Strength
src/main/java/org/opcfoundation/ua/transport/security/PrivKey.java Missing Cryptographic Step CWE-325 CWE-325: Missing Cryptographic Step https://cwe.mitre.org/data/definitions/325.html 'init' method of 'javax.crypto.Cipher' uses a crypto algorithm in a wrong way. AES key is not properly generated Low Java* javax.crypto.Cipher.init
src/main/java/org/opcfoundation/ua/transport/security/PrivKey.java Insecure Cryptographic Algorithm CWE-327 CWE-327: Use of a Broken or Risky Cryptographic Algorithm https://cwe.mitre.org/data/definitions/327.html 'getInstance' method of 'javax.crypto.Cipher' uses a non-recommended crypto algorithm. Medium Java* javax.crypto.Cipher.getInstance
@jouniaro
Copy link
Contributor

jouniaro commented Dec 16, 2024

Thanks for the notes!

The first one means we use SHA-1. It's in the createThumbprint method, which is used to identify the certificates - not to create any cryptographic signatures.

SHA-1 is also in the OPC UA specifications and the respective policies are deprecated. But, of course, we need to implement those policies still. Anyway, this note is not about those, I think.

The second one, I don't understand. Do you have more information, why it's wrong?

The third one is related to the algorithm ("AES/CBC/PKCS5Padding") used to store the private key secretly. I guess, it refers to a possible padding oracle, but this is only related to storing the key in a file (data at rest), so there is no way to use the oracle for it. See the following answer, for example:

https://crypto.stackexchange.com/questions/72444/is-the-java-function-aes-cbc-pkcs5padding-vulnerable-to-padding-oracle

@pradeipk
Copy link
Contributor Author

Hi @jouniaro , Further details on the issue reported is as below

  • 'init' method of 'javax.crypto.Cipher' uses a crypto algorithm in a wrong way. AES key is not properly generated

src/main/java/org/opcfoundation/ua/transport/security/PrivKey.java
line 210 : cipher.init(Cipher.DECRYPT_MODE, keySpec, ivSpec);

  • 'update' method of 'org.bouncycastle.crypto.signers.PSSSigner' uses a crypto algorithm in a wrong way. Signer updation w/o valid initialization.

src/main/java/org/opcfoundation/ua/transport/security/BcCryptoProvider.java
line 268 signer.update(dataToSign, 0, dataToSign.length);
line 310 : signer.update(dataToVerify, 0, dataToVerify.length);

To my assumption the scan tool is talking about signer.init(forSigning, params) which is invoked before the update method in getAsymmetricSigner method at line 407.

Kindly share your feedback on these matters—do you consider them noise or genuine issues that need to be addressed? Additionally, we have raised PRs for some of the issues identified by the scan tool. Please review and approve the PRs.

@pradeipk
Copy link
Contributor Author

#244 (comment)

@jouniaro
Copy link
Contributor

Yeah, I guess the 'signer.update' note is due to the initialisation being done in 'getAsymmetricSigner' method, instead of here.

The 'cipher.init' note I don't understand, since the key is initialized in the method.

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

No branches or pull requests

2 participants