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 new check for weak RSA keys and padding modes #1737

Merged
merged 6 commits into from
Nov 14, 2022

Conversation

presidentbeef
Copy link
Owner

@presidentbeef presidentbeef commented Oct 24, 2022

Fixes #1736

Warns about these patterns:

  • OpenSSL::PKey::RSA.new or OpenSSL::PKey::RSA.generate with key size < 2048 bits
  • OpenSSL::PKey::RSA#public_encrypt, OpenSSL::PKey::RSA#public_decrypt, OpenSSL::PKey::RSA#private_encrypt, OpenSSL::PKey::RSA#private_decrypt with no padding mode specified (in which case it's PKCS1), with OpenSSL::PKey::RSA::PKCS1, or OpenSSL::PKey::RSA::NO_PADDING

@bdewater
Copy link

bdewater commented Oct 25, 2022

Thank you for working on this! Some feedback:

https://cwe.mitre.org/data/definitions/780.html is quite strong in calling anything other than OAEP padding secure, should we follow that recommendation? I hope nobody picks for example SSLV23_PADDING on purpose, but then again who knows if people don't know why PKCS1_PADDING is insecure.

The PR currently does not seem to detect calls using the generic PKey interface introduced in OpenSSL gem 3.0:

weak_rsa = OpenSSL::PKey.generate_key("RSA", rsa_keygen_bits: 1024)
weak_encrypted = weak_rsa.encrypt("data", rsa_padding_mode: "pkcs1")
weak_signature_digest = weak_rsa.sign("SHA256", "data", rsa_padding_mode: "pkcs1")
weak_signature_raw = weak_rsa.sign_raw(nil, "data", rsa_padding_mode: "pkcs1")

Edit: the new interface takes both Symbol and String-keyed hashes btw.

@presidentbeef
Copy link
Owner Author

Makes sense, I can add those in as well.

@presidentbeef
Copy link
Owner Author

Now warns on key size for OpenSSL::PKey.generate_key.

Warns on any padding mode other than OAEP.

Added padding checks for encrypt, decrypt, sign, verify, sign_raw, verify_raw.

@presidentbeef presidentbeef merged commit 4998655 into main Nov 14, 2022
@presidentbeef presidentbeef deleted the add_check_for_weak_pkey branch November 14, 2022 05:41
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 this pull request may close these issues.

Add checks for weak public-key cryptography
2 participants