[stable-2.18] Preserve `_ansible_no_log` from action result; fix `include_vars` to set properly (#84143) (#84179)

* 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 c9ac477e53)

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)

lib/ansible/executor/task_executor.py

@@ -684,8 +684,8 @@ class TaskExecutor:
                 self._handler.cleanup()
             display.debug("handler run complete")
 
-            # preserve 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"] = no_log
+            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)

lib/ansible/plugins/action/include_vars.py

@@ -237,7 +237,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()

test/integration/targets/include_vars-ad-hoc/dir/encrypted.yml

@@ -0,0 +1,6 @@
+$ANSIBLE_VAULT;1.1;AES256
+31613539636636336264396235633933633839646337323533316638633336653461393036336664
+3939386435313638366366626566346135623932653238360a366261303663343034633865626132
+31646231623630333636383636383833656331643164656366623332396439306132663264663131
+6439633766376261320a616265306430366530363866356433366430633265353739373732646536
+37623661333064306162373463616231636365373231313939373230643936313362

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' "$@"
+echo "single file include"
-ansible testhost -i ../../inventory -m include_vars -a 'dir=dir' "$@"
+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

test/integration/targets/include_vars-ad-hoc/vaultpass

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+echo supersecurepassword

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

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

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:
+      raw: echo {{ var_does_not_exist }}
-        msg: "SHOW {{ 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:
+      raw: echo {{ item }}
-        msg: "SHOW {{ 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:
+      raw: echo {{ var_does_not_exist }}
-        msg: "SHOW {{ 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

test/integration/targets/no_log/no_log_config.yml

@@ -10,4 +10,4 @@
     - debug:
 
     - debug:
-      loop: '{{ range(3) }}'
+      loop: '{{ range(3) | list }}'

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" ]
+      with_items: [ "{{log_me_prefix}}ITEM", "{{log_me_prefix}}SKIPPED", "{{log_me_prefix}}ITEM_FAILED" ]
-      when: item != "LOG_ME_SKIPPED"
+      when: item != log_me_prefix ~ "SKIPPED"
-      failed_when: item == "LOG_ME_ITEM_FAILED"
+      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

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
+        secret: "{{ s104 }}"
-        subopt_dict: str_sub_opt1=FLICK
+        subopt_dict: str_sub_opt1={{ s105 }}

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: "{{ s211 }}"
-          - subopt1: FREEFALL
+          - subopt1: "{{ s212 }}"

test/integration/targets/no_log/runme.sh

@@ -1,26 +1,32 @@
 #!/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.
 [ "$(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; }')" = "27/0" ]
+'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" ]
+[ "$(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

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