FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Java: Query to detect weak encryption: insufficient key size by luchua-bc · Pull Request #4926 · github/codeql · GitHub

/ codeql Public

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

Merged
aschackmull merged 7 commits into
github:mainfrom
luchua-bc:java/insufficient-key-size
Feb 3, 2021
ConversationCommits (7)ChecksFindingsFiles changed
Merged

Java: Query to detect weak encryption: insufficient key size#4926
aschackmull merged 7 commits into
github:mainfrom
luchua-bc:java/insufficient-key-size

Conversation

luchua-bc commented Jan 7, 2021

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

smowton commented Jan 11, 2021

Copy link
Copy Markdown
Contributor

OK sure that's enough factoring, running tests

JarLob commented Jan 14, 2021

Copy link
Copy Markdown
Contributor

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 commented Jan 15, 2021

Copy link
Copy Markdown
Contributor Author

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

smowton commented Jan 27, 2021

Copy link
Copy Markdown
Contributor

Please rebase. Pinged a codeowner for final review.

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

luchua-bc commented Jan 28, 2021

Copy link
Copy Markdown
Contributor Author

Thanks @smowton. I've rebased this branch.

aschackmull requested changes Feb 2, 2021
View reviewed changes

aschackmull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks mostly good to me. Just a few comments.

Comment thread java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java Outdated
Comment thread java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java Outdated
Comment thread java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp Outdated
Comment thread java/ql/src/semmle/code/java/security/Encryption.qll Outdated

luchua-bc commented Feb 2, 2021

Copy link
Copy Markdown
Contributor Author

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

aschackmull merged commit 40d02e7 into github:main Feb 3, 2021
luchua-bc deleted the java/insufficient-key-size branch February 3, 2021 15:14
JarLob mentioned this pull request Jun 8, 2021
Closed
1 task
jcogs33 mentioned this pull request Oct 19, 2022
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aschackmull aschackmull approved these changes
+2 more reviewers
smowton smowton left review comments
JarLob JarLob left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

documentation Java

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL