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

Add new rule file_cron_allow_exists #11441

Merged
merged 3 commits into from
Jan 11, 2024
Merged

Conversation

jan-cerny
Copy link
Collaborator

Add new rule file_cron_allow_exists and add it to CIS profiles because the CIS RHEL Benchmark requires the file /etc/cron.allow to exist.

The other rules within the control that check the ownership and permissions on /etc/cron.allow are passing if this file doesn't exist. The file doesn't exist by default. To ensure the file exists, we add a new rule that creates it.

Resolves: https://issues.redhat.com/browse/RHEL-1314

Add new rule file_cron_allow_exists and add it to CIS profiles
because the CIS RHEL Benchmark requires the file /etc/cron.allow
to exist.

The other rules within the control that check the ownership and
permissions on /etc/cron.allow are passing if this file doesn't exist.
The file doesn't exist by default. To ensure the file exists, we add a
new rule that creates it.

Resolves: https://issues.redhat.com/browse/RHEL-1314
@jan-cerny jan-cerny added New Rule Issues or pull requests related to new Rules. RHEL9 Red Hat Enterprise Linux 9 product related. RHEL7 Red Hat Enterprise Linux 7 product related. RHEL8 Red Hat Enterprise Linux 8 product related. CIS CIS Benchmark related. labels Jan 11, 2024
@jan-cerny jan-cerny added this to the 0.1.72 milestone Jan 11, 2024
@jan-cerny jan-cerny requested a review from a team as a code owner January 11, 2024 12:31
Copy link

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

rhel8 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@Mab879 Mab879 self-assigned this Jan 11, 2024
@@ -0,0 +1,2 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Is the name dne.fail.sh intentional? Maybe we can rename the file to more easily reflect the test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

dne is acronym for "does not exist"

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok. Thanks. It was not obvious to me. : ) So, all fine to keep this name if you prefer.

and the output should list the file.

template:
name: file_existence
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to extend this template to also accept mode and ownership properties as parameters for the remediation when creating the file. This would avoid the rules file_groupowner_cron_allow, file_owner_cron_allow and file_permissions_cron_allow to fail after creating the file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, that'd be amazing!

@jan-cerny
Copy link
Collaborator Author

Now there is a problem with the dependency between rules - during the initial scan, the file_permissions_cron_allow passed because /etc/cron.allow doesn't exist but then in the remediation phase the remediation for the rule file_cron_allow_exists creates /etc/cron.allow but remediation for file_permissions_cron_allow isn't executed because it passed during the inital scan so consequently in the final scan the file exists but with default permissions so the rule file_permissions_cron_allow fails.

@marcusburghardt
Copy link
Member

Now there is a problem with the dependency between rules - during the initial scan, the file_permissions_cron_allow passed because /etc/cron.allow doesn't exist but then in the remediation phase the remediation for the rule file_cron_allow_exists creates /etc/cron.allow but remediation for file_permissions_cron_allow isn't executed because it passed during the inital scan so consequently in the final scan the file exists but with default permissions so the rule file_permissions_cron_allow fails.

I recently commented about this: #11441 (comment)
It seems a better alternative to extend the template. Otherwise the user would need to scan and remediate the system twice.

This patch adds 2 new parameters fileuid and filemode to the
file_existence template. They will be used only on remediations. They
will enable us to create the file with correct permissions from the
beginning which helps avoid remediation time collision with other rules
that check file permissions and file ownership.
This way we will ensure that the file is created with correct
owner and permissions from the beginning.

Uses the new parameters of the file_existence template.
These options aren't checked by OVAL, they are only used by
remediations.
Copy link

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

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_file_at_deny_not_exist' differs.
--- xccdf_org.ssgproject.content_rule_file_at_deny_not_exist
+++ xccdf_org.ssgproject.content_rule_file_at_deny_not_exist
@@ -1,11 +1,7 @@
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
-#!/bin/bash
-
-
-
-    if [[ -f  /etc/at.deny ]]; then
+if [[ -f  /etc/at.deny ]]; then
         rm /etc/at.deny
     fi
 

bash remediation for rule 'xccdf_org.ssgproject.content_rule_file_cron_deny_not_exist' differs.
--- xccdf_org.ssgproject.content_rule_file_cron_deny_not_exist
+++ xccdf_org.ssgproject.content_rule_file_cron_deny_not_exist
@@ -1,11 +1,7 @@
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
-#!/bin/bash
-
-
-
-    if [[ -f  /etc/cron.deny ]]; then
+if [[ -f  /etc/cron.deny ]]; then
         rm /etc/cron.deny
     fi
 

Copy link

codeclimate bot commented Jan 11, 2024

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

View more on Code Climate.

@jan-cerny
Copy link
Collaborator Author

I have improved the file_existence template so that the remediations will create files with given uid and permissions and I have used this in the rule file_cron_allow_exists to help avoid the aforementioned remediation time conflict.

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for the PR!

Waving the SLES Automatus failure as this rule is only applicable to RHEL currently.

@Mab879 Mab879 merged commit 69fcc7d into ComplianceAsCode:master Jan 11, 2024
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CIS CIS Benchmark related. New Rule Issues or pull requests related to new Rules. RHEL7 Red Hat Enterprise Linux 7 product related. RHEL8 Red Hat Enterprise Linux 8 product related. RHEL9 Red Hat Enterprise Linux 9 product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants