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.

Sign up for GitHub

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

Jump to bottom

align template systemd_dropin_configuration #12054

Merged
merged 13 commits into from
Jul 16, 2024
Merged

align template systemd_dropin_configuration #12054

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5ac946f
systemd_dropin_congiruation template OVAL: honor the section of the s…
vojtapolasek Jun 5, 2024
00ed030
systemd_dropin_configuration bash: rewrite using already existing macro
vojtapolasek Jun 5, 2024
f4dfdda
honor the section of the systemd file also in tests and rules
vojtapolasek Jun 5, 2024
a0980ac
macro bash_ensure_ini_config: do not put space around equals sign
vojtapolasek Jun 5, 2024
ad9b073
update Ansible remediation of the template to be aligned with Bash
vojtapolasek Jul 4, 2024
0226a25
honor the no_quotes parameter also when remediating
vojtapolasek Jul 9, 2024
3413ae9
remove the component field from the template instantiation because it…
vojtapolasek Jul 9, 2024
27d55c0
add documentation for systemd_dropin_configuration template
vojtapolasek Jul 9, 2024
719bf66
modify the Ansible remediation so that Jinja is not evaluated in the …
vojtapolasek Jul 10, 2024
0808946
change the file name used in remediations
vojtapolasek Jul 12, 2024
bab20e9
Ansible: update regex which checks for the section
vojtapolasek Jul 12, 2024
7c709f1
update test scenarios to not preserve previous file content
vojtapolasek Jul 12, 2024
4c5541b
Update docs/templates/template_reference.md
vojtapolasek Jul 15, 2024
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
14 changes: 13 additions & 1 deletion shared/macros/10-bash.jinja
View file
Open in desktop
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 •
edited
Loading

Choose a reason for hiding this comment

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

FYI @teacup-on-rockingchair

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
{{% 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
{{% 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")"
{{% if no_quotes %}}
echo -e "[{{{ section }}}]\n{{{ key }}}={{{ value }}}" >> "$file"
{{% else %}}
echo -e '[{{{ section }}}]\n{{{ key }}}="{{{ value }}}"' >> "$file"
{{% endif %}}
fi
{{%- endmacro %}}

Expand Down
8 changes: 8 additions & 0 deletions shared/templates/systemd_dropin_configuration/ansible.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
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}}"
Expand All @@ -33,7 +37,11 @@
path: "{{{ DROPIN_DIR }}}/oscap-remedy.conf"
section: {{{ SECTION }}}
option: {{{ PARAM }}}
{{% if NO_QUOTES %}}
value: "{{{ VALUE }}}"
{{% else %}}
value: '"{{{ VALUE }}}"'
{{% endif %}}
state: present
no_extra_spaces: true
create: true
Expand Down
3 changes: 2 additions & 1 deletion shared/templates/systemd_dropin_configuration/bash.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
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
value=VALUE,
no_quotes=NO_QUOTES
) }}}
4 changes: 4 additions & 0 deletions shared/templates/systemd_dropin_configuration/tests/correct_dir.pass.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ PARAM="{{{ PARAM }}}"
VALUE="{{{ VALUE }}}"
DROPIN_DIR="{{{ DROPIN_DIR }}}"
[ -d $DROPIN_DIR ] || mkdir -p $DROPIN_DIR
{{% 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 %}}
4 changes: 4 additions & 0 deletions shared/templates/systemd_dropin_configuration/tests/correct_master.pass.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@ SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
VALUE="{{{ VALUE }}}"
MASTER_CFG_FILE="{{{ 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 %}}
6 changes: 6 additions & 0 deletions shared/templates/systemd_dropin_configuration/tests/multiple_vals.fail.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ VALUE="{{{ VALUE }}}"
DROPIN_DIR="{{{ DROPIN_DIR }}}"
MASTER_CFG_FILE="{{{ MASTER_CFG_FILE }}}"
[ -d $DROPIN_DIR ] || mkdir -p $DROPIN_DIR
{{% 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 %}}
4 changes: 4 additions & 0 deletions shared/templates/systemd_dropin_configuration/tests/wrong_dir.fail.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@ SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
DROPIN_DIR="{{{ DROPIN_DIR }}}"
[ -d $DROPIN_DIR ] || mkdir -p $DROPIN_DIR
{{% 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 %}}
4 changes: 4 additions & 0 deletions shared/templates/systemd_dropin_configuration/tests/wrong_master.fail.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@
SECTION="{{{ SECTION }}}"
PARAM="{{{ PARAM }}}"
MASTER_CFG_FILE="{{{ 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 %}}