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

align template systemd_dropin_configuration #12054

Merged

Conversation

vojtapolasek
Copy link
Collaborator

@vojtapolasek vojtapolasek commented Jun 10, 2024

Description:

  • correct and simplify the bash remediation for the template
  • modify test scenarios
  • update Ansible remediation to be aligned with Bash

Rationale:

Review Hints:

  • test the systemd_dropin_configuration template
  • also test rules journald_compress and journald_storage

What remains to be done?

  • documentation of the template

@vojtapolasek vojtapolasek added the Bash Bash remediation update. label Jun 10, 2024
@vojtapolasek vojtapolasek added this to the 0.1.74 milestone Jun 10, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Jun 10, 2024
Copy link

openshift-ci bot commented Jun 10, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

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

github-actions bot commented Jun 10, 2024

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_dnf-automatic_apply_updates' differs.
--- xccdf_org.ssgproject.content_rule_dnf-automatic_apply_updates
+++ xccdf_org.ssgproject.content_rule_dnf-automatic_apply_updates
@@ -9,12 +9,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[commands\]([^\n\[]*\n+)+?[[:space:]]*apply_updates" "$f"; then
-            sed -i "s/apply_updates[^(\n)]*/apply_updates = yes/" "$f"
+
+            sed -i "s/apply_updates[^(\n)]*/apply_updates=yes/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[commands\]" "$f"; then
-            sed -i "/[[:space:]]*\[commands\]/a apply_updates = yes" "$f"
+
+            sed -i "/[[:space:]]*\[commands\]/a apply_updates=yes" "$f"
+
             found=true
     fi
 done
@@ -23,5 +27,7 @@
 if ! $found ; then
     file=$(echo "/etc/dnf/automatic.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[commands]\napply_updates = yes" >> "$file"
+
+    echo -e "[commands]\napply_updates=yes" >> "$file"
+
 fi

bash remediation for rule 'xccdf_org.ssgproject.content_rule_dnf-automatic_security_updates_only' differs.
--- xccdf_org.ssgproject.content_rule_dnf-automatic_security_updates_only
+++ xccdf_org.ssgproject.content_rule_dnf-automatic_security_updates_only
@@ -9,12 +9,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[commands\]([^\n\[]*\n+)+?[[:space:]]*upgrade_type" "$f"; then
-            sed -i "s/upgrade_type[^(\n)]*/upgrade_type = security/" "$f"
+
+            sed -i "s/upgrade_type[^(\n)]*/upgrade_type=security/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[commands\]" "$f"; then
-            sed -i "/[[:space:]]*\[commands\]/a upgrade_type = security" "$f"
+
+            sed -i "/[[:space:]]*\[commands\]/a upgrade_type=security" "$f"
+
             found=true
     fi
 done
@@ -23,5 +27,7 @@
 if ! $found ; then
     file=$(echo "/etc/dnf/automatic.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[commands]\nupgrade_type = security" >> "$file"
+
+    echo -e "[commands]\nupgrade_type=security" >> "$file"
+
 fi

bash remediation for rule 'xccdf_org.ssgproject.content_rule_journald_compress' differs.
--- xccdf_org.ssgproject.content_rule_journald_compress
+++ xccdf_org.ssgproject.content_rule_journald_compress
@@ -1,46 +1,38 @@
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
-function remove_journald_Compress_configuration {
-    local COMPONENT_PARAM_CONFIG
-    mapfile -t COMPONENT_PARAM_CONFIG < <(ls /etc/systemd/journald.conf.d/*.conf)
-    COMPONENT_PARAM_CONFIG+=("/etc/systemd/journald.conf")
+found=false
 
-    for f in "${COMPONENT_PARAM_CONFIG[@]}"
-    do
-        sed -i "/^\s*Compress\s*=\s*/d" "$f"
-        # make sure file has newline at the end
-        sed -i -e '$a\' "$f"
-    done
-    sed -i -e '$a\' "/etc/systemd/journald.conf"
-}
+# set value in all files if they contain section or key
+for f in $(echo -n "/etc/systemd/journald.conf.d/complianceascode_hardening.conf /etc/systemd/journald.conf.d/*.conf /etc/systemd/journald.conf"); do
+    if [ ! -e "$f" ]; then
+        continue
+    fi
 
-function journald_Compress_add_configuration {
-    local COMPONENT_PARAM_REMEDY_CFG
-    mkdir -p "/etc/systemd/journald.conf.d"
-    COMPONENT_PARAM_REMEDY_CFG="/etc/systemd/journald.conf.d/oscap-remedy.conf"
+    # find key in section and change value
+    if grep -qzosP "[[:space:]]*\[Journal\]([^\n\[]*\n+)+?[[:space:]]*Compress" "$f"; then
 
-    if [ ! -f "${COMPONENT_PARAM_REMEDY_CFG}" ] ; then
-        touch "${COMPONENT_PARAM_REMEDY_CFG}"
+            sed -i "s/Compress[^(\n)]*/Compress=yes/" "$f"
+
+            found=true
+
+    # find section and add key = value to it
+    elif grep -qs "[[:space:]]*\[Journal\]" "$f"; then
+
+            sed -i "/[[:space:]]*\[Journal\]/a Compress=yes" "$f"
+
+            found=true
     fi
-    cp "${COMPONENT_PARAM_REMEDY_CFG}" "${COMPONENT_PARAM_REMEDY_CFG}.bak"
-    # Insert before the line matching the regex '^#\s*Compress'.
-    line_number="$(LC_ALL=C grep -n "^#\s*Compress" "${COMPONENT_PARAM_REMEDY_CFG}.bak" | LC_ALL=C sed 's/:.*//g')"
-    if [ -z "$line_number" ]; then
-       # There was no match of '^#\s*Compress', insert at
-       # the end of the file.
-       printf '%s\n' "Compress=yes" >> "${COMPONENT_PARAM_REMEDY_CFG}"
-    else
-        head -n "$(( line_number - 1 ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" > "${COMPONENT_PARAM_REMEDY_CFG}"
-        printf '%s\n' "Compress=yes" >> "/etc/systemd/journald.conf"
-        tail -n "+$(( line_number ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" >> "${COMPONENT_PARAM_REMEDY_CFG}"
-    fi
-    # Clean up after ourselves.
-    rm "${COMPONENT_PARAM_REMEDY_CFG}.bak"
-}
+done
 
-remove_journald_Compress_configuration
-journald_Compress_add_configuration
+# if section not in any file, append section with key = value to FIRST file in files parameter
+if ! $found ; then
+    file=$(echo "/etc/systemd/journald.conf.d/complianceascode_hardening.conf /etc/systemd/journald.conf.d/*.conf /etc/systemd/journald.conf" | cut -f1 -d ' ')
+    mkdir -p "$(dirname "$file")"
+
+    echo -e "[Journal]\nCompress=yes" >> "$file"
+
+fi
 
 else
     >&2 echo 'Remediation is not applicable, nothing was done'

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_journald_compress' differs.
--- xccdf_org.ssgproject.content_rule_journald_compress
+++ xccdf_org.ssgproject.content_rule_journald_compress
@@ -1,12 +1,17 @@
-- name: Check for duplicate Compress values in master journald configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf
-    create: false
-    regexp: ^\s*Compress=
-    state: absent
-  check_mode: true
-  changed_when: false
-  register: dupes_master
+- name: Ensure journald is configured to compress large log files - Search for a section
+    in files
+  ansible.builtin.find:
+    paths: '{{item.path}}'
+    patterns: '{{item.pattern}}'
+    contains: ^\s*\[Journal\]
+    read_whole_file: true
+    use_regex: true
+  register: systemd_dropin_files_with_section
+  loop:
+  - path: '{{ ''/etc/systemd/journald.conf'' | dirname }}'
+    pattern: '{{ ''/etc/systemd/journald.conf'' | basename | regex_escape }}'
+  - path: /etc/systemd/journald.conf.d
+    pattern: .*\.conf
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   tags:
   - CCE-85930-6
@@ -17,30 +22,11 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Deduplicate Compress values from journald master configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf
-    create: false
-    regexp: ^\s*Compress=
-    state: absent
-  when:
-  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
-  - dupes_master.found is defined and dupes_master.found > 1
-  tags:
-  - CCE-85930-6
-  - journald_compress
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - no_reboot_needed
-  - restrict_strategy
-
-- name: Collect all config journald files which configure Compress
-  ansible.builtin.find:
-    paths: /etc/systemd/journald.conf.d
-    contains: ^[\s]*Compress=.*$
-    patterns: '*.conf'
-  register: journald_Compress_dropin_config_files
+- name: Ensure journald is configured to compress large log files - Count number of
+    files which contain the correct section
+  ansible.builtin.set_fact:
+    count_of_systemd_dropin_files_with_section: '{{systemd_dropin_files_with_section.results
+      | map(attribute=''matched'') | list | map(''int'') | sum}}'
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   tags:
   - CCE-85930-6
@@ -51,14 +37,20 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Deduplicate values from journald Compress dropin configuration
-  ansible.builtin.lineinfile:
-    path: '{{ item.path }}'
-    create: false
-    regexp: ^\s*Compress=
-    state: absent
-  loop: '{{  journald_Compress_dropin_config_files.files }}'
-  when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+- name: Ensure journald is configured to compress large log files - Add missing configuration
+    to correct section
+  ini_file:
+    path: '{{item}}'
+    section: Journal
+    option: Compress
+    value: 'yes'
+    state: present
+    no_extra_spaces: true
+  when:
+  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  - count_of_systemd_dropin_files_with_section | int > 0
+  loop: '{{systemd_dropin_files_with_section.results | sum(attribute=''files'', start=[])
+    | map(attribute=''path'') | list }}'
   tags:
   - CCE-85930-6
   - journald_compress
@@ -68,16 +60,19 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Insert correct line to journald Compress configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf.d/oscap-remedy.conf
+- name: Ensure journald is configured to compress large log files - Add configuration
+    to new remediation file
+  ini_file:
+    path: /etc/systemd/journald.conf.d/complianceascode_hardening.conf
+    section: Journal
+    option: Compress
+    value: 'yes'
+    state: present
+    no_extra_spaces: true
     create: true
-    regexp: ^\s*Compress=
-    line: Compress=yes
-    state: present
-    insertbefore: ^# Compress
-    validate: bash -n %s
-  when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  when:
+  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  - count_of_systemd_dropin_files_with_section | int == 0
   tags:
   - CCE-85930-6
   - journald_compress

bash remediation for rule 'xccdf_org.ssgproject.content_rule_journald_forward_to_syslog' differs.
--- xccdf_org.ssgproject.content_rule_journald_forward_to_syslog
+++ xccdf_org.ssgproject.content_rule_journald_forward_to_syslog
@@ -1,46 +1,38 @@
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
-function remove_journald_ForwardToSyslog_configuration {
-    local COMPONENT_PARAM_CONFIG
-    mapfile -t COMPONENT_PARAM_CONFIG < <(ls /etc/systemd/journald.conf.d/*.conf)
-    COMPONENT_PARAM_CONFIG+=("/etc/systemd/journald.conf")
+found=false
 
-    for f in "${COMPONENT_PARAM_CONFIG[@]}"
-    do
-        sed -i "/^\s*ForwardToSyslog\s*=\s*/d" "$f"
-        # make sure file has newline at the end
-        sed -i -e '$a\' "$f"
-    done
-    sed -i -e '$a\' "/etc/systemd/journald.conf"
-}
+# set value in all files if they contain section or key
+for f in $(echo -n "/etc/systemd/journald.conf.d/complianceascode_hardening.conf /etc/systemd/journald.conf.d/*.conf /etc/systemd/journald.conf"); do
+    if [ ! -e "$f" ]; then
+        continue
+    fi
 
-function journald_ForwardToSyslog_add_configuration {
-    local COMPONENT_PARAM_REMEDY_CFG
-    mkdir -p "/etc/systemd/journald.conf.d"
-    COMPONENT_PARAM_REMEDY_CFG="/etc/systemd/journald.conf.d/oscap-remedy.conf"
+    # find key in section and change value
+    if grep -qzosP "[[:space:]]*\[Journal\]([^\n\[]*\n+)+?[[:space:]]*ForwardToSyslog" "$f"; then
 
-    if [ ! -f "${COMPONENT_PARAM_REMEDY_CFG}" ] ; then
-        touch "${COMPONENT_PARAM_REMEDY_CFG}"
+            sed -i "s/ForwardToSyslog[^(\n)]*/ForwardToSyslog=yes/" "$f"
+
+            found=true
+
+    # find section and add key = value to it
+    elif grep -qs "[[:space:]]*\[Journal\]" "$f"; then
+
+            sed -i "/[[:space:]]*\[Journal\]/a ForwardToSyslog=yes" "$f"
+
+            found=true
     fi
-    cp "${COMPONENT_PARAM_REMEDY_CFG}" "${COMPONENT_PARAM_REMEDY_CFG}.bak"
-    # Insert before the line matching the regex '^#\s*Compress'.
-    line_number="$(LC_ALL=C grep -n "^#\s*ForwardToSyslog" "${COMPONENT_PARAM_REMEDY_CFG}.bak" | LC_ALL=C sed 's/:.*//g')"
-    if [ -z "$line_number" ]; then
-       # There was no match of '^#\s*ForwardToSyslog', insert at
-       # the end of the file.
-       printf '%s\n' "ForwardToSyslog=yes" >> "${COMPONENT_PARAM_REMEDY_CFG}"
-    else
-        head -n "$(( line_number - 1 ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" > "${COMPONENT_PARAM_REMEDY_CFG}"
-        printf '%s\n' "ForwardToSyslog=yes" >> "/etc/systemd/journald.conf"
-        tail -n "+$(( line_number ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" >> "${COMPONENT_PARAM_REMEDY_CFG}"
-    fi
-    # Clean up after ourselves.
-    rm "${COMPONENT_PARAM_REMEDY_CFG}.bak"
-}
+done
 
-remove_journald_ForwardToSyslog_configuration
-journald_ForwardToSyslog_add_configuration
+# if section not in any file, append section with key = value to FIRST file in files parameter
+if ! $found ; then
+    file=$(echo "/etc/systemd/journald.conf.d/complianceascode_hardening.conf /etc/systemd/journald.conf.d/*.conf /etc/systemd/journald.conf" | cut -f1 -d ' ')
+    mkdir -p "$(dirname "$file")"
+
+    echo -e "[Journal]\nForwardToSyslog=yes" >> "$file"
+
+fi
 
 else
     >&2 echo 'Remediation is not applicable, nothing was done'

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_journald_forward_to_syslog' differs.
--- xccdf_org.ssgproject.content_rule_journald_forward_to_syslog
+++ xccdf_org.ssgproject.content_rule_journald_forward_to_syslog
@@ -1,12 +1,17 @@
-- name: Check for duplicate ForwardToSyslog values in master journald configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf
-    create: false
-    regexp: ^\s*ForwardToSyslog=
-    state: absent
-  check_mode: true
-  changed_when: false
-  register: dupes_master
+- name: Ensure journald is configured to send logs to rsyslog - Search for a section
+    in files
+  ansible.builtin.find:
+    paths: '{{item.path}}'
+    patterns: '{{item.pattern}}'
+    contains: ^\s*\[Journal\]
+    read_whole_file: true
+    use_regex: true
+  register: systemd_dropin_files_with_section
+  loop:
+  - path: '{{ ''/etc/systemd/journald.conf'' | dirname }}'
+    pattern: '{{ ''/etc/systemd/journald.conf'' | basename | regex_escape }}'
+  - path: /etc/systemd/journald.conf.d
+    pattern: .*\.conf
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   tags:
   - CCE-85995-9
@@ -17,30 +22,11 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Deduplicate ForwardToSyslog values from journald master configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf
-    create: false
-    regexp: ^\s*ForwardToSyslog=
-    state: absent
-  when:
-  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
-  - dupes_master.found is defined and dupes_master.found > 1
-  tags:
-  - CCE-85995-9
-  - journald_forward_to_syslog
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - no_reboot_needed
-  - restrict_strategy
-
-- name: Collect all config journald files which configure ForwardToSyslog
-  ansible.builtin.find:
-    paths: /etc/systemd/journald.conf.d
-    contains: ^[\s]*ForwardToSyslog=.*$
-    patterns: '*.conf'
-  register: journald_ForwardToSyslog_dropin_config_files
+- name: Ensure journald is configured to send logs to rsyslog - Count number of files
+    which contain the correct section
+  ansible.builtin.set_fact:
+    count_of_systemd_dropin_files_with_section: '{{systemd_dropin_files_with_section.results
+      | map(attribute=''matched'') | list | map(''int'') | sum}}'
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   tags:
   - CCE-85995-9
@@ -51,14 +37,20 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Deduplicate values from journald ForwardToSyslog dropin configuration
-  ansible.builtin.lineinfile:
-    path: '{{ item.path }}'
-    create: false
-    regexp: ^\s*ForwardToSyslog=
-    state: absent
-  loop: '{{  journald_ForwardToSyslog_dropin_config_files.files }}'
-  when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+- name: Ensure journald is configured to send logs to rsyslog - Add missing configuration
+    to correct section
+  ini_file:
+    path: '{{item}}'
+    section: Journal
+    option: ForwardToSyslog
+    value: 'yes'
+    state: present
+    no_extra_spaces: true
+  when:
+  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  - count_of_systemd_dropin_files_with_section | int > 0
+  loop: '{{systemd_dropin_files_with_section.results | sum(attribute=''files'', start=[])
+    | map(attribute=''path'') | list }}'
   tags:
   - CCE-85995-9
   - journald_forward_to_syslog
@@ -68,16 +60,19 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Insert correct line to journald ForwardToSyslog configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf.d/oscap-remedy.conf
+- name: Ensure journald is configured to send logs to rsyslog - Add configuration
+    to new remediation file
+  ini_file:
+    path: /etc/systemd/journald.conf.d/complianceascode_hardening.conf
+    section: Journal
+    option: ForwardToSyslog
+    value: 'yes'
+    state: present
+    no_extra_spaces: true
     create: true
-    regexp: ^\s*ForwardToSyslog=
-    line: ForwardToSyslog=yes
-    state: present
-    insertbefore: ^# ForwardToSyslog
-    validate: bash -n %s
-  when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  when:
+  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  - count_of_systemd_dropin_files_with_section | int == 0
   tags:
   - CCE-85995-9
   - journald_forward_to_syslog

bash remediation for rule 'xccdf_org.ssgproject.content_rule_journald_storage' differs.
--- xccdf_org.ssgproject.content_rule_journald_storage
+++ xccdf_org.ssgproject.content_rule_journald_storage
@@ -1,46 +1,38 @@
 # Remediation is applicable only in certain platforms
 if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then
 
-function remove_journald_Storage_configuration {
-    local COMPONENT_PARAM_CONFIG
-    mapfile -t COMPONENT_PARAM_CONFIG < <(ls /etc/systemd/journald.conf.d/*.conf)
-    COMPONENT_PARAM_CONFIG+=("/etc/systemd/journald.conf")
+found=false
 
-    for f in "${COMPONENT_PARAM_CONFIG[@]}"
-    do
-        sed -i "/^\s*Storage\s*=\s*/d" "$f"
-        # make sure file has newline at the end
-        sed -i -e '$a\' "$f"
-    done
-    sed -i -e '$a\' "/etc/systemd/journald.conf"
-}
+# set value in all files if they contain section or key
+for f in $(echo -n "/etc/systemd/journald.conf.d/complianceascode_hardening.conf /etc/systemd/journald.conf.d/*.conf /etc/systemd/journald.conf"); do
+    if [ ! -e "$f" ]; then
+        continue
+    fi
 
-function journald_Storage_add_configuration {
-    local COMPONENT_PARAM_REMEDY_CFG
-    mkdir -p "/etc/systemd/journald.conf.d"
-    COMPONENT_PARAM_REMEDY_CFG="/etc/systemd/journald.conf.d/oscap-remedy.conf"
+    # find key in section and change value
+    if grep -qzosP "[[:space:]]*\[Journal\]([^\n\[]*\n+)+?[[:space:]]*Storage" "$f"; then
 
-    if [ ! -f "${COMPONENT_PARAM_REMEDY_CFG}" ] ; then
-        touch "${COMPONENT_PARAM_REMEDY_CFG}"
+            sed -i "s/Storage[^(\n)]*/Storage=persistent/" "$f"
+
+            found=true
+
+    # find section and add key = value to it
+    elif grep -qs "[[:space:]]*\[Journal\]" "$f"; then
+
+            sed -i "/[[:space:]]*\[Journal\]/a Storage=persistent" "$f"
+
+            found=true
     fi
-    cp "${COMPONENT_PARAM_REMEDY_CFG}" "${COMPONENT_PARAM_REMEDY_CFG}.bak"
-    # Insert before the line matching the regex '^#\s*Compress'.
-    line_number="$(LC_ALL=C grep -n "^#\s*Storage" "${COMPONENT_PARAM_REMEDY_CFG}.bak" | LC_ALL=C sed 's/:.*//g')"
-    if [ -z "$line_number" ]; then
-       # There was no match of '^#\s*Storage', insert at
-       # the end of the file.
-       printf '%s\n' "Storage=persistent" >> "${COMPONENT_PARAM_REMEDY_CFG}"
-    else
-        head -n "$(( line_number - 1 ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" > "${COMPONENT_PARAM_REMEDY_CFG}"
-        printf '%s\n' "Storage=persistent" >> "/etc/systemd/journald.conf"
-        tail -n "+$(( line_number ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" >> "${COMPONENT_PARAM_REMEDY_CFG}"
-    fi
-    # Clean up after ourselves.
-    rm "${COMPONENT_PARAM_REMEDY_CFG}.bak"
-}
+done
 
-remove_journald_Storage_configuration
-journald_Storage_add_configuration
+# if section not in any file, append section with key = value to FIRST file in files parameter
+if ! $found ; then
+    file=$(echo "/etc/systemd/journald.conf.d/complianceascode_hardening.conf /etc/systemd/journald.conf.d/*.conf /etc/systemd/journald.conf" | cut -f1 -d ' ')
+    mkdir -p "$(dirname "$file")"
+
+    echo -e "[Journal]\nStorage=persistent" >> "$file"
+
+fi
 
 else
     >&2 echo 'Remediation is not applicable, nothing was done'

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_journald_storage' differs.
--- xccdf_org.ssgproject.content_rule_journald_storage
+++ xccdf_org.ssgproject.content_rule_journald_storage
@@ -1,12 +1,17 @@
-- name: Check for duplicate Storage values in master journald configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf
-    create: false
-    regexp: ^\s*Storage=
-    state: absent
-  check_mode: true
-  changed_when: false
-  register: dupes_master
+- name: Ensure journald is configured to write log files to persistent disk - Search
+    for a section in files
+  ansible.builtin.find:
+    paths: '{{item.path}}'
+    patterns: '{{item.pattern}}'
+    contains: ^\s*\[Journal\]
+    read_whole_file: true
+    use_regex: true
+  register: systemd_dropin_files_with_section
+  loop:
+  - path: '{{ ''/etc/systemd/journald.conf'' | dirname }}'
+    pattern: '{{ ''/etc/systemd/journald.conf'' | basename | regex_escape }}'
+  - path: /etc/systemd/journald.conf.d
+    pattern: .*\.conf
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   tags:
   - CCE-86045-2
@@ -17,30 +22,11 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Deduplicate Storage values from journald master configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf
-    create: false
-    regexp: ^\s*Storage=
-    state: absent
-  when:
-  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
-  - dupes_master.found is defined and dupes_master.found > 1
-  tags:
-  - CCE-86045-2
-  - journald_storage
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - no_reboot_needed
-  - restrict_strategy
-
-- name: Collect all config journald files which configure Storage
-  ansible.builtin.find:
-    paths: /etc/systemd/journald.conf.d
-    contains: ^[\s]*Storage=.*$
-    patterns: '*.conf'
-  register: journald_Storage_dropin_config_files
+- name: Ensure journald is configured to write log files to persistent disk - Count
+    number of files which contain the correct section
+  ansible.builtin.set_fact:
+    count_of_systemd_dropin_files_with_section: '{{systemd_dropin_files_with_section.results
+      | map(attribute=''matched'') | list | map(''int'') | sum}}'
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   tags:
   - CCE-86045-2
@@ -51,14 +37,20 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Deduplicate values from journald Storage dropin configuration
-  ansible.builtin.lineinfile:
-    path: '{{ item.path }}'
-    create: false
-    regexp: ^\s*Storage=
-    state: absent
-  loop: '{{  journald_Storage_dropin_config_files.files }}'
-  when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+- name: Ensure journald is configured to write log files to persistent disk - Add
+    missing configuration to correct section
+  ini_file:
+    path: '{{item}}'
+    section: Journal
+    option: Storage
+    value: persistent
+    state: present
+    no_extra_spaces: true
+  when:
+  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  - count_of_systemd_dropin_files_with_section | int > 0
+  loop: '{{systemd_dropin_files_with_section.results | sum(attribute=''files'', start=[])
+    | map(attribute=''path'') | list }}'
   tags:
   - CCE-86045-2
   - journald_storage
@@ -68,16 +60,19 @@
   - no_reboot_needed
   - restrict_strategy
 
-- name: Insert correct line to journald Storage configuration
-  ansible.builtin.lineinfile:
-    path: /etc/systemd/journald.conf.d/oscap-remedy.conf
+- name: Ensure journald is configured to write log files to persistent disk - Add
+    configuration to new remediation file
+  ini_file:
+    path: /etc/systemd/journald.conf.d/complianceascode_hardening.conf
+    section: Journal
+    option: Storage
+    value: persistent
+    state: present
+    no_extra_spaces: true
     create: true
-    regexp: ^\s*Storage=
-    line: Storage=persistent
-    state: present
-    insertbefore: ^# Storage
-    validate: bash -n %s
-  when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  when:
+  - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+  - count_of_systemd_dropin_files_with_section | int == 0
   tags:
   - CCE-86045-2
   - journald_storage

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_certificate_verification' differs.
--- xccdf_org.ssgproject.content_rule_sssd_certificate_verification
+++ xccdf_org.ssgproject.content_rule_sssd_certificate_verification
@@ -21,12 +21,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[sssd\]([^\n\[]*\n+)+?[[:space:]]*certificate_verification" "$f"; then
-            sed -i "s/certificate_verification[^(\n)]*/certificate_verification = ocsp_dgst=$var_sssd_certificate_verification_digest_function/" "$f"
+
+            sed -i "s/certificate_verification[^(\n)]*/certificate_verification=ocsp_dgst=$var_sssd_certificate_verification_digest_function/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[sssd\]" "$f"; then
-            sed -i "/[[:space:]]*\[sssd\]/a certificate_verification = ocsp_dgst=$var_sssd_certificate_verification_digest_function" "$f"
+
+            sed -i "/[[:space:]]*\[sssd\]/a certificate_verification=ocsp_dgst=$var_sssd_certificate_verification_digest_function" "$f"
+
             found=true
     fi
 done
@@ -35,7 +39,9 @@
 if ! $found ; then
     file=$(echo "$MAIN_CONF /etc/sssd/sssd.conf /etc/sssd/conf.d/*.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[sssd]\ncertificate_verification = ocsp_dgst=$var_sssd_certificate_verification_digest_function" >> "$file"
+
+    echo -e "[sssd]\ncertificate_verification=ocsp_dgst=$var_sssd_certificate_verification_digest_function" >> "$file"
+
 fi
 
 umask $OLD_UMASK

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_enable_smartcards' differs.
--- xccdf_org.ssgproject.content_rule_sssd_enable_smartcards
+++ xccdf_org.ssgproject.content_rule_sssd_enable_smartcards
@@ -16,12 +16,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[pam\]([^\n\[]*\n+)+?[[:space:]]*pam_cert_auth" "$f"; then
-            sed -i "s/pam_cert_auth[^(\n)]*/pam_cert_auth = True/" "$f"
+
+            sed -i "s/pam_cert_auth[^(\n)]*/pam_cert_auth=True/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[pam\]" "$f"; then
-            sed -i "/[[:space:]]*\[pam\]/a pam_cert_auth = True" "$f"
+
+            sed -i "/[[:space:]]*\[pam\]/a pam_cert_auth=True" "$f"
+
             found=true
     fi
 done
@@ -30,7 +34,9 @@
 if ! $found ; then
     file=$(echo "/etc/sssd/sssd.conf /etc/sssd/conf.d/*.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[pam]\npam_cert_auth = True" >> "$file"
+
+    echo -e "[pam]\npam_cert_auth=True" >> "$file"
+
 fi
 
 umask $OLD_UMASK

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_memcache_timeout' differs.
--- xccdf_org.ssgproject.content_rule_sssd_memcache_timeout
+++ xccdf_org.ssgproject.content_rule_sssd_memcache_timeout
@@ -19,12 +19,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[nss\]([^\n\[]*\n+)+?[[:space:]]*memcache_timeout" "$f"; then
-            sed -i "s/memcache_timeout[^(\n)]*/memcache_timeout = $var_sssd_memcache_timeout/" "$f"
+
+            sed -i "s/memcache_timeout[^(\n)]*/memcache_timeout=$var_sssd_memcache_timeout/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[nss\]" "$f"; then
-            sed -i "/[[:space:]]*\[nss\]/a memcache_timeout = $var_sssd_memcache_timeout" "$f"
+
+            sed -i "/[[:space:]]*\[nss\]/a memcache_timeout=$var_sssd_memcache_timeout" "$f"
+
             found=true
     fi
 done
@@ -33,7 +37,9 @@
 if ! $found ; then
     file=$(echo "/etc/sssd/sssd.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[nss]\nmemcache_timeout = $var_sssd_memcache_timeout" >> "$file"
+
+    echo -e "[nss]\nmemcache_timeout=$var_sssd_memcache_timeout" >> "$file"
+
 fi
 
 umask $OLD_UMASK

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_offline_cred_expiration' differs.
--- xccdf_org.ssgproject.content_rule_sssd_offline_cred_expiration
+++ xccdf_org.ssgproject.content_rule_sssd_offline_cred_expiration
@@ -16,12 +16,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[pam\]([^\n\[]*\n+)+?[[:space:]]*offline_credentials_expiration" "$f"; then
-            sed -i "s/offline_credentials_expiration[^(\n)]*/offline_credentials_expiration = 1/" "$f"
+
+            sed -i "s/offline_credentials_expiration[^(\n)]*/offline_credentials_expiration=1/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[pam\]" "$f"; then
-            sed -i "/[[:space:]]*\[pam\]/a offline_credentials_expiration = 1" "$f"
+
+            sed -i "/[[:space:]]*\[pam\]/a offline_credentials_expiration=1" "$f"
+
             found=true
     fi
 done
@@ -30,7 +34,9 @@
 if ! $found ; then
     file=$(echo "/etc/sssd/sssd.conf /etc/sssd/conf.d/*.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[pam]\noffline_credentials_expiration = 1" >> "$file"
+
+    echo -e "[pam]\noffline_credentials_expiration=1" >> "$file"
+
 fi
 
 umask $OLD_UMASK

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_run_as_sssd_user' differs.
--- xccdf_org.ssgproject.content_rule_sssd_run_as_sssd_user
+++ xccdf_org.ssgproject.content_rule_sssd_run_as_sssd_user
@@ -18,12 +18,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[sssd\]([^\n\[]*\n+)+?[[:space:]]*user" "$f"; then
-            sed -i "s/user[^(\n)]*/user = sssd/" "$f"
+
+            sed -i "s/user[^(\n)]*/user=sssd/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[sssd\]" "$f"; then
-            sed -i "/[[:space:]]*\[sssd\]/a user = sssd" "$f"
+
+            sed -i "/[[:space:]]*\[sssd\]/a user=sssd" "$f"
+
             found=true
     fi
 done
@@ -32,7 +36,9 @@
 if ! $found ; then
     file=$(echo "$MAIN_CONF /etc/sssd/sssd.conf /etc/sssd/conf.d/*.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[sssd]\nuser = sssd" >> "$file"
+
+    echo -e "[sssd]\nuser=sssd" >> "$file"
+
 fi
 
 umask $OLD_UMASK

bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_ssh_known_hosts_timeout' differs.
--- xccdf_org.ssgproject.content_rule_sssd_ssh_known_hosts_timeout
+++ xccdf_org.ssgproject.content_rule_sssd_ssh_known_hosts_timeout
@@ -19,12 +19,16 @@
 
     # find key in section and change value
     if grep -qzosP "[[:space:]]*\[ssh\]([^\n\[]*\n+)+?[[:space:]]*ssh_known_hosts_timeout" "$f"; then
-            sed -i "s/ssh_known_hosts_timeout[^(\n)]*/ssh_known_hosts_timeout = $var_sssd_ssh_known_hosts_timeout/" "$f"
+
+            sed -i "s/ssh_known_hosts_timeout[^(\n)]*/ssh_known_hosts_timeout=$var_sssd_ssh_known_hosts_timeout/" "$f"
+
             found=true
 
     # find section and add key = value to it
     elif grep -qs "[[:space:]]*\[ssh\]" "$f"; then
-            sed -i "/[[:space:]]*\[ssh\]/a ssh_known_hosts_timeout = $var_sssd_ssh_known_hosts_timeout" "$f"
+
+            sed -i "/[[:space:]]*\[ssh\]/a ssh_known_hosts_timeout=$var_sssd_ssh_known_hosts_timeout" "$f"
+
             found=true
     fi
 done
@@ -33,7 +37,9 @@
 if ! $found ; then
     file=$(echo "/etc/sssd/sssd.conf" | cut -f1 -d ' ')
     mkdir -p "$(dirname "$file")"
-    echo -e "[ssh]\nssh_known_hosts_timeout = $var_sssd_ssh_known_hosts_timeout" >> "$file"
+
+    echo -e "[ssh]\nssh_known_hosts_timeout=$var_sssd_ssh_known_hosts_timeout" >> "$file"
+
 fi
 
 umask $OLD_UMASK

Copy link

github-actions bot commented Jun 10, 2024

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

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

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

@marcusburghardt marcusburghardt self-assigned this Jun 17, 2024
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.

For Ansible remediation, I believe you can use a combination of find and ini_file modules. You can also use the macro ansible_ini_file_set in the Playbook or create a new macro.

shared/macros/10-oval.jinja Show resolved Hide resolved
@vojtapolasek
Copy link
Collaborator Author

@marcusburghardt I updated the Ansible remediation.

@vojtapolasek vojtapolasek marked this pull request as ready for review July 9, 2024 08:21
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Jul 9, 2024
@vojtapolasek
Copy link
Collaborator Author

OK, it seems Jinja is not permitted to be used in "when" conditionals, I will modify the Ansible.

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 also tested the journald_storage rule locally and the remediation is failing with wrong_master.fail.sh test scenario. The cause is that the test scenario script duplicates the section and parameter in the file and the remediation can't deal with this. We should review if this test scenario script is incorrect or if this is a valid scenario that the remediation should deal with.

@@ -2097,7 +2097,7 @@ Example macro invocation(s)::
:type value: str

#}}
{{% macro bash_ensure_ini_config(files, section, key, value) -%}}
{{% macro bash_ensure_ini_config(files, section, key, value, no_quotes=true) -%}}
Copy link
Member

Choose a reason for hiding this comment

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

This no_quotes argument seems unnecessary since we can already determine if quotes are used or not by checking the value. It would simplify the template call. Asking the user to specify the value and no_quotes seems redundant. Could you treat this within the macro without including an additional parameter? Also, do you have any example where no_quotes would be false?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My idea behind this change was alignment with the OVAL check counterpart of this macro in the scope of systemd_dropin_configuration template; oval_check_dropin_file. This macro honors no_quotes. Therefore, I think the remediation should honor no_quotes as well. Moreover, the macro I modify here is more generic, it is an ini file check. There is no standard if values should be quoted or not (https://en.wikipedia.org/wiki/INI_file#Quoted_values).

Copy link
Member

Choose a reason for hiding this comment

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

It seems a good case for a new macro specific for systemd files, to treat the quotes and call the systemd_dropin_configuration macro. But I see your point regarding the alignment to OVAL.

The ideal approach, in my perspective, would be to remove this unnecessary parameter in the whole template and macros and treat these predictable cases within the macro. However this is not the scope of this PR, not a simple refactoring and can be done later.

Copy link
Member

Choose a reason for hiding this comment

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

By the way, the no_quotes is not even used in oval_check_dropin_file macro. It is used quotes, which is not declared. We should definitely review these macros soon.

Copy link
Member

@marcusburghardt marcusburghardt Jul 16, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @marcusburghardt , see proposed change to oval_check_dropin_file in #12173

@marcusburghardt marcusburghardt added the Ansible Ansible remediation update. label Jul 12, 2024
@marcusburghardt marcusburghardt added the Update Template Issues or pull requests related to Templates updates. label Jul 12, 2024
This file name is used in case no other existing file can be used as a target for the remediation.
The section has to be at the begining of line, indentation is permitted.
@vojtapolasek
Copy link
Collaborator Author

@marcusburghardt regarding the test which fails for you... What I did was that I modified test scenarios to overwrite the configuration file. I think this is standard.
I also inquired about the situation when there are multiple sections with the same name within one Systemd configuration file. This is valid, although probably not encouraged and I don't think we have to test for this case. Just fyi, sections with the same name are concatenated when interpreting the file.

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 tested all the rules using this template and everything is green. There is only an small update pending in the documentation, regarding the file name.

@marcusburghardt
Copy link
Member

INFO - xccdf_org.ssgproject.content_rule_journald_compress
INFO - Script wrong_dir.fail.sh using profile (all) OK
INFO - Script multiple_vals.fail.sh using profile (all) OK
INFO - Script correct_dir.pass.sh using profile (all) OK
INFO - Script correct_master.pass.sh using profile (all) OK
INFO - Script wrong_master.fail.sh using profile (all) OK
INFO - Script correct_value_in_quotes.fail.sh using profile (all) OK
INFO - xccdf_org.ssgproject.content_rule_journald_forward_to_syslog
INFO - Script wrong_dir.fail.sh using profile (all) OK
INFO - Script multiple_vals.fail.sh using profile (all) OK
INFO - Script correct_dir.pass.sh using profile (all) OK
INFO - Script correct_master.pass.sh using profile (all) OK
INFO - Script wrong_master.fail.sh using profile (all) OK
INFO - xccdf_org.ssgproject.content_rule_journald_storage
INFO - Script wrong_dir.fail.sh using profile (all) OK
INFO - Script multiple_vals.fail.sh using profile (all) OK
INFO - Script correct_dir.pass.sh using profile (all) OK
INFO - Script correct_master.pass.sh using profile (all) OK
INFO - Script wrong_master.fail.sh using profile (all) OK
INFO - Script correct_value_in_quotes.fail.sh using profile (all) OK

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
Copy link

codeclimate bot commented Jul 15, 2024

Code Climate has analyzed commit 4c5541b 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

/packit test

@marcusburghardt
Copy link
Member

The failing CI tests are optional and I confirmed the failures are not related to this PR so they can be safely waived in this specific PR.

@marcusburghardt marcusburghardt merged commit 8a8b6ba into ComplianceAsCode:master Jul 16, 2024
91 of 95 checks passed
teacup-on-rockingchair added a commit to teacup-on-rockingchair/content that referenced this pull request Jul 17, 2024
Handle correctly the no_quotes parameter of the macro, so it affects the quotes parameter of called oval_line_in_file_state macro
Thanks to @marcusburghardt and @vojtapolasek for their feedback on this in context of ComplianceAsCode#12054 🙇
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. Update Template Issues or pull requests related to Templates updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants