Review and fix sssd rules in ANSSI control file

[marcusburghardt]

trafficstars

Description:

When reviewing #12351 I noticed some issues with sssd related rules. This PR:

• Remove unnecessary steps in sssd_enable_pam_services bash remediation
• Updates the id_provider value and fix the sssd package name in some test scenarios
• Simplify and align the test scenarios logic
• Create applicability to check if SSSD configuration files are present
• Use the applicability in sssd_enable_pam_services for better reporting

Rationale:

Better test scenarios and remediation for sssd related rules used in ANSSI control file.

Review Hints:

./build_product rhel9
./tests/automatus.py rule --libvirt qemu:///session rhel9 --datastream build/ssg-rhel9-ds.xml --dontclean --remediate-using bash --debug service_sssd_enabled
./tests/automatus.py rule --libvirt qemu:///session rhel9 --datastream build/ssg-rhel9-ds.xml --dontclean --remediate-using bash --debug sssd_enable_pam_services

[github-actions[bot]]

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

[github-actions[bot]]

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

Click here to see the full diff

New content has different text for rule 'xccdf_org.ssgproject.content_rule_sssd_enable_pam_services'.
--- xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
+++ xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
@@ -9,6 +9,9 @@
 /etc/sssd/sssd.conf. For example:
 [sssd]
 services = sudo, autofs, pam
+
+[warning]:
+This rule will report as "notapplicable" if there is no SSSD configuration file present in the system. The SSSD configuration might be different for each site and therefore a new configuration file is not automatically created.
 
 [reference]:
 1

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_enable_pam_services' differs.
--- xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
+++ xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
@@ -1,53 +1,43 @@
 # Remediation is applicable only in certain platforms
 if rpm --quiet -q sssd-common; then
-
-# sssd configuration files must be created with 600 permissions if they don't exist
-# otherwise the sssd module fails to start
-OLD_UMASK=$(umask)
-umask u=rw,go=
 
 SSSD_CONF="/etc/sssd/sssd.conf"
 SSSD_CONF_DIR="/etc/sssd/conf.d/*.conf"
 
 if [ ! -f "$SSSD_CONF" ] && [ ! -f "$SSSD_CONF_DIR" ]; then
-    mkdir -p /etc/sssd
-    touch "$SSSD_CONF"
-fi
+    echo "
+    sssd configuration files not found. Ensure a valid configuration is present.
+    Manual modification of configuration files may be necessary to align with site policies."
+else
+    # Flag to check if there is already services with pam
+    service_already_exist=false
+    for f in $SSSD_CONF $SSSD_CONF_DIR; do
+        if [ ! -e "$f" ]; then
+            continue
+        fi
+        # finds all services entries under [sssd] configuration category, get a unique list so it doesn't add redundant fix
+        services_list=$( awk '/^\s*\[/{f=0} /^\s*\[sssd\]/{f=1}f' $f | grep -P '^services[ \t]*=' | uniq )
+        if [ -z "$services_list" ]; then
+            continue
+        fi
 
-# Flag to check if there is already services with pam
-service_already_exist=false
-for f in $SSSD_CONF $SSSD_CONF_DIR; do
-	if [ ! -e "$f" ]; then
-		continue
-	fi
-	# finds all services entries under [sssd] configuration category, get a unique list so it doesn't add redundant fix
-	services_list=$( awk '/^\s*\[/{f=0} /^\s*\[sssd\]/{f=1}f' $f | grep -P '^services[ \t]*=' | uniq )
-    if [ -z "$services_list" ]; then
-        continue
-    fi
+        while IFS= read -r services; do
+            if [[ ! $services =~ "pam" ]]; then
+                sed -i "s/$services$/&, pam/" $f
+            fi
+            # Either pam service was already there or got added now
+            service_already_exist=true
+        done <<< "$services_list"
+    done
 
-	while IFS= read -r services; do
-		if [[ ! $services =~ "pam" ]]; then
-			sed -i "s/$services$/&, pam/" $f
-		fi
-        # Either pam service was already there or got added now
-        service_already_exist=true
-	done <<< "$services_list"
-
-done
-
-# If there was no service in [sssd], add it to first config
-if [ "$service_already_exist" = false ]; then
-    for f in $SSSD_CONF $SSSD_CONF_DIR; do
-        cat << EOF >> "$f"
+    # If there was no service in [sssd], add it to first config
+    if [ "$service_already_exist" = false ]; then
+        cat << EOF >> "$SSSD_CONF"
 [sssd]
 services = pam
 EOF
-        break
-    done
+    fi
 fi
-
-umask $OLD_UMASK
 
 else
     >&2 echo 'Remediation is not applicable, nothing was done'

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_enable_pam_services' differs.
--- xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
+++ xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
@@ -12,7 +12,7 @@
   - no_reboot_needed
   - sssd_enable_pam_services
 
-- name: Configure PAM in SSSD Services - Find all the conf files inside the /etc/sssd/conf.d/
+- name: Configure PAM in SSSD Services - Find all conf files inside the /etc/sssd/conf.d/
     directory
   ansible.builtin.find:
     paths:
@@ -31,7 +31,7 @@
   - no_reboot_needed
   - sssd_enable_pam_services
 
-- name: Configure PAM in SSSD Services - Modify lines in files in the /etc/sssd/conf.d/
+- name: Configure PAM in SSSD Services - Modify lines in files found in the /etc/sssd/conf.d/
     directory
   ansible.builtin.replace:
     path: '{{ item }}'
@@ -53,7 +53,7 @@
   - no_reboot_needed
   - sssd_enable_pam_services
 
-- name: Configure PAM in SSSD Services - Find /etc/sssd/sssd.conf
+- name: Configure PAM in SSSD Services - Check existence of /etc/sssd/sssd.conf
   ansible.builtin.stat:
     path: /etc/sssd/sssd.conf
   register: sssd_conf_file
@@ -89,16 +89,17 @@
   - no_reboot_needed
   - sssd_enable_pam_services
 
-- name: Configure PAM in SSSD Services - Find services key in /etc/sssd/sssd.conf
-  ansible.builtin.replace:
+- name: Configure PAM in SSSD Services - Ensure services entry in sssd section of
+    /etc/sssd/sssd.conf
+  ini_file:
     path: /etc/sssd/sssd.conf
-    regexp: ^\s*\[sssd\][^\[\]]*?(?:\n(?!\[)[^\n]*?services\s*=)+
-    replace: ''
-  changed_when: false
-  check_mode: true
-  register: sssd_conf_file_services
+    section: sssd
+    option: services
+    value: pam
   when:
   - '"sssd-common" in ansible_facts.packages'
+  - not modify_lines_sssd_conf_d_files.changed
+  - not modify_lines_sssd_conf_file.changed
   - sssd_conf_file.stat.exists
   tags:
   - CCE-82446-6
@@ -110,26 +111,3 @@
   - medium_severity
   - no_reboot_needed
   - sssd_enable_pam_services
-
-- name: Configure PAM in SSSD Services - Insert entry to /etc/sssd/sssd.conf
-  ini_file:
-    path: /etc/sssd/sssd.conf
-    section: sssd
-    option: services
-    value: pam
-  when:
-  - '"sssd-common" in ansible_facts.packages'
-  - not modify_lines_sssd_conf_d_files.changed
-  - not modify_lines_sssd_conf_file.changed
-  - (sssd_conf_file_services.msg is defined and "replacements" not in sssd_conf_file_services.msg)
-    or not sssd_conf_file.stat.exists
-  tags:
-  - CCE-82446-6
-  - NIST-800-53-CM-6(a)
-  - NIST-800-53-IA-2(1)
-  - configure_strategy
-  - low_complexity
-  - medium_disruption
-  - medium_severity
-  - no_reboot_needed
-  - sssd_enable_pam_services

Platform has been changed for rule 'xccdf_org.ssgproject.content_rule_sssd_enable_pam_services'
--- xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
+++ xccdf_org.ssgproject.content_rule_sssd_enable_pam_services
@@ -1 +1 @@
-
+oval:ssg-sssd_conf_files_present:def:1

[github-actions[bot]]

:robot: A k8s content image for this PR is available at: ghcr.io/complianceascode/k8scontent:12378 This image was built from commit: 319c8aa176b52ddfda3d3f5691ea14f653ae70d6

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

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

[marcusburghardt]

CI tests for ANSSI are returning error with rules service_sssd_enabled and sssd_enable_pam_services. The remediation of sssd_enable_pam_services is not supposed to create sssd conf files, as it was incorrectly doing before the PR. It should only update existing files.

Now, since the VM doesn't have sssd conf files created by default, the rules keep failing after remediation. It is expected, but we need to figure out how to treat this.

[marcusburghardt]

CI tests for ANSSI are returning error with rules service_sssd_enabled and sssd_enable_pam_services. The remediation of sssd_enable_pam_services is not supposed to create sssd conf files, as it was incorrectly doing before the PR. It should only update existing files.

Now, since the VM doesn't have sssd conf files created by default, the rules keep failing after remediation. It is expected, but we need to figure out how to treat this.

I created a new applicability to check if any SSSD configuration file is already present in the system. If so, the rule will check and, if necessary, remediate. If not, the rule will report notapplicable. It was also included warning in the rule description informing this behavior.

[marcusburghardt]

I saw that other SSSD related rules could benefit of the applicability introduced here. I have plan to check these rules in another opportunity. Each rule uses a different approach. Ideally we should create a template (maybe some macros are enough) for SSSD and align all the SSSD related rules to the same approach. Unfortunately, this would demand more time than I have right now. So, for this PR I focused only on rules used in ANSSI control file.

[Mab879]

@marcusburghardt Do you plan on workig on this? How can we move this along?

[qlty-cloud-legacy[bot]]

Code Climate has analyzed commit 6e3a4972 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 60.9% (0.0% change).

View more on Code Climate.

[openshift-ci[bot]]

@marcusburghardt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/4.19-images	6e3a497286a7a0336bf6a40bc892e1da89853772	link	true	/test 4.19-images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[jan-cerny]

Closing because of no activity in last year.