Add support for XCCDF variables into sshd_lineinfile template #12251

vojtapolasek · 2024-08-01T09:27:50Z

Description:

modify the sshd_lineinfile template and related code so that the value of sshd parameter can be specified through an xccdf variable
- create new macros for lineinfile states which use XCCDF variable
- modify the OVAL, Bash and Ansible code used in the template
- modify all templated test scenarios so that they use the variable
- add a new required parameter datatype. It allows to create correct test scenarios. Note that this could be probably omitted, but I think that having proper support for data types in this template could allow us greater flexibility in the future, for example checking if some value is greater or equal.
modify all rules templated with sshd_lineinfile to include the datatype parameter
template the rule sshd_set_keepalive as a working POC
update the sshd_lineinfile template documentation

Rationale:

The sshd_lineinfile is very robust and it has support for distributed sshd configuration.
There are rules regarding sshd configuration which utilize XCCDF variables.
But these rules are not using the sshd_lineinfile template and therefore they don't respect presence of distributed sshd configuration. This creates problems because if two categories of rules mentioned above are combined, then remediations can end up in different files; some in the main sshd_config and some in a file within the sshd_config.d directory. This is not desired.

Please note that I did not template all relevant rules, I modified only one to keep the PR size down. The rest will follow up in a separate PR.

Review Hints:

running automatus in template mode should be sufficient

openshift-ci · 2024-08-01T09:27:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

github-actions · 2024-08-01T09:29:42Z

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 · 2024-08-01T09:32:23Z

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

Click here to see the full diff

OVAL for rule 'xccdf_org.ssgproject.content_rule_sshd_set_keepalive' differs.
--- oval:ssg-sshd_set_keepalive:def:1
+++ oval:ssg-sshd_set_keepalive:def:1
@@ -5,5 +5,7 @@
 criteria AND
 extend_definition oval:ssg-sshd_required_or_unset:def:1
 extend_definition oval:ssg-package_openssh-server_installed:def:1
-criteria OR
-criterion oval:ssg-test_sshd_set_keepalive_clientalivecountmax:tst:1
+criteria AND
+criteria AND
+criterion oval:ssg-test_sshd_set_keepalive:tst:1
+criterion oval:ssg-test_ClientAliveCountMax_present_sshd_set_keepalive:tst:1

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sshd_set_keepalive' differs.
--- xccdf_org.ssgproject.content_rule_sshd_set_keepalive
+++ xccdf_org.ssgproject.content_rule_sshd_set_keepalive
@@ -2,6 +2,7 @@
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
 var_sshd_set_keepalive=''
+
 
 
 if [ -e "/etc/ssh/sshd_config" ] ; then

github-actions · 2024-08-01T09:48:28Z

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:12251
This image was built from commit: b7b820d

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:12251

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:12251 make deploy-local

Mab879 · 2024-08-01T17:18:08Z

Please double check your commits, there are commits on this PR that don't seem to belong.

vojtapolasek · 2024-08-02T06:39:22Z

Sorry for inconvenience, fixed.

jan-cerny

It's really amazing.

Please resolve the Code Climate problems. Then see the comments below.

jan-cerny · 2024-08-02T07:05:27Z

linux_os/guide/services/ssh/ssh_server/sshd_set_keepalive/rule.yml

+    name: sshd_lineinfile
+    vars:
+        parameter: ClientAliveCountMax
+        rule_id: sshd_set_keepalive


I found it weird that the rule_id is passed as a parameter here because the rule ID is known to the templating engine.

I confirm it is not used. I fixed it in the sshd_set_keepalive. I also made a commit which removes it from other templated rules, just in case it would feel too much out of scope here.

jan-cerny · 2024-08-02T07:09:48Z

shared/macros/10-oval.jinja

@@ -336,6 +336,21 @@ Generates the :code:`<affected>` tag for OVAL check using correct product platfo
 </ind:textfilecontent54_state>
 {{%- endmacro %}}

+{{%- macro oval_line_in_file_state_xccdf_variable(var_name='', id_stem='', datatype='') -%}}
+{{%- set id_stem = id_stem or rule_id -%}}
+<local_variable id="local_var_regex_{{{ id_stem }}}"


I have discovered that eg. in build/rhel9/checks/oval/sshd_set_keepalive.xml this generates 2 local variables that reference the same external variable. It probably isn't a problem.

I think I solved it by 444811a

jan-cerny · 2024-08-02T07:12:47Z

shared/templates/sshd_lineinfile/tests/wrong_value_different_includings.fail.sh

@@ -2,8 +2,9 @@

 # platform = Oracle Linux 8,Oracle Linux 9

-SSHD_PARAM={{{ PARAMETER }}}
-SSHD_VAL="bad_val"
+{{% if XCCDF_VARIABLE %}}


Would it make sense to extend all the Oracle test scenarios also to RHEL? How can we test that these scenarios work well and don't break Oracle?

I don't think this is in scope of this PR. Oracle has an extra test in the oval which has something to do with the fact that there should be correct include directive present in /etc/ssh/sshd_config. It might be worth adding also to our OVAL, but I think this needs an extra PR.

vojtapolasek · 2024-08-08T09:54:22Z

Hello @jan-cerny , I think I addressed your feedback.

jan-cerny

@vojtapolasek Please resolve the Code Climate problems, but overall it looks great.

vojtapolasek · 2024-08-09T09:21:09Z

Hello @jan-cerny I am not sure if fixing the remaining Codeclimate issue would not be actually contraproductive... I am afraid it would decrease readability of the set_variables_for_test_scenarios function. I will add comment which might help to understand its functionality.

implement notion of XCCDF variables also start using a new template parameter datatype. It is used to generate proper templated test scenarios, see the template.py of the sysctl template.

…f supplied

…ger as well the + character kinda forced the value to be a string because it refused to concatenate non-string instances of the value

…e template

… same time

It is effectivelly not used and will be removed in the future. The build system does not need this passed, it knows it already.

…pendently of the OVAL state. This prevents creating of duplicate local_variables which in the end reference the same XCCDF variable (through external_variable).

extract setting of test scenario variables to a separate function split long strings to multiple lines

jan-cerny · 2024-08-09T09:41:06Z

@vojtapolasek I concur, thanks for your analysis.

codeclimate · 2024-08-09T10:05:40Z

Code Climate has analyzed commit b7b820d and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1

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 59.4% (0.0% change).

View more on Code Climate.

vojtapolasek added Ansible Ansible remediation update. OVAL OVAL update. Related to the systems assessments. Bash Bash remediation update. Update Rule Issues or pull requests related to Rules updates. Update Template Issues or pull requests related to Templates updates. labels Aug 1, 2024

vojtapolasek added this to the 0.1.75 milestone Aug 1, 2024

openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 1, 2024

vojtapolasek force-pushed the sshd_lineinfile_variables branch from 8cf4050 to 7d232bc Compare August 1, 2024 14:03

vojtapolasek changed the title ~~WIP: add support for variables into sshd_lineinfile template~~ Add support for XCCDF variables into sshd_lineinfile template Aug 1, 2024

vojtapolasek marked this pull request as ready for review August 1, 2024 14:11

vojtapolasek requested a review from a team as a code owner August 1, 2024 14:11

openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 1, 2024

jan-cerny self-assigned this Aug 2, 2024

vojtapolasek force-pushed the sshd_lineinfile_variables branch from cd37810 to c7cf89c Compare August 2, 2024 06:38

jan-cerny reviewed Aug 2, 2024

View reviewed changes

vojtapolasek force-pushed the sshd_lineinfile_variables branch from a1516f4 to 7384483 Compare August 8, 2024 12:26

jan-cerny requested changes Aug 8, 2024

View reviewed changes

vojtapolasek force-pushed the sshd_lineinfile_variables branch 2 times, most recently from 3488d2f to 16205f2 Compare August 9, 2024 07:25

vojtapolasek added 3 commits August 9, 2024 11:23

extend template.py of sshd_lineinfile

acf787b

implement notion of XCCDF variables also start using a new template parameter datatype. It is used to generate proper templated test scenarios, see the template.py of the sysctl template.

create new oval macro for lineinfile state which uses XCCDF variable

cba06ad

modify sshd_lineinfile OVAL template so that it uses XCCDF variable i…

5cb119a

…f supplied

vojtapolasek added 13 commits August 9, 2024 11:23

update sshd_lineinfile bash template

b2f4d76

bash macro set_config_file: essentially allow the value to be an inte…

05f8984

…ger as well the + character kinda forced the value to be a string because it refused to concatenate non-string instances of the value

modify the sshd_lineinfile ansible template to handle variables

170fcdc

update test scenarios so that they consider XCCDF variable

a4fc19d

convert an example rule to templated one; sshd_set_keepalive

fc7e0ec

add the datatype parameter to all rules templated with sshd_lineinfil…

c5091bb

…e template

update the documentation of sshd_lineinfile template.

8b77313

document new parameters for the sshd_oval_check macro

956fffa

template.py: prevent specification of value and xccdf_variable at the…

3ac6f5d

… same time

do not specify rule_id as a template parameter.

c453156

It is effectivelly not used and will be removed in the future. The build system does not need this passed, it knows it already.

modify oval_line_in_file_macros so that the variable is declared inde…

f250db8

…pendently of the OVAL state. This prevents creating of duplicate local_variables which in the end reference the same XCCDF variable (through external_variable).

apply suggestions provided by Codeclimate

337e837

extract setting of test scenario variables to a separate function split long strings to multiple lines

remove rule_id template parameter from rremaining rules

b7b820d

vojtapolasek force-pushed the sshd_lineinfile_variables branch from 16205f2 to b7b820d Compare August 9, 2024 09:23

jan-cerny approved these changes Aug 9, 2024

View reviewed changes

jan-cerny merged commit 94ae023 into ComplianceAsCode:master Aug 9, 2024
93 of 97 checks passed

This was referenced Sep 25, 2024

Review sshd_set_maxstartups rule #12419

Merged

Ensure consistency among SSH related rules #12420

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for XCCDF variables into sshd_lineinfile template #12251

Add support for XCCDF variables into sshd_lineinfile template #12251

vojtapolasek commented Aug 1, 2024 •

edited

Loading

openshift-ci bot commented Aug 1, 2024

github-actions bot commented Aug 1, 2024

github-actions bot commented Aug 1, 2024 •

edited

Loading

github-actions bot commented Aug 1, 2024 •

edited

Loading

Mab879 commented Aug 1, 2024

vojtapolasek commented Aug 2, 2024

jan-cerny left a comment

jan-cerny Aug 2, 2024

vojtapolasek Aug 8, 2024

jan-cerny Aug 2, 2024

vojtapolasek Aug 8, 2024

jan-cerny Aug 2, 2024

vojtapolasek Aug 8, 2024

vojtapolasek commented Aug 8, 2024

jan-cerny left a comment

vojtapolasek commented Aug 9, 2024

jan-cerny commented Aug 9, 2024

codeclimate bot commented Aug 9, 2024

Add support for XCCDF variables into sshd_lineinfile template #12251

Add support for XCCDF variables into sshd_lineinfile template #12251

Conversation

vojtapolasek commented Aug 1, 2024 • edited Loading

Description:

Rationale:

Review Hints:

openshift-ci bot commented Aug 1, 2024

github-actions bot commented Aug 1, 2024

github-actions bot commented Aug 1, 2024 • edited Loading

github-actions bot commented Aug 1, 2024 • edited Loading

Mab879 commented Aug 1, 2024

vojtapolasek commented Aug 2, 2024

jan-cerny left a comment

Choose a reason for hiding this comment

jan-cerny Aug 2, 2024

Choose a reason for hiding this comment

vojtapolasek Aug 8, 2024

Choose a reason for hiding this comment

jan-cerny Aug 2, 2024

Choose a reason for hiding this comment

vojtapolasek Aug 8, 2024

Choose a reason for hiding this comment

jan-cerny Aug 2, 2024

Choose a reason for hiding this comment

vojtapolasek Aug 8, 2024

Choose a reason for hiding this comment

vojtapolasek commented Aug 8, 2024

jan-cerny left a comment

Choose a reason for hiding this comment

vojtapolasek commented Aug 9, 2024

jan-cerny commented Aug 9, 2024

codeclimate bot commented Aug 9, 2024

vojtapolasek commented Aug 1, 2024 •

edited

Loading

github-actions bot commented Aug 1, 2024 •

edited

Loading

github-actions bot commented Aug 1, 2024 •

edited

Loading