SLE15 audit rules mac modification usr share depends on selinux policy packages #10883

teacup-on-rockingchair · 2023-07-20T10:14:46Z

Description:

In SLE15 environment we have separate package for selinux policy

Rationale:

In order rules that audit selinux policy configuration to be relevant we need one of the dedicated selinux-policy packages to be installed otherwise the rule is irrelevant

openshift-ci · 2023-07-20T10:14:50Z

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 · 2023-07-20T10:16:19Z

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

jan-cerny

My main concern is that the change of the XCCDF manual file isn't mentioned in the PR description. It gives a false impression that this PR is solely about the single audit rule. Such a big change as XCCDF manual update should be submitted as a separate PR instead.

jan-cerny · 2023-07-21T08:31:12Z

...ystem/auditing/auditd_configure_rules/audit_rules_mac_modification_usr_share/oval/shared.xml

+{{%- if 'sle15' in product -%}}
+      <criteria operator="AND">
+        <criterion test_ref="check_selinux_policy_devel_installed"
+          comment="Check no selinux-policy-devel package installed" negate="true"/>
+        <criterion test_ref="check_selinux_policy_targeted_installed"
+          comment="Check no selinux-policy-targeted package installed" negate="true"/>
+      </criteria>
+{{%- endif -%}}


Have you considered creating a platform restriction in the rule.yml instad of modifying the OVAL? What are the reasons for this solution?

The main reason here is that for sle platform, not sure about the others, we need to have some of the selinux-policy packages installed in order the whole rule to be relevant because otherwise even if we have selinux installed with no policies there is nothing to audit

That doesn't explain why you chose this way, you can get the same result with using platform expressions. Also if you used platforms you would get "notapplicable" results if the package isn't present, and that way you can get more understandable user experience in the HTML report.

So I assume your suggestion to rework it, so the rule is limited by platform clause:
platform: package[selinux-policy-devel] or [selinux-policy-targeted] ,
and add declaration of the relevant packages for all linux flavours?

jan-cerny · 2023-07-21T08:34:13Z

...ystem/auditing/auditd_configure_rules/audit_rules_mac_modification_usr_share/oval/shared.xml

@@ -22,6 +30,11 @@
    </criteria>
  </definition>

+{{%- if 'sle15' in product -%}}
+  {{{ oval_test_package_installed(package="selinux-policy-devel", evr=EVR, test_id="check_selinux_policy_devel_installed") }}}


The EVR variable isn't defined. You probably wanted to set it to None or you can remove the evr=EVR, completely because it's a parameter with a default value (see the definition of the oval_test_package_installed macro).

@jan-cerny

Thanks to @jan-cerny for the tip

jan-cerny · 2023-07-24T08:28:36Z

...ystem/auditing/auditd_configure_rules/audit_rules_mac_modification_usr_share/oval/shared.xml

+{{%- if 'sle15' in product -%}}
+      <criteria operator="AND">
+        <criterion test_ref="check_selinux_policy_devel_installed"
+          comment="Check no selinux-policy-devel package installed" negate="true"/>
+        <criterion test_ref="check_selinux_policy_targeted_installed"
+          comment="Check no selinux-policy-targeted package installed" negate="true"/>
+      </criteria>
+{{%- endif -%}}


That doesn't explain why you chose this way, you can get the same result with using platform expressions. Also if you used platforms you would get "notapplicable" results if the package isn't present, and that way you can get more understandable user experience in the HTML report.

jan-cerny · 2023-07-24T08:43:31Z

...ystem/auditing/auditd_configure_rules/audit_rules_mac_modification_usr_share/oval/shared.xml

@@ -4,6 +4,14 @@
      mandatory access controls (SELinux) in usr/share/selinux are enabled.") }}}

    <criteria operator="OR">
+{{%- if 'sle15' in product -%}}


This condition also matches any other string that contains "sle15" as a substring, eg. "sle158", "ssle15". That means it can match unwanted products in future. Normally, you should do product == "sle15", or better, product in ["sle15"]. The latter will allow people do have an easier expansion in future.

Thanks for the tip changed it in c3a8fb7

@jan-cerny

Thanks to @jan-cerny for the tip

codeclimate · 2023-07-27T07:12:40Z

Code Climate has analyzed commit c3a8fb7 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 53.2% (0.0% change).

View more on Code Climate.

openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Jul 20, 2023

teacup-on-rockingchair marked this pull request as ready for review July 20, 2023 12:16

openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Jul 20, 2023

jan-cerny reviewed Jul 21, 2023

View reviewed changes

SLE15 selinux policy directories depend on separate packages

c59f17e

teacup-on-rockingchair force-pushed the sle15_audit_rules_mac_modification_usr_share_depends_selinux_policy branch from ff68aba to c59f17e Compare July 22, 2023 05:27

Use explicit empty value for oval_test_package_installed's EVR parameter

b0b0b49

Thanks to @jan-cerny for the tip

jan-cerny reviewed Jul 24, 2023

View reviewed changes

jan-cerny self-assigned this Jul 24, 2023

jan-cerny added this to the 0.1.70 milestone Jul 24, 2023

jan-cerny added the OVAL OVAL update. Related to the systems assessments. label Jul 25, 2023

More robust jinja product condition

c3a8fb7

Thanks to @jan-cerny for the tip

jan-cerny approved these changes Jul 28, 2023

View reviewed changes

teacup-on-rockingchair merged commit 8d96a74 into ComplianceAsCode:master Jul 29, 2023
33 of 34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SLE15 audit rules mac modification usr share depends on selinux policy packages #10883

SLE15 audit rules mac modification usr share depends on selinux policy packages #10883

teacup-on-rockingchair commented Jul 20, 2023

openshift-ci bot commented Jul 20, 2023

github-actions bot commented Jul 20, 2023

jan-cerny left a comment

jan-cerny Jul 21, 2023

teacup-on-rockingchair Jul 22, 2023

jan-cerny Jul 24, 2023

teacup-on-rockingchair Jul 27, 2023 •

edited

jan-cerny Jul 21, 2023

jan-cerny Jul 24, 2023

jan-cerny Jul 24, 2023

teacup-on-rockingchair Jul 27, 2023

codeclimate bot commented Jul 27, 2023

SLE15 audit rules mac modification usr share depends on selinux policy packages #10883

SLE15 audit rules mac modification usr share depends on selinux policy packages #10883

Conversation

teacup-on-rockingchair commented Jul 20, 2023

Description:

Rationale:

openshift-ci bot commented Jul 20, 2023

github-actions bot commented Jul 20, 2023

jan-cerny left a comment

Choose a reason for hiding this comment

jan-cerny Jul 21, 2023

Choose a reason for hiding this comment

teacup-on-rockingchair Jul 22, 2023

Choose a reason for hiding this comment

jan-cerny Jul 24, 2023

Choose a reason for hiding this comment

teacup-on-rockingchair Jul 27, 2023 • edited

Choose a reason for hiding this comment

jan-cerny Jul 21, 2023

Choose a reason for hiding this comment

jan-cerny Jul 24, 2023

Choose a reason for hiding this comment

jan-cerny Jul 24, 2023

Choose a reason for hiding this comment

teacup-on-rockingchair Jul 27, 2023

Choose a reason for hiding this comment

codeclimate bot commented Jul 27, 2023

teacup-on-rockingchair Jul 27, 2023 •

edited