Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add checks for weak public-key cryptography #1736

Closed
bdewater opened this issue Oct 20, 2022 · 2 comments · Fixed by #1737
Closed

Add checks for weak public-key cryptography #1736

bdewater opened this issue Oct 20, 2022 · 2 comments · Fixed by #1737

Comments

@bdewater
Copy link

bdewater commented Oct 20, 2022

Is your feature request related to a problem? Please describe.
Similar to check_weak_hash.rb and inspired by ruby/openssl#546 - it is quite easy to use RSA and other asymmetric algorithms insecurely. Some ideas that can be checked for:

  • Small key sizes (eg RSA < 2048 bits = CWE-326) when generating keys
  • Insecure padding modes (eg RSA PKCS#1 v1.5 = CWE-780) when doing operations

Describe the solution you'd like
Flag the code like this blog post which specifically showcases CWE-780:

def encrypt(payload)
  public_key = OpenSSL::PKey::RSA.new("grab the public 4096 bit key")
  Base64.encode64(public_key.public_encrypt(payload.to_json)
end

OpenSSL::PKey::RSA#public_encrypt defaults to insecure PKCS#1 v1.5 padding. On modern OpenSSL gems (>= 3.0) this is a wrapper for OpenSSL::PKey::PKey#encrypt, so effectively the following code is called:

public_key.encrypt(payload.to_json, "rsa_padding_mode" => "pkcs1")

Both types of calls should be flagged.

Describe alternatives you've considered
N/A

@presidentbeef
Copy link
Owner

Nice suggestion!

@iamjohnford
Copy link

The current checks don't appear to catch the following version of the weak RSA padding mode:

public_key.encrypt(payload.to_json, rsa_padding_mode: "pkcs1")

Should a check for that be added as well?

Repository owner locked and limited conversation to collaborators May 9, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants