[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 7 commits into from
Feb 3, 2021
Merged

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

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.

@luchua-bc luchua-bc requested a review from a team as a code owner January 7, 2021 21:31
@luchua-bc luchua-bc mentioned this pull request Jan 7, 2021
Closed
1 task
smowton
smowton reviewed Jan 8, 2021
View reviewed changes
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql Outdated Show resolved Hide resolved
@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
Copy link
Contributor Author

Thanks @smowton. I've rebased this branch.

aschackmull
aschackmull requested changes Feb 2, 2021
View reviewed changes
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.

java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java Outdated Show resolved Hide resolved
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp Outdated Show resolved Hide resolved
java/ql/src/semmle/code/java/security/Encryption.qll Outdated Show resolved Hide resolved
@luchua-bc
Copy link
Contributor Author

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

@aschackmull aschackmull merged commit 40d02e7 into github:main Feb 3, 2021
@luchua-bc luchua-bc deleted the java/insufficient-key-size branch February 3, 2021 15:14
@JarLob JarLob mentioned this pull request Jun 8, 2021
Closed
1 task
@jcogs33 jcogs33 mentioned this pull request Oct 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smowton smowton smowton left review comments

@JarLob JarLob JarLob left review comments

@aschackmull aschackmull aschackmull approved these changes

Assignees
No one assigned
Labels
documentation Java
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@luchua-bc @smowton @JarLob @aschackmull