Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

OpenSSH audit: range tests. #288

Merged
merged 3 commits into from
Apr 3, 2020

Conversation

a-martynovich
Copy link
Contributor

@cla-bot cla-bot bot added the cla-signed label Mar 24, 2020
@a-martynovich a-martynovich requested a review from rptrchv March 24, 2020 08:51
@a-martynovich a-martynovich force-pushed the 277-cis-openssh branch 2 times, most recently from a661d24 to e53e2c6 Compare March 24, 2020 14:24
self._safe = safe
self.is_safe = {self.COMPARE.MATCH: self.match,
self.COMPARE.RANGE: self.range}[compare]
self._safe_value = (safe if compare == self.COMPARE.MATCH else str(self._safe[1]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant parentheses

'openssh-agent-forwarding':
('OpenSSH: Disable agent forwarding', 'AllowAgentForwarding'),
'openssh-protocol':
('\tOpenSSH: Force protocol version 2', 'Protocol'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this '\t' here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To align the output in the console. Otherwise it's hard to read.

def __init__(self, default, safe, compare=COMPARE.MATCH):
self.default = default
self._safe = safe
self.is_safe = {self.COMPARE.MATCH: self.match,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls comment

@@ -362,7 +395,7 @@ def patch_sshd_config(patch_param):
logger.exception('sshd or service executable not found')
return

safe_value_string = '\n# Added by wott-agent on {}\n{} {}\n'.format(time.ctime(), patch_param, safe_value)
safe_value_string = '\n# Added by wott-agent on {}\n{} {}\n'.format(time.ctime(), patch_param, param_info.safe_value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too long line

if not replaced and not safe and default_value != safe_value:
logger.info('{}: replacing default "{}" with "{}"'.format(patch_param, default_value, safe_value))
if not replaced and not safe and not param_info.is_safe(param_info.default):
logger.info('{}: replacing default "{}" with "{}"'.format(patch_param, param_info.default, param_info.safe_value))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too long line

@a-martynovich a-martynovich force-pushed the 277-cis-openssh branch 2 times, most recently from 19b8a45 to 08cc181 Compare April 2, 2020 13:46
@a-martynovich a-martynovich requested a review from rptrchv April 2, 2020 13:46
Copy link
Contributor

@rptrchv rptrchv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls be consistent in logger strings formatting. I.e. always use %s

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 3, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@a-martynovich
Copy link
Contributor Author

I'm doing this to silence Codacy warnings. It says i should always use %-formatting for logging. It is a valid point, though

@a-martynovich a-martynovich requested a review from rptrchv April 3, 2020 04:10
@vpetersson vpetersson merged commit 68dfb24 into WoTTsecurity:master Apr 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants