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

Fixed the remediation for rsyslog_files_permissions #4906

Merged
merged 1 commit into from
Nov 5, 2019

Conversation

matejak
Copy link
Member

@matejak matejak commented Oct 7, 2019

  • Stripped quotes and brackets from extracted paths.
  • Dropped apparent config files from extracted list of logfile paths.

The old remediation shows errors on a fresh RHEL8 install.

@matejak matejak added the Bash Bash remediation update. label Oct 7, 2019
@matejak matejak added this to the 0.1.47 milestone Oct 7, 2019
@yuumasato
Copy link
Member

@matejak Thanks for the fix, would you consider adding test scenarios?

@matejak matejak closed this Oct 9, 2019
@matejak matejak deleted the rsyslog_perms branch October 9, 2019 11:46
@matejak matejak restored the rsyslog_perms branch October 9, 2019 11:47
@matejak
Copy link
Member Author

matejak commented Oct 9, 2019

Oops, accidental branch removal

@matejak matejak reopened this Oct 9, 2019
- Stripped quotes and brackets from extracted paths.
- Dropped apparent config files from extracted list of logfile paths.
- Improved test scenarios.
@matejak
Copy link
Member Author

matejak commented Oct 11, 2019

I have updated some test scenarios, but existing scenarios fail unexpectedly, and it is not trivial to fix them. So we can either merge the PR as-is, or wait a bit until the OVAL is fixed to be more robust.

@jan-cerny jan-cerny self-assigned this Oct 15, 2019
@@ -9,10 +9,13 @@ source $SHARED/rsyslog_log_utils.sh
PERMS=0600

# setup test data
create_rsyslog_test_logs 1
create_rsyslog_test_logs 4
Copy link
Collaborator

Choose a reason for hiding this comment

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

This scenario also fails for me on RHEL 8.

ERROR - Script perms_0600.pass.sh using profile xccdf_org.ssgproject.content_profile_pci-dss found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage 
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_permissions'.

The verbose log contains:

Variable 'oval:ssg-var_rfp_include_config_regex:var:1' has no values.

This should contain the matches of ^\$IncludeConfig[\s]+([^\s;]+) on /etc/rsyslog.conf. However, on RHEL8 this regex doesn't work because rsyslog in RHEL8 uses the modern syntax,

eg.

# Include all config files in /etc/rsyslog.d/
include(file="/etc/rsyslog.d/*.conf" mode="optional")

From this I guess RHEL8 is out of scope of this rule. However, I can see this rule in RHEL8 profile in rhel8/profiles/pci-dss.profile. Could you please check if this rule applies to RHEL8 or should apply to RHEL8 or shouldn't apply to RHEL8? Then we can decide on further steps.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Aha, I see that this will be addressed by #4379

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that another PR is what I was referring to as waiting for more robust OVAL.
However, this PR fixes errors in remediation, so I propose to get it merged as-is.

@@ -22,4 +25,12 @@ cat << EOF > $RSYSLOG_CONF

*.* ${RSYSLOG_TEST_LOGS[0]}

*.* ${RSYSLOG_TEST_LOGS[1]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that there is a problem that the OVAL for this rule always returns false if /etc/rsyslog.conf doesn't contain any inlcusion. Please add \$IncludeConfig /etc/rsyslog.d/*.conf to both test scenarios.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@matejak this is the problem

Copy link
Member

Choose a reason for hiding this comment

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

I think that there is a problem that the OVAL for this rule always returns false if /etc/rsyslog.conf doesn't contain any inlcusion.

This may be an actual finding in the OVAL check.
If rsyslog.conf or /etc/rsyslog.d/*.conf without includes are correct, the OVAL check should just pass.

@jvymazal Are includes for rsyslog *.conf file mandatory?

Copy link
Member Author

@matejak matejak Nov 4, 2019

Choose a reason for hiding this comment

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

After consulting with @jvymazal, rsystlog will run even if there is no include directive present. So the test case is valid, and it is our OVAL which is not able to cope with this configuration.
As an interesting fact, in a default installation, the directive is there, but it doesn't do anything, because the /etc/rsyslog.d/ directory is empty.

@jan-cerny
Copy link
Collaborator

We have discussed this with @matejak . We think that the remediation fix is basically a display fix. Currently, by looking to the HTML report people might think that the remediation is defective. If the patch is applied, it will not make the rule passing. The report from oscap xccdf eval --remediate will still show error, because the scan on RHEL 8 will fail, because OVAL expects IncludeConfig directive in /etc/rsyslog.conf and RHEL 8 uses the new syntax with include() instead. The remediation fix will only reveal the OVAL is defective.

We think that it isn't a release blocker, and it's not a regression from previous released version of RHEL8 content.

@jan-cerny
Copy link
Collaborator

On my F30 laptop it doesn't produce the weird messages s. a. /tmp/oscap.3j86mL/fix-XXJMZJ3X: line 48: [: : integer expression expected anymore.

@jan-cerny jan-cerny modified the milestones: 0.1.47, 0.1.48 Nov 5, 2019
@jan-cerny jan-cerny merged commit 7978939 into ComplianceAsCode:master Nov 5, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bash Bash remediation update.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants