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.

Sign up for GitHub

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

Jump to bottom

More flexibility for login banners #7690

Merged
merged 4 commits into from
Oct 11, 2021
Merged

More flexibility for login banners #7690

merged 4 commits into from
Oct 11, 2021

Conversation

marcusburghardt
Copy link
Member

Description:

While some Security Guidelines require a specific content for login banners, like STIG, others are less restrictive, like CIS.
Currently, the variable login_banner_text expects predefined content for login banners, being prone to report incorrect assessment of relevant rules when CIS profile is chosen.

Rationale:

The login_banner_text variable was updated to ensure compliance with CIS, only restricting technical information from the login banners when CIS profile is used.

It was explicitly defined the correspondent content for each profile which uses login banner rules. Mainly the STIG profiles didn't have this variable defined, but were working because dod_banners was selected as default. Therefore, it was also created an explicit default for this variable, to not depend on ordering of the available options. The default content was the same used for CIS since it is short and generic.

Finally, this patch also fixes the RHBZ#1983061.

@JAORMX
Copy link
Contributor

JAORMX commented Oct 4, 2021

Mind modifying the rhcos4 moderate profile? we'd probably use dod_banners as well

@marcusburghardt
Copy link
Member Author

Mind modifying the rhcos4 moderate profile? we'd probably use dod_banners as well

Done @JAORMX . Please, take a look and let me know if it is fine or any additional update is necessary for rhcos4. Thanks

@JAORMX
Copy link
Contributor

JAORMX commented Oct 5, 2021

/retest

1 similar comment
@JAORMX
Copy link
Contributor

JAORMX commented Oct 5, 2021

/retest

@marcusburghardt marcusburghardt added the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 6, 2021
@marcusburghardt
Copy link
Member Author

With this patch, our test scenarios detected issues related to /etc/motd. It was because the test scripts for the rule banner_etc_motd were assuming the banner text from dod_banners were expected. However, STIG doesn't even mention /etc/motd. I investigated the existing profiles and only CIS is actually using the rule banner_etc_motd. Due that, I will reduce the test scenarios scripts from this rule to the only necessary ones to validate the rule itself and expected content from CIS.

@marcusburghardt
More flexibility for login banners
76a10d9 
Also fixed RHBZ#1983061 and defined a generic text as default
content where not explicitly selected. Previously the DOD banners
would be applied as default.
@marcusburghardt
Defined login_banner_text for rhcos4 moderate profile
1f8c241
@marcusburghardt
banner_etc_motd rule reviewed and properly adjusted
330c0fb 
Removed unnecessary test scenarios and improved the necessary ones.
Adjusted OVAL to allow absent /etc/motd file.
marcusburghardt
marcusburghardt commented Oct 7, 2021
View reviewed changes
linux_os/guide/system/accounts/accounts-banners/banner_etc_motd/oval/shared.xml
@@ -6,7 +6,7 @@
</criteria>
</definition>

<ind:textfilecontent54_test check="all" check_existence="all_exist" comment="correct banner in /etc/motd" id="test_banner_etc_motd" version="1">
<ind:textfilecontent54_test check="all" check_existence="any_exist" comment="correct banner in /etc/motd" id="test_banner_etc_motd" version="1">
Copy link
Member Author

@marcusburghardt marcusburghardt Oct 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Different from /etc/issue, /etc/motd can be either removed or present with the expected content. So, absent file is also fine.

@marcusburghardt
Fixed ansible remediation macro for login banners
020a69a 
Correctly split login_banner_text variable when multiple banners are defined.
@marcusburghardt
Copy link
Member Author

/retest

@marcusburghardt marcusburghardt removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 7, 2021
@marcusburghardt marcusburghardt added RHEL8 CIS Alignment Update Rule Issues or pull requests related to Rules updates. labels Oct 8, 2021
ggbecker
ggbecker approved these changes Oct 11, 2021
View reviewed changes
Copy link
Member

@ggbecker ggbecker left a comment

Choose a reason for hiding this comment

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

LGTM

@ggbecker ggbecker merged commit ef1deb9 into ComplianceAsCode:master Oct 11, 2021
@yuumasato yuumasato added this to the 0.1.59 milestone Oct 12, 2021
@marcusburghardt marcusburghardt deleted the BZ1983061 branch November 2, 2021 15:49
@matusmarhefka matusmarhefka mentioned this pull request Nov 9, 2021
Closed
@marcusburghardt marcusburghardt added RHEL7 Red Hat Enterprise Linux 7 product related. RHEL8 Red Hat Enterprise Linux 8 product related. CIS CIS Benchmark related. labels Jun 15, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggbecker ggbecker ggbecker approved these changes

Assignees
No one assigned
Labels
CIS CIS Benchmark related. RHEL7 Red Hat Enterprise Linux 7 product related. RHEL8 Red Hat Enterprise Linux 8 product related. Update Rule Issues or pull requests related to Rules updates.
Projects
None yet
Milestone
0.1.59
Development

Successfully merging this pull request may close these issues.
4 participants
@marcusburghardt @JAORMX @ggbecker @yuumasato