-
Notifications
You must be signed in to change notification settings - Fork 685
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 root user to interactive users #11729
Conversation
This datastream diff is auto generated by the check Click here to see the full diffNew content has different text for rule 'xccdf_org.ssgproject.content_rule_file_permission_user_init_files'.
--- xccdf_org.ssgproject.content_rule_file_permission_user_init_files
+++ xccdf_org.ssgproject.content_rule_file_permission_user_init_files
@@ -16,12 +16,6 @@
[reference]:
R50
-[reference]:
-RHEL-09-232045
-
-[reference]:
-SV-257889r925654_rule
-
[rationale]:
Local initialization files are used to configure the user's shell environment
upon logon. Malicious modification of these files could compromise accounts upon
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_file_permission_user_init_files' differs.
--- xccdf_org.ssgproject.content_rule_file_permission_user_init_files
+++ xccdf_org.ssgproject.content_rule_file_permission_user_init_files
@@ -10,7 +10,6 @@
database: passwd
tags:
- CCE-83637-9
- - DISA-STIG-RHEL-09-232045
- file_permission_user_init_files
- low_complexity
- low_disruption
@@ -33,7 +32,6 @@
register: found_init_files
tags:
- CCE-83637-9
- - DISA-STIG-RHEL-09-232045
- file_permission_user_init_files
- low_complexity
- low_disruption
@@ -50,7 +48,6 @@
{''skip_missing'': True}) }}'
tags:
- CCE-83637-9
- - DISA-STIG-RHEL-09-232045
- file_permission_user_init_files
- low_complexity
- low_disruption |
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
Tests pass locally
|
shared/macros/10-oval.jinja
Outdated
</set> | ||
</unix:password_object> | ||
|
||
<unix:password_object id="{{{ object_id }}}_root" version="1"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are more rules using this macro. We should ensure the remediation on them are aligned to the OVAL assessment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes already working on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scan has option for root, but the remediation doesn't. Is that expected.
readarray -t interactive_users < <(awk -F: '$3>={{{ uid_min }}} {print $1}' /etc/passwd) | ||
readarray -t interactive_users_home < <(awk -F: '$3>={{{ uid_min }}} {print $6}' /etc/passwd) | ||
readarray -t interactive_users_shell < <(awk -F: '$3>={{{ uid_min }}} {print $7}' /etc/passwd) | ||
readarray -t interactive_users < <(awk -F: '$3==0 || $3>={{{ uid_min }}} {print $1}' /etc/passwd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This always checks root do we want that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think yes - I think that is the point of the issue that the root's files were not evaluated and were not remediatied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The testing farm failure for RHEL 7 looks valid, please take look.
The rule passes after first remediation and passes after second remediation but suddenly it fails in the final scan. I suspect something happens during the third reboot. I will try to identify the cause. |
The problem is that it conflicts with the rule rpm_verify_permissions. The root's dot files are owned by the rootfiles RPM package and they're shipped with different permissions. The first remediation changes the permissions in the code for rule file_permission_user_init_files, but that makes the rule rpm_verify_permissions fail and this fail is fixed during the second remediation and that makes the rule file_permission_user_init_files fail. |
We're trying to fix a RHEL 9 STIG misalignement. But the conflict with rpm_verify_permissions happens on RHEL 7. The reason is that RHEL 9 STIG doesn't contain rpm_verify_permissions. So we can solve this situation this way: We won't change rule file_permission_user_init_files. We will create a new rule that will be similar but will check the root files. We will replace the file_permission_user_init_files in RHEL 9 STIG with the new rule. We will select the new rule only in RHEL 9 STIG. This way, we won't break other products. What do you think? |
That seems like a fine solution. |
OK I will proceed with my proposal |
The rule file_permission_user_init_files now checks only dot files of users with UID greater than or equal 1000. But according to RHEL 9 STIG and CIS benchmarks it should check also the root user's dot files. This commit creates a new rule file_permission_user_init_files_root which is almost the same as file_permission_user_init_files, but the new rule accounts also for the root user and his init files. We also change the OVAL jinja macro. This change will include the root user to the user list only if needed. We will use the root in the rule file_permission_user_init_file. But we will not use the root in accounts_user_interactive_home_directory_defined where we keep the old behavior. The commit also adds a simple test scenario covering this situation. Fixes: ComplianceAsCode#11699
Code Climate has analyzed commit af615e2 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 59.3% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks
The rule file_permission_user_init_files now checks only dot files of users with UID greater than or equal 1000. But according to RHEL 9 STIG and CIS benchmarks it should check also the root user's dot files. This commit extends the rule to account also for the root user and adds a simple test scenario covering this situation.
Fixes #11699