no_log mask suboption fallback values and defaults CVE-2021-20228 (#73487) (#73492)

(cherry picked from commit 0cdc410dce6658e93c06fa27e0100ddbb11e7015)

7 files changed, 125 insertions, 11 deletions

diff --git a/changelogs/fragments/no_log-fallback.yml b/changelogs/fragments/no_log-fallback.yml
new file mode 100644
index 0000000000..6947531358
--- /dev/null
+++ b/changelogs/fragments/no_log-fallback.yml
@@ -0,0 +1,2 @@
+security_fixes:
+  - '**security issue** - Mask default and fallback values for ``no_log`` module options (CVE-2021-20228)'
diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py
index a5c45393c0..5dfcad268f 100644
--- a/lib/ansible/module_utils/basic.py
+++ b/lib/ansible/module_utils/basic.py
@@ -720,6 +720,9 @@ class AnsibleModule(object):
                 if k not in self.argument_spec:
                     self.argument_spec[k] = v
 
+        # Save parameter values that should never be logged
+        self.no_log_values = set()
+
         self._load_params()
         self._set_fallbacks()
 
@@ -731,8 +734,6 @@ class AnsibleModule(object):
             print('\n{"failed": true, "msg": "Module alias error: %s"}' % to_native(e))
             sys.exit(1)
 
-        # Save parameter values that should never be logged
-        self.no_log_values = set()
         self._handle_no_log_values()
 
         # check the locale as set by the current environment, and reset to
@@ -1893,14 +1894,15 @@ class AnsibleModule(object):
             param = self.params
         for (k, v) in spec.items():
             default = v.get('default', None)
-            if pre is True:
-                # this prevents setting defaults on required items
-                if default is not None and k not in param:
-                    param[k] = default
-            else:
-                # make sure things without a default still get set None
-                if k not in param:
-                    param[k] = default
+
+            # This prevents setting defaults on required items on the 1st run,
+            # otherwise will set things without a default to None on the 2nd.
+            if k not in param and (default is not None or not pre):
+                # Make sure any default value for no_log fields are masked.
+                if v.get('no_log', False) and default:
+                    self.no_log_values.add(default)
+
+                param[k] = default
 
     def _set_fallbacks(self, spec=None, param=None):
         if spec is None:
@@ -1920,9 +1922,13 @@ class AnsibleModule(object):
                     else:
                         fallback_args = item
                 try:
-                    param[k] = fallback_strategy(*fallback_args, **fallback_kwargs)
+                    fallback_value = fallback_strategy(*fallback_args, **fallback_kwargs)
                 except AnsibleFallbackNotFound:
                     continue
+                else:
+                    if v.get('no_log', False) and fallback_value:
+                        self.no_log_values.add(fallback_value)
+                    param[k] = fallback_value
 
     def _load_params(self):
         ''' read the input and set the params attribute.
diff --git a/test/integration/targets/module_utils/callback/pure_json.py b/test/integration/targets/module_utils/callback/pure_json.py
new file mode 100644
index 0000000000..1723d7bbe8
--- /dev/null
+++ b/test/integration/targets/module_utils/callback/pure_json.py
@@ -0,0 +1,31 @@
+# (c) 2021 Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+DOCUMENTATION = '''
+    name: pure_json
+    type: stdout
+    short_description: only outputs the module results as json
+'''
+
+import json
+
+from ansible.plugins.callback import CallbackBase
+
+
+class CallbackModule(CallbackBase):
+
+    CALLBACK_VERSION = 2.0
+    CALLBACK_TYPE = 'stdout'
+    CALLBACK_NAME = 'pure_json'
+
+    def v2_runner_on_failed(self, result, ignore_errors=False):
+        self._display.display(json.dumps(result._result))
+
+    def v2_runner_on_ok(self, result):
+        self._display.display(json.dumps(result._result))
+
+    def v2_runner_on_skipped(self, result):
+        self._display.display(json.dumps(result._result))
diff --git a/test/integration/targets/module_utils/library/test_no_log.py b/test/integration/targets/module_utils/library/test_no_log.py
new file mode 100644
index 0000000000..770e0b3a17
--- /dev/null
+++ b/test/integration/targets/module_utils/library/test_no_log.py
@@ -0,0 +1,35 @@
+#!/usr/bin/python
+# (c) 2021 Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+
+from ansible.module_utils.basic import AnsibleModule, env_fallback
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict(
+            explicit_pass=dict(type='str', no_log=True),
+            fallback_pass=dict(type='str', no_log=True, fallback=(env_fallback, ['SECRET_ENV'])),
+            default_pass=dict(type='str', no_log=True, default='zyx'),
+            normal=dict(type='str', default='plaintext'),
+            suboption=dict(
+                type='dict',
+                options=dict(
+                    explicit_sub_pass=dict(type='str', no_log=True),
+                    fallback_sub_pass=dict(type='str', no_log=True, fallback=(env_fallback, ['SECRET_SUB_ENV'])),
+                    default_sub_pass=dict(type='str', no_log=True, default='xvu'),
+                    normal=dict(type='str', default='plaintext'),
+                ),
+            ),
+        ),
+    )
+
+    module.exit_json(changed=False)
+
+
+if __name__ == '__main__':
+    main()
diff --git a/test/integration/targets/module_utils/module_utils_test_no_log.yml b/test/integration/targets/module_utils/module_utils_test_no_log.yml
new file mode 100644
index 0000000000..bad2efd495
--- /dev/null
+++ b/test/integration/targets/module_utils/module_utils_test_no_log.yml
@@ -0,0 +1,9 @@
+# This is called by module_utils_vvvvv.yml with a custom callback
+- hosts: testhost
+  gather_facts: no
+  tasks:
+    - name: Check no_log invocation results
+      test_no_log:
+        explicit_pass: abc
+        suboption:
+          explicit_sub_pass: def
diff --git a/test/integration/targets/module_utils/module_utils_vvvvv.yml b/test/integration/targets/module_utils/module_utils_vvvvv.yml
new file mode 100644
index 0000000000..1fd91d252f
--- /dev/null
+++ b/test/integration/targets/module_utils/module_utils_vvvvv.yml
@@ -0,0 +1,27 @@
+- hosts: testhost
+  gather_facts: no
+  tasks:
+  # Invocation usually is output with 3vs or more, our callback plugin displays it anyway
+  - name: Check no_log invocation results
+    command: ansible-playbook -i {{ inventory_file }} module_utils_test_no_log.yml
+    environment:
+      ANSIBLE_CALLBACK_PLUGINS: callback
+      ANSIBLE_STDOUT_CALLBACK: pure_json
+      SECRET_ENV: ghi
+      SECRET_SUB_ENV: jkl
+    register: no_log_invocation
+
+  - set_fact:
+      no_log_invocation: '{{ no_log_invocation.stdout | trim | from_json }}'
+
+  - name: check no log values from fallback or default are masked
+    assert:
+      that:
+      - no_log_invocation.invocation.module_args.default_pass == 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+      - no_log_invocation.invocation.module_args.explicit_pass == 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+      - no_log_invocation.invocation.module_args.fallback_pass == 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+      - no_log_invocation.invocation.module_args.normal == 'plaintext'
+      - no_log_invocation.invocation.module_args.suboption.default_sub_pass == 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+      - no_log_invocation.invocation.module_args.suboption.explicit_sub_pass == 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+      - no_log_invocation.invocation.module_args.suboption.fallback_sub_pass == 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
+      - no_log_invocation.invocation.module_args.suboption.normal == 'plaintext'
diff --git a/test/integration/targets/module_utils/runme.sh b/test/integration/targets/module_utils/runme.sh
index efbb7f8208..6adb84560b 100755
--- a/test/integration/targets/module_utils/runme.sh
+++ b/test/integration/targets/module_utils/runme.sh
@@ -2,5 +2,9 @@
 
 set -eux
 
+# Keep the -vvvvv here. This acts as a test for testing that higher verbosity
+# doesn't traceback with unicode in the custom module_utils directory path.
+ansible-playbook module_utils_vvvvv.yml -i ../../inventory -vvvvv "$@"
+
 ansible-playbook module_utils_test.yml -i ../../inventory -v "$@"
 ANSIBLE_MODULE_UTILS=other_mu_dir ansible-playbook module_utils_envvar.yml -i ../../inventory -v "$@"