From f8ff395d817c3eddc050f809919c15dfb5796120 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Sat, 6 Feb 2021 03:35:30 +1000 Subject: no_log mask suboption fallback values and defaults CVE-2021-20228 (#73487) (#73492) (cherry picked from commit 0cdc410dce6658e93c06fa27e0100ddbb11e7015) --- changelogs/fragments/no_log-fallback.yml | 2 ++ lib/ansible/module_utils/basic.py | 28 ++++++++++------- .../targets/module_utils/callback/pure_json.py | 31 +++++++++++++++++++ .../targets/module_utils/library/test_no_log.py | 35 ++++++++++++++++++++++ .../module_utils/module_utils_test_no_log.yml | 9 ++++++ .../targets/module_utils/module_utils_vvvvv.yml | 27 +++++++++++++++++ test/integration/targets/module_utils/runme.sh | 4 +++ 7 files changed, 125 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/no_log-fallback.yml create mode 100644 test/integration/targets/module_utils/callback/pure_json.py create mode 100644 test/integration/targets/module_utils/library/test_no_log.py create mode 100644 test/integration/targets/module_utils/module_utils_test_no_log.yml create mode 100644 test/integration/targets/module_utils/module_utils_vvvvv.yml 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 "$@" -- cgit v1.2.1