Refactor Ansible remediations that search local file systems #10912
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are some rules which need to locate files and directories in a system but should only consider local files and directories, ignoring remote file systems and some known local directories with pseudo file systems. Recently the Ansible approach was refactored in two rules in order to make the remediation more robust and more efficient. A third rule was discovered with the same issue so this macro was created to make the remediation aligned among all relevant rules and to make the same approach easier to be adopted by future rules.
Adopted the new Ansible macro to efficiently identify relevant local paths. The previous remediation wasn't even working as expected and failing in test scenarios. The issue was fixed by using the new macro.
Also aligned the remaining tasks with project Style Guides
marcusburghardt
added
bugfix
|
Start a new ephemeral environment with changes proposed in this pull request: rhel8 (from CTF) Environment (using Fedora as testing environment) Fedora Testing Environment Oracle Linux 8 Environment |
|
This datastream diff is auto generated by the check Click here to see the full diffansible remediation for rule 'xccdf_org.ssgproject.content_rule_set_password_hashing_min_rounds_logindefs' differs.
--- xccdf_org.ssgproject.content_rule_set_password_hashing_min_rounds_logindefs
+++ xccdf_org.ssgproject.content_rule_set_password_hashing_min_rounds_logindefs
@@ -1,5 +1,6 @@
-- name: Ensure SHA_CRYPT_MIN_ROUNDS has minimum value of 5000
- replace:
+- name: Set Password Hashing Rounds in /etc/login.defs - Ensure SHA_CRYPT_MIN_ROUNDS
+ has Minimum Value of 5000
+ ansible.builtin.replace:
path: /etc/login.defs
regexp: (^\s*SHA_CRYPT_MIN_ROUNDS\s+)(?!(?:[5-9]\d{3,}|\d{5,}))\S*(\s*$)
replace: \g<1>5000\g<2>
@@ -14,11 +15,13 @@
- restrict_strategy
- set_password_hashing_min_rounds_logindefs
-- name: Check to see if SHA_CRYPT_MIN_ROUNDS is explicitly configured
- shell: |
- set -o pipefail
- grep -e '^\s*SHA_CRYPT_MIN_ROUNDS\s\+' /etc/login.defs || true
- register: check_sha_crypt_min_rounds_result
+- name: Set Password Hashing Rounds in /etc/login.defs - Ensure SHA_CRYPT_MAX_ROUNDS
+ has Minimum Value of 5000
+ ansible.builtin.replace:
+ path: /etc/login.defs
+ regexp: (^\s*SHA_CRYPT_MAX_ROUNDS\s+)(?!(?:[5-9]\d{3,}|\d{5,}))\S*(\s*$)
+ replace: \g<1>5000\g<2>
+ backup: false
tags:
- CCE-89707-4
- DISA-STIG-RHEL-08-010130
@@ -28,20 +31,3 @@
- no_reboot_needed
- restrict_strategy
- set_password_hashing_min_rounds_logindefs
-
-- name: Ensure SHA_CRYPT_MAX_ROUNDS has minimum value of 5000
- replace:
- path: /etc/login.defs
- regexp: (^\s*SHA_CRYPT_MAX_ROUNDS\s+)(?!(?:[5-9]\d{3,}|\d{5,}))\S*(\s*$)
- replace: \g<1>5000\g<2>
- backup: false
- when: '"SHA_CRYPT_MIN_ROUNDS" not in check_sha_crypt_min_rounds_result.stdout'
- tags:
- - CCE-89707-4
- - DISA-STIG-RHEL-08-010130
- - low_complexity
- - low_disruption
- - medium_severity
- - no_reboot_needed
- - restrict_strategy
- - set_password_hashing_min_rounds_logindefs
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_dir_perms_world_writable_root_owned' differs.
--- xccdf_org.ssgproject.content_rule_dir_perms_world_writable_root_owned
+++ xccdf_org.ssgproject.content_rule_dir_perms_world_writable_root_owned
@@ -27,7 +27,6 @@
- run
- sys
search_paths: []
- world_writable_dirs: []
tags:
- CCE-83375-6
- DISA-STIG-RHEL-08-010700
@@ -106,6 +105,20 @@
- no_reboot_needed
- restrict_strategy
+- name: Ensure All World-Writable Directories Are Owned by root User - Define Rule
+ Specific Facts
+ ansible.builtin.set_fact:
+ world_writable_dirs: []
+ tags:
+ - CCE-83375-6
+ - DISA-STIG-RHEL-08-010700
+ - dir_perms_world_writable_root_owned
+ - low_complexity
+ - medium_disruption
+ - medium_severity
+ - no_reboot_needed
+ - restrict_strategy
+
- name: Ensure All World-Writable Directories Are Owned by root User - Find All Uncompliant
Directories in Local File Systems
ansible.builtin.command:
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_no_host_based_files' differs.
--- xccdf_org.ssgproject.content_rule_no_host_based_files
+++ xccdf_org.ssgproject.content_rule_no_host_based_files
@@ -27,8 +27,6 @@
- run
- sys
search_paths: []
- shosts_equiv_files:
- - /shosts.equiv
tags:
- CCE-84055-3
- DISA-STIG-RHEL-08-010460
@@ -107,6 +105,20 @@
- no_reboot_needed
- restrict_strategy
+- name: Remove Host-Based Authentication Files - Define Rule Specific Facts
+ ansible.builtin.set_fact:
+ shosts_equiv_files:
+ - /shosts.equiv
+ tags:
+ - CCE-84055-3
+ - DISA-STIG-RHEL-08-010460
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_host_based_files
+ - no_reboot_needed
+ - restrict_strategy
+
- name: Remove Host-Based Authentication Files - Find All shosts.equiv Files in Local
File Systems
ansible.builtin.command:
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_no_user_host_based_files' differs.
--- xccdf_org.ssgproject.content_rule_no_user_host_based_files
+++ xccdf_org.ssgproject.content_rule_no_user_host_based_files
@@ -1,8 +1,32 @@
-- name: Find local mount points
- shell: |
- set -o pipefail
- df --local | awk '{print $6}' | grep -v Mounted | grep -v '^/dev' || true
- register: local_mount_points
+- name: Remove User Host-Based Authentication Files - Define Excluded (Non-Local)
+ File Systems and Paths
+ ansible.builtin.set_fact:
+ excluded_fstypes:
+ - afs
+ - ceph
+ - cifs
+ - smb3
+ - smbfs
+ - sshfs
+ - ncpfs
+ - ncp
+ - nfs
+ - nfs4
+ - gfs
+ - gfs2
+ - glusterfs
+ - gpfs
+ - pvfs2
+ - ocfs2
+ - lustre
+ - davfs
+ - fuse.sshfs
+ excluded_paths:
+ - dev
+ - proc
+ - run
+ - sys
+ search_paths: []
tags:
- CCE-84056-1
- DISA-STIG-RHEL-08-010470
@@ -13,17 +37,15 @@
- no_user_host_based_files
- restrict_strategy
-- name: Detect the .shosts files on the system
- find:
- paths: '{{ item }}'
- recurse: true
- patterns:
- - .shosts
+- name: Remove User Host-Based Authentication Files - Find Relevant Root Directories
+ Ignoring Pre-Defined Excluded Paths
+ ansible.builtin.find:
+ paths: /
+ file_type: directory
+ excludes: '{{ excluded_paths }}'
hidden: true
- file_type: file
- check_mode: false
- with_items: '{{ local_mount_points.stdout_lines }}'
- register: shosts_locations
+ recurse: false
+ register: result_relevant_root_dirs
tags:
- CCE-84056-1
- DISA-STIG-RHEL-08-010470
@@ -34,12 +56,11 @@
- no_user_host_based_files
- restrict_strategy
-- name: Remove .shosts Files
- file:
- path: '{{ item.path }}'
- state: absent
- with_items: '{{ shosts_locations.results | map(attribute=''files'') | list }}'
- when: shosts_locations is success
+- name: Remove User Host-Based Authentication Files - Include Relevant Root Directories
+ in a List of Paths to be Searched
+ ansible.builtin.set_fact:
+ search_paths: '{{ search_paths | union([item.path]) }}'
+ loop: '{{ result_relevant_root_dirs.files }}'
tags:
- CCE-84056-1
- DISA-STIG-RHEL-08-010470
@@ -49,3 +70,99 @@
- no_reboot_needed
- no_user_host_based_files
- restrict_strategy
+
+- name: Remove User Host-Based Authentication Files - Increment Search Paths List
+ with Local Partitions Mount Points
+ ansible.builtin.set_fact:
+ search_paths: '{{ search_paths | union([item.mount]) }}'
+ loop: '{{ ansible_mounts }}'
+ when:
+ - item.fstype not in excluded_fstypes
+ - item.mount != '/'
+ tags:
+ - CCE-84056-1
+ - DISA-STIG-RHEL-08-010470
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_reboot_needed
+ - no_user_host_based_files
+ - restrict_strategy
+
+- name: Remove User Host-Based Authentication Files - Increment Search Paths List
+ with Local NFS File System Targets
+ ansible.builtin.set_fact:
+ search_paths: '{{ search_paths | union([item.device.split('':'')[1]]) }}'
+ loop: '{{ ansible_mounts }}'
+ when: item.device is search("localhost:")
+ tags:
+ - CCE-84056-1
+ - DISA-STIG-RHEL-08-010470
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_reboot_needed
+ - no_user_host_based_files
+ - restrict_strategy
+
+- name: Remove User Host-Based Authentication Files - Define Rule Specific Facts
+ ansible.builtin.set_fact:
+ user_shosts_files:
+ - /.shosts
+ tags:
+ - CCE-84056-1
+ - DISA-STIG-RHEL-08-010470
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_reboot_needed
+ - no_user_host_based_files
+ - restrict_strategy
+
+- name: Remove User Host-Based Authentication Files - Find All .shosts Files in Local
+ File Systems
+ ansible.builtin.command:
+ cmd: find {{ item }} -xdev -type f -name ".shosts"
+ loop: '{{ search_paths }}'
+ changed_when: false
+ register: result_found_shosts_files
+ tags:
+ - CCE-84056-1
+ - DISA-STIG-RHEL-08-010470
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_reboot_needed
+ - no_user_host_based_files
+ - restrict_strategy
+
+- name: Remove User Host-Based Authentication Files - Create List of .shosts Files
+ Present in Local File Systems
+ ansible.builtin.set_fact:
+ user_shosts_files: '{{ user_shosts_files | union(item.stdout_lines) | list }}'
+ loop: '{{ result_found_shosts_files.results }}'
+ tags:
+ - CCE-84056-1
+ - DISA-STIG-RHEL-08-010470
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_reboot_needed
+ - no_user_host_based_files
+ - restrict_strategy
+
+- name: Remove User Host-Based Authentication Files - Ensure No .shosts Files Are
+ Present in the System
+ ansible.builtin.file:
+ path: '{{ item }}'
+ state: absent
+ loop: '{{ user_shosts_files }}'
+ tags:
+ - CCE-84056-1
+ - DISA-STIG-RHEL-08-010470
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_reboot_needed
+ - no_user_host_based_files
+ - restrict_strategy |
jan-cerny
reviewed
Jul 27, 2023
| when: | ||
| - item.fstype not in excluded_fstypes | ||
| - item.mount != '/' | ||
| {{{ ansible_create_list_of_local_paths(list_name="search_paths") }}} |
There was a problem hiding this comment.
I think that the fail of the test scenario world_writable_dir_on_nonlocal_fs.fail.sh on the CI is caused by the invocation of systemctl command in this scenario, which hits the problem that there is no Dbus in the container back end environment that we use in CI.
+ systemctl restart nfs-server
System has not been booted with systemd as init system (PID 1). Can't operate.
Failed to connect to bus: Host is down
When executed local with a virtual machine back end the test is passing.
[jcerny@fedora scap-security-guide{pr/10912}]$ python3 tests/automatus.py rule --remediate-using ansible --libvirt qemu:///system ssgts_rhel9 dir_perms_world_writable_root_owned
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/jcerny/work/git/scap-security-guide/logs/rule-custom-2023-07-27-1438/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_dir_perms_world_writable_root_owned
INFO - Script world_writable_dir_on_nonlocal_fs.fail.sh using profile (all) OK
INFO - Script world_writable_dir_owned_by_uid_2.fail.sh using profile (all) OK
INFO - Script all_dirs_ok.pass.sh using profile (all) OK
I think the problem isn't related to the contents of this PR.
| grep -e '^\s*SHA_CRYPT_MIN_ROUNDS\s\+' /etc/login.defs || true | ||
| register: check_sha_crypt_min_rounds_result | ||
|
|
||
| # NOTE(gyee): there's a possibility that the value of SHA_CRYPT_MIN_ROUNDS is |
There was a problem hiding this comment.
Was the comment useful?
There was a problem hiding this comment.
SHA_CRYPT_MIN_ROUNDS is specific for SHA-512-based passwords so it is not supported in all cases. In most of the cases (and distros) it is not expected to be set. I think the comments were unnecessary and also the case is now represented in test scenarios. IMO the comment won't be missed.
|
|
||
| - name: "{{{ rule_title }}} - Find All .shosts Files in Local File Systems" | ||
| ansible.builtin.command: | ||
| cmd: find {{ item }} -xdev -type f -name ".shosts" |
There was a problem hiding this comment.
Will this be reported as not permitted shell command?
There was a problem hiding this comment.
No. It is using the cmd module and the test only cares about shell module, so it won't be reported as an issue.
|
Code Climate has analyzed commit 5eafed2 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 53.2% (0.0% change). View more on Code Climate. |
|
/packit retest-failed |
1 similar comment
|
/packit retest-failed |
jan-cerny
approved these changes
Jul 28, 2023
|
/packit retest-failed |
1 similar comment
|
/packit retest-failed |
|
/packit test |
This was referenced Aug 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
There are some rules which need to locate files and directories in a system but should only consider local files and directories, ignoring remote file systems and some known local directories with pseudo file systems.
Recently the Ansible approach in
dir_perms_world_writable_root_ownedandno_host_based_fileswas refactored in order to make the remediation more robust and more efficient.A third rule (
no_user_host_based_files) was discovered with the same issue.Therefore a macro was created to make the remediation aligned among all relevant rules and to make the same approach easier to be adopted by future rules.
This PR also refactors the Ansible remediation in
set_password_hashing_min_rounds_logindefsrule by removing unnecessary tasks and align the playbook to the project Style Guides. Finally, the test scenarios were also improved.Rationale:
Review Hints:
Automatus tests using Ansible remediation should be enough.