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

Avoid Ansible shell module if not necessary #10887

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

marcusburghardt
Copy link
Member

Description:

The Ansible remediation of two rules was updated in this PR:

  • dconf_db_up_to_date
    • shell module simply replaced by command module and simplified.
  • no_host_based_files
    • Ansible remediation was refactored to improve efficiency on detecting local partitions and performance on searching files.

Rationale:

Review Hints:

automatus tests with these rules should be enough. The same result is expected after the improvements.

The used approach was not efficient on detecting relevant local
partitions and the performance of the find module was not optimized
since it would search some directories multiple times. The Playbook was
refactored to efficiently detect local partitions and locate
shosts.equiv files on them. Note that this Playbook is sharing many
code with the dir_perms_world_writable_root_owned rule which was also
refactored by me some days ago. In a separate PR I indend to move this
logic to a macro and avoid code duplication.
@marcusburghardt marcusburghardt added enhancement General enhancements to the project. Ansible Ansible remediation update. labels Jul 20, 2023
@marcusburghardt marcusburghardt added this to the 0.1.70 milestone Jul 20, 2023
@github-actions
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@github-actions
Copy link

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_dconf_db_up_to_date' differs.
--- xccdf_org.ssgproject.content_rule_dconf_db_up_to_date
+++ xccdf_org.ssgproject.content_rule_dconf_db_up_to_date
@@ -13,9 +13,8 @@
   - unknown_strategy
 
 - name: Run dconf update
-  shell: |-
-    set -o pipefail
-    dconf update
+  ansible.builtin.command:
+    cmd: dconf update
   when:
   - '"gdm" in ansible_facts.packages'
   - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]

New content has different text for rule 'xccdf_org.ssgproject.content_rule_no_host_based_files'.
--- xccdf_org.ssgproject.content_rule_no_host_based_files
+++ xccdf_org.ssgproject.content_rule_no_host_based_files
@@ -3,10 +3,8 @@
 Remove Host-Based Authentication Files
 
 [description]:
-The shosts.equiv file list remote hosts
-and users that are trusted by the local system.
-To remove these files, run the following command to delete them from any
-location:
+The shosts.equiv file lists remote hosts and users that are trusted by the local
+system. To remove these files, run the following command to delete them from any location:
 $ sudo rm /[path]/[to]/[file]/shosts.equiv
 
 [reference]:
@@ -22,11 +20,10 @@
 SV-230283r627750_rule
 
 [rationale]:
-The shosts.equiv files are used to configure host-based authentication for the
-system via SSH. Host-based authentication is not sufficient for preventing
-unauthorized access to the system, as it does not require interactive
-identification and authentication of a connection request, or for the use of
-two-factor authentication.
+The shosts.equiv files are used to configure host-based authentication for the system via SSH.
+Host-based authentication is not sufficient for preventing unauthorized access to the system,
+as it does not require interactive identification and authentication of a connection request,
+or for the use of two-factor authentication.
 
 [ident]:
 CCE-84055-3

OCIL for rule 'xccdf_org.ssgproject.content_rule_no_host_based_files' differs.
--- ocil:ssg-no_host_based_files_ocil:questionnaire:1
+++ ocil:ssg-no_host_based_files_ocil:questionnaire:1
@@ -1,5 +1,4 @@
-Verify that there are no shosts.equiv files
-on the system, run the following command:
+Verify that there are no shosts.equiv files on the system, run the following command:
 $ find / -name shosts.equiv
       Is it the case that shosts.equiv files exist?
       
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
@@ -1,8 +1,34 @@
-- 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 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: []
+    shosts_equiv_files:
+    - /shosts.equiv
   tags:
   - CCE-84055-3
   - DISA-STIG-RHEL-08-010460
@@ -13,16 +39,15 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Detect the shosts.equiv files on the system
-  find:
-    paths: '{{ item }}'
-    recurse: true
-    patterns:
-    - shosts.equiv
-    file_type: file
-  check_mode: false
-  with_items: '{{ local_mount_points.stdout_lines }}'
-  register: shosts_equiv_locations
+- name: Remove 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
+    recurse: false
+  register: result_relevant_root_dirs
   tags:
   - CCE-84055-3
   - DISA-STIG-RHEL-08-010460
@@ -33,13 +58,11 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Remove shosts.equiv Files
-  file:
-    path: '{{ item.path }}'
-    state: absent
-  with_items: '{{ shosts_equiv_locations.results | map(attribute=''files'') | list
-    }}'
-  when: shosts_equiv_locations is success
+- name: Remove 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-84055-3
   - DISA-STIG-RHEL-08-010460
@@ -49,3 +72,85 @@
   - no_host_based_files
   - no_reboot_needed
   - restrict_strategy
+
+- name: Remove 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-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 - 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-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:
+    cmd: find {{ item }} -xdev -type f -name "shosts.equiv"
+  loop: '{{ search_paths }}'
+  changed_when: false
+  register: result_found_shosts_equiv_files
+  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 - Create List of shosts.equiv Files
+    Present in Local File Systems
+  ansible.builtin.set_fact:
+    shosts_equiv_files: '{{ shosts_equiv_files | union(item.stdout_lines) | list }}'
+  loop: '{{ result_found_shosts_equiv_files.results }}'
+  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 - Ensure No shosts.equiv Files Are
+    Present in the System
+  ansible.builtin.file:
+    path: '{{ item }}'
+    state: absent
+  loop: '{{ shosts_equiv_files }}'
+  tags:
+  - CCE-84055-3
+  - DISA-STIG-RHEL-08-010460
+  - high_severity
+  - low_complexity
+  - low_disruption
+  - no_host_based_files
+  - no_reboot_needed
+  - restrict_strategy

@codeclimate
Copy link

codeclimate bot commented Jul 20, 2023

Code Climate has analyzed commit 7aef752 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.

@Mab879 Mab879 self-assigned this Jul 20, 2023
@Mab879 Mab879 merged commit d99f979 into ComplianceAsCode:master Jul 20, 2023
33 of 34 checks passed
@marcusburghardt marcusburghardt deleted the ansible_shell branch July 21, 2023 05:00
@marcusburghardt marcusburghardt modified the milestones: 0.1.70, 0.1.69 Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ansible Ansible remediation update. enhancement General enhancements to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansible remediations are using not permitted shell commands
2 participants