ansible/CVE-2024-8775.patch

From: Matt Davis <6775756+nitzmahone@users.noreply.github.com>
Date: Thu, 24 Oct 2024 15:56:54 -0700
Subject: CVE-2024-8775 Preserve `_ansible_no_log` from action result;
 fix `include_vars` to set properly (#84143)

* 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

[backport]
 "grep -q" exits on the first match, so the preceding
process in the pipeline gets SIGPIPE.  Changing "grep -q
'porter.*cable'" to "grep 'porter.*cable' >/dev/null" makes that test
pass.

origin: backport, https://github.com/ansible/ansible/commit/c9ac477e53a99e95781f333eec3329a935c1bf95
bug: https://github.com/ansible/ansible/issues/84217
bug-debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1086551
---
 changelogs/fragments/cve-2024-8775.yml             |  5 ++++
 lib/ansible/executor/task_executor.py              |  3 +-
 lib/ansible/plugins/action/include_vars.py         |  3 +-
 .../targets/include_vars-ad-hoc/dir/encrypted.yml  |  6 ++++
 .../targets/include_vars-ad-hoc/runme.sh           | 22 +++++++++++++--
 .../targets/include_vars-ad-hoc/vaultpass          |  3 ++
 .../no_log/action_plugins/action_sets_no_log.py    |  8 ++++++
 .../targets/no_log/ansible_no_log_in_result.yml    | 13 +++++++++
 test/integration/targets/no_log/dynamic.yml        | 29 +++++++++++++++-----
 test/integration/targets/no_log/no_log_config.yml  |  2 +-
 test/integration/targets/no_log/no_log_local.yml   | 15 ++++++----
 .../targets/no_log/no_log_suboptions.yml           | 14 +++++-----
 .../targets/no_log/no_log_suboptions_invalid.yml   | 29 +++++++++++---------
 test/integration/targets/no_log/runme.sh           | 16 +++++++----
 test/integration/targets/no_log/runme.sh.orig      | 26 ++++++++++++++++++
 test/integration/targets/no_log/secretvars.yml     | 32 ++++++++++++++++++++++
 16 files changed, 182 insertions(+), 44 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 100755 test/integration/targets/no_log/runme.sh.orig
 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 0000000..a292c99
--- /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 65bcacf..957d1bf 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -677,8 +677,9 @@ class TaskExecutor:
                 self._handler.cleanup()
             display.debug("handler run complete")
 
+            # propagate no log to result- the action can set this, so only overwrite it with the task's value if missing or falsey
             # preserve no log
-            result["_ansible_no_log"] = self._play_context.no_log
+            result["_ansible_no_log"] = bool(self._play_context.no_log or result.get('_ansible_no_log', False))
 
             # 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 0723453..bd67d01 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 0000000..328f180
--- /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 51b68d2..41a0929 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 'porter.*cable' 2>&1
+
+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 0000000..1f78d41
--- /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 0000000..cb42616
--- /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 0000000..a80a4a7
--- /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 4a1123d..9523677 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 8a50880..165f4e0 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 aacf7de..d2bc5ae 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 e67ecfe..338a871 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 933a8a9..1092cf5 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 8bfe019..d6476ac 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/runme.sh.orig b/test/integration/targets/no_log/runme.sh.orig
new file mode 100755
index 0000000..8bfe019
--- /dev/null
+++ b/test/integration/targets/no_log/runme.sh.orig
@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+
+set -eux
+
+# This test expects 7 loggable vars and 0 non-loggable ones.
+# If either mismatches it fails, run the ansible-playbook command to debug.
+[ "$(ansible-playbook no_log_local.yml -i ../../inventory -vvvvv "$@" | awk \
+'BEGIN { logme = 0; nolog = 0; } /LOG_ME/ { logme += 1;} /DO_NOT_LOG/ { nolog += 1;} END { printf "%d/%d", logme, nolog; }')" = "26/0" ]
+
+# 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" ]
+
+# 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" ]
+
+# 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" ]
+
+# 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" ]
diff --git a/test/integration/targets/no_log/secretvars.yml b/test/integration/targets/no_log/secretvars.yml
new file mode 100644
index 0000000..0030d74
--- /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