Skip to content

Ruby: add CryptoAlgorithms library #7273

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
Merged
alexrford merged 11 commits into from
Dec 22, 2021
Merged

Ruby: add CryptoAlgorithms library #7273

alexrford merged 11 commits into from
Dec 22, 2021

Conversation

@alexrford
Copy link
Contributor

@alexrford alexrford commented Nov 30, 2021 •
edited
Loading

This PR copies CryptoAlgorithms.qll to Ruby and extends it with some additional encryption algorithms (ARIA, IDEA, SM4, SEED, GOST, CAST, CHACHA, CAMELLIA - all classified as strong) and block modes (ECB - classified as weak, and CBC, CCM, CFB, CTR, GCM, OFB - classified as strong).

It then adds a new library, OpenSSL.qll, for Ruby that models parts of the OpenSSL::Cipher library component. The aim here is mostly to be able to take a string cipherName that can be used to construct an OpenSSL::Cipher instance, s.t. we can pass judgement on whether that cipher is "strong" or "weak". This is used in #7313 to decide if this is a safe way to encrypt cookies or not. Some of the implementation here is taken from the Java Encryption.qll library, in particular the regex matching code.

The OpenSSL Ruby library itself wraps a C OpenSSL/LibreSSL library. The known ciphers here were generated using OpenSSL 1.1.1l and LibreSSL 3.4.1, which each support some algorithms that the other does not.

Original description

Adds a CryptoAlgorithms.qll library for Ruby. This is similar to the libraries of the same name for JS and Python, but currently not identical to them. It would be good to unify as much of these as possible, but there may be some parts of this Ruby implementation that are language specific and need to move out into a separate file.

The main change here, relative to the JS/Python version, is the addition of an OpenSSL module. This is specifically to support an upcoming PR which will use this to determine strong/weak ciphers supported by OpenSSL. These ciphers can include both the underlying block encryption algorithm and also the block mode (e.g. aes-256-ecb) - which in particular means that we say that a cipher is weak if either of these components are considered weak. These ciphers are encoded in the OpenSSLCipher class, which is added here as a type of CryptographicAlgorithm.

I've uploaded the diff of this change relative to the existing versions at https://gist.github.com/alexrford/92426d26cb6b0def5d8967d49a0be0a4. In addition to the OpenSSL related changes, I've pulled in some of the regex matching code from the C++ Encryption.qll library, and added a few more encryption algorithms (Cast, GOST, ChaCha).

@RasmusWL I believe you worked on unifying the JS and Python versions of this library? I'd be interested to get your thoughts on what to do in this case, in particular if:

  1. the OpenSSL related code here could make sense in a shared version of this library, or
  2. if it should be factored out into a separate file and Ruby should use the existing CryptoAlgorithms.qll as it is (perhaps with Cast/GOST/ChaCha added)?

@alexrford alexrford added the Ruby label Nov 30, 2021
@alexrford alexrford marked this pull request as ready for review December 5, 2021 23:45
@alexrford alexrford requested a review from a team as a code owner December 5, 2021 23:45
@alexrford alexrford requested review from a team as code owners December 9, 2021 17:42
@alexrford
Copy link
Contributor Author

alexrford commented Dec 9, 2021

I've revisited this since the first version and changed it s.t. CryptoAlgorithms.qll is shared with the Python/JS versions and the OpenSSL related code is moved out to a separate file. This necessitated making the AlgorithmNames module public, which I'm not entirely satisfied with as it seems mostly like an implementation detail. I'm not sure of a good alternative, though.

nickrolfe
nickrolfe reviewed Dec 10, 2021
View reviewed changes
@alexrford
Copy link
Contributor Author

alexrford commented Dec 11, 2021

Updated again, with a few significant changes:

  • Ciphers in the OpenSSL module are based on those available using gems built for both OpenSSL 1.1.1 and for LibreSSL 3.4.1.
  • This in turn adds a few more encryption algorithms: ARIA, IDEA, SEED, and SM4, and block modes: CBC, CFB, CTR, and OFB. These are all classified as strong as I haven't been able to find any attacks that seem to weaken these more than any other algorithms that we've already marked as strong, but I'm of course open to be corrected on this.
  • QLDoc fixes.

@alexrford alexrford force-pushed the ruby/crypto-algorithms branch from b80633a to 0481385 Compare December 11, 2021 16:15
@alexrford alexrford mentioned this pull request Dec 16, 2021
Merged
nickrolfe
nickrolfe reviewed Dec 16, 2021
View reviewed changes
Copy link
Contributor

@nickrolfe nickrolfe left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM. It'd be good to get sign-off from JS/Python as well.

@erik-krogh
Copy link
Contributor

erik-krogh commented Dec 17, 2021

You add the isStrongBlockMode() predicate, but you never use it?

@alexrford
Copy link
Contributor Author

alexrford commented Dec 20, 2021

You add the isStrongBlockMode() predicate, but you never use it?

True, I added this for symmetry with isWeakBlockMode() but on reflection it's not necessary. We use the block mode as a reason to disqualify a cipher from being marked as strong. However, the block mode isn't sufficient to mark a cipher as strong and isn't always present in the cipher name (particularly for stream ciphers like ChaCha where a block mode wouldn't make sense).

I've removed this predicate.

aibaars
aibaars reviewed Dec 22, 2021
View reviewed changes
aibaars
aibaars previously approved these changes Dec 22, 2021
View reviewed changes
alexrford and others added 11 commits December 22, 2021 16:38
@alexrford
Ruby: add CryptoAlgorithms library
1156581
@alexrford
Ruby: add empty ruby file to avoid DataFlowConsistency failure
0303c27
@alexrford
Ruby: split OpenSSL parts from CryptoALgorithms.qll and sync with JS/…
bdb2d8b 
…Python version
@alexrford
Add more encryption algorithms and modes to CryptoAlgorithms::Algorit…
d3af687 
…hmNames

Strong encryption algorithms: ARIA, IDEA, SEED, SM4
Strong block modes: CBC, CFB, CTR, OFB
@alexrford
Ruby: Base OpenSSL supported algorithms on OpenSSL 1.1.1 and LibreSSL…
e6bc45e 
… 3.4.1
@alexrford
Ruby: OpenSSL and CryptoAlgorithms test update
97c75de
@alexrford
Ruby: OpenSSL QLDoc fixes
27a40fb
@alexrford @nickrolfe
Update ruby/ql/lib/codeql/ruby/security/OpenSSL.qll
df0da98 
Co-authored-by: Nick Rolfe <nickrolfe@github.com>
@alexrford
Remove unused isStrongBlockMode predicate from CryptoAlgorithms.qll
f16d776
@alexrford
Move CryptoAlgorithms::AlgorithmsName into a separate internal/Crypto…
a2104de 
…AlgorithmNames.qll
@alexrford
Bump a date
3da98ec
@alexrford alexrford force-pushed the ruby/crypto-algorithms branch from cb516f8 to 3da98ec Compare December 22, 2021 16:38
@alexrford alexrford merged commit 0cbf136 into main Dec 22, 2021
@alexrford alexrford deleted the ruby/crypto-algorithms branch December 22, 2021 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickrolfe nickrolfe nickrolfe left review comments

+1 more reviewer

@aibaars aibaars aibaars approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

JS Python Ruby

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@alexrford @erik-krogh @nickrolfe @aibaars