Rust: Weak encryption algorithm query.

[geoffw0]

New weak encryption algorithm query for Rust (rust/weak-cryptographic-algorithm). This uses the same framework as for javascript, python and ruby - in particular ConceptsShared.qll, CryptoAlgorithms.qll and CryptoAlgorithmNames.qll - which paves the way for similar queries such as weak hashing and ... something to do with http as well by the looks of it.

Limitations:

• only one cryptography library is modelled, though I think it's the main one.
• no models / tests for block modes, so we wont get any results for ECB encryption yet.
• the path matching in frameworks/RustCrypto.qll is really clunky at the moment. We need to figure out a way to do this that is both (1) more readable and (2) more reliable - one of the tests defeats it with a simple type alias.

TODO:

• DCA run
• code review
• docs review

@RasmusWL - I'm told you're the right person to talk to about the shared concepts stuff. I'm interested to hear what you think our next priority should be.

[github-actions]

QHelp previews:

rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.qhelp

Use of a broken or weak cryptographic algorithm

Using broken or weak cryptographic algorithms can leave data vulnerable to being decrypted or forged by an attacker.

Many cryptographic algorithms provided by cryptography libraries are known to be weak, or flawed. Using such an algorithm means that encrypted or hashed data is less secure than it appears to be.

This query alerts on any use of a weak cryptographic algorithm, that is not a hashing algorithm. Use of broken or weak cryptographic hash functions are handled by the rust/weak-sensitive-data-hashing query.

Recommendation

Ensure that you use a strong, modern cryptographic algorithm, such as AES-128 or RSA-2048.

Example

The following code uses the des crate from the RustCrypto family to encrypt some secret data. The DES algorithm is old and considered very weak.

let des_cipher = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // BAD: weak encryption
let encryption_result = des_cipher.encrypt_padded_mut::<des::cipher::block_padding::Pkcs7>(data, data_len);

Instead, we should use a strong modern algorithm. In this case, we have selected the 256-bit version of the AES algorithm.

let aes_cipher = cbc::Encryptor::<aes::Aes256>::new(key.into(), iv.into()); // GOOD: strong encryption
let encryption_result = aes_cipher.encrypt_padded_mut::<aes::cipher::block_padding::Pkcs7>(data, data_len);

References

• NIST, FIPS 140 Annex A: Approved Security Functions.
• NIST, SP 800-131A Revision 2: Transitioning the Use of Cryptographic Algorithms and Key Lengths.
• OWASP: Cryptographic Storage Cheat Sheet - Algorithms.
• Common Weakness Enumeration: CWE-327.

[mchammer01]

LGTM ✨
A few comments for your consideration. Nothing major, apart from the link to the archive publication.

[geoffw0]

Thanks for your review @mchammer01. All suggestions accepted.

[mchammer01]

LGTM

[geoffw0]

DCA LGTM.

[aibaars]

This looks good to me. Syntax highlighting is missing for the Rust samples in the rendered QHelp. The blocks start with ```none instead of ```rust . I suppose we'll need to teach the qhelp renderer that .rs files are rust.

[aibaars]

This query doesn't exist yet, but I guess you are planning to add it shortly, so it's fine to leave it like this,

[geoffw0]

Also a good point. We do indeed plan to add it soon, but I'll make a note to modify this if we do not.

[hvitved]

Some minor QL stylistic comments.

[geoffw0]

Syntax highlighting is missing for the Rust samples in the rendered QHelp.

Good point. I've created an issue for this, I think I know what to do but I don't have time to get on it right now.

[geoffw0]

Thanks for the reviews - I've accepted the code suggestions, I think the other points warrant follow-up work.

[aibaars]

@geoffw0 I re-ran the qhelp preview workflow, the syntax highlighting looks better now.

[aibaars]

Don't forget to change the sync'd copies as well.

[geoffw0]

Are you happy that we do that? I didn't want to interfere with other languages unless there's some level of agreement.

[aibaars]

I'm happy with that, although if you like to leave things as-is, that's also fine with me.

[geoffw0]

I've done the change.

[geoffw0]

@paldepind the interface is still showing you as "requested changes", please could you approve ... or point out what I've missed?