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

fix ansible SLES stig remediations in check mode #11248

Conversation

teacup-on-rockingchair
Copy link
Contributor

Description:

  • When run in check mode ansible remediation scripts should be more isolated and need extra checks instead of assumptions

Rationale:

  • Fix ansible remediations run in check mode for rules part of the SLE STIG profile

  • Add var_accounts_tmout variable definition for PCI profile used in accounts_tmout rule

@teacup-on-rockingchair teacup-on-rockingchair added Ansible Ansible remediation update. SLES SUSE Linux Enterprise Server product related. STIG STIG Benchmark related. pci-dss labels Nov 5, 2023
@teacup-on-rockingchair teacup-on-rockingchair requested a review from a team as a code owner November 5, 2023 14:45
Copy link

github-actions bot commented Nov 5, 2023

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

Copy link

github-actions bot commented Nov 5, 2023

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_aide_check_audit_tools' differs.
--- xccdf_org.ssgproject.content_rule_aide_check_audit_tools
+++ xccdf_org.ssgproject.content_rule_aide_check_audit_tools
@@ -1,3 +1,20 @@
+- name: Configure AIDE to Verify the Audit Tools - Gather List of Packages
+  tags:
+  - CCE-85964-5
+  - DISA-STIG-RHEL-08-030650
+  - NIST-800-53-AU-9(3)
+  - NIST-800-53-AU-9(3).1
+  - aide_check_audit_tools
+  - aide_check_audit_tools
+  - low_complexity
+  - low_disruption
+  - medium_severity
+  - no_reboot_needed
+  - restrict_strategy
+  ansible.builtin.package_facts:
+    manager: auto
+  when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+
 - name: Ensure aide is installed
   package:
     name: '{{ item }}'

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_aide_verify_acls' differs.
--- xccdf_org.ssgproject.content_rule_aide_verify_acls
+++ xccdf_org.ssgproject.content_rule_aide_verify_acls
@@ -45,7 +45,7 @@
     replace: \g<1>\g<2>+acl
   when:
   - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
-  - '''aide'' in ansible_facts.packages'
+  - find_rules_groups_results is not skipped and "'aide' in ansible_facts.packages"
   with_items: '{{ find_rules_groups_results.stdout_lines | map(''trim'') | list }}'
   tags:
   - CCE-84220-3

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_aide_verify_ext_attributes' differs.
--- xccdf_org.ssgproject.content_rule_aide_verify_ext_attributes
+++ xccdf_org.ssgproject.content_rule_aide_verify_ext_attributes
@@ -45,7 +45,7 @@
     replace: \g<1>\g<2>+xattrs
   when:
   - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
-  - '''aide'' in ansible_facts.packages'
+  - find_rules_groups_results is not skipped and "'aide' in ansible_facts.packages"
   with_items: '{{ find_rules_groups_results.stdout_lines | map(''trim'') | list }}'
   tags:
   - CCE-83733-6

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_no_empty_passwords_etc_shadow' differs.
--- xccdf_org.ssgproject.content_rule_no_empty_passwords_etc_shadow
+++ xccdf_org.ssgproject.content_rule_no_empty_passwords_etc_shadow
@@ -22,7 +22,7 @@
   with_items: '{{ users_nopasswd.stdout_lines }}'
   when:
   - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
-  - users_nopasswd.stdout_lines | length > 0
+  - users_nopasswd is not skipped and users_nopasswd.stdout_lines | length > 0
   tags:
   - CCE-85953-8
   - DISA-STIG-RHEL-08-010121

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_audit_rules_suid_privilege_function' differs.
--- xccdf_org.ssgproject.content_rule_audit_rules_suid_privilege_function
+++ xccdf_org.ssgproject.content_rule_audit_rules_suid_privilege_function
@@ -100,7 +100,7 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Update Update /etc/audit/audit.rules to audit privileged functions
+- name: Update /etc/audit/audit.rules to audit privileged functions
   ansible.builtin.lineinfile:
     path: /etc/audit/audit.rules
     line: '{{  item.rule  }}'

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_network_sniffer_disabled' differs.
--- xccdf_org.ssgproject.content_rule_network_sniffer_disabled
+++ xccdf_org.ssgproject.content_rule_network_sniffer_disabled
@@ -25,7 +25,7 @@
   loop: '{{ network_interfaces.stdout_lines }}'
   when:
   - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
-  - item.split(':')
+  - network_interfaces.stdout_lines is defined and "item.split(':') | length == 3"
   tags:
   - CCE-82283-3
   - DISA-STIG-RHEL-08-040330

@marcusburghardt marcusburghardt self-assigned this Nov 8, 2023
@vojtapolasek vojtapolasek added this to the 0.1.72 milestone Nov 29, 2023
@marcusburghardt
Copy link
Member

@teacup-on-rockingchair , could you resolve the conflict in this PR, please?

@openshift-merge-robot openshift-merge-robot added the needs-rebase Used by openshift-ci bot. label Dec 9, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Used by openshift-ci bot. label Dec 10, 2023
@teacup-on-rockingchair
Copy link
Contributor Author

@teacup-on-rockingchair , could you resolve the conflict in this PR, please?

Hi @marcusburghardt any issues with the PR that disturbs you?

Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

I have some comments about the new criteria included in the conditionals. Could you take a look, please?

@marcusburghardt
Copy link
Member

In automatus tests for CS8, CS9 and Fedora this error is appearing:
ERROR - Rule 'network_sniffer_disabled' test setup script 'promisc_interface_exists.fail.sh' failed with exit code 2
It is likely related to restrictions in containers. Possibly enabling the CAP_NET_ADMIN capability should allow the network interfaces inside a container to be set to promiscuous mode but it is not necessary for testing purposes. In VMs, the test scenario works as expected. So the error for this specific test scenario can be safely waived here.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Used by openshift-ci bot. label Jan 28, 2024
@marcusburghardt marcusburghardt modified the milestones: 0.1.72, 0.1.73 Jan 29, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Used by openshift-ci bot. label Jan 30, 2024
@jan-cerny
Copy link
Collaborator

/packit retest-failed

@marcusburghardt
Copy link
Member

/packit test

@teacup-on-rockingchair
Copy link
Contributor Author

/packit test

Hi @marcusburghardt I cannot seem to understand how the changes made have connection to the failing tests, that are preventing the PR for merging can you give me some hints here, so we can push this one through?

@marcusburghardt
Copy link
Member

/packit test

Hi @marcusburghardt I cannot seem to understand how the changes made have connection to the failing tests, that are preventing the PR for merging can you give me some hints here, so we can push this one through?

Hi @teacup-on-rockingchair , I noticed a weakness in network_sniffer_disabled rule that was causing the error while executing the Ansible Playbook in CI tests. It should be fixed by #11657

Once this PR is merged, I would recommend to rebase your PR.

Copy link

github-actions bot commented Mar 7, 2024

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:11248
This image was built from commit: 81add9c

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11248

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11248 make deploy-local

Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

I couldn't reproduce errors in check mode even before the changes. Some changes are interesting in this PR, but I believe most of the changes are not necessary since they have no effect. Maybe you could share more details on how to reproduce the errors in check mode, please?

@teacup-on-rockingchair
Copy link
Contributor Author

I couldn't reproduce errors in check mode even before the changes. Some changes are interesting in this PR, but I believe most of the changes are not necessary since they have no effect. Maybe you could share more details on how to reproduce the errors in check mode, please?

Thanks for the help and feedback @marcusburghardt I rebased at 6abcf65 and removed irrelevant changes, and applied the suggestion to use file instead of fileglob. In my setup the ansible playbooks in check mode now work ok for stig and pci-dss profiles. which was the initial intention.

@marcusburghardt
Copy link
Member

@teacup-on-rockingchair , I am finishing some local tests but so far it is good.
Could you only take a look in the "Automatus SLE15" failure, please? It seems legit:

ERROR - Terminating due to error: Couldn't install required packages: passwd.

Make sure that dependencies on passwd package are interpreted for that platform.
Thanks to @marcusburghardt for raising the flag on that 🙇
Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

Great. Now it is only necessary to update the references for stability tests and the required tests should pass.

Copy link

codeclimate bot commented Apr 8, 2024

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

@teacup-on-rockingchair
Copy link
Contributor Author

Great. Now it is only necessary to update the references for stability tests and the required tests should pass.

Thanks @marcusburghardt 🙇 I think the only issue in tests seemed to be network_sniffer_disabled test, which is container related if I understand it correctly

Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

Thanks

@marcusburghardt
Copy link
Member

Overriding CODEOWNERS as @teacup-on-rockingchair can't approve his own PR.

@marcusburghardt marcusburghardt merged commit bec7734 into ComplianceAsCode:master Apr 9, 2024
41 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ansible Ansible remediation update. pci-dss SLES SUSE Linux Enterprise Server product related. STIG STIG Benchmark related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants