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 S507 (ssh_no_host_key_verification) #7528

Merged
merged 1 commit into from
Sep 20, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_bandit/S507.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from paramiko import client
from paramiko.client import AutoAddPolicy, WarningPolicy

ssh_client = client.SSHClient()

# OK
ssh_client.set_missing_host_key_policy(policy=foo)
ssh_client.set_missing_host_key_policy(client.MissingHostKeyPolicy)
ssh_client.set_missing_host_key_policy()
ssh_client.set_missing_host_key_policy(foo)

# Errors
ssh_client.set_missing_host_key_policy(client.AutoAddPolicy)
ssh_client.set_missing_host_key_policy(client.WarningPolicy)
ssh_client.set_missing_host_key_policy(AutoAddPolicy)
ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy)
ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy)
ssh_client.set_missing_host_key_policy(policy=WarningPolicy)

# Unrelated
set_missing_host_key_policy(client.AutoAddPolicy)
foo.set_missing_host_key_policy(client.AutoAddPolicy)
ssh_client = 1
ssh_client.set_missing_host_key_policy(client.AutoAddPolicy)
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::ParamikoCall) {
flake8_bandit::rules::paramiko_call(checker, func);
}
if checker.enabled(Rule::SSHNoHostKeyVerification) {
flake8_bandit::rules::ssh_no_host_key_verification(checker, call);
}
if checker.enabled(Rule::LoggingConfigInsecureListen) {
flake8_bandit::rules::logging_config_insecure_listen(checker, call);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bandit, "324") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::HashlibInsecureHashFunction),
(Flake8Bandit, "501") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::RequestWithNoCertValidation),
(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),
(Flake8Bandit, "509") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::SnmpWeakCryptography),
(Flake8Bandit, "601") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::ParamikoCall),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ mod tests {
#[test_case(Rule::ParamikoCall, Path::new("S601.py"))]
#[test_case(Rule::RequestWithNoCertValidation, Path::new("S501.py"))]
#[test_case(Rule::RequestWithoutTimeout, Path::new("S113.py"))]
#[test_case(Rule::SSHNoHostKeyVerification, Path::new("S507.py"))]
#[test_case(Rule::SnmpInsecureVersion, Path::new("S508.py"))]
#[test_case(Rule::SnmpWeakCryptography, Path::new("S509.py"))]
#[test_case(Rule::StartProcessWithAShell, Path::new("S605.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_bandit/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub(crate) use request_without_timeout::*;
pub(crate) use shell_injection::*;
pub(crate) use snmp_insecure_version::*;
pub(crate) use snmp_weak_cryptography::*;
pub(crate) use ssh_no_host_key_verification::*;
pub(crate) use suspicious_function_call::*;
pub(crate) use try_except_continue::*;
pub(crate) use try_except_pass::*;
Expand All @@ -41,6 +42,7 @@ mod request_without_timeout;
mod shell_injection;
mod snmp_insecure_version;
mod snmp_weak_cryptography;
mod ssh_no_host_key_verification;
mod suspicious_function_call;
mod try_except_continue;
mod try_except_pass;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprAttribute, ExprCall, Stmt, StmtAssign};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for uses of policies disabling SSH verification in Paramiko.
///
/// ## Why is this bad?
/// By default, Paramiko checks the identity of remote host when establishing
/// an SSH connection. Disabling the verification might lead to the client
/// connecting to a malicious host, without the client knowing.
///
/// ## Example
/// ```python
/// from paramiko import client
///
/// ssh_client = client.SSHClient()
/// ssh_client.set_missing_host_key_policy(client.AutoAddPolicy)
/// ```
///
/// Use instead:
/// ```python
/// from paramiko import client
///
/// ssh_client = client.SSHClient()
/// ssh_client.set_missing_host_key_policy()
/// ```
///
/// ## References
/// - [Paramiko documentation: set_missing_host_key_policy](https://docs.paramiko.org/en/latest/api/client.html#paramiko.client.SSHClient.set_missing_host_key_policy)
#[violation]
pub struct SSHNoHostKeyVerification;

impl Violation for SSHNoHostKeyVerification {
#[derive_message_formats]
fn message(&self) -> String {
format!("Paramiko call with policy set to automatically trust the unknown host key")
}
}

/// S507
pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCall) {
let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else {
return;
};

if attr.as_str() != "set_missing_host_key_policy" {
return;
}

let Some(policy_argument) = call.arguments.find_argument("policy", 0) else {
return;
};

if !checker
.semantic()
.resolve_call_path(policy_argument)
.is_some_and(|call_path| {
matches!(
call_path.as_slice(),
["paramiko", "client", "AutoAddPolicy" | "WarningPolicy"]
)
})
{
return;
}

let Expr::Name(name) = value.as_ref() else {
return;
};

if let Some(binding_id) = checker.semantic().resolve_name(name) {
if let Some(Stmt::Assign(StmtAssign { value, .. })) = checker
.semantic()
.binding(binding_id)
.statement(checker.semantic())
{
if let Expr::Call(ExprCall { func, .. }) = value.as_ref() {
if checker
.semantic()
.resolve_call_path(func)
.is_some_and(|call_path| {
matches!(call_path.as_slice(), ["paramiko", "client", "SSHClient"])
})
{
checker.diagnostics.push(Diagnostic::new(
SSHNoHostKeyVerification,
policy_argument.range(),
));
}
}
}
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
---
source: crates/ruff/src/rules/flake8_bandit/mod.rs
---
S507.py:13:40: S507 Paramiko call with policy set to automatically trust the unknown host key
|
12 | # Errors
13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy)
| ^^^^^^^^^^^^^^^^^^^^ S507
14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy)
15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy)
|

S507.py:14:40: S507 Paramiko call with policy set to automatically trust the unknown host key
|
12 | # Errors
13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy)
14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy)
| ^^^^^^^^^^^^^^^^^^^^ S507
15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy)
16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy)
|

S507.py:15:40: S507 Paramiko call with policy set to automatically trust the unknown host key
|
13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy)
14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy)
15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy)
| ^^^^^^^^^^^^^ S507
16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy)
17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy)
|

S507.py:16:47: S507 Paramiko call with policy set to automatically trust the unknown host key
|
14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy)
15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy)
16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy)
| ^^^^^^^^^^^^^^^^^^^^ S507
17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy)
18 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy)
|

S507.py:17:47: S507 Paramiko call with policy set to automatically trust the unknown host key
|
15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy)
16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy)
17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy)
| ^^^^^^^^^^^^^^^^^^^^ S507
18 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy)
|

S507.py:18:47: S507 Paramiko call with policy set to automatically trust the unknown host key
|
16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy)
17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy)
18 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy)
| ^^^^^^^^^^^^^ S507
19 |
20 | # Unrelated
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.