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

Java: Query to detect weak encryption: insufficient key size #4926

Merged
merged 7 commits into from
Feb 3, 2021

Conversation

luchua-bc
Copy link
Contributor

Strength of cryptographic algorithms depends on a sufficient key size to be robust against brute-force attacks. A theoretically sound encryption scheme with an insufficient key size can still be subjected to brute force attacks that have a reasonable chance of succeeding using current attack methods and resources.

This type of issues is categorized as CWE-326: Inadequate Encryption Strength.

This query finds uses of strong encryption algorithms (AES, DSA, RSA, and EC) with too small a key size.

Please consider to merge the PR. Thanks.

@smowton
Copy link
Contributor

smowton commented Jan 11, 2021

OK sure that's enough factoring, running tests

@JarLob
Copy link
Contributor

JarLob commented Jan 14, 2021

Hey @luchua-bc,

  1. OWASP recommends minimum 256 bits for ECDH and ECDSA. Could you please bump the requirement to 256 bits?
  2. It looks like algorithms like "prime192v2" can be also specified without the "X9.62 ". Could you please handle these cases too??
  3. Please add "c2tnb" family to the list.
  4. The algorithm name for KeyPairGenerator.getInstance is not case sensitive.
  5. Please add "DH" algorithm name to the "DSA" check.

@luchua-bc
Copy link
Contributor Author

@JarLob I've made requested changes. Please review. Thanks.

@smowton
Copy link
Contributor

smowton commented Jan 27, 2021

Please rebase. Pinged a codeowner for final review.

@luchua-bc luchua-bc force-pushed the java/insufficient-key-size branch from bddd738 to ab7d257 Compare January 28, 2021 04:08
@luchua-bc
Copy link
Contributor Author

Thanks @smowton. I've rebased this branch.

Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good to me. Just a few comments.

@luchua-bc
Copy link
Contributor Author

Thanks @aschackmull for reviewing this PR. I've made requested text changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants