File 0003-Preserve-ANSIBLE_NO_LOG-CVE-2024-8775.patch of Package ansible
From ea8910e28a3ed3aa724adb7f24cb9971943a8202 Mon Sep 17 00:00:00 2001
From: Matt Davis <6775756+nitzmahone@users.noreply.github.com>
Date: Tue, 29 Oct 2024 09:41:27 -0700
Subject: [PATCH] [stable-2.14] Preserve `_ansible_no_log` from action result;
fix `include_vars` to set properly (#84143) (#84183)
* fixes for CVE-2024-8775
* propagate truthy `_ansible_no_log` in action result (previously superseded by task-calculated value)
* always mask entire `include_vars` action result if any file loaded had a false `show_content` flag (previously used only the flag value from the last file loaded)
* update no_log tests for CVE-2024-8775
* include validation of _ansible_no_log preservation when set by actions
* replace static values with dynamic for increased robustness to logging/display/callback changes (but still using grep counts :( )
* changelog
* use ternary, coerce to bool explicitly
(cherry picked from commit c9ac477e53a99e95781f333eec3329a935c1bf95)
---
changelogs/fragments/cve-2024-8775.yml | 5 +++
lib/ansible/executor/task_executor.py | 9 ++++--
lib/ansible/plugins/action/include_vars.py | 3 +-
.../include_vars-ad-hoc/dir/encrypted.yml | 6 ++++
.../targets/include_vars-ad-hoc/runme.sh | 22 +++++++++++--
.../targets/include_vars-ad-hoc/vaultpass | 3 ++
.../action_plugins/action_sets_no_log.py | 8 +++++
.../no_log/ansible_no_log_in_result.yml | 13 ++++++++
test/integration/targets/no_log/dynamic.yml | 29 +++++++++++++----
.../targets/no_log/no_log_config.yml | 2 +-
.../targets/no_log/no_log_local.yml | 15 +++++----
.../targets/no_log/no_log_suboptions.yml | 14 ++++----
.../no_log/no_log_suboptions_invalid.yml | 29 +++++++++--------
test/integration/targets/no_log/runme.sh | 16 +++++++---
.../integration/targets/no_log/secretvars.yml | 32 +++++++++++++++++++
15 files changed, 161 insertions(+), 45 deletions(-)
create mode 100644 changelogs/fragments/cve-2024-8775.yml
create mode 100644 test/integration/targets/include_vars-ad-hoc/dir/encrypted.yml
create mode 100755 test/integration/targets/include_vars-ad-hoc/vaultpass
create mode 100644 test/integration/targets/no_log/action_plugins/action_sets_no_log.py
create mode 100644 test/integration/targets/no_log/ansible_no_log_in_result.yml
create mode 100644 test/integration/targets/no_log/secretvars.yml
diff --git a/changelogs/fragments/cve-2024-8775.yml b/changelogs/fragments/cve-2024-8775.yml
new file mode 100644
index 0000000000..a292c99704
--- /dev/null
+++ b/changelogs/fragments/cve-2024-8775.yml
@@ -0,0 +1,5 @@
+security_fixes:
+ - task result processing - Ensure that action-sourced result masking (``_ansible_no_log=True``)
+ is preserved. (CVE-2024-8775)
+ - include_vars action - Ensure that result masking is correctly requested when vault-encrypted
+ files are read. (CVE-2024-8775)
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 7b8a74bba7..077b4e1a45 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -549,6 +549,8 @@ class TaskExecutor:
# skipping this task during the conditional evaluation step
context_validation_error = e
+ no_log = self._play_context.no_log
+
# Evaluate the conditional (if any) for this task, which we do before running
# the final task post-validation. We do this before the post validation due to
# the fact that the conditional may specify that the task be skipped due to a
@@ -673,8 +675,11 @@ class TaskExecutor:
self._handler.cleanup()
display.debug("handler run complete")
- # preserve no log
- result["_ansible_no_log"] = self._play_context.no_log
+ # propagate no log to result- the action can set this, so only overwrite it with the task's value if missing or falsey
+ result["_ansible_no_log"] = bool(no_log or result.get('_ansible_no_log', False))
+
+ if self._task.action not in C._ACTION_WITH_CLEAN_FACTS:
+ result = wrap_var(result)
# update the local copy of vars with the registered value, if specified,
# or any facts which may have been generated by the module execution
diff --git a/lib/ansible/plugins/action/include_vars.py b/lib/ansible/plugins/action/include_vars.py
index 07234537ac..bd67d01cf2 100644
--- a/lib/ansible/plugins/action/include_vars.py
+++ b/lib/ansible/plugins/action/include_vars.py
@@ -225,7 +225,8 @@ class ActionModule(ActionBase):
b_data, show_content = self._loader._get_file_contents(filename)
data = to_text(b_data, errors='surrogate_or_strict')
- self.show_content = show_content
+ self.show_content &= show_content # mask all results if any file was encrypted
+
data = self._loader.load(data, file_name=filename, show_content=show_content)
if not data:
data = dict()
diff --git a/test/integration/targets/include_vars-ad-hoc/dir/encrypted.yml b/test/integration/targets/include_vars-ad-hoc/dir/encrypted.yml
new file mode 100644
index 0000000000..328f18082a
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/dir/encrypted.yml
@@ -0,0 +1,6 @@
+$ANSIBLE_VAULT;1.1;AES256
+31613539636636336264396235633933633839646337323533316638633336653461393036336664
+3939386435313638366366626566346135623932653238360a366261303663343034633865626132
+31646231623630333636383636383833656331643164656366623332396439306132663264663131
+6439633766376261320a616265306430366530363866356433366430633265353739373732646536
+37623661333064306162373463616231636365373231313939373230643936313362
diff --git a/test/integration/targets/include_vars-ad-hoc/runme.sh b/test/integration/targets/include_vars-ad-hoc/runme.sh
index 51b68d2134..5956e1f42b 100755
--- a/test/integration/targets/include_vars-ad-hoc/runme.sh
+++ b/test/integration/targets/include_vars-ad-hoc/runme.sh
@@ -1,6 +1,22 @@
#!/usr/bin/env bash
-set -eux
+set -eux -o pipefail
-ansible testhost -i ../../inventory -m include_vars -a 'dir/inc.yml' "$@"
-ansible testhost -i ../../inventory -m include_vars -a 'dir=dir' "$@"
+echo "single file include"
+ansible testhost -i ../../inventory -m include_vars -a 'dir/inc.yml' -vvv 2>&1 | grep -q 'porter.*cable'
+
+echo "single file encrypted include"
+ansible testhost -i ../../inventory -m include_vars -a 'dir/encrypted.yml' -vvv --vault-password-file vaultpass > output.txt 2>&1
+
+echo "directory include with encrypted"
+ansible testhost -i ../../inventory -m include_vars -a 'dir=dir' -vvv --vault-password-file vaultpass >> output.txt 2>&1
+
+grep -q 'output has been hidden' output.txt
+
+# all content should be masked if any file is encrypted
+if grep -e 'i am a secret' -e 'porter.*cable' output.txt; then
+ echo "FAIL: vault masking failed"
+ exit 1
+fi
+
+echo PASS
diff --git a/test/integration/targets/include_vars-ad-hoc/vaultpass b/test/integration/targets/include_vars-ad-hoc/vaultpass
new file mode 100755
index 0000000000..1f78d41e66
--- /dev/null
+++ b/test/integration/targets/include_vars-ad-hoc/vaultpass
@@ -0,0 +1,3 @@
+#!/bin/sh
+
+echo supersecurepassword
diff --git a/test/integration/targets/no_log/action_plugins/action_sets_no_log.py b/test/integration/targets/no_log/action_plugins/action_sets_no_log.py
new file mode 100644
index 0000000000..cb42616875
--- /dev/null
+++ b/test/integration/targets/no_log/action_plugins/action_sets_no_log.py
@@ -0,0 +1,8 @@
+from __future__ import annotations
+
+from ansible.plugins.action import ActionBase
+
+
+class ActionModule(ActionBase):
+ def run(self, tmp=None, task_vars=None):
+ return dict(changed=False, failed=False, msg="action result should be masked", _ansible_no_log="yeppers") # ensure that a truthy non-bool works here
diff --git a/test/integration/targets/no_log/ansible_no_log_in_result.yml b/test/integration/targets/no_log/ansible_no_log_in_result.yml
new file mode 100644
index 0000000000..a80a4a782e
--- /dev/null
+++ b/test/integration/targets/no_log/ansible_no_log_in_result.yml
@@ -0,0 +1,13 @@
+- hosts: localhost
+ gather_facts: no
+ tasks:
+ - action_sets_no_log:
+ register: res_action
+
+ - assert:
+ that:
+ - res_action.msg == "action result should be masked"
+
+ - action_sets_no_log:
+ loop: [1, 2, 3]
+ register: res_action
diff --git a/test/integration/targets/no_log/dynamic.yml b/test/integration/targets/no_log/dynamic.yml
index 4a1123d574..95236779b0 100644
--- a/test/integration/targets/no_log/dynamic.yml
+++ b/test/integration/targets/no_log/dynamic.yml
@@ -1,27 +1,42 @@
- name: test dynamic no log
hosts: testhost
gather_facts: no
- ignore_errors: yes
tasks:
- name: no loop, task fails, dynamic no_log
- debug:
- msg: "SHOW {{ var_does_not_exist }}"
+ raw: echo {{ var_does_not_exist }}
no_log: "{{ not (unsafe_show_logs|bool) }}"
+ ignore_errors: yes
+ register: result
+
+ - assert:
+ that:
+ - result is failed
+ - result.results is not defined
- name: loop, task succeeds, dynamic does no_log
- debug:
- msg: "SHOW {{ item }}"
+ raw: echo {{ item }}
loop:
- a
- b
- c
no_log: "{{ not (unsafe_show_logs|bool) }}"
+ register: result
+
+ - assert:
+ that:
+ - result.results | length == 3
- name: loop, task fails, dynamic no_log
- debug:
- msg: "SHOW {{ var_does_not_exist }}"
+ raw: echo {{ var_does_not_exist }}
loop:
- a
- b
- c
no_log: "{{ not (unsafe_show_logs|bool) }}"
+ ignore_errors: yes
+ register: result
+
+ - assert:
+ that:
+ - result is failed
+ - result.results is not defined # DT needs result.results | length == 3
diff --git a/test/integration/targets/no_log/no_log_config.yml b/test/integration/targets/no_log/no_log_config.yml
index 8a5088059d..165f4e07c5 100644
--- a/test/integration/targets/no_log/no_log_config.yml
+++ b/test/integration/targets/no_log/no_log_config.yml
@@ -10,4 +10,4 @@
- debug:
- debug:
- loop: '{{ range(3) }}'
+ loop: '{{ range(3) | list }}'
diff --git a/test/integration/targets/no_log/no_log_local.yml b/test/integration/targets/no_log/no_log_local.yml
index aacf7de276..d2bc5ae482 100644
--- a/test/integration/targets/no_log/no_log_local.yml
+++ b/test/integration/targets/no_log/no_log_local.yml
@@ -4,19 +4,22 @@
hosts: testhost
gather_facts: no
tasks:
+ - include_vars: secretvars.yml
+ no_log: true
+
- name: args should be logged in the absence of no_log
- shell: echo "LOG_ME_TASK_SUCCEEDED"
+ shell: echo "{{log_me_prefix}}TASK_SUCCEEDED"
- name: failed args should be logged in the absence of no_log
- shell: echo "LOG_ME_TASK_FAILED"
+ shell: echo "{{log_me_prefix}}TASK_FAILED"
failed_when: true
ignore_errors: true
- name: item args should be logged in the absence of no_log
shell: echo {{ item }}
- with_items: [ "LOG_ME_ITEM", "LOG_ME_SKIPPED", "LOG_ME_ITEM_FAILED" ]
- when: item != "LOG_ME_SKIPPED"
- failed_when: item == "LOG_ME_ITEM_FAILED"
+ with_items: [ "{{log_me_prefix}}ITEM", "{{log_me_prefix}}SKIPPED", "{{log_me_prefix}}ITEM_FAILED" ]
+ when: item != log_me_prefix ~ "SKIPPED"
+ failed_when: item == log_me_prefix ~ "ITEM_FAILED"
ignore_errors: true
- name: args should not be logged when task-level no_log set
@@ -61,7 +64,7 @@
no_log: true
- name: args should be logged when task-level no_log overrides play-level
- shell: echo "LOG_ME_OVERRIDE"
+ shell: echo "{{log_me_prefix}}OVERRIDE"
no_log: false
- name: Add a fake host for next play
diff --git a/test/integration/targets/no_log/no_log_suboptions.yml b/test/integration/targets/no_log/no_log_suboptions.yml
index e67ecfe21b..338a871eed 100644
--- a/test/integration/targets/no_log/no_log_suboptions.yml
+++ b/test/integration/targets/no_log/no_log_suboptions.yml
@@ -5,20 +5,20 @@
tasks:
- name: Task with suboptions
module:
- secret: GLAMOROUS
+ secret: "{{ s106 }}"
subopt_dict:
- str_sub_opt1: AFTERMATH
+ str_sub_opt1: "{{ s107 }}"
str_sub_opt2: otherstring
nested_subopt:
- n_subopt1: MANPOWER
+ n_subopt1: "{{ s101 }}"
subopt_list:
- - subopt1: UNTAPPED
+ - subopt1: "{{ s102 }}"
subopt2: thridstring
- - subopt1: CONCERNED
+ - subopt1: "{{ s103 }}"
- name: Task with suboptions as string
module:
- secret: MARLIN
- subopt_dict: str_sub_opt1=FLICK
+ secret: "{{ s104 }}"
+ subopt_dict: str_sub_opt1={{ s105 }}
diff --git a/test/integration/targets/no_log/no_log_suboptions_invalid.yml b/test/integration/targets/no_log/no_log_suboptions_invalid.yml
index 933a8a9bb2..1092cf5fbe 100644
--- a/test/integration/targets/no_log/no_log_suboptions_invalid.yml
+++ b/test/integration/targets/no_log/no_log_suboptions_invalid.yml
@@ -4,42 +4,45 @@
ignore_errors: yes
tasks:
+ - include_vars: secretvars.yml
+ no_log: true
+
- name: Task with suboptions and invalid parameter
module:
- secret: SUPREME
+ secret: "{{ s201 }}"
invalid: param
subopt_dict:
- str_sub_opt1: IDIOM
+ str_sub_opt1: "{{ s202 }}"
str_sub_opt2: otherstring
nested_subopt:
- n_subopt1: MOCKUP
+ n_subopt1: "{{ s203 }}"
subopt_list:
- - subopt1: EDUCATED
+ - subopt1: "{{ s204 }}"
subopt2: thridstring
- - subopt1: FOOTREST
+ - subopt1: "{{ s205 }}"
- name: Task with suboptions as string with invalid parameter
module:
- secret: FOOTREST
+ secret: "{{ s213 }}"
invalid: param
- subopt_dict: str_sub_opt1=CRAFTY
+ subopt_dict: str_sub_opt1={{ s206 }}
- name: Task with suboptions with dict instead of list
module:
- secret: FELINE
+ secret: "{{ s207 }}"
subopt_dict:
- str_sub_opt1: CRYSTAL
+ str_sub_opt1: "{{ s208 }}"
str_sub_opt2: otherstring
nested_subopt:
- n_subopt1: EXPECTANT
+ n_subopt1: "{{ s209 }}"
subopt_list:
foo: bar
- name: Task with suboptions with incorrect data type
module:
- secret: AGROUND
+ secret: "{{ s210 }}"
subopt_dict: 9068.21361
subopt_list:
- - subopt1: GOLIATH
- - subopt1: FREEFALL
+ - subopt1: "{{ s211 }}"
+ - subopt1: "{{ s212 }}"
diff --git a/test/integration/targets/no_log/runme.sh b/test/integration/targets/no_log/runme.sh
index 8bfe019bb9..d6476ac69c 100755
--- a/test/integration/targets/no_log/runme.sh
+++ b/test/integration/targets/no_log/runme.sh
@@ -1,6 +1,12 @@
#!/usr/bin/env bash
-set -eux
+set -eux -o pipefail
+
+# ensure _ansible_no_log returned by actions is actually respected
+ansible-playbook ansible_no_log_in_result.yml -vvvvv > "${OUTPUT_DIR}/output.log" 2> /dev/null
+
+[ "$(grep -c "action result should be masked" "${OUTPUT_DIR}/output.log")" = "0" ]
+[ "$(grep -c "the output has been hidden" "${OUTPUT_DIR}/output.log")" = "4" ]
# This test expects 7 loggable vars and 0 non-loggable ones.
# If either mismatches it fails, run the ansible-playbook command to debug.
@@ -9,18 +15,18 @@ set -eux
# deal with corner cases with no log and loops
# no log enabled, should produce 6 censored messages
-[ "$(ansible-playbook dynamic.yml -i ../../inventory -vvvvv "$@" -e unsafe_show_logs=no|grep -c 'output has been hidden')" = "6" ]
+[ "$(ansible-playbook dynamic.yml -i ../../inventory -vvvvv "$@" -e unsafe_show_logs=no|grep -c 'output has been hidden')" = "6" ] # DT needs 7
# no log disabled, should produce 0 censored
[ "$(ansible-playbook dynamic.yml -i ../../inventory -vvvvv "$@" -e unsafe_show_logs=yes|grep -c 'output has been hidden')" = "0" ]
# test no log for sub options
-[ "$(ansible-playbook no_log_suboptions.yml -i ../../inventory -vvvvv "$@" | grep -Ec '(MANPOWER|UNTAPPED|CONCERNED|MARLIN|FLICK)')" = "0" ]
+[ "$(ansible-playbook no_log_suboptions.yml -i ../../inventory -vvvvv "$@" | grep -Ec 'SECRET')" = "0" ]
# test invalid data passed to a suboption
-[ "$(ansible-playbook no_log_suboptions_invalid.yml -i ../../inventory -vvvvv "$@" | grep -Ec '(SUPREME|IDIOM|MOCKUP|EDUCATED|FOOTREST|CRAFTY|FELINE|CRYSTAL|EXPECTANT|AGROUND|GOLIATH|FREEFALL)')" = "0" ]
+[ "$(ansible-playbook no_log_suboptions_invalid.yml -i ../../inventory -vvvvv "$@" | grep -Ec 'SECRET')" = "0" ]
# test variations on ANSIBLE_NO_LOG
[ "$(ansible-playbook no_log_config.yml -i ../../inventory -vvvvv "$@" | grep -Ec 'the output has been hidden')" = "1" ]
[ "$(ANSIBLE_NO_LOG=0 ansible-playbook no_log_config.yml -i ../../inventory -vvvvv "$@" | grep -Ec 'the output has been hidden')" = "1" ]
-[ "$(ANSIBLE_NO_LOG=1 ansible-playbook no_log_config.yml -i ../../inventory -vvvvv "$@" | grep -Ec 'the output has been hidden')" = "6" ]
+[ "$(ANSIBLE_NO_LOG=1 ansible-playbook no_log_config.yml -i ../../inventory -vvvvv "$@" | grep -Ec 'the output has been hidden')" = "6" ] # DT needs 5
diff --git a/test/integration/targets/no_log/secretvars.yml b/test/integration/targets/no_log/secretvars.yml
new file mode 100644
index 0000000000..0030d747d7
--- /dev/null
+++ b/test/integration/targets/no_log/secretvars.yml
@@ -0,0 +1,32 @@
+# These values are in a separate vars file and referenced dynamically to avoid spurious counts from contextual error messages
+# that show the playbook contents inline (since unencrypted playbook contents are not considered secret).
+log_me_prefix: LOG_ME_
+
+# Unique values are used for each secret below to ensure that one secret "learned" does not cause another non-secret
+# value to be considered secret simply because they share the same value. A common substring is, however, present in
+# each one to simplify searching for secret values in test output. Having a unique value for each also helps in
+# debugging when unexpected output is encountered.
+
+# secrets for no_log_suboptions.yml
+s101: SECRET101
+s102: SECRET102
+s103: SECRET103
+s104: SECRET104
+s105: SECRET105
+s106: SECRET106
+s107: SECRET107
+
+# secrets for no_log_suboptions_invalid.yml
+s201: SECRET201
+s202: SECRET202
+s203: SECRET203
+s204: SECRET204
+s205: SECRET205
+s206: SECRET206
+s207: SECRET207
+s208: SECRET208
+s209: SECRET209
+s210: SECRET210
+s211: SECRET211
+s212: SECRET212
+s213: SECRET213
--
2.48.1
