Skip to content
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.

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

feat(rules): implement flake8-bandit S505 #7703

Merged
merged 3 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S505.py
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()
)
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,3 +48,4 @@ mod suspicious_function_call;
mod try_except_continue;
mod try_except_pass;
mod unsafe_yaml_load;
mod weak_cryptographic_key;
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()))
}
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.
|


Loading
Loading