-
Notifications
You must be signed in to change notification settings - Fork 717
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
Extends rsyslog_logfiles_attributes_modify template for permissions #10139
Extends rsyslog_logfiles_attributes_modify template for permissions #10139
Conversation
The main differences among the test scenarios are related to the legacy and RainerScript syntax. Therefore, they were segmented in three prefixes: legacy, rainer and mixed. This should make them easier to review and maintain.
Aligned the Ansible remediation to the Bash, which was already working as expected when the include statement was declared in multilines.
The rule will use the rsyslog_logfiles_attributes_modify template which already covers the necessary test scenarios for this rule. One single test scenario script was created in the rule level to test stricter permissions, something not valid for owner and group-owner rules also covered by the template.
The rsyslog_files_permissions rule is now using the rsyslog_logfiles_attributes_modify template.
Minor extension in the regex to match rsyslog log files. Test scenarions relealed a case where log files defined in log lines which uses brackets where not matching. Ensured the match for this case and reviewed the respective test scenario.
FYI @dodys and @teacup-on-rockingchair |
This datastream diff is auto generated by the check Click here to see the full diffbash remediation for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_groupownership' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_files_groupownership
+++ xccdf_org.ssgproject.content_rule_rsyslog_files_groupownership
@@ -49,7 +49,8 @@
# * Strip quotes and closing brackets from paths.
# * Ignore paths that match /dev|/etc.*\.conf, as those are paths, but likely not log files
# * From the remaining valid rows select only fields constituting a log file path
- # Text file column is understood to represent a log file path if and only if all of the following are met:
+ # Text file column is understood to represent a log file path if and only if all of the
+ # following are met:
# * it contains at least one slash '/' character,
# * it is preceded by space
# * it doesn't contain space (' '), colon (':'), and semicolon (';') characters
@@ -61,8 +62,8 @@
FILTERED_PATHS=$(awk '{if(NF>=2&&($NF~/^\//||$NF~/^-\//)){sub(/^-\//,"/",$NF);print $NF}}' <<< "${LINES_WITH_PATHS}")
CLEANED_PATHS=$(sed -e "s/[\"')]//g; /\\/etc.*\.conf/d; /\\/dev\\//d" <<< "${FILTERED_PATHS}")
MATCHED_ITEMS=$(sed -e "/^$/d" <<< "${CLEANED_PATHS}")
- # Since above sed command might return more than one item (delimited by newline), split the particular
- # matches entries into new array specific for this log file
+ # Since above sed command might return more than one item (delimited by newline), split
+ # the particular matches entries into new array specific for this log file
readarray -t ARRAY_FOR_LOG_FILE <<< "$MATCHED_ITEMS"
# Concatenate the two arrays - previous content of $LOG_FILE_PATHS array with
# items from newly created array for this log file
@@ -72,7 +73,8 @@
fi
done
-# Check for RainerScript action log format which might be also multiline so grep regex is a bit curly
+# Check for RainerScript action log format which might be also multiline so grep regex is a bit
+# curly:
# extract possibly multiline action omfile expressions
# extract File="logfile" expression
# match only "logfile" expression
@@ -83,22 +85,8 @@
LOG_FILE_PATHS+=("$(echo "${OMFILE_LINES}"| grep -oE "\"([/[:alnum:][:punct:]]*)\""|tr -d "\"")")
done
-FILE_PARAM="groupowner"
-FILE_CMD=""
-case "$FILE_PARAM" in
- "groupowner")
- FILE_CMD=$(which chgrp)
- ;;
- "owner")
- FILE_CMD=$(which chown)
- ;;
- *)
- echo -n "Not supported file attribute! "
- exit 1
- ;;
-esac
-
-# Correct the form o
+# Ensure the correct attribute if file exists
+FILE_CMD="chgrp"
for LOG_FILE_PATH in "${LOG_FILE_PATHS[@]}"
do
# Sanity check - if particular $LOG_FILE_PATH is empty string, skip it from further processing
@@ -106,8 +94,7 @@
then
continue
fi
-
- $FILE_CMD "+0" "$LOG_FILE_PATH"
+ $FILE_CMD "0" "$LOG_FILE_PATH"
done
else
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_groupownership' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_files_groupownership
+++ xccdf_org.ssgproject.content_rule_rsyslog_files_groupownership
@@ -39,7 +39,7 @@
- name: Ensure Log Files Are Owned By Appropriate Group - Get include files directives
ansible.builtin.shell: |
set -o pipefail
- grep -oP '^\s*include\s*\(\s*file.*' {{ rsyslog_etc_config }} |cut -d"\"" -f 2 || true
+ awk '/)/{f=0} /include\(/{f=1} f{nf=gensub("^(include\\(|\\s*)file=\"(\\S+)\".*","\\2",1); if($0!=nf){print nf}}' {{ rsyslog_etc_config }} || true
register: rsyslog_new_inc
changed_when: false
when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
@@ -162,8 +162,7 @@
- name: Ensure Log Files Are Owned By Appropriate Group -Setup log files attribute
ansible.builtin.file:
path: '{{ item }}'
- owner: '{{ ( "groupowner" is match("owner")) | ternary(0, omit) }}'
- group: '{{ ( "groupowner" is match("groupowner")) | ternary(0 , omit) }}'
+ group: 0
state: file
loop: '{{ log_files | list | flatten | unique }}'
failed_when: false
bash remediation for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_ownership' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_files_ownership
+++ xccdf_org.ssgproject.content_rule_rsyslog_files_ownership
@@ -49,7 +49,8 @@
# * Strip quotes and closing brackets from paths.
# * Ignore paths that match /dev|/etc.*\.conf, as those are paths, but likely not log files
# * From the remaining valid rows select only fields constituting a log file path
- # Text file column is understood to represent a log file path if and only if all of the following are met:
+ # Text file column is understood to represent a log file path if and only if all of the
+ # following are met:
# * it contains at least one slash '/' character,
# * it is preceded by space
# * it doesn't contain space (' '), colon (':'), and semicolon (';') characters
@@ -61,8 +62,8 @@
FILTERED_PATHS=$(awk '{if(NF>=2&&($NF~/^\//||$NF~/^-\//)){sub(/^-\//,"/",$NF);print $NF}}' <<< "${LINES_WITH_PATHS}")
CLEANED_PATHS=$(sed -e "s/[\"')]//g; /\\/etc.*\.conf/d; /\\/dev\\//d" <<< "${FILTERED_PATHS}")
MATCHED_ITEMS=$(sed -e "/^$/d" <<< "${CLEANED_PATHS}")
- # Since above sed command might return more than one item (delimited by newline), split the particular
- # matches entries into new array specific for this log file
+ # Since above sed command might return more than one item (delimited by newline), split
+ # the particular matches entries into new array specific for this log file
readarray -t ARRAY_FOR_LOG_FILE <<< "$MATCHED_ITEMS"
# Concatenate the two arrays - previous content of $LOG_FILE_PATHS array with
# items from newly created array for this log file
@@ -72,7 +73,8 @@
fi
done
-# Check for RainerScript action log format which might be also multiline so grep regex is a bit curly
+# Check for RainerScript action log format which might be also multiline so grep regex is a bit
+# curly:
# extract possibly multiline action omfile expressions
# extract File="logfile" expression
# match only "logfile" expression
@@ -83,22 +85,8 @@
LOG_FILE_PATHS+=("$(echo "${OMFILE_LINES}"| grep -oE "\"([/[:alnum:][:punct:]]*)\""|tr -d "\"")")
done
-FILE_PARAM="owner"
-FILE_CMD=""
-case "$FILE_PARAM" in
- "groupowner")
- FILE_CMD=$(which chgrp)
- ;;
- "owner")
- FILE_CMD=$(which chown)
- ;;
- *)
- echo -n "Not supported file attribute! "
- exit 1
- ;;
-esac
-
-# Correct the form o
+# Ensure the correct attribute if file exists
+FILE_CMD="chown"
for LOG_FILE_PATH in "${LOG_FILE_PATHS[@]}"
do
# Sanity check - if particular $LOG_FILE_PATH is empty string, skip it from further processing
@@ -106,8 +94,7 @@
then
continue
fi
-
- $FILE_CMD "+0" "$LOG_FILE_PATH"
+ $FILE_CMD "0" "$LOG_FILE_PATH"
done
else
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_ownership' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_files_ownership
+++ xccdf_org.ssgproject.content_rule_rsyslog_files_ownership
@@ -39,7 +39,7 @@
- name: Ensure Log Files Are Owned By Appropriate User - Get include files directives
ansible.builtin.shell: |
set -o pipefail
- grep -oP '^\s*include\s*\(\s*file.*' {{ rsyslog_etc_config }} |cut -d"\"" -f 2 || true
+ awk '/)/{f=0} /include\(/{f=1} f{nf=gensub("^(include\\(|\\s*)file=\"(\\S+)\".*","\\2",1); if($0!=nf){print nf}}' {{ rsyslog_etc_config }} || true
register: rsyslog_new_inc
changed_when: false
when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
@@ -162,8 +162,7 @@
- name: Ensure Log Files Are Owned By Appropriate User -Setup log files attribute
ansible.builtin.file:
path: '{{ item }}'
- owner: '{{ ( "owner" is match("owner")) | ternary(0, omit) }}'
- group: '{{ ( "owner" is match("groupowner")) | ternary(0 , omit) }}'
+ owner: 0
state: file
loop: '{{ log_files | list | flatten | unique }}'
failed_when: false
New content has different text for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_permissions'.
--- xccdf_org.ssgproject.content_rule_rsyslog_files_permissions
+++ xccdf_org.ssgproject.content_rule_rsyslog_files_permissions
@@ -12,7 +12,7 @@
$ ls -l LOGFILE
If the permissions are not 600 or more restrictive, run the following
command to correct this:
-$ sudo chmod 0600 LOGFILE"
+$ sudo chmod 600 LOGFILE"
[reference]:
BP28(R36)
bash remediation for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_permissions' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_files_permissions
+++ xccdf_org.ssgproject.content_rule_rsyslog_files_permissions
@@ -49,7 +49,8 @@
# * Strip quotes and closing brackets from paths.
# * Ignore paths that match /dev|/etc.*\.conf, as those are paths, but likely not log files
# * From the remaining valid rows select only fields constituting a log file path
- # Text file column is understood to represent a log file path if and only if all of the following are met:
+ # Text file column is understood to represent a log file path if and only if all of the
+ # following are met:
# * it contains at least one slash '/' character,
# * it is preceded by space
# * it doesn't contain space (' '), colon (':'), and semicolon (';') characters
@@ -61,8 +62,8 @@
FILTERED_PATHS=$(awk '{if(NF>=2&&($NF~/^\//||$NF~/^-\//)){sub(/^-\//,"/",$NF);print $NF}}' <<< "${LINES_WITH_PATHS}")
CLEANED_PATHS=$(sed -e "s/[\"')]//g; /\\/etc.*\.conf/d; /\\/dev\\//d" <<< "${FILTERED_PATHS}")
MATCHED_ITEMS=$(sed -e "/^$/d" <<< "${CLEANED_PATHS}")
- # Since above sed command might return more than one item (delimited by newline), split the particular
- # matches entries into new array specific for this log file
+ # Since above sed command might return more than one item (delimited by newline), split
+ # the particular matches entries into new array specific for this log file
readarray -t ARRAY_FOR_LOG_FILE <<< "$MATCHED_ITEMS"
# Concatenate the two arrays - previous content of $LOG_FILE_PATHS array with
# items from newly created array for this log file
@@ -72,9 +73,20 @@
fi
done
-DESIRED_PERM_MOD=600
+# Check for RainerScript action log format which might be also multiline so grep regex is a bit
+# curly:
+# extract possibly multiline action omfile expressions
+# extract File="logfile" expression
+# match only "logfile" expression
+for LOG_FILE in "${RSYSLOG_CONFIG_FILES[@]}"
+do
+ ACTION_OMFILE_LINES=$(grep -ozP "action\s*\(\s*type\s*=\s*\"omfile\"[^\)]*\)" "${LOG_FILE}")
+ OMFILE_LINES=$(echo "${ACTION_OMFILE_LINES}"| grep -aoP "File\s*=\s*\"([/[:alnum:][:punct:]]*)\"\s*\)")
+ LOG_FILE_PATHS+=("$(echo "${OMFILE_LINES}"| grep -oE "\"([/[:alnum:][:punct:]]*)\""|tr -d "\"")")
+done
-# Correct the form o
+# Ensure the correct attribute if file exists
+FILE_CMD="chmod"
for LOG_FILE_PATH in "${LOG_FILE_PATHS[@]}"
do
# Sanity check - if particular $LOG_FILE_PATH is empty string, skip it from further processing
@@ -82,12 +94,7 @@
then
continue
fi
-
- # Also for each log file check if its permissions differ from 600. If so, correct them
- if [ -f "$LOG_FILE_PATH" ] && [ "$(/usr/bin/stat -c %a "$LOG_FILE_PATH")" -ne $DESIRED_PERM_MOD ]
- then
- /bin/chmod $DESIRED_PERM_MOD "$LOG_FILE_PATH"
- fi
+ $FILE_CMD "0600" "$LOG_FILE_PATH"
done
else
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_rsyslog_files_permissions' differs.
--- xccdf_org.ssgproject.content_rule_rsyslog_files_permissions
+++ xccdf_org.ssgproject.content_rule_rsyslog_files_permissions
@@ -1,7 +1,7 @@
-- name: Set rsyslog logfile configuration facts
- set_fact:
+- name: Ensure System Log Files Have Correct Permissions - Set rsyslog logfile configuration
+ facts
+ ansible.builtin.set_fact:
rsyslog_etc_config: /etc/rsyslog.conf
- desired_perm_mode: '600'
when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
tags:
- CCE-80862-6
@@ -16,8 +16,8 @@
- no_reboot_needed
- rsyslog_files_permissions
-- name: Get IncludeConfig directive
- shell: |
+- name: Ensure System Log Files Have Correct Permissions - Get IncludeConfig directive
+ ansible.builtin.shell: |
set -o pipefail
grep -e '$IncludeConfig' {{ rsyslog_etc_config }} | cut -d ' ' -f 2 || true
register: rsyslog_old_inc
@@ -36,10 +36,10 @@
- no_reboot_needed
- rsyslog_files_permissions
-- name: Get include files directives
- shell: |
+- name: Ensure System Log Files Have Correct Permissions - Get include files directives
+ ansible.builtin.shell: |
set -o pipefail
- grep -oP '^\s*include\s*\(\s*file.*' {{ rsyslog_etc_config }} |cut -d"\"" -f 2 || true
+ awk '/)/{f=0} /include\(/{f=1} f{nf=gensub("^(include\\(|\\s*)file=\"(\\S+)\".*","\\2",1); if($0!=nf){print nf}}' {{ rsyslog_etc_config }} || true
register: rsyslog_new_inc
changed_when: false
when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
@@ -56,12 +56,10 @@
- no_reboot_needed
- rsyslog_files_permissions
-- name: Expand glob expressions
- shell: |
- set -o pipefail
- eval printf '%s\\n' {{ item }}
- register: include_config_output
- loop: '{{ rsyslog_old_inc.stdout_lines + rsyslog_new_inc.stdout_lines }}'
+- name: Ensure System Log Files Have Correct Permissions - Aggregate rsyslog includes
+ ansible.builtin.set_fact:
+ include_config_output: '{{ rsyslog_old_inc.stdout_lines + rsyslog_new_inc.stdout_lines
+ }}'
when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
tags:
- CCE-80862-6
@@ -76,10 +74,12 @@
- no_reboot_needed
- rsyslog_files_permissions
-- name: List all config files
- shell: find {{ item }} -not -path "*/.*" -type f
- loop: '{{ include_config_output.results|map(attribute=''stdout_lines'')|list|flatten
- }}'
+- name: Ensure System Log Files Have Correct Permissions - List all config files
+ ansible.builtin.find:
+ paths: '{{ include_config_output | list | map(''dirname'') }}'
+ patterns: '{{ include_config_output | list | map(''basename'') }}'
+ hidden: false
+ follow: true
register: rsyslog_config_files
failed_when: false
changed_when: false
@@ -97,13 +97,13 @@
- no_reboot_needed
- rsyslog_files_permissions
-- name: Extract log files
- shell: |
+- name: Ensure System Log Files Have Correct Permissions - Extract log files old format
+ ansible.builtin.shell: |
set -o pipefail
grep -oP '^[^(\s|#|\$)]+[\s]+.*[\s]+-?(/+[^:;\s]+);*\.*$' {{ item }} |awk '{print $NF}'|sed -e 's/^-//' || true
- loop: '{{ rsyslog_config_files.results|map(attribute=''stdout_lines'')|list|flatten|unique
+ loop: '{{ rsyslog_config_files.files|map(attribute=''path'')|list|flatten|unique
+ [ rsyslog_etc_config ] }}'
- register: log_files
+ register: log_files_old
changed_when: false
when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
tags:
@@ -119,13 +119,14 @@
- no_reboot_needed
- rsyslog_files_permissions
-- name: Setup log files permissions
- ignore_errors: true
- file:
- path: '{{ item }}'
- mode: '{{ desired_perm_mode }}'
- loop: '{{ log_files.results|map(attribute=''stdout_lines'')|list|flatten|unique
- }}'
+- name: Ensure System Log Files Have Correct Permissions - Extract log files new format
+ ansible.builtin.shell: |
+ set -o pipefail
+ grep -ozP "action\s*\(\s*type\s*=\s*\"omfile\"[^\)]*\)" {{ item }} | grep -aoP "File\s*=\s*\"([/[:alnum:][:punct:]]*)\"\s*\)"|grep -oE "\"([/[:alnum:][:punct:]]*)\"" |tr -d "\""|| true
+ loop: '{{ rsyslog_config_files.files|map(attribute=''path'')|list|flatten|unique
+ + [ rsyslog_etc_config ] }}'
+ register: log_files_new
+ changed_when: false
when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
tags:
- CCE-80862-6
@@ -139,3 +140,42 @@
- medium_severity
- no_reboot_needed
- rsyslog_files_permissions
+
+- name: Ensure System Log Files Have Correct Permissions - Sum all log files found
+ ansible.builtin.set_fact:
+ log_files: '{{ log_files_new.results|map(attribute=''stdout_lines'')|list|flatten|unique
+ + log_files_old.results|map(attribute=''stdout_lines'')|list|flatten|unique }}'
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-80862-6
+ - NIST-800-53-AC-6(1)
+ - NIST-800-53-CM-6(a)
+ - PCI-DSS-Req-10.5.1
+ - PCI-DSS-Req-10.5.2
+ - configure_strategy
+ - low_complexity
+ - medium_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_files_permissions
+
+- name: Ensure System Log Files Have Correct Permissions -Setup log files attribute
+ ansible.builtin.file:
+ path: '{{ item }}'
+ mode: 384
+ state: file
+ loop: '{{ log_files | list | flatten | unique }}'
+ failed_when: false
+ when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ tags:
+ - CCE-80862-6
+ - NIST-800-53-AC-6(1)
+ - NIST-800-53-CM-6(a)
+ - PCI-DSS-Req-10.5.1
+ - PCI-DSS-Req-10.5.2
+ - configure_strategy
+ - low_complexity
+ - medium_disruption
+ - medium_severity
+ - no_reboot_needed
+ - rsyslog_files_permissions |
|
While the which command is common in most of the systems, it might not be installed out-of-box in some environments. On the other hand, chown, chgrp and chmod are more likely to be present. The bash remediation already uses some commands without the absolut path, so it was also removed for the three commands previously mentioned.
In any case, I fixed this in the last commit. |
/retest |
/retest |
shared/templates/rsyslog_logfiles_attributes_modify/bash.template
Outdated
Show resolved
Hide resolved
shared/templates/rsyslog_logfiles_attributes_modify/ansible.template
Outdated
Show resolved
Hide resolved
...uide/system/logging/ensure_rsyslog_log_file_configuration/rsyslog_files_permissions/rule.yml
Show resolved
Hide resolved
shared/templates/rsyslog_logfiles_attributes_modify/tests/legafy_incorrect_attr.fail.sh
Outdated
Show resolved
Hide resolved
ATTR_VALUE="root" | ||
{{% else %}} | ||
CHATTR="chmod" | ||
ATTR_VALUE="0600" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The incorrect attribute values will also need to vary per product.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In all current cases, the expected permission is 0600
or 0640
and a stricter permission is accepted. So for the test scenarios in template
, using 0600
is enough for the correct permission. In a similar logic, in all current cases, no permission for "others" is allowed. So 0666
is also sufficient for the test scenarios in this template.
Something interesting would be to include an additional test scenario specific to Ubuntu and SLE in the rsyslog_files_permissions
rule. I will include.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
…mplate Co-authored-by: Watson Yuuma Sato <wsato@redhat.com>
b02ab3c
to
ed237b8
Compare
The values are already known during the build time. Therefore, the conditions to define which command to execute are unnecessary in run time.
Some distros accept 0640 permission for rsyslog log files. A new test scenario was included in the rule level to cover these cases.
The expected permission is different among some distros. A variable was conditionally defined for a more accurate rule description. It was also included the proper parameters when calling the template.
ed237b8
to
30b923a
Compare
Code Climate has analyzed commit 30b923a 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.5% (0.0% change). View more on Code Climate. |
/packit retest-failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work
The failure on c9s is unrelated to this PR:
|
Description:
The PR #9789 introduced a new template to check and remediate rsyslog log files attributes.
The initial template covers
ownership
andgroup-ownership
attributes, which satisfy these two rules:The template is already compatible to both legacy and RainerScript syntax.
This PR is a extension of the
rsyslog_logfiles_attributes_modify
template in order to also coverpermissions
attributes of rsyslog log files, which satisfy the following additional rule:Rationale:
These are the only three relevant rules for rsyslog log files attributes, which are now aligned to the same approach through this new template. In addition:
Review Hints:
./build_product rhel7 rhel8 rhel9 ...
Example of
automatus
command for testing these 3 rules: