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

Improve remediation for enable_authselect #12038

Merged

Conversation

marcusburghardt
Copy link
Member

@marcusburghardt marcusburghardt commented May 30, 2024

Description:

This is a supportive rule intended to ensure authselect is used in a system, specially in fresh installations in some specific scenarios where authselect is not enabled by default. In this case, the preferred profile is selected.

However, in a scenario where a custom profile is used, even when based on the preferred profile, this rule would move again from the custom profile to the preferred profile. This is usually not a desired behavior because may cause inconsistencies between the profiles.
Custom profiles are usually there because a requirement cannot be satisfied by a default profile.

Rationale:

Review Hints:

CI tests in containers are expected to fail because the containers usually don't use authselect.

The simplest test is to ensure authselect is enabled with some features.

authselect select minimal
authselect enable-feature with-mkhomedir

Then execute the enable_authselect remediation informing a different profile for var_authselect_profile variable.
A hint is to build the content, copy and edit the enable_authselect.yml Playbook and execute it directly.

./build_product rhel9
cp build/rhel9/fixes/ansible/enable_authselect.yml /tmp/enable_authselect.yml

Edit the Playbook to make it executable directly via ansible-playbook. Here is an example:

- name: Manual test of Ansible Remediation
  hosts: all
  remote_user: root
  vars:
    - var_authselect_profile: sssd
  tasks:
    - name: Enable authselect - Check Current authselect Profile
      ansible.builtin.command:
        cmd: authselect current
      register: result_authselect_current
      changed_when: false
      failed_when: false
      tags:
      - CCE-89732-2
      - DISA-STIG-needed_rules
      - NIST-800-53-AC-3
      - PCI-DSSv4-8.3.4
      - configure_strategy
      - enable_authselect
      - low_complexity
      - medium_disruption
      - medium_severity
      - no_reboot_needed

...

Then execute it in a VM.

cd /tmp
ansible-playbook -i <VM IP>, enable_authselect.yml

Check the authselect settings on the VM.

This is a supportive rule intended to ensure authselect is used in a
system, specially in fresh installations in some specific scenarios
where authselect is not enabled by default. In this case, the preferred
profile is selected. However, in a scenario where a custom profile is
used, even based on the preferred profile, this rule would move again
from the custom profile to the preferred profile. This is usually not a
desired bahavior because may cause inconsistencies between the
profiles since custom profiles are usually there because a requirement
cannot be satisfied by a default profile.
Avoid showing task as changed when nothing was actually changed.
@marcusburghardt marcusburghardt added bugfix Fixes to reported bugs. Ansible Ansible remediation update. Bash Bash remediation update. labels May 30, 2024
@marcusburghardt marcusburghardt added this to the 0.1.74 milestone May 30, 2024
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

Copy link

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

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_enable_authselect' differs.
--- xccdf_org.ssgproject.content_rule_enable_authselect
+++ xccdf_org.ssgproject.content_rule_enable_authselect
@@ -2,12 +2,16 @@
 var_authselect_profile=''
 
 
-authselect select "$var_authselect_profile"
+authselect current
 
 if test "$?" -ne 0; then
-    if rpm --quiet --verify pam; then
-        authselect select --force "$var_authselect_profile"
-    else
-	echo "Files in the 'pam' package have been altered, so the authselect configuration won't be forced" >&2
+    authselect select "$var_authselect_profile"
+
+    if test "$?" -ne 0; then
+        if rpm --quiet --verify pam; then
+            authselect select --force "$var_authselect_profile"
+        else
+	        echo "authselect is not used but files from the 'pam' package have been altered, so the authselect configuration won't be forced." >&2
+        fi
     fi
 fi

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_enable_authselect' differs.
--- xccdf_org.ssgproject.content_rule_enable_authselect
+++ xccdf_org.ssgproject.content_rule_enable_authselect
@@ -4,10 +4,11 @@
   tags:
     - always
 
-- name: Enable authselect - Select authselect profile
+- name: Enable authselect - Check Current authselect Profile
   ansible.builtin.command:
-    cmd: authselect select "{{ var_authselect_profile }}"
-  register: result_authselect_select
+    cmd: authselect current
+  register: result_authselect_current
+  changed_when: false
   failed_when: false
   tags:
   - CCE-88248-0
@@ -20,12 +21,13 @@
   - medium_severity
   - no_reboot_needed
 
-- name: Enable authselect - Verify if PAM has been altered
+- name: Enable authselect - Try to Select an authselect Profile
   ansible.builtin.command:
-    cmd: rpm -qV pam
-  register: result_altered_authselect
+    cmd: authselect select "{{ var_authselect_profile }}"
+  register: result_authselect_select
+  changed_when: result_authselect_select.rc == 0
   failed_when: false
-  when: result_authselect_select.rc != 0
+  when: result_authselect_current.rc != 0
   tags:
   - CCE-88248-0
   - NIST-800-53-AC-3
@@ -37,14 +39,15 @@
   - medium_severity
   - no_reboot_needed
 
-- name: Enable authselect - Informative message based on the authselect integrity
-    check
-  ansible.builtin.assert:
-    that:
-    - result_altered_authselect is skipped or result_altered_authselect.rc == 0
-    fail_msg:
-    - Files in the 'pam' package have been altered, so the authselect configuration
-      won't be forced.
+- name: Enable authselect - Verify If pam Has Been Altered
+  ansible.builtin.command:
+    cmd: rpm -qV pam
+  register: result_altered_authselect
+  changed_when: false
+  failed_when: false
+  when:
+  - result_authselect_select is not skipped
+  - result_authselect_select.rc != 0
   tags:
   - CCE-88248-0
   - NIST-800-53-AC-3
@@ -56,12 +59,14 @@
   - medium_severity
   - no_reboot_needed
 
-- name: Enable authselect - Force authselect profile select
-  ansible.builtin.command:
-    cmd: authselect select --force "{{ var_authselect_profile }}"
-  when:
-  - result_authselect_select.rc != 0
-  - result_altered_authselect is skipped or result_altered_authselect.rc == 0
+- name: Enable authselect - Informative Message Based on authselect Integrity Check
+  ansible.builtin.assert:
+    that:
+    - result_authselect_current.rc == 0 or result_altered_authselect is skipped or
+      result_altered_authselect.rc == 0
+    fail_msg:
+    - authselect is not used but files from the 'pam' package have been altered, so
+      the authselect configuration won't be forced.
   tags:
   - CCE-88248-0
   - NIST-800-53-AC-3
@@ -72,3 +77,21 @@
   - medium_disruption
   - medium_severity
   - no_reboot_needed
+
+- name: Enable authselect - Force authselect Profile Selection
+  ansible.builtin.command:
+    cmd: authselect select --force "{{ var_authselect_profile }}"
+  when:
+  - result_authselect_current.rc != 0
+  - result_authselect_select.rc != 0
+  - result_altered_authselect.rc == 0
+  tags:
+  - CCE-88248-0
+  - NIST-800-53-AC-3
+  - PCI-DSSv4-8.3.4
+  - configure_strategy
+  - enable_authselect
+  - low_complexity
+  - medium_disruption
+  - medium_severity
+  - no_reboot_needed

Copy link

Change in Ansible shell module found.

Please consider using more suitable Ansible module than shell if possible.

Copy link

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:12038
This image was built from commit: 4f3deb2

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:12038

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

@marcusburghardt marcusburghardt requested a review from a team May 30, 2024 12:08
Copy link

codeclimate bot commented May 30, 2024

Code Climate has analyzed commit 4f3deb2 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.4% (0.0% change).

View more on Code Climate.

@marcusburghardt
Copy link
Member Author

/packit build

@Mab879 Mab879 self-assigned this May 31, 2024
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

LGTM.

Tests pass locally.

@Mab879 Mab879 merged commit b5e9db9 into ComplianceAsCode:master May 31, 2024
110 of 113 checks passed
@marcusburghardt marcusburghardt deleted the enable_authselect_new_case branch June 3, 2024 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ansible Ansible remediation update. Bash Bash remediation update. bugfix Fixes to reported bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants