-
Notifications
You must be signed in to change notification settings - Fork 879
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Part of #1646. ## Summary Implement `S505` ([`weak_cryptographic_key`](https://bandit.readthedocs.io/en/latest/plugins/b505_weak_cryptographic_key.html)) rule from `bandit`. For this rule, `bandit` [reports the issue with](https://github.com/PyCQA/bandit/blob/1.7.5/bandit/plugins/weak_cryptographic_key.py#L47-L56): - medium severity for DSA/RSA < 2048 bits and EC < 224 bits - high severity for DSA/RSA < 1024 bits and EC < 160 bits Since Ruff does not handle severities for `bandit`-related rules, we could either report the issue if we have lower values than medium severity, or lower values than high one. Two reasons led me to choose the first option: - a medium severity issue is still a security issue we would want to report to the user, who can then decide to either handle the issue or ignore it - `bandit` [maps the EC key algorithms to their respective key lengths in bits](https://github.com/PyCQA/bandit/blob/1.7.5/bandit/plugins/weak_cryptographic_key.py#L112-L133), but there is no value below 160 bits, so technically `bandit` would never report medium severity issues for EC keys, only high ones Another consideration is that as shared just above, for EC key algorithms, `bandit` has a mapping to map the algorithms to their respective key lengths. In the implementation in Ruff, I rather went with an explicit list of EC algorithms known to be vulnerable (which would thus be reported) rather than implementing a mapping to retrieve the associated key length and comparing it with the minimum value. ## Test Plan Snapshot tests from https://github.com/PyCQA/bandit/blob/1.7.5/examples/weak_cryptographic_key_sizes.py.
- Loading branch information
1 parent
c2a9cf8
commit 5989745
Showing
8 changed files
with
366 additions
and
0 deletions.
There are no files selected for viewing
54 changes: 54 additions & 0 deletions
54
crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
from cryptography.hazmat import backends | ||
from cryptography.hazmat.primitives.asymmetric import dsa | ||
from cryptography.hazmat.primitives.asymmetric import ec | ||
from cryptography.hazmat.primitives.asymmetric import rsa | ||
from Crypto.PublicKey import DSA as pycrypto_dsa | ||
from Crypto.PublicKey import RSA as pycrypto_rsa | ||
from Cryptodome.PublicKey import DSA as pycryptodomex_dsa | ||
from Cryptodome.PublicKey import RSA as pycryptodomex_rsa | ||
|
||
# OK | ||
dsa.generate_private_key(key_size=2048, backend=backends.default_backend()) | ||
ec.generate_private_key(curve=ec.SECP384R1, backend=backends.default_backend()) | ||
rsa.generate_private_key( | ||
public_exponent=65537, key_size=2048, backend=backends.default_backend() | ||
) | ||
pycrypto_dsa.generate(bits=2048) | ||
pycrypto_rsa.generate(bits=2048) | ||
pycryptodomex_dsa.generate(bits=2048) | ||
pycryptodomex_rsa.generate(bits=2048) | ||
dsa.generate_private_key(2048, backends.default_backend()) | ||
ec.generate_private_key(ec.SECP256K1, backends.default_backend()) | ||
rsa.generate_private_key(3, 2048, backends.default_backend()) | ||
pycrypto_dsa.generate(2048) | ||
pycrypto_rsa.generate(2048) | ||
pycryptodomex_dsa.generate(2048) | ||
pycryptodomex_rsa.generate(2048) | ||
|
||
# Errors | ||
dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) | ||
ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) | ||
rsa.generate_private_key( | ||
public_exponent=65537, key_size=2047, backend=backends.default_backend() | ||
) | ||
pycrypto_dsa.generate(bits=2047) | ||
pycrypto_rsa.generate(bits=2047) | ||
pycryptodomex_dsa.generate(bits=2047) | ||
pycryptodomex_rsa.generate(bits=2047) | ||
dsa.generate_private_key(2047, backends.default_backend()) | ||
ec.generate_private_key(ec.SECT163R2, backends.default_backend()) | ||
rsa.generate_private_key(3, 2047, backends.default_backend()) | ||
pycrypto_dsa.generate(2047) | ||
pycrypto_rsa.generate(2047) | ||
pycryptodomex_dsa.generate(2047) | ||
pycryptodomex_rsa.generate(2047) | ||
|
||
# Don't crash when the size is variable. | ||
rsa.generate_private_key( | ||
public_exponent=65537, key_size=some_key_size, backend=backends.default_backend() | ||
) | ||
|
||
# Can't reliably know which curve was passed, in some cases like below. | ||
ec.generate_private_key( | ||
curve=curves[self.curve]["create"](self.size), backend=backends.default_backend() | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
162 changes: 162 additions & 0 deletions
162
crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,162 @@ | ||
use std::fmt::{Display, Formatter}; | ||
|
||
use ruff_diagnostics::{Diagnostic, Violation}; | ||
use ruff_macros::{derive_message_formats, violation}; | ||
use ruff_python_ast::{self as ast, Constant, Expr, ExprAttribute, ExprCall}; | ||
use ruff_text_size::{Ranged, TextRange}; | ||
|
||
use crate::checkers::ast::Checker; | ||
|
||
/// ## What it does | ||
/// Checks for uses of cryptographic keys with vulnerable key sizes. | ||
/// | ||
/// ## Why is this bad? | ||
/// Small keys are easily breakable. For DSA and RSA, keys should be at least | ||
/// 2048 bits long. For EC, keys should be at least 224 bits long. | ||
/// | ||
/// ## Example | ||
/// ```python | ||
/// from cryptography.hazmat.primitives.asymmetric import dsa, ec | ||
/// | ||
/// dsa.generate_private_key(key_size=512) | ||
/// ec.generate_private_key(curve=ec.SECT163K1) | ||
/// ``` | ||
/// | ||
/// Use instead: | ||
/// ```python | ||
/// from cryptography.hazmat.primitives.asymmetric import dsa, ec | ||
/// | ||
/// dsa.generate_private_key(key_size=4096) | ||
/// ec.generate_private_key(curve=ec.SECP384R1) | ||
/// ``` | ||
/// | ||
/// ## References | ||
/// - [CSRC: Transitioning the Use of Cryptographic Algorithms and Key Lengths](https://csrc.nist.gov/pubs/sp/800/131/a/r2/final) | ||
#[violation] | ||
pub struct WeakCryptographicKey { | ||
cryptographic_key: CryptographicKey, | ||
} | ||
|
||
impl Violation for WeakCryptographicKey { | ||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
let WeakCryptographicKey { cryptographic_key } = self; | ||
let minimum_key_size = cryptographic_key.minimum_key_size(); | ||
format!( | ||
"{cryptographic_key} key sizes below {minimum_key_size} bits are considered breakable" | ||
) | ||
} | ||
} | ||
|
||
/// S505 | ||
pub(crate) fn weak_cryptographic_key(checker: &mut Checker, call: &ExprCall) { | ||
let Some((cryptographic_key, range)) = extract_cryptographic_key(checker, call) else { | ||
return; | ||
}; | ||
|
||
if cryptographic_key.is_vulnerable() { | ||
checker.diagnostics.push(Diagnostic::new( | ||
WeakCryptographicKey { cryptographic_key }, | ||
range, | ||
)); | ||
} | ||
} | ||
|
||
#[derive(Debug, PartialEq, Eq)] | ||
enum CryptographicKey { | ||
Dsa { key_size: u16 }, | ||
Ec { algorithm: String }, | ||
Rsa { key_size: u16 }, | ||
} | ||
|
||
impl CryptographicKey { | ||
const fn minimum_key_size(&self) -> u16 { | ||
match self { | ||
Self::Dsa { .. } | Self::Rsa { .. } => 2048, | ||
Self::Ec { .. } => 224, | ||
} | ||
} | ||
|
||
fn is_vulnerable(&self) -> bool { | ||
match self { | ||
Self::Dsa { key_size } | Self::Rsa { key_size } => key_size < &self.minimum_key_size(), | ||
Self::Ec { algorithm } => { | ||
matches!(algorithm.as_str(), "SECP192R1" | "SECT163K1" | "SECT163R2") | ||
} | ||
} | ||
} | ||
} | ||
|
||
impl Display for CryptographicKey { | ||
fn fmt(&self, fmt: &mut Formatter) -> std::fmt::Result { | ||
match self { | ||
CryptographicKey::Dsa { .. } => fmt.write_str("DSA"), | ||
CryptographicKey::Ec { .. } => fmt.write_str("EC"), | ||
CryptographicKey::Rsa { .. } => fmt.write_str("RSA"), | ||
} | ||
} | ||
} | ||
|
||
fn extract_cryptographic_key( | ||
checker: &mut Checker, | ||
call: &ExprCall, | ||
) -> Option<(CryptographicKey, TextRange)> { | ||
let call_path = checker.semantic().resolve_call_path(&call.func)?; | ||
match call_path.as_slice() { | ||
["cryptography", "hazmat", "primitives", "asymmetric", function, "generate_private_key"] => { | ||
match *function { | ||
"dsa" => { | ||
let (key_size, range) = extract_int_argument(call, "key_size", 0)?; | ||
Some((CryptographicKey::Dsa { key_size }, range)) | ||
} | ||
"rsa" => { | ||
let (key_size, range) = extract_int_argument(call, "key_size", 1)?; | ||
Some((CryptographicKey::Rsa { key_size }, range)) | ||
} | ||
"ec" => { | ||
let argument = call.arguments.find_argument("curve", 0)?; | ||
let ExprAttribute { attr, value, .. } = argument.as_attribute_expr()?; | ||
let call_path = checker.semantic().resolve_call_path(value)?; | ||
if matches!( | ||
call_path.as_slice(), | ||
["cryptography", "hazmat", "primitives", "asymmetric", "ec"] | ||
) { | ||
Some(( | ||
CryptographicKey::Ec { | ||
algorithm: attr.to_string(), | ||
}, | ||
argument.range(), | ||
)) | ||
} else { | ||
None | ||
} | ||
} | ||
_ => None, | ||
} | ||
} | ||
["Crypto" | "Cryptodome", "PublicKey", function, "generate"] => match *function { | ||
"DSA" => { | ||
let (key_size, range) = extract_int_argument(call, "bits", 0)?; | ||
Some((CryptographicKey::Dsa { key_size }, range)) | ||
} | ||
"RSA" => { | ||
let (key_size, range) = extract_int_argument(call, "bits", 0)?; | ||
Some((CryptographicKey::Dsa { key_size }, range)) | ||
} | ||
_ => None, | ||
}, | ||
_ => None, | ||
} | ||
} | ||
|
||
fn extract_int_argument(call: &ExprCall, name: &str, position: usize) -> Option<(u16, TextRange)> { | ||
let argument = call.arguments.find_argument(name, position)?; | ||
let Expr::Constant(ast::ExprConstant { | ||
value: Constant::Int(i), | ||
.. | ||
}) = argument | ||
else { | ||
return None; | ||
}; | ||
Some((i.as_u16()?, argument.range())) | ||
} |
142 changes: 142 additions & 0 deletions
142
...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
--- | ||
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs | ||
--- | ||
S505.py:29:35: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
28 | # Errors | ||
29 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) | ||
| ^^^^ S505 | ||
30 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) | ||
31 | rsa.generate_private_key( | ||
| | ||
|
||
S505.py:30:31: S505 EC key sizes below 224 bits are considered breakable | ||
| | ||
28 | # Errors | ||
29 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) | ||
30 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) | ||
| ^^^^^^^^^^^^ S505 | ||
31 | rsa.generate_private_key( | ||
32 | public_exponent=65537, key_size=2047, backend=backends.default_backend() | ||
| | ||
|
||
S505.py:32:37: S505 RSA key sizes below 2048 bits are considered breakable | ||
| | ||
30 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) | ||
31 | rsa.generate_private_key( | ||
32 | public_exponent=65537, key_size=2047, backend=backends.default_backend() | ||
| ^^^^ S505 | ||
33 | ) | ||
34 | pycrypto_dsa.generate(bits=2047) | ||
| | ||
|
||
S505.py:34:28: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
32 | public_exponent=65537, key_size=2047, backend=backends.default_backend() | ||
33 | ) | ||
34 | pycrypto_dsa.generate(bits=2047) | ||
| ^^^^ S505 | ||
35 | pycrypto_rsa.generate(bits=2047) | ||
36 | pycryptodomex_dsa.generate(bits=2047) | ||
| | ||
|
||
S505.py:35:28: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
33 | ) | ||
34 | pycrypto_dsa.generate(bits=2047) | ||
35 | pycrypto_rsa.generate(bits=2047) | ||
| ^^^^ S505 | ||
36 | pycryptodomex_dsa.generate(bits=2047) | ||
37 | pycryptodomex_rsa.generate(bits=2047) | ||
| | ||
|
||
S505.py:36:33: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
34 | pycrypto_dsa.generate(bits=2047) | ||
35 | pycrypto_rsa.generate(bits=2047) | ||
36 | pycryptodomex_dsa.generate(bits=2047) | ||
| ^^^^ S505 | ||
37 | pycryptodomex_rsa.generate(bits=2047) | ||
38 | dsa.generate_private_key(2047, backends.default_backend()) | ||
| | ||
|
||
S505.py:37:33: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
35 | pycrypto_rsa.generate(bits=2047) | ||
36 | pycryptodomex_dsa.generate(bits=2047) | ||
37 | pycryptodomex_rsa.generate(bits=2047) | ||
| ^^^^ S505 | ||
38 | dsa.generate_private_key(2047, backends.default_backend()) | ||
39 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) | ||
| | ||
|
||
S505.py:38:26: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
36 | pycryptodomex_dsa.generate(bits=2047) | ||
37 | pycryptodomex_rsa.generate(bits=2047) | ||
38 | dsa.generate_private_key(2047, backends.default_backend()) | ||
| ^^^^ S505 | ||
39 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) | ||
40 | rsa.generate_private_key(3, 2047, backends.default_backend()) | ||
| | ||
|
||
S505.py:39:25: S505 EC key sizes below 224 bits are considered breakable | ||
| | ||
37 | pycryptodomex_rsa.generate(bits=2047) | ||
38 | dsa.generate_private_key(2047, backends.default_backend()) | ||
39 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) | ||
| ^^^^^^^^^^^^ S505 | ||
40 | rsa.generate_private_key(3, 2047, backends.default_backend()) | ||
41 | pycrypto_dsa.generate(2047) | ||
| | ||
|
||
S505.py:40:29: S505 RSA key sizes below 2048 bits are considered breakable | ||
| | ||
38 | dsa.generate_private_key(2047, backends.default_backend()) | ||
39 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) | ||
40 | rsa.generate_private_key(3, 2047, backends.default_backend()) | ||
| ^^^^ S505 | ||
41 | pycrypto_dsa.generate(2047) | ||
42 | pycrypto_rsa.generate(2047) | ||
| | ||
|
||
S505.py:41:23: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
39 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) | ||
40 | rsa.generate_private_key(3, 2047, backends.default_backend()) | ||
41 | pycrypto_dsa.generate(2047) | ||
| ^^^^ S505 | ||
42 | pycrypto_rsa.generate(2047) | ||
43 | pycryptodomex_dsa.generate(2047) | ||
| | ||
|
||
S505.py:42:23: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
40 | rsa.generate_private_key(3, 2047, backends.default_backend()) | ||
41 | pycrypto_dsa.generate(2047) | ||
42 | pycrypto_rsa.generate(2047) | ||
| ^^^^ S505 | ||
43 | pycryptodomex_dsa.generate(2047) | ||
44 | pycryptodomex_rsa.generate(2047) | ||
| | ||
|
||
S505.py:43:28: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
41 | pycrypto_dsa.generate(2047) | ||
42 | pycrypto_rsa.generate(2047) | ||
43 | pycryptodomex_dsa.generate(2047) | ||
| ^^^^ S505 | ||
44 | pycryptodomex_rsa.generate(2047) | ||
| | ||
|
||
S505.py:44:28: S505 DSA key sizes below 2048 bits are considered breakable | ||
| | ||
42 | pycrypto_rsa.generate(2047) | ||
43 | pycryptodomex_dsa.generate(2047) | ||
44 | pycryptodomex_rsa.generate(2047) | ||
| ^^^^ S505 | ||
45 | | ||
46 | # Don't crash when the size is variable. | ||
| | ||
|
||
|
Oops, something went wrong.