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

Update existing rule for RHEL-08-020320 #7303

Merged
merged 1 commit into from Aug 17, 2021
Merged

Update existing rule for RHEL-08-020320 #7303

merged 1 commit into from Aug 17, 2021

Conversation

Mab879
Copy link
Member

@Mab879 Mab879 commented Jul 29, 2021

Description:

Add to profile, clean up, add tests, and moved the rule to a better location. Keeping under SAP did not seem like a good idea.

Rationale:

@Mab879 Mab879 added DISA RHEL8 STIG Alignment Update Rule Issues or pull requests related to Rules updates. labels Jul 29, 2021
@Mab879 Mab879 added this to the 0.1.58 milestone Jul 29, 2021
@Mab879 Mab879 requested a review from ggbecker July 29, 2021 20:50
@Mab879 Mab879 force-pushed the RHEL-08-020320 branch 3 times, most recently from da0a600 to ffd88d2 Compare July 29, 2021 21:06
Mab879
Mab879 commented Jul 30, 2021
View reviewed changes
...ide/system/accounts/accounts-restrictions/accounts_authorized_local_users/ansible/shared.yml Outdated
@@ -1,4 +1,4 @@
# platform = multi_platform_sle
# platform = multi_platform_sle,multi_platform_rhel
# reboot = false
# strategy = restrict
# complexity = low
Copy link
Member Author

Choose a reason for hiding this comment

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

This seem low to me, do we want to raise it to medium or high? Looking for more guidance on this.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know how to answer this. Following the documentation here it indicates it's just informative and there is no real impact on anything. This is translated into ansible tags but I doubt people are actually using this meta information in any way.

The estimated complexity or difficulty of applying the fix to the target. Only informative for now

Copy link
Member Author

Choose a reason for hiding this comment

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

My issue is that without tailoring this rule will cause issues when remediated. Setting disruption to medium or high might be good idea or add warning like in some other rules.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. You can add a warning to the rule.yml similar as it's done in: https://github.com/ComplianceAsCode/content/blob/master/linux_os/guide/system/software/sudo/sudo_dedicated_group/rule.yml#L14

Copy link
Member Author

Choose a reason for hiding this comment

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

The more I think about it, just deleting users doesn't sit right with me, do want to disable remediation by default?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The more I think about it, just deleting users doesn't sit right with me, do want to disable remediation by default?

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

By default this will delete all user added local users on the box, for a new system that is fine, but for existing systems this will be highly distributive.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, so please remove it. Also, please make sure that the test scenarios will pass.

@Mab879 Mab879 force-pushed the RHEL-08-020320 branch 2 times, most recently from 23e0a78 to ffd88d2 Compare July 30, 2021 18:17
jan-cerny
jan-cerny previously requested changes Aug 3, 2021
View reviewed changes
...ide/system/accounts/accounts-restrictions/accounts_authorized_local_users/ansible/shared.yml Outdated
@@ -1,4 +1,4 @@
# platform = multi_platform_sle
# platform = multi_platform_sle,multi_platform_rhel
# reboot = false
# strategy = restrict
# complexity = low
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, so please remove it. Also, please make sure that the test scenarios will pass.

@Mab879 Mab879 requested a review from jan-cerny August 3, 2021 22:01
@jan-cerny
Copy link
Collaborator

unfortunately we've got some conflicting files, could you fix that?

@Mab879
Copy link
Member Author

Mab879 commented Aug 5, 2021

@jan-cerny Done, thanks for pointing that you.

@openshift-ci
Copy link

openshift-ci bot commented Aug 6, 2021

@Mab879: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-rhcos4-e8 d6f7334 link /test e2e-aws-rhcos4-e8
ci/prow/e2e-aws-rhcos4-moderate d6f7334 link /test e2e-aws-rhcos4-moderate

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@Mab879 Mab879 requested a review from ggbecker August 9, 2021 14:23
@ggbecker
Copy link
Member

ggbecker commented Aug 9, 2021

changes look good... I just wonder if we are using the correct users in the regex. From the STIG, it states that:

Accounts such as "games" and "gopher" are not authorized accounts as they do not support authorized system functions.

And in the default RHEL8 there is an account called games as described in the STIG and it should not be allowed... on the other hand this is on a RHEL default installation and we are not providing the remediation because it can be dangerous to remove legit accounts. We should gather more opinions on this. @yuumasato What do you think about this?

@ggbecker
Copy link
Member

changes look good... I just wonder if we are using the correct users in the regex. From the STIG, it states that:

Accounts such as "games" and "gopher" are not authorized accounts as they do not support authorized system functions.

And in the default RHEL8 there is an account called games as described in the STIG and it should not be allowed... on the other hand this is on a RHEL default installation and we are not providing the remediation because it can be dangerous to remove legit accounts. We should gather more opinions on this. @yuumasato What do you think about this?

I have found a few discussions around this topic. It's not conclusive and mostly system users are legacy.
https://access.redhat.com/discussions/685053
https://access.redhat.com/solutions/31669

Since there is a XCCDF variable for users allowed, we can move forward with this. The user's list can be tailored according to the need.

@ggbecker ggbecker dismissed jan-cerny’s stale review August 17, 2021 10:56

requests have been addressed

@ggbecker ggbecker self-assigned this Aug 17, 2021
@ggbecker ggbecker merged commit 7223401 into ComplianceAsCode:master Aug 17, 2021
@marcusburghardt marcusburghardt added RHEL8 Red Hat Enterprise Linux 8 product related. STIG STIG Benchmark related. labels Jun 23, 2022
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

@jan-cerny jan-cerny Awaiting requested review from jan-cerny

Assignees

@ggbecker ggbecker

Labels
RHEL8 Red Hat Enterprise Linux 8 product related. STIG STIG Benchmark related. Update Rule Issues or pull requests related to Rules updates.
Projects
None yet
Milestone
0.1.58
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Mab879 @jan-cerny @ggbecker @marcusburghardt