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
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ template:
component: journald
master_cfg_file: /etc/systemd/journald.conf
dropin_dir: {{{ journald_conf_dir_path }}}
section: Journal
param: Compress
value: yes
no_quotes: 'true'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

# This scenario is a regression test for https://bugzilla.redhat.com/show_bug.cgi?id=2193169

echo "Compress='yes'" > "/etc/systemd/journald.conf"
echo -e "[Journal]\nCompress='yes'" > "/etc/systemd/journald.conf"
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ template:
component: journald
master_cfg_file: /etc/systemd/journald.conf
dropin_dir: {{{ journald_conf_dir_path }}}
section: Journal
param: ForwardToSyslog
value: yes
no_quotes: 'true'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ template:
component: journald
master_cfg_file: /etc/systemd/journald.conf
dropin_dir: {{{ journald_conf_dir_path }}}
section: Journal
param: Storage
value: persistent
no_quotes: 'true'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

# This scenario is a regression test for https://bugzilla.redhat.com/show_bug.cgi?id=2169857

echo "Storage='persistent'" > "/etc/systemd/journald.conf"
echo -e "[Journal]\nStorage='persistent'" > "/etc/systemd/journald.conf"
20 changes: 16 additions & 4 deletions shared/macros/10-bash.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -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

found=false

# set value in all files if they contain section or key
Expand All @@ -2108,12 +2108,20 @@ for f in $(echo -n "{{{ files }}}"); do

# find key in section and change value
if grep -qzosP "[[:space:]]*\[{{{ section }}}\]([^\n\[]*\n+)+?[[:space:]]*{{{ key }}}" "$f"; then
sed -i "s/{{{ key }}}[^(\n)]*/{{{ key }}} = {{{ value }}}/" "$f"
{{% if no_quotes %}}
sed -i "s/{{{ key }}}[^(\n)]*/{{{ key }}}={{{ value }}}/" "$f"
{{% else %}}
sed -i 's/{{{ key }}}[^(\n)]*/{{{ key }}}="{{{ value }}}"/' "$f"
{{% endif %}}
found=true

# find section and add key = value to it
elif grep -qs "[[:space:]]*\[{{{ section }}}\]" "$f"; then
sed -i "/[[:space:]]*\[{{{ section }}}\]/a {{{ key }}} = {{{ value }}}" "$f"
{{% if no_quotes %}}
sed -i "/[[:space:]]*\[{{{ section }}}\]/a {{{ key }}}={{{ value }}}" "$f"
{{% else %}}
sed -i '/[[:space:]]*\[{{{ section }}}\]/a {{{ key }}}="{{{ value }}}"' "$f"
{{% endif %}}
found=true
fi
done
Expand All @@ -2122,7 +2130,11 @@ done
if ! $found ; then
file=$(echo "{{{ files }}}" | cut -f1 -d ' ')
mkdir -p "$(dirname "$file")"
echo -e "[{{{ section }}}]\n{{{ key }}} = {{{ value }}}" >> "$file"
{{% if no_quotes %}}
echo -e "[{{{ section }}}]\n{{{ key }}}={{{ value }}}" >> "$file"
{{% else %}}
echo -e '[{{{ section }}}]\n{{{ key }}}="{{{ value }}}"' >> "$file"
{{% endif %}}
fi
{{%- endmacro %}}

Expand Down
4 changes: 3 additions & 1 deletion shared/macros/10-oval.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,8 @@ Generates the :code:`<affected>` tag for OVAL check using correct product platfo
:type path: str
:param dropin_dir: Path to the dropin directory
:type dropin_dir: str
:param section: optional section if the file has an ini-like format
:type section: str
:param parameter: The shell variable name.
:type parameter: str
:param value: The variable value WITHOUT QUOTES.
Expand All @@ -604,7 +606,7 @@ Generates the :code:`<affected>` tag for OVAL check using correct product platfo
:type missing_config_file_fail: bool
#}}

{{%- macro oval_check_dropin_file(path, dropin_dir, parameter='', value='', application='', no_quotes=false, missing_parameter_pass=false, multi_value=false, missing_config_file_fail=false) %}}
{{%- macro oval_check_dropin_file(path, dropin_dir, section='', parameter='', value='', application='', no_quotes=false, missing_parameter_pass=false, multi_value=false, missing_config_file_fail=false) %}}
vojtapolasek marked this conversation as resolved.
Show resolved Hide resolved
<def-group>
{{%- set prefix_regex = "^[ \\t]*" -%}}
{{%- set separator_regex = '=' -%}}
Expand Down
75 changes: 38 additions & 37 deletions shared/templates/systemd_dropin_configuration/ansible.template
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,46 @@
# strategy = restrict
# complexity = low
# disruption = low
- name: Check for duplicate {{{ PARAM }}} values in master {{{ COMPONENT }}} configuration
ansible.builtin.lineinfile:
path: {{{ MASTER_CFG_FILE }}}
create: false
regexp: ^\s*{{{ PARAM }}}=
state: absent
check_mode: true
changed_when: false
register: dupes_master

- name: Deduplicate {{{ PARAM }}} values from {{{ COMPONENT }}} master configuration
ansible.builtin.lineinfile:
path: {{{ MASTER_CFG_FILE }}}
create: false
regexp: ^\s*{{{ PARAM }}}=
state: absent
when: dupes_master.found is defined and dupes_master.found > 1

- name: Collect all config {{{ COMPONENT }}} files which configure {{{ PARAM }}}
- name: "{{{ rule_title }}} - Search for a section in files"
ansible.builtin.find:
paths: {{{ DROPIN_DIR }}}
contains: ^[\s]*{{{ PARAM }}}=.*$
patterns: "*.conf"
register: {{{ COMPONENT }}}_{{{ PARAM }}}_dropin_config_files
paths: "{{item.path}}"
patterns: "{{item.pattern}}"
contains: "[{{{ SECTION }}}]"
read_whole_file: true
register: systemd_dropin_files_with_section
loop:
- path: "{{ '{{{ MASTER_CFG_FILE }}}' | dirname }}"
pattern: "{{ '{{{ MASTER_CFG_FILE }}}' | basename }}"
- path: "{{{ DROPIN_DIR }}}"
pattern: "*.conf"

- name: Deduplicate values from {{{ COMPONENT }}} {{{ PARAM }}} dropin configuration
ansible.builtin.lineinfile:
path: "{{ item.path }}"
create: false
regexp: ^\s*{{{ PARAM }}}=
state: absent
loop: "{{ {{{ COMPONENT }}}_{{{ PARAM }}}_dropin_config_files.files }}"
- name: "{{{ rule_title }}} - Add missing configuration to correct section"
ini_file:
path: "{{item}}"
section: {{{ SECTION }}}
option: {{{ PARAM }}}
{{% if NO_QUOTES %}}
value: "{{{ VALUE }}}"
{{% else %}}
value: '"{{{ VALUE }}}"'
{{% endif %}}
state: present
no_extra_spaces: true
when: "{{systemd_dropin_files_with_section.results | map(attribute='matched') | list | map('int') | sum > 0}}"
loop: "{{systemd_dropin_files_with_section.results | sum(attribute='files', start=[]) | map(attribute='path') | list }}"
marcusburghardt marked this conversation as resolved.
Show resolved Hide resolved

- name: Insert correct line to {{{ COMPONENT }}} {{{ PARAM }}} configuration
ansible.builtin.lineinfile:
path: {{{ DROPIN_DIR }}}/oscap-remedy.conf
create: true
regexp: ^\s*{{{ PARAM }}}=
line: {{{ PARAM }}}={{{ VALUE }}}
- name: "{{{ rule_title }}} - Add configuration to new remediation file"
ini_file:
path: "{{{ DROPIN_DIR }}}/oscap-remedy.conf"
section: {{{ SECTION }}}
option: {{{ PARAM }}}
{{% if NO_QUOTES %}}
value: "{{{ VALUE }}}"
{{% else %}}
value: '"{{{ VALUE }}}"'
{{% endif %}}
state: present
insertbefore: ^# {{{ PARAM }}}
validate: bash -n %s
no_extra_spaces: true
create: true
when: "{{systemd_dropin_files_with_section.results | map(attribute='matched') | list | map('int') | sum == 0}}"
47 changes: 7 additions & 40 deletions shared/templates/systemd_dropin_configuration/bash.template
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,10 @@
# complexity = low
# disruption = low

function remove_{{{ COMPONENT }}}_{{{ PARAM }}}_configuration {
local COMPONENT_PARAM_CONFIG
mapfile -t COMPONENT_PARAM_CONFIG < <(ls {{{ DROPIN_DIR }}}/*.conf)
COMPONENT_PARAM_CONFIG+=("{{{ MASTER_CFG_FILE }}}")

for f in "${COMPONENT_PARAM_CONFIG[@]}"
do
sed -i "/^\s*{{{ PARAM }}}\s*=\s*/d" "$f"
# make sure file has newline at the end
sed -i -e '$a\' "$f"
done
sed -i -e '$a\' "{{{ MASTER_CFG_FILE }}}"
}

function {{{ COMPONENT }}}_{{{ PARAM }}}_add_configuration {
local COMPONENT_PARAM_REMEDY_CFG
mkdir -p "{{{ DROPIN_DIR }}}"
COMPONENT_PARAM_REMEDY_CFG="{{{ DROPIN_DIR }}}/oscap-remedy.conf"

if [ ! -f "${COMPONENT_PARAM_REMEDY_CFG}" ] ; then
touch "${COMPONENT_PARAM_REMEDY_CFG}"
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*{{{ PARAM }}}" "${COMPONENT_PARAM_REMEDY_CFG}.bak" | LC_ALL=C sed 's/:.*//g')"
if [ -z "$line_number" ]; then
# There was no match of '^#\s*{{{ PARAM }}}', insert at
# the end of the file.
printf '%s\n' "{{{ PARAM }}}={{{ VALUE }}}" >> "${COMPONENT_PARAM_REMEDY_CFG}"
else
head -n "$(( line_number - 1 ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" > "${COMPONENT_PARAM_REMEDY_CFG}"
printf '%s\n' "{{{ PARAM }}}={{{ VALUE }}}" >> "{{{ MASTER_CFG_FILE }}}"
tail -n "+$(( line_number ))" "${COMPONENT_PARAM_REMEDY_CFG}.bak" >> "${COMPONENT_PARAM_REMEDY_CFG}"
fi
# Clean up after ourselves.
rm "${COMPONENT_PARAM_REMEDY_CFG}.bak"
}

remove_{{{ COMPONENT }}}_{{{ PARAM }}}_configuration
{{{ COMPONENT }}}_{{{ PARAM }}}_add_configuration
{{{ bash_ensure_ini_config(
files=DROPIN_DIR+"/oscap-remedy.conf "+DROPIN_DIR+"/*.conf "+MASTER_CFG_FILE,
vojtapolasek marked this conversation as resolved.
Show resolved Hide resolved
section=SECTION,
key=PARAM,
value=VALUE,
no_quotes=NO_QUOTES
) }}}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
oval_check_dropin_file(
path=MASTER_CFG_FILE,
dropin_dir=DROPIN_DIR,
section=SECTION,
parameter=PARAM,
value=VALUE,
no_quotes=NO_QUOTES,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
#!/bin/bash
SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
VALUE="{{{ VALUE }}}"
DROPIN_DIR="{{{ DROPIN_DIR }}}"
[ -d $DROPIN_DIR ] || mkdir -p $DROPIN_DIR
echo "$PARAM=$VALUE" >> "$DROPIN_DIR/ssg.conf"
{{% if NO_QUOTES %}}
echo -e "[$SECTION]\n$PARAM=$VALUE" >> "$DROPIN_DIR/ssg.conf"
{{% else %}}
echo -e "[$SECTION]\n$PARAM=\"$VALUE\"" >> "$DROPIN_DIR/ssg.conf"
{{% endif %}}
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#!/bin/bash
SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
VALUE="{{{ VALUE }}}"
MASTER_CFG_FILE="{{{ MASTER_CFG_FILE }}}"
echo "$PARAM=$VALUE" >> "$MASTER_CFG_FILE"
{{% if NO_QUOTES %}}
echo -e "[$SECTION]\n$PARAM=$VALUE" >> "$MASTER_CFG_FILE"
{{% else %}}
echo -e "[$SECTION]\n$PARAM=\"$VALUE\"" >> "$MASTER_CFG_FILE"
{{% endif %}}
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
#!/bin/bash
SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
VALUE="{{{ VALUE }}}"
DROPIN_DIR="{{{ DROPIN_DIR }}}"
MASTER_CFG_FILE="{{{ MASTER_CFG_FILE }}}"
[ -d $DROPIN_DIR ] || mkdir -p $DROPIN_DIR
echo "$PARAM=$VALUE" >> "$DROPIN_DIR/ssg.conf"
echo "$PARAM=badval" >> "$DROPIN_DIR/gss.conf"
echo "$PARAM=foobarzoo" >> "$MASTER_CFG_FILE"
{{% if NO_QUOTES %}}
echo -e "[$SECTION]\n$PARAM=$VALUE" >> "$DROPIN_DIR/ssg.conf"
echo -e "[$SECTION]\n$PARAM=badval" >> "$DROPIN_DIR/gss.conf"
echo -e "[$SECTION]\n$PARAM=foobarzoo" >> "$MASTER_CFG_FILE"
{{% else %}}
echo -e "[$SECTION]\n$PARAM=\"$VALUE\"" >> "$DROPIN_DIR/ssg.conf"
echo -e "[$SECTION]\n$PARAM=\"badval\"" >> "$DROPIN_DIR/gss.conf"
echo -e "[$SECTION]\n$PARAM=\"foobarzoo\"" >> "$MASTER_CFG_FILE"
{{% endif %}}
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#!/bin/bash
SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
DROPIN_DIR="{{{ DROPIN_DIR }}}"
[ -d $DROPIN_DIR ] || mkdir -p $DROPIN_DIR
echo "$PARAM=badval" >> "$DROPIN_DIR/ssg.conf"
{{% if NO_QUOTES %}}
echo -e "[$SECTION]\n$PARAM=badval" >> "$DROPIN_DIR/ssg.conf"
{{% else %}}
echo -e "[$SECTION]\n$PARAM=\"badval\"" >> "$DROPIN_DIR/ssg.conf"
{{% endif %}}
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/bin/bash
SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
VALUE="{{{ VALUE }}}"
MASTER_CFG_FILE="{{{ MASTER_CFG_FILE }}}"
echo "$PARAM=badval" >> "$MASTER_CFG_FILE"
{{% if NO_QUOTES %}}
echo -e "[$SECTION]\n$PARAM=badval" >> "$MASTER_CFG_FILE"
{{% else %}}
echo -e "[$SECTION]\n$PARAM=\"badval\"" >> "$MASTER_CFG_FILE"
{{% endif %}}
18 changes: 9 additions & 9 deletions tests/unit/bash/test_bash_ensure_ini_config.bats.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ teardown() {

@test "bash_ensure_ini_config - Basic value remediation" {
printf "[pam]\npam_cert_auth = false\n" > sssd_test/sssd.conf
expected_output="[pam]\npam_cert_auth = true\n"
expected_output="[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"

Expand All @@ -57,7 +57,7 @@ teardown() {
@test "bash_ensure_ini_config - Value remediation in multiple files" {
printf "[pam]\npam_cert_auth = false\n" > sssd_test/sssd.conf
printf "[pam]\npam_cert_auth = false\n" > pam_cert_auth.conf
expected_output="[pam]\npam_cert_auth = true\n"
expected_output="[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf pam_cert_auth.conf" "pam" "pam_cert_auth" "true"

Expand All @@ -70,7 +70,7 @@ teardown() {

@test "bash_ensure_ini_config - No remediation happened" {
printf "[pam]\npam_cert_auth = true\n" > sssd_test/sssd.conf
expected_output="[pam]\npam_cert_auth = true\n"
expected_output="[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"

Expand All @@ -80,7 +80,7 @@ teardown() {

@test "bash_ensure_ini_config - Append section with option to empty file" {
printf "" > sssd_test/sssd.conf
expected_output="[pam]\npam_cert_auth = true\n"
expected_output="[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"

Expand All @@ -89,7 +89,7 @@ teardown() {
}

@test "bash_ensure_ini_config - Create file with section and option" {
expected_output="[pam]\npam_cert_auth = true\n"
expected_output="[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"

Expand All @@ -99,7 +99,7 @@ teardown() {

@test "bash_ensure_ini_config - Append option to section" {
printf "[pam]\n" > sssd_test/sssd.conf
expected_output="[pam]\npam_cert_auth = true\n"
expected_output="[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"

Expand All @@ -109,7 +109,7 @@ teardown() {

@test "bash_ensure_ini_config - Append option to section when section is substring of option" {
printf "[pam]\n" > sssd_test/sssd.conf
expected_output="[pam]\npam_verbosity = 1\npam_cert_auth = true\n"
expected_output="[pam]\npam_verbosity=1\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"
call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_verbosity" "1"
Expand All @@ -121,7 +121,7 @@ teardown() {
@test "bash_ensure_ini_config - Append option to section in multiple files" {
printf "[pam]\n" > sssd_test/sssd.conf
printf "[pam]\n" > pam_cert_auth.conf
expected_output="[pam]\npam_cert_auth = true\n"
expected_output="[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "pam_cert_auth.conf sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"

Expand All @@ -134,7 +134,7 @@ teardown() {

@test "bash_ensure_ini_config - Append section with option to non-empty file" {
printf "[section]\nkey = value\n" > sssd_test/sssd.conf
expected_output="[section]\nkey = value\n[pam]\npam_cert_auth = true\n"
expected_output="[section]\nkey = value\n[pam]\npam_cert_auth=true\n"

call_bash_ensure_ini_config "sssd_test/sssd.conf" "pam" "pam_cert_auth" "true"

Expand Down