Skip to content

Commit 6dfc1cc

Browse files
authored
[flake8-bandit] Implement S502 SslInsecureVersion rule (#9390)
## Summary Adds S502 rule for the [flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin port. Checks for calls to any function with keywords arguments `ssl_version` or `method` or for kwargs `method` in calls to `OpenSSL.SSL.Context` and `ssl_version` in calls to `ssl.wrap_socket` which have an insecure ssl_version valu. See also https://bandit.readthedocs.io/en/latest/_modules/bandit/plugins/insecure_ssl_tls.html#ssl_with_bad_version ## Test Plan Fixture added ## Issue Link Refers: #1646
1 parent 60ba7a7 commit 6dfc1cc

File tree

8 files changed

+203
-0
lines changed

8 files changed

+203
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import ssl
2+
from ssl import wrap_socket
3+
from OpenSSL import SSL
4+
from OpenSSL.SSL import Context
5+
6+
wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502
7+
ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502
8+
ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502
9+
SSL.Context(method=SSL.SSLv2_METHOD) # S502
10+
SSL.Context(method=SSL.SSLv23_METHOD) # S502
11+
Context(method=SSL.SSLv3_METHOD) # S502
12+
Context(method=SSL.TLSv1_METHOD) # S502
13+
14+
wrap_socket(ssl_version=ssl.PROTOCOL_TLS_CLIENT) # OK
15+
SSL.Context(method=SSL.TLS_SERVER_METHOD) # OK
16+
func(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

+3
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
968968
if checker.enabled(Rule::SslWithNoVersion) {
969969
flake8_bandit::rules::ssl_with_no_version(checker, call);
970970
}
971+
if checker.enabled(Rule::SslInsecureVersion) {
972+
flake8_bandit::rules::ssl_insecure_version(checker, call);
973+
}
971974
}
972975
Expr::Dict(dict) => {
973976
if checker.any_enabled(&[

crates/ruff_linter/src/codes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
642642
(Flake8Bandit, "413") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousPycryptoImport),
643643
(Flake8Bandit, "415") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousPyghmiImport),
644644
(Flake8Bandit, "501") => (RuleGroup::Stable, rules::flake8_bandit::rules::RequestWithNoCertValidation),
645+
(Flake8Bandit, "502") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslInsecureVersion),
645646
(Flake8Bandit, "504") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslWithNoVersion),
646647
(Flake8Bandit, "505") => (RuleGroup::Preview, rules::flake8_bandit::rules::WeakCryptographicKey),
647648
(Flake8Bandit, "506") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnsafeYAMLLoad),

crates/ruff_linter/src/rules/flake8_bandit/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ mod tests {
3636
#[test_case(Rule::SSHNoHostKeyVerification, Path::new("S507.py"))]
3737
#[test_case(Rule::SnmpInsecureVersion, Path::new("S508.py"))]
3838
#[test_case(Rule::SnmpWeakCryptography, Path::new("S509.py"))]
39+
#[test_case(Rule::SslInsecureVersion, Path::new("S502.py"))]
3940
#[test_case(Rule::SslWithNoVersion, Path::new("S504.py"))]
4041
#[test_case(Rule::StartProcessWithAShell, Path::new("S605.py"))]
4142
#[test_case(Rule::StartProcessWithNoShell, Path::new("S606.py"))]

crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub(crate) use shell_injection::*;
2020
pub(crate) use snmp_insecure_version::*;
2121
pub(crate) use snmp_weak_cryptography::*;
2222
pub(crate) use ssh_no_host_key_verification::*;
23+
pub(crate) use ssl_insecure_version::*;
2324
pub(crate) use ssl_with_no_version::*;
2425
pub(crate) use suspicious_function_call::*;
2526
pub(crate) use suspicious_imports::*;
@@ -51,6 +52,7 @@ mod shell_injection;
5152
mod snmp_insecure_version;
5253
mod snmp_weak_cryptography;
5354
mod ssh_no_host_key_verification;
55+
mod ssl_insecure_version;
5456
mod ssl_with_no_version;
5557
mod suspicious_function_call;
5658
mod suspicious_imports;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
use ruff_diagnostics::{Diagnostic, Violation};
2+
use ruff_macros::{derive_message_formats, violation};
3+
use ruff_python_ast::{self as ast, Expr, ExprCall};
4+
use ruff_text_size::Ranged;
5+
6+
use crate::checkers::ast::Checker;
7+
8+
/// ## What it does
9+
/// Checks for function calls with parameters that indicate the use of insecure
10+
/// SSL and TLS protocol versions.
11+
///
12+
/// ## Why is this bad?
13+
/// Several highly publicized exploitable flaws have been discovered in all
14+
/// versions of SSL and early versions of TLS. The following versions are
15+
/// considered insecure, and should be avoided:
16+
/// - SSL v2
17+
/// - SSL v3
18+
/// - TLS v1
19+
/// - TLS v1.1
20+
///
21+
/// This method supports detection on the Python's built-in `ssl` module and
22+
/// the `pyOpenSSL` module.
23+
///
24+
/// ## Example
25+
/// ```python
26+
/// import ssl
27+
///
28+
/// ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1)
29+
/// ```
30+
///
31+
/// Use instead:
32+
/// ```python
33+
/// import ssl
34+
///
35+
/// ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2)
36+
/// ```
37+
#[violation]
38+
pub struct SslInsecureVersion {
39+
protocol: String,
40+
}
41+
42+
impl Violation for SslInsecureVersion {
43+
#[derive_message_formats]
44+
fn message(&self) -> String {
45+
let SslInsecureVersion { protocol } = self;
46+
format!("Call made with insecure SSL protocol: `{protocol}`")
47+
}
48+
}
49+
50+
/// S502
51+
pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) {
52+
let Some(keyword) = checker
53+
.semantic()
54+
.resolve_call_path(call.func.as_ref())
55+
.and_then(|call_path| match call_path.as_slice() {
56+
["ssl", "wrap_socket"] => Some("ssl_version"),
57+
["OpenSSL", "SSL", "Context"] => Some("method"),
58+
_ => None,
59+
})
60+
else {
61+
return;
62+
};
63+
64+
let Some(keyword) = call.arguments.find_keyword(keyword) else {
65+
return;
66+
};
67+
68+
match &keyword.value {
69+
Expr::Name(ast::ExprName { id, .. }) => {
70+
if is_insecure_protocol(id) {
71+
checker.diagnostics.push(Diagnostic::new(
72+
SslInsecureVersion {
73+
protocol: id.to_string(),
74+
},
75+
keyword.range(),
76+
));
77+
}
78+
}
79+
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
80+
if is_insecure_protocol(attr) {
81+
checker.diagnostics.push(Diagnostic::new(
82+
SslInsecureVersion {
83+
protocol: attr.to_string(),
84+
},
85+
keyword.range(),
86+
));
87+
}
88+
}
89+
_ => {}
90+
}
91+
}
92+
93+
/// Returns `true` if the given protocol name is insecure.
94+
fn is_insecure_protocol(name: &str) -> bool {
95+
matches!(
96+
name,
97+
"PROTOCOL_SSLv2"
98+
| "PROTOCOL_SSLv3"
99+
| "PROTOCOL_TLSv1"
100+
| "PROTOCOL_TLSv1_1"
101+
| "SSLv2_METHOD"
102+
| "SSLv23_METHOD"
103+
| "SSLv3_METHOD"
104+
| "TLSv1_METHOD"
105+
| "TLSv1_1_METHOD"
106+
)
107+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
3+
---
4+
S502.py:6:13: S502 Call made with insecure SSL protocol: `PROTOCOL_SSLv3`
5+
|
6+
4 | from OpenSSL.SSL import Context
7+
5 |
8+
6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502
9+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502
10+
7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502
11+
8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502
12+
|
13+
14+
S502.py:7:17: S502 Call made with insecure SSL protocol: `PROTOCOL_TLSv1`
15+
|
16+
6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502
17+
7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502
19+
8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502
20+
9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502
21+
|
22+
23+
S502.py:8:17: S502 Call made with insecure SSL protocol: `PROTOCOL_SSLv2`
24+
|
25+
6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502
26+
7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502
27+
8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502
29+
9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502
30+
10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502
31+
|
32+
33+
S502.py:9:13: S502 Call made with insecure SSL protocol: `SSLv2_METHOD`
34+
|
35+
7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502
36+
8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502
37+
9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502
38+
| ^^^^^^^^^^^^^^^^^^^^^^^ S502
39+
10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502
40+
11 | Context(method=SSL.SSLv3_METHOD) # S502
41+
|
42+
43+
S502.py:10:13: S502 Call made with insecure SSL protocol: `SSLv23_METHOD`
44+
|
45+
8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502
46+
9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502
47+
10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502
48+
| ^^^^^^^^^^^^^^^^^^^^^^^^ S502
49+
11 | Context(method=SSL.SSLv3_METHOD) # S502
50+
12 | Context(method=SSL.TLSv1_METHOD) # S502
51+
|
52+
53+
S502.py:11:9: S502 Call made with insecure SSL protocol: `SSLv3_METHOD`
54+
|
55+
9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502
56+
10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502
57+
11 | Context(method=SSL.SSLv3_METHOD) # S502
58+
| ^^^^^^^^^^^^^^^^^^^^^^^ S502
59+
12 | Context(method=SSL.TLSv1_METHOD) # S502
60+
|
61+
62+
S502.py:12:9: S502 Call made with insecure SSL protocol: `TLSv1_METHOD`
63+
|
64+
10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502
65+
11 | Context(method=SSL.SSLv3_METHOD) # S502
66+
12 | Context(method=SSL.TLSv1_METHOD) # S502
67+
| ^^^^^^^^^^^^^^^^^^^^^^^ S502
68+
13 |
69+
14 | wrap_socket(ssl_version=ssl.PROTOCOL_TLS_CLIENT) # OK
70+
|
71+
72+

ruff.schema.json

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)