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

Rewrite remediations for rsyslog_remote_tls #9866

Conversation

vojtapolasek
Copy link
Collaborator

@vojtapolasek vojtapolasek commented Nov 23, 2022

Description:

  • add few tests to test also files in /etc/rsyslog.d
  • completely rewrite Bash and Ansible remediations
  • now remediations try to file the omfwd line in tact as much as possible. They first try to correct wrong config options and also add missing config options to the omfwd directive.
  • If no omfwd directive is found, they create a new one.

Rationale:

Fixes: #9621

@vojtapolasek vojtapolasek added the Ansible Ansible remediation update. label Nov 23, 2022
@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

github-actions bot commented Nov 23, 2022

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_rsyslog_remote_tls' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_remote_tls
+++ xccdf_org.ssgproject.content_rule_rsyslog_remote_tls
@@ -3,19 +3,23 @@
 
 rsyslog_remote_loghost_address=''
 
+params_to_add_if_missing=("protocol" "target" "port" "StreamDriver" "StreamDriverMode" "StreamDriverAuthMode" "streamdriver.CheckExtendedKeyPurpose")
+values_to_add_if_missing=("tcp" "$rsyslog_remote_loghost_address" "6514" "gtls" "1" "x509/name" "on")
+params_to_replace_if_wrong_value=("protocol" "StreamDriver" "StreamDriverMode" "StreamDriverAuthMode" "streamdriver.CheckExtendedKeyPurpose")
+values_to_replace_if_wrong_value=("tcp" "gtls" "1" "x509/name" "on")
 
-# Get omfwd configuration directive
-OMFWD_CONFIG_OUTPUT=`grep -Pzo '^(?s)action\s*\(\s*type\s*=\s*"omfwd".*\)' /etc/rsyslog.conf /etc/rsyslog.d/*.conf`
-OMFWD_CONFIG=`echo "$OMFWD_CONFIG_OUTPUT"| awk 'BEGIN {FS=":"; RS=")\n"}; {print $2}'`
-OMFWD_CONFIG_FILE=`echo "$OMFWD_CONFIG_OUTPUT"| awk 'BEGIN {FS=":"; RS=")\n"}; {print $1}'`
-if ! [ -z "$OMFWD_CONFIG" ]; then
- OMFWD_TLS_STREAM=`echo "$OMFWD_CONFIG"|grep 'StreamDriver="gtls"'`
- if ! [ -z "${OMFWD_TLS_STREAM}" ]; then
- exit 0
- else
- # insert TLS stream param
- sed -i 's/action\s*(\s*type\s*=\s*"omfwd"/action(type="omfwd"\ StreamDriver="gtls"\ /' $OMFWD_CONFIG_FILE
- fi
+files_containing_omfwd=("$(grep -ilE '^[^#]*\s*action\s*\(\s*type\s*=\s*"omfwd".*' /etc/rsyslog.conf /etc/rsyslog.d/*.conf)")
+if [ -n "${files_containing_omfwd[*]}" ]; then
+ for file in "${files_containing_omfwd[@]}"; do
+ for ((i=0; i<${#params_to_replace_if_wrong_value[@]}; i++)); do
+ sed -i -E -e 'H;$!d;x;s/^\n//' -e "s|(\s*action\s*\(\s*type\s*=\s*[\"]omfwd[\"].*?)${params_to_replace_if_wrong_value[$i]}\s*=\s*[\"]\S*[\"](.*\))|\1${params_to_replace_if_wrong_value[$i]}=\"${values_to_replace_if_wrong_value[$i]}\"\2|gI" "$file"
+ done
+ for ((i=0; i<${#params_to_add_if_missing[@]}; i++)); do
+ if ! grep -qPzi "(?s)\s*action\s*\(\s*type\s*=\s*[\"]omfwd[\"].*?${params_to_add_if_missing[$i]}.*?\).*" "$file"; then
+ sed -i -E -e 'H;$!d;x;s/^\n//' -e "s|(\s*action\s*\(\s*type\s*=\s*[\"]omfwd[\"])|\1\n${params_to_add_if_missing[$i]}=\"${values_to_add_if_missing[$i]}\"|gI" "$file"
+ fi
+ done
+ done
 else
 echo "action(type=\"omfwd\" protocol=\"tcp\" Target=\"$rsyslog_remote_loghost_address\" port=\"6514\" StreamDriver=\"gtls\" StreamDriverMode=\"1\" StreamDriverAuthMode=\"x509/name\" streamdriver.CheckExtendedKeyPurpose=\"on\")" >> /etc/rsyslog.conf
 fi

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_rsyslog_remote_tls' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_remote_tls
+++ xccdf_org.ssgproject.content_rule_rsyslog_remote_tls
@@ -4,42 +4,188 @@
 tags:
 - always
 
-- name: Get omfwd configuration directive
- shell: sed -e '/^action\s*(\s*type\s*=\s*"omfwd"/,/)/!d' /etc/rsyslog.conf /etc/rsyslog.d/*.conf
- || true
- register: include_omfwd_config_output
- when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
- tags:
- - CCE-82457-3
- - NIST-800-53-AU-9(3)
- - NIST-800-53-CM-6(a)
- - configure_strategy
- - low_complexity
- - low_disruption
- - medium_severity
- - no_reboot_needed
- - rsyslog_remote_tls
-
-- name: Get include files directives
- shell: |
- set -o pipefail echo \"{{ include_omfwd_config_output.stdout }}\"|grep 'StreamDriver=\"gtls\"'
- register: include_omfwd_gtls_config_output
+- name: 'Configure TLS for rsyslog remote logging: search for omfwd action directive
+ in rsyslog include files'
+ ansible.builtin.find:
+ paths: /etc/rsyslog.d/
+ pattern: '*.conf'
+ contains: ^\s*action\s*\(\s*type\s*=\s*"omfwd".*
+ register: rsyslog_includes_with_directive
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: search for omfwd action directive
+ in rsyslog main config file'
+ ansible.builtin.find:
+ paths: /etc
+ pattern: rsyslog.conf
+ contains: ^\s*action\s*\(\s*type\s*=\s*"omfwd".*
+ register: rsyslog_main_file_with_directive
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: declare Rsyslog option parameters
+ to be inserted if entirely missing'
+ ansible.builtin.set_fact:
+ rsyslog_parameters_to_add_if_missing:
+ - protocol
+ - target
+ - port
+ - StreamDriver
+ - StreamDriverMode
+ - StreamDriverAuthMode
+ - streamdriver.CheckExtendedKeyPurpose
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: declare Rsyslog option values to
+ be inserted if entirely missing'
+ ansible.builtin.set_fact:
+ rsyslog_values_to_add_if_missing:
+ - tcp
+ - '{{ rsyslog_remote_loghost_address }}'
+ - '6514'
+ - gtls
+ - '1'
+ - x509/name
+ - 'on'
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: declare Rsyslog option parameters
+ to be replaced if defined with wrong values'
+ ansible.builtin.set_fact:
+ rsyslog_parameters_to_replace_if_wrong_value:
+ - protocol
+ - StreamDriver
+ - StreamDriverMode
+ - StreamDriverAuthMode
+ - streamdriver.CheckExtendedKeyPurpose
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: declare Rsyslog option values to
+ be replaced when having wrong value'
+ ansible.builtin.set_fact:
+ rsyslog_values_to_replace_if_wrong_value:
+ - tcp
+ - gtls
+ - '1'
+ - x509/name
+ - 'on'
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: assemble list of files with existing
+ directives'
+ ansible.builtin.set_fact:
+ rsyslog_files: '{{ rsyslog_includes_with_directive.files | map(attribute=''path'')
+ | list + rsyslog_main_file_with_directive.files | map(attribute=''path'') |
+ list }}'
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: try to fix existing directives'
+ block:
+
+ - name: 'Configure TLS for rsyslog remote logging: Fix existing omfwd directives
+ by adjusting the value'
+ ansible.builtin.replace:
+ path: '{{ item[0] }}'
+ regexp: (?i)^(\s*action\s*\(\s*type\s*=\s*"omfwd"[\s\S]*)({{ item[1][0] | regex_escape()
+ }}\s*=\s*"\S*")([\s\S]*\))$
+ replace: \1{{ item[1][0] }}="{{ item[1][1] }}"\3
+ loop: '{{ rsyslog_files | product (rsyslog_parameters_to_replace_if_wrong_value
+ | zip(rsyslog_values_to_replace_if_wrong_value)) | list }}'
+
+ - name: 'Configure TLS for rsyslog remote logging: Fix existing omfwd directives
+ by adding parameter and value'
+ ansible.builtin.replace:
+ path: '{{ item[0] }}'
+ regexp: (?i)^(\s*action\s*\(\s*type\s*=\s*"omfwd"(?:[\s\S](?!{{ item[1][0] |
+ regex_escape() }}))*.)(\))$
+ replace: \1 {{ item[1][0] }}="{{ item[1][1] }}" \2
+ loop: '{{ rsyslog_files | product (rsyslog_parameters_to_add_if_missing | zip(rsyslog_values_to_add_if_missing))
+ | list }}'
 when:
 - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
- - (include_omfwd_config_output.stdout_lines| length > 0)
- tags:
- - CCE-82457-3
- - NIST-800-53-AU-9(3)
- - NIST-800-53-CM-6(a)
- - configure_strategy
- - low_complexity
- - low_disruption
- - medium_severity
- - no_reboot_needed
- - rsyslog_remote_tls
-
-- name: Set rsyslog omfwd to use TLS
- lineinfile:
+ - rsyslog_includes_with_directive.matched or rsyslog_main_file_with_directive.matched
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls
+
+- name: 'Configure TLS for rsyslog remote logging: Add missing rsyslog directive'
+ ansible.builtin.lineinfile:
 dest: /etc/rsyslog.conf
 line: action(type="omfwd" protocol="tcp" Target="{{ rsyslog_remote_loghost_address
 }}" port="6514" StreamDriver="gtls" StreamDriverMode="1" StreamDriverAuthMode="x509/name"
@@ -47,14 +193,14 @@
 create: true
 when:
 - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
- - (include_omfwd_gtls_config_output is skipped ) or ("gtls" not in include_omfwd_gtls_config_output.stdout)
- tags:
- - CCE-82457-3
- - NIST-800-53-AU-9(3)
- - NIST-800-53-CM-6(a)
- - configure_strategy
- - low_complexity
- - low_disruption
- - medium_severity
- - no_reboot_needed
- - rsyslog_remote_tls
+ - not rsyslog_includes_with_directive.matched and not rsyslog_main_file_with_directive.matched
+ tags:
+ - CCE-82457-3
+ - NIST-800-53-AU-9(3)
+ - NIST-800-53-CM-6(a)
+ - configure_strategy
+ - low_complexity
+ - low_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_remote_tls

@vojtapolasek vojtapolasek force-pushed the rsyslog_remote_tls_ansible_remove_shell branch from 5f6502f to 078ff71 Compare November 23, 2022 12:20
@marcusburghardt marcusburghardt self-assigned this Nov 23, 2022
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.

The remediation is working fine after the changes.
Could you use the complete name for the modules, please?
They are all part of the ansible.builtin collection.

@@ -0,0 +1,12 @@
#!/bin/bash
# remediation = none
Copy link
Member

Choose a reason for hiding this comment

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

I saw most of the failing test scenarios are not remediating because of the # remediation = none header.
Is that intentional? Why the remediation is not executed in these cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure why it is like that. It has been proposed by @jan-cerny in #4788
Jan, do you remember what was the reason to disable remediations during some test scenarios?

Copy link
Member

Choose a reason for hiding this comment

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

@jan-cerny , do you remember the context where it was decided to not remediate in some test scenarios? I would be fine to treat this in a separate PR/issue, but it would be good to have this context so we can actually track it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that it was to prevent the test errors because at that moment there was no remediation for this rule, because these scenarios were introduced in 2019 but the remediation was introduced in 2022.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @jan-cerny . It makes sense. It also makes sense to remove the header now. @vojtapolasek , are you comfortable to update this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, yes, I can update that, but it will probably require more thorough update (maybe rewrite) of remediations. But that's good.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks.

@marcusburghardt marcusburghardt added this to the 0.1.66 milestone Nov 29, 2022
@marcusburghardt
Copy link
Member

/packit retest-failed

@marcusburghardt
Copy link
Member

/packit build

@marcusburghardt
Copy link
Member

/packit retest-failed

@vojtapolasek
Copy link
Collaborator Author

I am still working on this. I need to rewrite remediations to be able to remediate multiline expressions.

@marcusburghardt
Copy link
Member

I am still working on this. I need to rewrite remediations to be able to remediate multiline expressions.

Thanks for the update @vojtapolasek

@marcusburghardt
Copy link
Member

I am still working on this. I need to rewrite remediations to be able to remediate multiline expressions.

@vojtapolasek , could you move this PR to Draft since you are still working on it, please?

@vojtapolasek vojtapolasek marked this pull request as draft January 3, 2023 12:59
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Jan 3, 2023
@vojtapolasek vojtapolasek changed the title do not use shell commands in Ansible remediation of rsyslog_remote_tls WIP: do not use shell commands in Ansible remediation of rsyslog_remote_tls Jan 3, 2023
@vojtapolasek vojtapolasek force-pushed the rsyslog_remote_tls_ansible_remove_shell branch from 6c54f6c to ac512a9 Compare January 6, 2023 14:00
@vojtapolasek vojtapolasek changed the title WIP: do not use shell commands in Ansible remediation of rsyslog_remote_tls Rewrite remediations for rsyslog_remote_tls Jan 6, 2023
@vojtapolasek vojtapolasek marked this pull request as ready for review January 6, 2023 14:04
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Jan 6, 2023
Now the remediation checks first for /etc/rsyslog.conf and /etc/rsyslog.d/*.conf files
it tries to find an already existing directive and fix it if it has only the gtls option missing
in case it finds such a directive, it fixes it
if no "fixable" directive is found, the new correct directive is added to /etc/rsyslog.conf
the remediation tries to keep original config line in tact if possible
the remediation is on par with ansible. It tries to fix already entered values or add missing values before dumping new omfwd directive.
This is an attempt to preserve user configuration.

Note that here sed is used to read whole file and search for patterns. The magic behind it is explained here:
https://unix.stackexchange.com/a/669938
@vojtapolasek vojtapolasek force-pushed the rsyslog_remote_tls_ansible_remove_shell branch from ac512a9 to ec54d74 Compare January 6, 2023 14:16
@vojtapolasek
Copy link
Collaborator Author

@marcusburghardt I consider this read to be reviewed.

@marcusburghardt
Copy link
Member

@marcusburghardt I consider this read to be reviewed.

Great @vojtapolasek . I should manage to review it on Monday. Thanks

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.

Testes are working but there are some small adjustments for the Ansible remediation.

vojtapolasek and others added 2 commits January 9, 2023 14:41
Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
@vojtapolasek
Copy link
Collaborator Author

I think I fixed problems you pointed out.

@codeclimate
Copy link

codeclimate bot commented Jan 9, 2023

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

View more on Code Climate.

@marcusburghardt
Copy link
Member

/packit retest-failed

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 @vojtapolasek . Good job!

@marcusburghardt marcusburghardt merged commit 3ef431b into ComplianceAsCode:master Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ansible Ansible remediation update.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansible remediation of rsyslog_remote_tls contains unnecessary shell commands
3 participants