Skip to content

Commit 26527fc

Browse files
andreashaerterandreashaerter
committed
run role: improve script quoting
Ensure safer handling of input by adding ansible.builtin.quote where appropriate. Even if inputs are user-controlled or derived from internal variables, it's better to be safe and prevent potential issues with shell parsing.
1 parent 9467167 commit 26527fc

File tree

7 files changed

+81
-84
lines changed

7 files changed

+81
-84
lines changed

‎roles/run/handlers/main.yml‎

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,55 +29,46 @@
2929
3030
# Set ownership of files and directories: service user and group
3131
find \
32-
'{{ run_acmesh_cfg_home }}' \
33-
'{{ run_acmesh_cfg_config_home }}' \
34-
'{{ run_acmesh_cfg_cert_home }}' \
35-
-exec chown -c "{{ run_acmesh_user }}:{{ run_acmesh_group }}" {} \;
36-
chown -c "{{ run_acmesh_user }}:{{ run_acmesh_group }}" '{{ run_acmesh_cfg_config_home }}/account.conf'
37-
chown -c "{{ run_acmesh_user }}:{{ run_acmesh_group }}" '{{ run_acmesh_cfg_logfile }}'
32+
{{ run_acmesh_cfg_home | ansible.builtin.quote }} \
33+
{{ run_acmesh_cfg_config_home | ansible.builtin.quote }} \
34+
{{ run_acmesh_cfg_cert_home | ansible.builtin.quote }} \
35+
-exec chown -c {{ (run_acmesh_user ~ ':' ~ run_acmesh_group) | ansible.builtin.quote }} {} \;
36+
chown -c {{ (run_acmesh_user ~ ':' ~ run_acmesh_group) | ansible.builtin.quote }} {{ (run_acmesh_cfg_config_home ~ '/account.conf') |
37+
ansible.builtin.quote }}
38+
chown -c {{ (run_acmesh_user ~ ':' ~ run_acmesh_group) | ansible.builtin.quote }} {{ run_acmesh_cfg_logfile | ansible.builtin.quote }}
3839
3940
# Set permissions of directories: 0750 / u=rwx,g=rx,o=
4041
find \
41-
'{{ run_acmesh_cfg_home }}' \
42-
'{{ run_acmesh_cfg_config_home }}' \
43-
'{{ run_acmesh_cfg_cert_home }}' \
42+
{{ run_acmesh_cfg_home | ansible.builtin.quote }} \
43+
{{ run_acmesh_cfg_config_home | ansible.builtin.quote }} \
44+
{{ run_acmesh_cfg_cert_home | ansible.builtin.quote }} \
4445
-type d \
4546
-exec chmod -c 0750 {} \;
4647
4748
# Set permissions of non-executables: 0640 / u=rw,g=r,o=
4849
# The account.conf will be handled later
4950
find \
50-
'{{ run_acmesh_cfg_home }}' \
51-
'{{ run_acmesh_cfg_config_home }}' \
52-
'{{ run_acmesh_cfg_cert_home }}' \
51+
{{ run_acmesh_cfg_home | ansible.builtin.quote }} \
52+
{{ run_acmesh_cfg_config_home | ansible.builtin.quote }} \
53+
{{ run_acmesh_cfg_cert_home | ansible.builtin.quote }} \
5354
-type f \
5455
-not -name '*\.sh' \
55-
-not -wholename '{{ run_acmesh_cfg_config_home }}/account.conf' \
56+
-not -wholename {{ (run_acmesh_cfg_config_home ~ '/account.conf') | ansible.builtin.quote }} \
5657
-exec chmod -c 0640 {} \;
5758
5859
# Set permissions of executables: 0750 / u=rwx,g=rx,o=
59-
chmod -c 0750 '{{ run_acmesh_cfg_home }}/acme.sh'
60+
chmod -c 0750 {{ (run_acmesh_cfg_home ~ '/acme.sh') | ansible.builtin.quote }}
6061
find \
61-
'{{ run_acmesh_cfg_home }}/deploy' \
62-
'{{ run_acmesh_cfg_home }}/dnsapi' \
63-
'{{ run_acmesh_cfg_home }}/notify' \
64-
-type f \
65-
-name '*\.sh' \
66-
-exec chmod -c 0750 {} \;
67-
68-
# Set permissions of executables: 0750 / u=rwx,g=rx,o=
69-
chmod -c 0750 '{{ run_acmesh_cfg_home }}/acme.sh'
70-
find \
71-
'{{ run_acmesh_cfg_home }}/deploy' \
72-
'{{ run_acmesh_cfg_home }}/dnsapi' \
73-
'{{ run_acmesh_cfg_home }}/notify' \
62+
{{ (run_acmesh_cfg_home ~ '/deploy') | ansible.builtin.quote }} \
63+
{{ (run_acmesh_cfg_home ~ '/dnsapi') | ansible.builtin.quote }} \
64+
{{ (run_acmesh_cfg_home ~ '/notify') | ansible.builtin.quote }} \
7465
-type f \
7566
-name '*\.sh' \
7667
-exec chmod -c 0750 {} \;
7768
7869
# Set permissions of sensitive files directories: 0600 / u=rw,g=,o=
79-
chmod -c 0600 '{{ run_acmesh_cfg_config_home }}/account.conf' # may contain credentials (e.g. DNS API or notify credentials)
80-
chmod -c 0600 '{{ run_acmesh_cfg_logfile }}'
70+
chmod -c 0600 {{ (run_acmesh_cfg_config_home ~ '/account.conf') | ansible.builtin.quote }} # may contain credentials (e.g. DNS API or notify credentials)
71+
chmod -c 0600 {{ run_acmesh_cfg_logfile | ansible.builtin.quote }}
8172
8273
exit 0
8374
executable: "/bin/bash" # this inline script was only tested with Bash yet, so hardcode the shell

‎roles/run/tasks/cert/default.yml‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -105,29 +105,29 @@
105105
cmd: >
106106
./acme.sh
107107
--issue
108-
--config-home "{{ run_acmesh_cfg_config_home }}"
109-
--certhome "{{ run_acmesh_cfg_cert_home }}"
108+
--config-home {{ run_acmesh_cfg_config_home | ansible.builtin.quote }}
109+
--certhome {{ run_acmesh_cfg_cert_home | ansible.builtin.quote }}
110110
{% for item_domain in item['domains'] %}
111-
--domain "{{ item_domain['name'] }}"
112-
{{ '--dns "' ~ item_domain['challenge']['dns_provider'] ~ '"'
111+
--domain {{ item_domain['name'] | ansible.builtin.quote }}
112+
{{ '--dns ' ~ (item_domain['challenge']['dns_provider'] | ansible.builtin.quote)
113113
if ((item_domain['challenge'] is defined) and
114114
(item_domain['challenge']['type'] is defined) and
115115
(item_domain['challenge']['type'] == 'dns') and
116116
(item_domain['challenge']['dns_provider'] is defined) and
117117
(item_domain['challenge']['dns_provider']))
118118
else
119119
'' }}
120-
{{ '--domain-alias "' ~ item_domain['challenge']['domain_alias'] ~ '"'
120+
{{ '--domain-alias ' ~ (item_domain['challenge']['domain_alias'] | ansible.builtin.quote)
121121
if ((item_domain['challenge']['domain_alias'] is defined) and
122122
(item_domain['challenge']['domain_alias']))
123123
else
124124
'' }}
125-
{{ '--challenge-alias "' ~ item_domain['challenge']['challenge_alias'] ~ '"'
125+
{{ '--challenge-alias ' ~ (item_domain['challenge']['challenge_alias'] | ansible.builtin.quote)
126126
if ((item_domain['challenge']['challenge_alias'] is defined) and
127127
(item_domain['challenge']['challenge_alias']))
128128
else
129129
'' }}
130-
{{ '--webroot "' ~ item_domain['challenge']['webroot'] ~ '"'
130+
{{ '--webroot ' ~ (item_domain['challenge']['webroot'] | ansible.builtin.quote)
131131
if ((item_domain['challenge'] is defined) and
132132
(item_domain['challenge']['type'] is defined) and
133133
(item_domain['challenge']['type'] == 'webroot') and
@@ -141,7 +141,7 @@
141141
(item_domain['challenge']['type'] == 'standalone'))
142142
else
143143
'' }}
144-
{{ '--httpport ' ~ item_domain['challenge']['httpport']
144+
{{ '--httpport ' ~ (item_domain['challenge']['httpport'] | ansible.builtin.quote)
145145
if ((item_domain['challenge']['httpport'] is defined) and
146146
(item_domain['challenge']['httpport']))
147147
else
@@ -152,7 +152,7 @@
152152
(item_domain['challenge']['type'] == 'alpn'))
153153
else
154154
'' }}
155-
{{ '--tlsport ' ~ item_domain['challenge']['tlsport']
155+
{{ '--tlsport ' ~ (item_domain['challenge']['tlsport'] | ansible.builtin.quote)
156156
if ((item_domain['challenge']['tlsport'] is defined) and
157157
(item_domain['challenge']['tlsport']))
158158
else
@@ -168,26 +168,26 @@
168168
(item['debug']))
169169
else
170170
'' }}
171-
{{ '--dnssleep ' ~ (item['dnssleep'] | int)
171+
{{ '--dnssleep ' ~ (item['dnssleep'] | ansible.builtin.quote)
172172
if (item['dnssleep'] is defined)
173173
else
174174
'' }}
175-
{{ '--pre-hook "' ~ item['pre_hook'] ~ '"'
175+
{{ '--pre-hook ' ~ (item['pre_hook'] | ansible.builtin.quote)
176176
if ((item['pre_hook'] is defined) and
177177
(item['pre_hook']))
178178
else
179179
'' }}
180-
{{ '--post-hook "' ~ item['post_hook'] ~ '"'
180+
{{ '--post-hook ' ~ (item['post_hook'] | ansible.builtin.quote)
181181
if ((item['post_hook'] is defined) and
182182
(item['post_hook']))
183183
else
184184
'' }}
185-
{{ '--renew-hook "' ~ item['renew_hook'] ~ '"'
185+
{{ '--renew-hook ' ~ (item['renew_hook'] | ansible.builtin.quote)
186186
if ((item['renew_hook'] is defined) and
187187
(item['renew_hook']))
188188
else
189189
'' }}
190-
{{ '--server "' ~ item['server'] ~ '"'
190+
{{ '--server ' ~ (item['server'] | ansible.builtin.quote)
191191
if ((item['server'] is defined) and
192192
(item['server']))
193193
else
@@ -275,30 +275,30 @@
275275
cmd: >
276276
./acme.sh
277277
--install-cert
278-
--config-home "{{ run_acmesh_cfg_config_home }}"
279-
--certhome "{{ run_acmesh_cfg_cert_home }}"
280-
--domain "{{ item['domains'][0]['name'] }}"
281-
{{ '--ca-file "' ~ item['install']['ca_file'] ~ '"'
278+
--config-home {{ run_acmesh_cfg_config_home | ansible.builtin.quote }}
279+
--certhome {{ run_acmesh_cfg_cert_home | ansible.builtin.quote }}
280+
--domain {{ item['domains'][0]['name'] | ansible.builtin.quote }}
281+
{{ '--ca-file ' ~ (item['install']['ca_file'] | ansible.builtin.quote)
282282
if ((item['install']['ca_file'] is defined) and
283283
(item['install']['ca_file']))
284284
else
285285
'' }}
286-
{{ '--cert-file "' ~ item['install']['cert_file'] ~ '"'
286+
{{ '--cert-file ' ~ (item['install']['cert_file'] | ansible.builtin.quote)
287287
if ((item['install']['cert_file'] is defined) and
288288
(item['install']['cert_file']))
289289
else
290290
'' }}
291-
{{ '--fullchain-file "' ~ item['install']['fullcain_file'] ~ '"'
291+
{{ '--fullchain-file ' ~ (item['install']['fullcain_file'] | ansible.builtin.quote)
292292
if ((item['install']['fullcain_file'] is defined) and
293293
(item['install']['fullcain_file']))
294294
else
295295
'' }}
296-
{{ '--key-file "' ~ item['install']['key_file'] ~ '"'
296+
{{ '--key-file ' ~ (item['install']['key_file'] | ansible.builtin.quote)
297297
if ((item['install']['key_file'] is defined) and
298298
(item['install']['key_file']))
299299
else
300300
'' }}
301-
{{ '--reloadcmd "' ~ item['install']['reloadcmd'] ~ '"'
301+
{{ '--reloadcmd ' ~ (item['install']['reloadcmd'] | ansible.builtin.quote)
302302
if ((item['install']['reloadcmd'] is defined) and
303303
(item['install']['reloadcmd']))
304304
else
@@ -333,7 +333,7 @@
333333
- name: "Cert | Default | Gather certificate information"
334334
ansible.builtin.command:
335335
cmd: >
336-
./acme.sh --list --certhome "{{ run_acmesh_cfg_cert_home }}"
336+
./acme.sh --list --certhome {{ run_acmesh_cfg_cert_home | ansible.builtin.quote }}
337337
args:
338338
chdir: "{{ run_acmesh_cfg_home }}"
339339
when:

‎roles/run/tasks/config/default.yml‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,22 @@
2222
set -u # fail on unset vars
2323
set -o pipefail # fail if any pipe cmd fails
2424
25-
__run_acmesh_checksum_before="$({{ __run_acmesh_sha1sum_executable }} '{{ run_acmesh_cfg_config_home }}/account.conf')"
25+
__run_acmesh_checksum_before=$({{ __run_acmesh_sha1sum_executable }} {{ (run_acmesh_cfg_config_home ~ '/account.conf') | ansible.builtin.quote }})
2626
2727
if ! ./acme.sh \
2828
--info \
29-
--home "{{ run_acmesh_cfg_home }}" \
30-
--config-home "{{ run_acmesh_cfg_config_home }}" \
31-
--certhome "{{ run_acmesh_cfg_cert_home }}" \
32-
--log "{{ run_acmesh_cfg_logfile }}" \
33-
--log-level {{ run_acmesh_cfg_log_level }} \
34-
--syslog {{ run_acmesh_cfg_syslog }} \
29+
--home {{ run_acmesh_cfg_home | ansible.builtin.quote }} \
30+
--config-home {{ run_acmesh_cfg_config_home | ansible.builtin.quote }} \
31+
--certhome {{ run_acmesh_cfg_cert_home | ansible.builtin.quote }} \
32+
--log {{ run_acmesh_cfg_logfile | ansible.builtin.quote }} \
33+
--log-level {{ run_acmesh_cfg_log_level | ansible.builtin.quote }} \
34+
--syslog {{ run_acmesh_cfg_syslog | ansible.builtin.quote }} \
3535
{{ '--auto-upgrade 1'
3636
if ((run_acmesh_autoupgrade is defined) and
3737
(run_acmesh_autoupgrade))
3838
else
3939
'--auto-upgrade 0' }} \
40-
{{ '--accountemail "' ~ run_acmesh_cfg_accountemail ~ '"'
40+
{{ '--accountemail ' ~ (run_acmesh_cfg_accountemail | ansible.builtin.quote)
4141
if ((run_acmesh_cfg_accountemail is defined) and
4242
(run_acmesh_cfg_accountemail))
4343
else
@@ -46,9 +46,10 @@
4646
exit 1
4747
fi
4848
49-
if [ "${__run_acmesh_checksum_before}" != "$({{ __run_acmesh_sha1sum_executable }} '{{ run_acmesh_cfg_config_home }}/account.conf')" ]
49+
__run_acmesh_checksum_after=$({{ __run_acmesh_sha1sum_executable }} {{ (run_acmesh_cfg_config_home ~ '/account.conf') | ansible.builtin.quote }})
50+
if [ "${__run_acmesh_checksum_before}" != "${__run_acmesh_checksum_after}" ]
5051
then
51-
printf 'Changed config file: {{ run_acmesh_cfg_config_home }}/account.conf'
52+
printf 'Changed config file: %s' {{ (run_acmesh_cfg_config_home ~ '/account.conf') | ansible.builtin.quote }}
5253
fi
5354
5455
exit 0

‎roles/run/tasks/init.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,19 @@
109109
cmd: |
110110
set -u # fail on unset vars
111111
set -o pipefail # fail if any pipe cmd fails
112+
112113
__run_acmesh_version_installed=""
113-
if command -v '{{ run_acmesh_cfg_home }}/acme.sh' > /dev/null 2>&1
114+
if command -v {{ (run_acmesh_cfg_home ~ '/acme.sh') | ansible.builtin.quote }} > /dev/null 2>&1
114115
then
115-
__run_acmesh_version_installed="$({{ run_acmesh_cfg_home }}/acme.sh --version | tail -1 | sed -e 's/^v//g')"
116+
__run_acmesh_version_installed=$({{ (run_acmesh_cfg_home ~ '/acme.sh' ) | ansible.builtin.quote }} --version | tail -1 | sed -e 's/^v//g')
116117
if ! printf '%s' "${__run_acmesh_version_installed}" | grep -E -q -e '^[[:digit:]\.]*$'
117118
then
118119
__run_acmesh_version_installed=''
119120
fi
120121
fi
122+
121123
printf '%s' "${__run_acmesh_version_installed}"
124+
exit 0
122125
executable: "/bin/bash" # this inline script was only tested with Bash yet, so hardcode the shell
123126
register: __run_acmesh_version_installed_result
124127
ignore_errors: true

‎roles/run/tasks/setup/install/default.yml‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -171,19 +171,19 @@
171171
cmd: >
172172
./acme.sh
173173
--install
174-
--home "{{ run_acmesh_cfg_home }}"
175-
--config-home "{{ run_acmesh_cfg_config_home }}"
176-
--certhome "{{ run_acmesh_cfg_cert_home }}"
177-
--log "{{ run_acmesh_cfg_logfile }}"
178-
--log-level {{ run_acmesh_cfg_log_level }}
179-
--syslog {{ run_acmesh_cfg_syslog }}
174+
--home {{ run_acmesh_cfg_home | ansible.builtin.quote }}
175+
--config-home {{ run_acmesh_cfg_config_home | ansible.builtin.quote }}
176+
--certhome {{ run_acmesh_cfg_cert_home | ansible.builtin.quote }}
177+
--log {{ run_acmesh_cfg_logfile | ansible.builtin.quote }}
178+
--log-level {{ run_acmesh_cfg_log_level | ansible.builtin.quote }}
179+
--syslog {{ run_acmesh_cfg_syslog | ansible.builtin.quote }}
180180
--nocron
181181
{{ '--auto-upgrade 1'
182182
if ((run_acmesh_autoupgrade is defined) and
183183
(run_acmesh_autoupgrade))
184184
else
185185
'--auto-upgrade 0' }}
186-
{{ '--accountemail "' ~ run_acmesh_cfg_accountemail ~ '"'
186+
{{ '--accountemail ' ~ (run_acmesh_cfg_accountemail | ansible.builtin.quote)
187187
if ((run_acmesh_cfg_accountemail is defined) and
188188
(run_acmesh_cfg_accountemail))
189189
else
@@ -214,18 +214,18 @@
214214
cmd: >
215215
./acme.sh
216216
--upgrade
217-
--home "{{ run_acmesh_cfg_home }}"
218-
--config-home "{{ run_acmesh_cfg_config_home }}"
219-
--certhome "{{ run_acmesh_cfg_cert_home }}"
220-
--log "{{ run_acmesh_cfg_logfile }}"
221-
--log-level {{ run_acmesh_cfg_log_level }}
222-
--syslog {{ run_acmesh_cfg_syslog }}
217+
--home {{ run_acmesh_cfg_home | ansible.builtin.quote }}
218+
--config-home {{ run_acmesh_cfg_config_home | ansible.builtin.quote }}
219+
--certhome {{ run_acmesh_cfg_cert_home | ansible.builtin.quote }}
220+
--log {{ run_acmesh_cfg_logfile | ansible.builtin.quote }}
221+
--log-level {{ run_acmesh_cfg_log_level | ansible.builtin.quote }}
222+
--syslog {{ run_acmesh_cfg_syslog | ansible.builtin.quote }}
223223
{{ '--auto-upgrade 1'
224224
if ((run_acmesh_autoupgrade is defined) and
225225
(run_acmesh_autoupgrade))
226226
else
227227
'--auto-upgrade 0' }}
228-
{{ '--accountemail "' ~ run_acmesh_cfg_accountemail ~ '"'
228+
{{ '--accountemail ' ~ (run_acmesh_cfg_accountemail | ansible.builtin.quote)
229229
if ((run_acmesh_cfg_accountemail is defined) and
230230
(run_acmesh_cfg_accountemail))
231231
else
@@ -260,15 +260,17 @@
260260
cmd: |
261261
set -u # fail on unset vars
262262
set -o pipefail # fail if any pipe cmd fails
263-
__run_acmesh_version_installed=''
264-
if command -v '{{ run_acmesh_cfg_home }}/acme.sh' > /dev/null 2>&1
263+
264+
__run_acmesh_version_installed=""
265+
if command -v {{ (run_acmesh_cfg_home ~ '/acme.sh') | ansible.builtin.quote }} > /dev/null 2>&1
265266
then
266-
__run_acmesh_version_installed="$({{ run_acmesh_cfg_home }}/acme.sh --version | tail -1 | sed -e 's/^v//g')"
267+
__run_acmesh_version_installed=$({{ (run_acmesh_cfg_home ~ '/acme.sh' ) | ansible.builtin.quote }} --version | tail -1 | sed -e 's/^v//g')
267268
if ! printf '%s' "${__run_acmesh_version_installed}" | grep -E -q -e '^[[:digit:]\.]*$'
268269
then
269270
__run_acmesh_version_installed=''
270271
fi
271272
fi
273+
272274
printf '%s' "${__run_acmesh_version_installed}"
273275
exit 0
274276
executable: "/bin/bash" # this inline script was only tested with Bash yet, so hardcode the shell

‎roles/run/tasks/setup/uninstall/default.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
# https://github.com/acmesh-official/acme.sh/wiki/Options-and-Params
1616
ansible.builtin.command:
1717
cmd: >
18-
{{ run_acmesh_cfg_home }}/acme.sh
18+
{{ (run_acmesh_cfg_home ~ '/acme.sh') | ansible.builtin.quote }}
1919
--uninstall
20-
--config-home "{{ run_acmesh_cfg_config_home }}"
20+
--config-home {{ run_acmesh_cfg_config_home | ansible.builtin.quote }}
2121
args:
2222
removes: "{{ run_acmesh_cfg_home }}/acme.sh"
2323
when:

‎roles/run/vars/main.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ __run_acmesh_systemd_renewal_servicefile: "/etc/systemd/system/acmesh-renewal.se
136136
__run_acmesh_systemd_renewal_timerfile: "/etc/systemd/system/acmesh-renewal.timer"
137137

138138

139-
# String. Path to the sha1sum executable
139+
# String. Path to the sha1sum executable.
140140
__run_acmesh_sha1sum_executable: "/usr/bin/sha1sum"
141141

142142

0 commit comments

Comments
 (0)