From 2001811586b6b86192f927d01782c4d1a6206e7c Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Thu, 28 Sep 2023 21:05:39 +0200 Subject: [PATCH 1/2] feat(rules): implement `flake8-bandit` `S505` --- .../test/fixtures/flake8_bandit/S505.py | 47 +++++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_bandit/mod.rs | 1 + .../src/rules/flake8_bandit/rules/mod.rs | 2 + .../rules/weak_cryptographic_key.rs | 172 ++++++++++++++++++ ...s__flake8_bandit__tests__S505_S505.py.snap | 143 +++++++++++++++ ruff.schema.json | 1 + 8 files changed, 370 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py new file mode 100644 index 0000000000000..71262866cbd93 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py @@ -0,0 +1,47 @@ +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 + + +# Okay +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()) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 77185c3c44231..1d8a45018cf64 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -592,6 +592,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::FlaskDebugTrue) { flake8_bandit::rules::flask_debug_true(checker, call); } + if checker.enabled(Rule::WeakCryptographicKey) { + flake8_bandit::rules::weak_cryptographic_key(checker, call); + } if checker.any_enabled(&[ Rule::SubprocessWithoutShellEqualsTrue, Rule::SubprocessPopenWithShellEqualsTrue, diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5de943cd83a6b..c6741ce9b8027 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -597,6 +597,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Bandit, "323") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::SuspiciousUnverifiedContextUsage), (Flake8Bandit, "324") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::HashlibInsecureHashFunction), (Flake8Bandit, "501") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::RequestWithNoCertValidation), + (Flake8Bandit, "505") => (RuleGroup::Preview, rules::flake8_bandit::rules::WeakCryptographicKey), (Flake8Bandit, "506") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::UnsafeYAMLLoad), (Flake8Bandit, "507") => (RuleGroup::Preview, rules::flake8_bandit::rules::SSHNoHostKeyVerification), (Flake8Bandit, "508") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::SnmpInsecureVersion), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs index 8c64fd4d9fdcd..1c721660ac779 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs @@ -47,6 +47,7 @@ mod tests { #[test_case(Rule::TryExceptPass, Path::new("S110.py"))] #[test_case(Rule::UnixCommandWildcardInjection, Path::new("S609.py"))] #[test_case(Rule::UnsafeYAMLLoad, Path::new("S506.py"))] + #[test_case(Rule::WeakCryptographicKey, Path::new("S505.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs index 8a467e169f7d2..24ad5a3721c85 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs @@ -22,6 +22,7 @@ pub(crate) use suspicious_function_call::*; pub(crate) use try_except_continue::*; pub(crate) use try_except_pass::*; pub(crate) use unsafe_yaml_load::*; +pub(crate) use weak_cryptographic_key::*; mod assert_used; mod bad_file_permissions; @@ -47,3 +48,4 @@ mod suspicious_function_call; mod try_except_continue; mod try_except_pass; mod unsafe_yaml_load; +mod weak_cryptographic_key; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs new file mode 100644 index 0000000000000..eb96510ea2423 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs @@ -0,0 +1,172 @@ +use once_cell::sync::Lazy; +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 rustc_hash::FxHashSet; +use similar::DiffableStr; +use std::fmt::{Display, Formatter}; + +use crate::checkers::ast::Checker; + +static VULNERABLE_ELLIPTIC_CURVE_KEYS: Lazy> = + Lazy::new(|| FxHashSet::from_iter(["SECP192R1", "SECT163K1", "SECT163R2"])); + +#[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 } => VULNERABLE_ELLIPTIC_CURVE_KEYS.contains(algorithm.as_str()), + } + } +} + +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"), + } + } +} + +/// ## What it does +/// Checks for uses of `cryptographic key lengths known to be vulnerable. +/// +/// ## Why is this bad? +/// Small key lengths can easily be breakable, as computational power +/// increases. For DSA and RSA keys, it is recommended to use key lengths equal +/// or higher to 2048 bits. For EC, it is recommended to use curves higher than +/// 224 bits. +/// +/// ## 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" + ) + } +} + +fn extract_int_argument(call: &ExprCall, name: &str, position: usize) -> Option<(u16, TextRange)> { + let Some(argument) = call.arguments.find_argument(name, position) else { + return None; + }; + let Expr::Constant(ast::ExprConstant { + value: Constant::Int(i), + .. + }) = argument + else { + return None; + }; + Some((i.as_u16()?, argument.range())) +} + +fn extract_cryptographic_key( + checker: &mut Checker, + call: &ExprCall, +) -> Option<(CryptographicKey, TextRange)> { + return checker + .semantic() + .resolve_call_path(&call.func) + .and_then(|call_path| match call_path.as_slice() { + ["cryptography", "hazmat", "primitives", "asymmetric", function, "generate_private_key"] => { + return match function.as_str() { + Some("dsa") => { + let Some((key_size, range)) = extract_int_argument(call, "key_size", 0) else {return None}; + return Some((CryptographicKey::Dsa { key_size }, range)); + }, + Some("rsa") => { + let Some((key_size, range)) = extract_int_argument(call, "key_size", 1) else {return None}; + return Some((CryptographicKey::Rsa { key_size }, range)); + }, + Some("ec") => { + let Some(argument) = call.arguments.find_argument("curve", 0) else { return None }; + let Expr::Attribute(ExprAttribute { attr, value, .. }) = argument else { return None }; + + if checker + .semantic() + .resolve_call_path(value) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["cryptography", "hazmat", "primitives", "asymmetric", "ec"])) + { + return Some((CryptographicKey::Ec{algorithm: attr.as_str().to_string()}, argument.range())); + } + return None; + }, + _ => None, + }; + }, + ["Crypto" | "Cryptodome", "PublicKey", function, "generate"] => { + return match function.as_str() { + Some("DSA") => { + let Some((key_size, range)) = extract_int_argument(call, "bits", 0) else {return None}; + return Some((CryptographicKey::Dsa { key_size }, range)); + }, + Some("RSA") => { + let Some((key_size, range)) = extract_int_argument(call, "bits", 0) else {return None}; + return Some((CryptographicKey::Dsa { key_size }, range)); + }, + _ => None, + }; + }, + _ => None, + }); +} + +/// S505 +pub(crate) fn weak_cryptographic_key(checker: &mut Checker, call: &ExprCall) { + let Expr::Attribute(_) = call.func.as_ref() else { + return; + }; + + 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, + )); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap new file mode 100644 index 0000000000000..b5d55bcd5614d --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap @@ -0,0 +1,143 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs +--- +S505.py:28:35: S505 DSA key sizes below 2048 bits are considered breakable + | +26 | # +27 | # # Errors +28 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) + | ^^^^ S505 +29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) +30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) + | + +S505.py:29:31: S505 EC key sizes below 224 bits are considered breakable + | +27 | # # Errors +28 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) +29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) + | ^^^^^^^^^^^^ S505 +30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) +31 | pycrypto_dsa.generate(bits=2047) + | + +S505.py:30:58: S505 RSA key sizes below 2048 bits are considered breakable + | +28 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) +29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) +30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) + | ^^^^ S505 +31 | pycrypto_dsa.generate(bits=2047) +32 | pycrypto_rsa.generate(bits=2047) + | + +S505.py:31:28: S505 DSA key sizes below 2048 bits are considered breakable + | +29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) +30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) +31 | pycrypto_dsa.generate(bits=2047) + | ^^^^ S505 +32 | pycrypto_rsa.generate(bits=2047) +33 | pycryptodomex_dsa.generate(bits=2047) + | + +S505.py:32:28: S505 DSA key sizes below 2048 bits are considered breakable + | +30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) +31 | pycrypto_dsa.generate(bits=2047) +32 | pycrypto_rsa.generate(bits=2047) + | ^^^^ S505 +33 | pycryptodomex_dsa.generate(bits=2047) +34 | pycryptodomex_rsa.generate(bits=2047) + | + +S505.py:33:33: S505 DSA key sizes below 2048 bits are considered breakable + | +31 | pycrypto_dsa.generate(bits=2047) +32 | pycrypto_rsa.generate(bits=2047) +33 | pycryptodomex_dsa.generate(bits=2047) + | ^^^^ S505 +34 | pycryptodomex_rsa.generate(bits=2047) +35 | dsa.generate_private_key(2047, backends.default_backend()) + | + +S505.py:34:33: S505 DSA key sizes below 2048 bits are considered breakable + | +32 | pycrypto_rsa.generate(bits=2047) +33 | pycryptodomex_dsa.generate(bits=2047) +34 | pycryptodomex_rsa.generate(bits=2047) + | ^^^^ S505 +35 | dsa.generate_private_key(2047, backends.default_backend()) +36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) + | + +S505.py:35:26: S505 DSA key sizes below 2048 bits are considered breakable + | +33 | pycryptodomex_dsa.generate(bits=2047) +34 | pycryptodomex_rsa.generate(bits=2047) +35 | dsa.generate_private_key(2047, backends.default_backend()) + | ^^^^ S505 +36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) +37 | rsa.generate_private_key(3, 2047, backends.default_backend()) + | + +S505.py:36:25: S505 EC key sizes below 224 bits are considered breakable + | +34 | pycryptodomex_rsa.generate(bits=2047) +35 | dsa.generate_private_key(2047, backends.default_backend()) +36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) + | ^^^^^^^^^^^^ S505 +37 | rsa.generate_private_key(3, 2047, backends.default_backend()) +38 | pycrypto_dsa.generate(2047) + | + +S505.py:37:29: S505 RSA key sizes below 2048 bits are considered breakable + | +35 | dsa.generate_private_key(2047, backends.default_backend()) +36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) +37 | rsa.generate_private_key(3, 2047, backends.default_backend()) + | ^^^^ S505 +38 | pycrypto_dsa.generate(2047) +39 | pycrypto_rsa.generate(2047) + | + +S505.py:38:23: S505 DSA key sizes below 2048 bits are considered breakable + | +36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) +37 | rsa.generate_private_key(3, 2047, backends.default_backend()) +38 | pycrypto_dsa.generate(2047) + | ^^^^ S505 +39 | pycrypto_rsa.generate(2047) +40 | pycryptodomex_dsa.generate(2047) + | + +S505.py:39:23: S505 DSA key sizes below 2048 bits are considered breakable + | +37 | rsa.generate_private_key(3, 2047, backends.default_backend()) +38 | pycrypto_dsa.generate(2047) +39 | pycrypto_rsa.generate(2047) + | ^^^^ S505 +40 | pycryptodomex_dsa.generate(2047) +41 | pycryptodomex_rsa.generate(2047) + | + +S505.py:40:28: S505 DSA key sizes below 2048 bits are considered breakable + | +38 | pycrypto_dsa.generate(2047) +39 | pycrypto_rsa.generate(2047) +40 | pycryptodomex_dsa.generate(2047) + | ^^^^ S505 +41 | pycryptodomex_rsa.generate(2047) + | + +S505.py:41:28: S505 DSA key sizes below 2048 bits are considered breakable + | +39 | pycrypto_rsa.generate(2047) +40 | pycryptodomex_dsa.generate(2047) +41 | pycryptodomex_rsa.generate(2047) + | ^^^^ S505 +42 | +43 | # Don't crash when the size is variable + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 9aa7c51714389..6187d070468a3 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3174,6 +3174,7 @@ "S5", "S50", "S501", + "S505", "S506", "S507", "S508", From e2e94f76e6d8bd8154c155aa8daeff2cbb7a461f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 28 Sep 2023 19:12:38 -0400 Subject: [PATCH 2/2] Misc. tweaks --- .../test/fixtures/flake8_bandit/S505.py | 27 ++- .../rules/weak_cryptographic_key.rs | 218 +++++++++--------- ...s__flake8_bandit__tests__S505_S505.py.snap | 167 +++++++------- 3 files changed, 204 insertions(+), 208 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py index 71262866cbd93..a21b28c9077f8 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py @@ -7,11 +7,12 @@ from Cryptodome.PublicKey import DSA as pycryptodomex_dsa from Cryptodome.PublicKey import RSA as pycryptodomex_rsa - -# Okay +# 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()) +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) @@ -23,11 +24,13 @@ pycrypto_rsa.generate(2048) pycryptodomex_dsa.generate(2048) pycryptodomex_rsa.generate(2048) -# -# # Errors + +# 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()) +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) @@ -40,8 +43,12 @@ 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()) +# 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()) +# 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() +) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs index eb96510ea2423..af4ae9f3d6342 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs @@ -1,58 +1,18 @@ -use once_cell::sync::Lazy; +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 rustc_hash::FxHashSet; -use similar::DiffableStr; -use std::fmt::{Display, Formatter}; use crate::checkers::ast::Checker; -static VULNERABLE_ELLIPTIC_CURVE_KEYS: Lazy> = - Lazy::new(|| FxHashSet::from_iter(["SECP192R1", "SECT163K1", "SECT163R2"])); - -#[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 } => VULNERABLE_ELLIPTIC_CURVE_KEYS.contains(algorithm.as_str()), - } - } -} - -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"), - } - } -} - /// ## What it does -/// Checks for uses of `cryptographic key lengths known to be vulnerable. +/// Checks for uses of cryptographic keys with vulnerable key sizes. /// /// ## Why is this bad? -/// Small key lengths can easily be breakable, as computational power -/// increases. For DSA and RSA keys, it is recommended to use key lengths equal -/// or higher to 2048 bits. For EC, it is recommended to use curves higher than -/// 224 bits. +/// 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 @@ -88,77 +48,8 @@ impl Violation for WeakCryptographicKey { } } -fn extract_int_argument(call: &ExprCall, name: &str, position: usize) -> Option<(u16, TextRange)> { - let Some(argument) = call.arguments.find_argument(name, position) else { - return None; - }; - let Expr::Constant(ast::ExprConstant { - value: Constant::Int(i), - .. - }) = argument - else { - return None; - }; - Some((i.as_u16()?, argument.range())) -} - -fn extract_cryptographic_key( - checker: &mut Checker, - call: &ExprCall, -) -> Option<(CryptographicKey, TextRange)> { - return checker - .semantic() - .resolve_call_path(&call.func) - .and_then(|call_path| match call_path.as_slice() { - ["cryptography", "hazmat", "primitives", "asymmetric", function, "generate_private_key"] => { - return match function.as_str() { - Some("dsa") => { - let Some((key_size, range)) = extract_int_argument(call, "key_size", 0) else {return None}; - return Some((CryptographicKey::Dsa { key_size }, range)); - }, - Some("rsa") => { - let Some((key_size, range)) = extract_int_argument(call, "key_size", 1) else {return None}; - return Some((CryptographicKey::Rsa { key_size }, range)); - }, - Some("ec") => { - let Some(argument) = call.arguments.find_argument("curve", 0) else { return None }; - let Expr::Attribute(ExprAttribute { attr, value, .. }) = argument else { return None }; - - if checker - .semantic() - .resolve_call_path(value) - .is_some_and(|call_path| matches!(call_path.as_slice(), ["cryptography", "hazmat", "primitives", "asymmetric", "ec"])) - { - return Some((CryptographicKey::Ec{algorithm: attr.as_str().to_string()}, argument.range())); - } - return None; - }, - _ => None, - }; - }, - ["Crypto" | "Cryptodome", "PublicKey", function, "generate"] => { - return match function.as_str() { - Some("DSA") => { - let Some((key_size, range)) = extract_int_argument(call, "bits", 0) else {return None}; - return Some((CryptographicKey::Dsa { key_size }, range)); - }, - Some("RSA") => { - let Some((key_size, range)) = extract_int_argument(call, "bits", 0) else {return None}; - return Some((CryptographicKey::Dsa { key_size }, range)); - }, - _ => None, - }; - }, - _ => None, - }); -} - /// S505 pub(crate) fn weak_cryptographic_key(checker: &mut Checker, call: &ExprCall) { - let Expr::Attribute(_) = call.func.as_ref() else { - return; - }; - let Some((cryptographic_key, range)) = extract_cryptographic_key(checker, call) else { return; }; @@ -170,3 +61,102 @@ pub(crate) fn weak_cryptographic_key(checker: &mut Checker, call: &ExprCall) { )); } } + +#[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())) +} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap index b5d55bcd5614d..bc14a8556f74a 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S505_S505.py.snap @@ -1,143 +1,142 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S505.py:28:35: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:29:35: S505 DSA key sizes below 2048 bits are considered breakable | -26 | # -27 | # # Errors -28 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) +28 | # Errors +29 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) | ^^^^ S505 -29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) -30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) +30 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) +31 | rsa.generate_private_key( | -S505.py:29:31: S505 EC key sizes below 224 bits are considered breakable +S505.py:30:31: S505 EC key sizes below 224 bits are considered breakable | -27 | # # Errors -28 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) -29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) +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 -30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) -31 | pycrypto_dsa.generate(bits=2047) +31 | rsa.generate_private_key( +32 | public_exponent=65537, key_size=2047, backend=backends.default_backend() | -S505.py:30:58: S505 RSA key sizes below 2048 bits are considered breakable +S505.py:32:37: S505 RSA key sizes below 2048 bits are considered breakable | -28 | dsa.generate_private_key(key_size=2047, backend=backends.default_backend()) -29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) -30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) - | ^^^^ S505 -31 | pycrypto_dsa.generate(bits=2047) -32 | pycrypto_rsa.generate(bits=2047) +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:31:28: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:34:28: S505 DSA key sizes below 2048 bits are considered breakable | -29 | ec.generate_private_key(curve=ec.SECT163R2, backend=backends.default_backend()) -30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) -31 | pycrypto_dsa.generate(bits=2047) +32 | public_exponent=65537, key_size=2047, backend=backends.default_backend() +33 | ) +34 | pycrypto_dsa.generate(bits=2047) | ^^^^ S505 -32 | pycrypto_rsa.generate(bits=2047) -33 | pycryptodomex_dsa.generate(bits=2047) +35 | pycrypto_rsa.generate(bits=2047) +36 | pycryptodomex_dsa.generate(bits=2047) | -S505.py:32:28: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:35:28: S505 DSA key sizes below 2048 bits are considered breakable | -30 | rsa.generate_private_key(public_exponent=65537, key_size=2047, backend=backends.default_backend()) -31 | pycrypto_dsa.generate(bits=2047) -32 | pycrypto_rsa.generate(bits=2047) +33 | ) +34 | pycrypto_dsa.generate(bits=2047) +35 | pycrypto_rsa.generate(bits=2047) | ^^^^ S505 -33 | pycryptodomex_dsa.generate(bits=2047) -34 | pycryptodomex_rsa.generate(bits=2047) +36 | pycryptodomex_dsa.generate(bits=2047) +37 | pycryptodomex_rsa.generate(bits=2047) | -S505.py:33:33: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:36:33: S505 DSA key sizes below 2048 bits are considered breakable | -31 | pycrypto_dsa.generate(bits=2047) -32 | pycrypto_rsa.generate(bits=2047) -33 | pycryptodomex_dsa.generate(bits=2047) +34 | pycrypto_dsa.generate(bits=2047) +35 | pycrypto_rsa.generate(bits=2047) +36 | pycryptodomex_dsa.generate(bits=2047) | ^^^^ S505 -34 | pycryptodomex_rsa.generate(bits=2047) -35 | dsa.generate_private_key(2047, backends.default_backend()) +37 | pycryptodomex_rsa.generate(bits=2047) +38 | dsa.generate_private_key(2047, backends.default_backend()) | -S505.py:34:33: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:37:33: S505 DSA key sizes below 2048 bits are considered breakable | -32 | pycrypto_rsa.generate(bits=2047) -33 | pycryptodomex_dsa.generate(bits=2047) -34 | pycryptodomex_rsa.generate(bits=2047) +35 | pycrypto_rsa.generate(bits=2047) +36 | pycryptodomex_dsa.generate(bits=2047) +37 | pycryptodomex_rsa.generate(bits=2047) | ^^^^ S505 -35 | dsa.generate_private_key(2047, backends.default_backend()) -36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) +38 | dsa.generate_private_key(2047, backends.default_backend()) +39 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) | -S505.py:35:26: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:38:26: S505 DSA key sizes below 2048 bits are considered breakable | -33 | pycryptodomex_dsa.generate(bits=2047) -34 | pycryptodomex_rsa.generate(bits=2047) -35 | dsa.generate_private_key(2047, backends.default_backend()) +36 | pycryptodomex_dsa.generate(bits=2047) +37 | pycryptodomex_rsa.generate(bits=2047) +38 | dsa.generate_private_key(2047, backends.default_backend()) | ^^^^ S505 -36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) -37 | rsa.generate_private_key(3, 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.py:36:25: S505 EC key sizes below 224 bits are considered breakable +S505.py:39:25: S505 EC key sizes below 224 bits are considered breakable | -34 | pycryptodomex_rsa.generate(bits=2047) -35 | dsa.generate_private_key(2047, backends.default_backend()) -36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) +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 -37 | rsa.generate_private_key(3, 2047, backends.default_backend()) -38 | pycrypto_dsa.generate(2047) +40 | rsa.generate_private_key(3, 2047, backends.default_backend()) +41 | pycrypto_dsa.generate(2047) | -S505.py:37:29: S505 RSA key sizes below 2048 bits are considered breakable +S505.py:40:29: S505 RSA key sizes below 2048 bits are considered breakable | -35 | dsa.generate_private_key(2047, backends.default_backend()) -36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) -37 | rsa.generate_private_key(3, 2047, backends.default_backend()) +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 -38 | pycrypto_dsa.generate(2047) -39 | pycrypto_rsa.generate(2047) +41 | pycrypto_dsa.generate(2047) +42 | pycrypto_rsa.generate(2047) | -S505.py:38:23: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:41:23: S505 DSA key sizes below 2048 bits are considered breakable | -36 | ec.generate_private_key(ec.SECT163R2, backends.default_backend()) -37 | rsa.generate_private_key(3, 2047, backends.default_backend()) -38 | pycrypto_dsa.generate(2047) +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 -39 | pycrypto_rsa.generate(2047) -40 | pycryptodomex_dsa.generate(2047) +42 | pycrypto_rsa.generate(2047) +43 | pycryptodomex_dsa.generate(2047) | -S505.py:39:23: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:42:23: S505 DSA key sizes below 2048 bits are considered breakable | -37 | rsa.generate_private_key(3, 2047, backends.default_backend()) -38 | pycrypto_dsa.generate(2047) -39 | pycrypto_rsa.generate(2047) +40 | rsa.generate_private_key(3, 2047, backends.default_backend()) +41 | pycrypto_dsa.generate(2047) +42 | pycrypto_rsa.generate(2047) | ^^^^ S505 -40 | pycryptodomex_dsa.generate(2047) -41 | pycryptodomex_rsa.generate(2047) +43 | pycryptodomex_dsa.generate(2047) +44 | pycryptodomex_rsa.generate(2047) | -S505.py:40:28: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:43:28: S505 DSA key sizes below 2048 bits are considered breakable | -38 | pycrypto_dsa.generate(2047) -39 | pycrypto_rsa.generate(2047) -40 | pycryptodomex_dsa.generate(2047) +41 | pycrypto_dsa.generate(2047) +42 | pycrypto_rsa.generate(2047) +43 | pycryptodomex_dsa.generate(2047) | ^^^^ S505 -41 | pycryptodomex_rsa.generate(2047) +44 | pycryptodomex_rsa.generate(2047) | -S505.py:41:28: S505 DSA key sizes below 2048 bits are considered breakable +S505.py:44:28: S505 DSA key sizes below 2048 bits are considered breakable | -39 | pycrypto_rsa.generate(2047) -40 | pycryptodomex_dsa.generate(2047) -41 | pycryptodomex_rsa.generate(2047) +42 | pycrypto_rsa.generate(2047) +43 | pycryptodomex_dsa.generate(2047) +44 | pycryptodomex_rsa.generate(2047) | ^^^^ S505 -42 | -43 | # Don't crash when the size is variable +45 | +46 | # Don't crash when the size is variable. |