WIP: sshd_lineinfile support variables

[maage]

Description:

Some sshd rules only support /etc/ssh/sshd_config

At least:

sshd_set_login_grace_time sshd_set_max_auth_tries sshd_set_max_sessions sshd_set_maxstartups

These also use variables.

This patch tries to make it possible to use variables with sshd_lineinfile template as it already support sshd_config.d.

Rationale:

If there is templating for a configuration all instances should use the templating as end result is less code.

WIP

Does not pass all tests and there seems to be some bugs.

[openshift-ci[bot]]

Hi @maage. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[github-actions[bot]]

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment)

Fedora Testing Environment

Oracle Linux 8 Environment

[github-actions[bot]]

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff

OVAL definition oval:ssg-sshd_set_login_grace_time:def:1 differs:
--- old datastream
+++ new datastream
- criterion oval:ssg-test_sshd_login_grace_time:tst:1
+ criteria OR
+ criterion oval:ssg-test_sshd_set_login_grace_time:tst:1
OCIL for rule 'xccdf_org.ssgproject.content_rule_sshd_set_login_grace_time' differs:
--- old datastream
+++ new datastream
@@ -1,8 +1,8 @@
-To ensure LoginGraceTime is set correctly, run the following command:
-$ sudo grep LoginGraceTime /etc/ssh/sshd_config
-If properly configured, the output should be:
-LoginGraceTime 
-If the option is set to a number greater than 0, then the unauthenticated session will be disconnected
-after the configured number seconds.
- Is it the case that it is commented out or not configured properly?
+To determine how the SSH daemon's LoginGraceTime option is set, run the following command:
+
+$ sudo grep -i LoginGraceTime /etc/ssh/sshd_config
+
+If a line indicating is returned, then the required value is set.
+
+ Is it the case that the required value is not set?
 
bash remediation for rule 'xccdf_org.ssgproject.content_rule_sshd_set_login_grace_time' differs:
--- old datastream
+++ new datastream
@@ -2,7 +2,6 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
 var_sshd_set_login_grace_time=''
-
 
 if [ -e "/etc/ssh/sshd_config" ] ; then
 
@@ -19,10 +18,10 @@
 if [ -z "$line_number" ]; then
 # There was no match of '^Match', insert at
 # the end of the file.
- printf '%s\n' "LoginGraceTime $var_sshd_set_login_grace_time" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "LoginGraceTime ${var_sshd_set_login_grace_time}" >> "/etc/ssh/sshd_config"
 else
 head -n "$(( line_number - 1 ))" "/etc/ssh/sshd_config.bak" > "/etc/ssh/sshd_config"
- printf '%s\n' "LoginGraceTime $var_sshd_set_login_grace_time" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "LoginGraceTime ${var_sshd_set_login_grace_time}" >> "/etc/ssh/sshd_config"
 tail -n "+$(( line_number ))" "/etc/ssh/sshd_config.bak" >> "/etc/ssh/sshd_config"
 fi
 # Clean up after ourselves.

OVAL definition oval:ssg-sshd_set_max_auth_tries:def:1 differs:
--- old datastream
+++ new datastream
- criterion oval:ssg-test_sshd_max_auth_tries:tst:1
+ criteria OR
+ criterion oval:ssg-test_sshd_set_max_auth_tries:tst:1
OCIL for rule 'xccdf_org.ssgproject.content_rule_sshd_set_max_auth_tries' differs:
--- old datastream
+++ new datastream
@@ -1,6 +1,8 @@
-To ensure the MaxAuthTries parameter is set, run the following command:
-$ sudo grep MaxAuthTries /etc/ssh/sshd_config
-If properly configured, output should be:
-MaxAuthTries 
- Is it the case that it is commented out or not configured properly?
+To determine how the SSH daemon's MaxAuthTries option is set, run the following command:
+
+$ sudo grep -i MaxAuthTries /etc/ssh/sshd_config
+
+If a line indicating is returned, then the required value is set.
+
+ Is it the case that the required value is not set?
 
bash remediation for rule 'xccdf_org.ssgproject.content_rule_sshd_set_max_auth_tries' differs:
--- old datastream
+++ new datastream
@@ -2,7 +2,6 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
 sshd_max_auth_tries_value=''
-
 
 if [ -e "/etc/ssh/sshd_config" ] ; then
 
@@ -19,10 +18,10 @@
 if [ -z "$line_number" ]; then
 # There was no match of '^Match', insert at
 # the end of the file.
- printf '%s\n' "MaxAuthTries $sshd_max_auth_tries_value" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "MaxAuthTries ${sshd_max_auth_tries_value}" >> "/etc/ssh/sshd_config"
 else
 head -n "$(( line_number - 1 ))" "/etc/ssh/sshd_config.bak" > "/etc/ssh/sshd_config"
- printf '%s\n' "MaxAuthTries $sshd_max_auth_tries_value" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "MaxAuthTries ${sshd_max_auth_tries_value}" >> "/etc/ssh/sshd_config"
 tail -n "+$(( line_number ))" "/etc/ssh/sshd_config.bak" >> "/etc/ssh/sshd_config"
 fi
 # Clean up after ourselves.

OVAL definition oval:ssg-sshd_set_max_sessions:def:1 differs:
--- old datastream
+++ new datastream
- criterion oval:ssg-test_sshd_max_sessions:tst:1
+ criteria OR
+ criterion oval:ssg-test_sshd_set_max_sessions:tst:1
OCIL for rule 'xccdf_org.ssgproject.content_rule_sshd_set_max_sessions' differs:
--- old datastream
+++ new datastream
@@ -1,6 +1,8 @@
-Run the following command to see what the max sessions number is:
-$ sudo grep MaxSessions /etc/ssh/sshd_config
-If properly configured, the output should be:
-MaxSessions 
- Is it the case that MaxSessions is not configured or not configured correctly?
+To determine how the SSH daemon's MaxSessions option is set, run the following command:
+
+$ sudo grep -i MaxSessions /etc/ssh/sshd_config
+
+If a line indicating is returned, then the required value is set.
+
+ Is it the case that the required value is not set?
 
bash remediation for rule 'xccdf_org.ssgproject.content_rule_sshd_set_max_sessions' differs:
--- old datastream
+++ new datastream
@@ -2,7 +2,6 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
 var_sshd_max_sessions=''
-
 
 if [ -e "/etc/ssh/sshd_config" ] ; then
 
@@ -19,10 +18,10 @@
 if [ -z "$line_number" ]; then
 # There was no match of '^Match', insert at
 # the end of the file.
- printf '%s\n' "MaxSessions $var_sshd_max_sessions" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "MaxSessions ${var_sshd_max_sessions}" >> "/etc/ssh/sshd_config"
 else
 head -n "$(( line_number - 1 ))" "/etc/ssh/sshd_config.bak" > "/etc/ssh/sshd_config"
- printf '%s\n' "MaxSessions $var_sshd_max_sessions" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "MaxSessions ${var_sshd_max_sessions}" >> "/etc/ssh/sshd_config"
 tail -n "+$(( line_number ))" "/etc/ssh/sshd_config.bak" >> "/etc/ssh/sshd_config"
 fi
 # Clean up after ourselves.

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_sshd_set_max_sessions' differs:
--- old datastream
+++ new datastream
@@ -37,9 +37,9 @@
 when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
 tags:
 - CCE-83357-4
- - configure_strategy
 - low_complexity
 - low_disruption
 - medium_severity
 - no_reboot_needed
+ - restrict_strategy
 - sshd_set_max_sessions

OVAL definition oval:ssg-sshd_set_maxstartups:def:1 differs:
--- old datastream
+++ new datastream
- criterion oval:ssg-tst_maxstartups_start_parameter:tst:1
- criterion oval:ssg-tst_maxstartups_rate_parameter:tst:1
- criterion oval:ssg-tst_maxstartups_full_parameter:tst:1
+ extend_definition oval:ssg-sshd_required_or_unset:def:1
+ extend_definition oval:ssg-package_openssh-server_installed:def:1
+ criteria OR
+ criteria AND
+ criterion oval:ssg-test_sshd_set_maxstartups_start_parameter:tst:1
+ criterion oval:ssg-test_sshd_set_maxstartups_rate_parameter:tst:1
+ criterion oval:ssg-test_sshd_set_maxstartups_full_parameter:tst:1
OCIL for rule 'xccdf_org.ssgproject.content_rule_sshd_set_maxstartups' differs:
--- old datastream
+++ new datastream
@@ -1,5 +1,8 @@
-To check if MaxStartups is configured, run the following command:
-$ sudo grep MaxStartups /etc/ssh/sshd_config
-If configured, this command should output the configuration.
- Is it the case that maxstartups is not configured?
+To determine how the SSH daemon's MaxStartups option is set, run the following command:
+
+$ sudo grep -i MaxStartups /etc/ssh/sshd_config
+
+If a line indicating is returned, then the required value is set.
+
+ Is it the case that the required value is not set?
 
bash remediation for rule 'xccdf_org.ssgproject.content_rule_sshd_set_maxstartups' differs:
--- old datastream
+++ new datastream
@@ -2,7 +2,6 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
 var_sshd_set_maxstartups=''
-
 
 if [ -e "/etc/ssh/sshd_config" ] ; then
 
@@ -19,10 +18,10 @@
 if [ -z "$line_number" ]; then
 # There was no match of '^Match', insert at
 # the end of the file.
- printf '%s\n' "MaxStartups $var_sshd_set_maxstartups" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "MaxStartups ${var_sshd_set_maxstartups}" >> "/etc/ssh/sshd_config"
 else
 head -n "$(( line_number - 1 ))" "/etc/ssh/sshd_config.bak" > "/etc/ssh/sshd_config"
- printf '%s\n' "MaxStartups $var_sshd_set_maxstartups" >> "/etc/ssh/sshd_config"
+ printf '%s\n' "MaxStartups ${var_sshd_set_maxstartups}" >> "/etc/ssh/sshd_config"
 tail -n "+$(( line_number ))" "/etc/ssh/sshd_config.bak" >> "/etc/ssh/sshd_config"
 fi
 # Clean up after ourselves.

[maage]

I'm not happy with comment strings etc. And probably there is some test issues, I just dropped all tests. Also it might be reasonable to change sshd_parameter setting block from 10-oval.jinja to sshd_lineinfile template.

It would also be nice to add sshd_config TIME FORMAT parser to convert pretty values like "4h" to seconds and then do int comparison. Current setup forces you to use plain seconds format only. For example ClientAliveInterval, LoginGraceTime and RekeyLimit enable this. For example rules would benefit for this: sshd_set_idle_timeout sshd_set_login_grace_time sshd_rekey_limit Implementation probably would be quite verbose when using regexp captures and arithmetic.

But it seems generally to work at least under Fedora.

I still wonder if this generic template macro system is feasible for each ssh config case. As seen you really do need to implement per config parameter criteria block and comparisons. In the end it still would have less code than current implementation. Around 14 rules needed to be changed still.

[maage]

Fixed issue where var_check was added unconditionally also when not having var_ref.

[qlty-cloud-legacy[bot]]

Code Climate has analyzed commit 1e91a9c3 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 42.7% (0.0% change).

View more on Code Climate.

[openshift-merge-robot]

@maage: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[matejak]

Sorry to keep you waiting, if you are still interested in this PR, then let's get it merged. I think that there are two major changes introduced in the PR:

• Introduction of variables instead of values to the template
• Extension of the mechanism to support other operations than simple equality.

I suggest to split the PR, so the first part may get merged soon, and we can then talk about the second aspect that looks quite tricky, and for example I am not in favor of rule-specific code in files that contain generic macros. Therefore, I would suggest that you rebase, and only consider commits that change templates and template tests, while you apply the new code to one rule only for the time being, so all those rule edits don't distract reviewers.

[matejak]

I close the PR for the time being, as it needs rebase, and it developed conflicts. Anybody interested in finalizing this don't hesitate to reopen the PR.