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

* no_log mask suboption fallback values and defaults

* Added changelog

* Remove lambda expression

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)'

lib/ansible/module_utils/basic.py

@@ -714,6 +714,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()
 
@@ -725,8 +728,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
@@ -1931,13 +1932,14 @@ 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:
+
+            # 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):
@@ -1958,9 +1960,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.

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

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()

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

test/integration/targets/module_utils/module_utils_vvvvv.yml

@@ -3,3 +3,28 @@
   tasks:
   - name: Use a specially crafted module to see if things were imported correctly
     test:
+
+  # 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'