Skip to content

Java: Promote insufficient key size query from experimental #10785

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

Merged
merged 59 commits into from
Nov 8, 2022

Conversation

jcogs33
Copy link
Contributor

@jcogs33 jcogs33 commented Oct 11, 2022

This PR promotes #4926 from experimental.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2022

QHelp previews:

java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.qhelp

Use of a cryptographic algorithm with insufficient key size

Modern encryption relies on the computational infeasibility of breaking a cipher and decoding its message without the key. As computational power increases, the ability to break ciphers grows, and key sizes need to become larger as a result. Cryptographic algorithms that use too small of a key size are vulnerable to brute force attacks, which can reveal sensitive data.

Recommendation

Use a key of the recommended size or larger. The key size should be at least 128 bits for AES encryption, 256 bits for elliptic-curve cryptography (ECC), and 2048 bits for RSA, DSA, or DH encryption.

Example

The following code uses cryptographic algorithms with insufficient key sizes.

    KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA");
    keyPairGen1.initialize(1024); // BAD: Key size is less than 2048

    KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("DSA");
    keyPairGen2.initialize(1024); // BAD: Key size is less than 2048

    KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DH");
    keyPairGen3.initialize(1024); // BAD: Key size is less than 2048

    KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("EC");
    ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1"); // BAD: Key size is less than 256
    keyPairGen4.initialize(ecSpec);

    KeyGenerator keyGen = KeyGenerator.getInstance("AES");
    keyGen.init(64); // BAD: Key size is less than 128

To fix the code, change the key sizes to be the recommended size or larger for each algorithm.

References

@jcogs33 jcogs33 changed the title Java: Add query to detect insufficient key size (globalflow keysize) Java: Add query to detect insufficient key size Oct 19, 2022
@jcogs33 jcogs33 changed the title Java: Add query to detect insufficient key size Java: Promote insufficient key size query from experimental Oct 19, 2022
@jcogs33 jcogs33 marked this pull request as ready for review October 19, 2022 15:46
@jcogs33 jcogs33 requested a review from a team as a code owner October 19, 2022 15:46
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Great use of flow states @jcogs33! 😄

I added some comments, but I'll probably give it another review if you decide to apply the suggested refactor (should be easier to review the details then).

Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Two more minor comments, but otherwise this LGTM. If DCA and MRVA are happy, let's ask for a docs review and then merge.

@jcogs33 jcogs33 added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Oct 31, 2022
@lucascosti
Copy link

👋 Docs first responder here! I've put this on our review board for a writer to pick up and review.

@mchammer01 mchammer01 self-requested a review November 3, 2022 11:51
mchammer01
mchammer01 previously approved these changes Nov 3, 2022
Copy link
Contributor

@mchammer01 mchammer01 left a comment

Choose a reason for hiding this comment

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

@jcogs33 👋🏻 - this looks good from a Docs point of view ✨
Added a couple of very minor comments (feel free to ignore them if you don't agree 🙂 )

Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

🎉

@jcogs33 jcogs33 merged commit cfbaf5e into github:main Nov 8, 2022
@jcogs33 jcogs33 deleted the insuff-key-size-globalflow-keysize branch November 10, 2022 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Java ready-for-doc-review This PR requires and is ready for review from the GitHub docs team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants