diff options
author | Sam Doran <sdoran@redhat.com> | 2019-10-02 18:04:08 -0400 |
---|---|---|
committer | Toshio Kuratomi <a.badger@gmail.com> | 2019-10-14 15:31:32 -0700 |
commit | 87f8d77d70476454f7fe2381bd363a329ce4266c (patch) | |
tree | e776ab43aae3e3a713d8a10ee06f6043c0a9600c | |
parent | 9849fc183d10656103f27163def2fe6b738fa44d (diff) | |
download | ansible-87f8d77d70476454f7fe2381bd363a329ce4266c.tar.gz |
[stable-2.6] Properly mask no_log values is sub parameters during failure (#63405)
ci_complete
(cherry picked from commit 99ddd411a6)
Co-authored-by: Sam Doran <sdoran@redhat.com>
-rw-r--r-- | changelogs/fragments/no-log-sub-options-invalid-parameter.yaml | 2 | ||||
-rw-r--r-- | lib/ansible/module_utils/basic.py | 29 | ||||
-rw-r--r-- | test/integration/targets/no_log/library/module.py | 45 | ||||
-rw-r--r-- | test/integration/targets/no_log/no_log_local.yml | 1 | ||||
-rw-r--r-- | test/integration/targets/no_log/no_log_suboptions.yml | 24 | ||||
-rw-r--r-- | test/integration/targets/no_log/no_log_suboptions_invalid.yml | 45 | ||||
-rwxr-xr-x | test/integration/targets/no_log/runme.sh | 6 |
7 files changed, 151 insertions, 1 deletions
diff --git a/changelogs/fragments/no-log-sub-options-invalid-parameter.yaml b/changelogs/fragments/no-log-sub-options-invalid-parameter.yaml new file mode 100644 index 0000000000..79019d64cf --- /dev/null +++ b/changelogs/fragments/no-log-sub-options-invalid-parameter.yaml @@ -0,0 +1,2 @@ +bugfixes: + - '**security issue** - properly hide parameters marked with ``no_log`` in suboptions when invalid parameters are passed to the module (CVE-2019-14858)' diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index a3e3916e71..b57a532a86 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1686,6 +1686,34 @@ class AnsibleModule(object): if no_log_object: self.no_log_values.update(return_values(no_log_object)) + # Get no_log values from suboptions + sub_argument_spec = arg_opts.get('options') + if sub_argument_spec is not None: + wanted_type = arg_opts.get('type') + sub_parameters = param.get(arg_name) + + if sub_parameters is not None: + if wanted_type == 'dict' or (wanted_type == 'list' and arg_opts.get('elements', '') == 'dict'): + # Sub parameters can be a dict or list of dicts. Ensure parameters are always a list. + if not isinstance(sub_parameters, list): + sub_parameters = [sub_parameters] + + for sub_param in sub_parameters: + # Validate dict fields in case they came in as strings + + if isinstance(sub_param, string_types): + sub_param = self._check_type_dict(sub_param) + + try: + if not isinstance(sub_param, Mapping): + raise TypeError("Value '{1}' in the sub parameter field '{0}' must by a {2}, " + "not '{1.__class__.__name__}'".format(arg_name, sub_param, wanted_type)) + except TypeError as te: + self.fail_json(msg="Failure when processing no_log parameters. Module invocation will be hidden. " + "%s" % to_native(te), invocation={'module_args': 'HIDDEN DUE TO FAILURE'}) + + self._handle_no_log_values(sub_argument_spec, sub_param) + if arg_opts.get('removed_in_version') is not None and arg_name in param: self._deprecations.append({ 'msg': "Param '%s' is deprecated. See the module docs for more information" % arg_name, @@ -2053,7 +2081,6 @@ class AnsibleModule(object): self._set_fallbacks(spec, param) options_aliases = self._handle_aliases(spec, param) - self._handle_no_log_values(spec, param) options_legal_inputs = list(spec.keys()) + list(options_aliases.keys()) self._check_arguments(self.check_invalid_arguments, spec, param, options_legal_inputs) diff --git a/test/integration/targets/no_log/library/module.py b/test/integration/targets/no_log/library/module.py new file mode 100644 index 0000000000..d4f3c565cf --- /dev/null +++ b/test/integration/targets/no_log/library/module.py @@ -0,0 +1,45 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- +# Copyright (c) 2019 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 + + +def main(): + module = AnsibleModule( + argument_spec={ + 'state': {}, + 'secret': {'no_log': True}, + 'subopt_dict': { + 'type': 'dict', + 'options': { + 'str_sub_opt1': {'no_log': True}, + 'str_sub_opt2': {}, + 'nested_subopt': { + 'type': 'dict', + 'options': { + 'n_subopt1': {'no_log': True}, + } + } + } + }, + 'subopt_list': { + 'type': 'list', + 'elements': 'dict', + 'options': { + 'subopt1': {'no_log': True}, + 'subopt2': {}, + } + } + + } + ) + module.exit_json(msg='done') + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/no_log/no_log_local.yml b/test/integration/targets/no_log/no_log_local.yml index aacf7de276..41b1858e62 100644 --- a/test/integration/targets/no_log/no_log_local.yml +++ b/test/integration/targets/no_log/no_log_local.yml @@ -47,6 +47,7 @@ poll: 1 shell: echo "DO_NOT_LOG_ASYNC_TASK_SUCCEEDED" no_log: true + ignore_errors: yes - name: play-level no_log set hosts: testhost diff --git a/test/integration/targets/no_log/no_log_suboptions.yml b/test/integration/targets/no_log/no_log_suboptions.yml new file mode 100644 index 0000000000..e67ecfe21b --- /dev/null +++ b/test/integration/targets/no_log/no_log_suboptions.yml @@ -0,0 +1,24 @@ +- name: test no log with suboptions + hosts: testhost + gather_facts: no + + tasks: + - name: Task with suboptions + module: + secret: GLAMOROUS + subopt_dict: + str_sub_opt1: AFTERMATH + str_sub_opt2: otherstring + nested_subopt: + n_subopt1: MANPOWER + + subopt_list: + - subopt1: UNTAPPED + subopt2: thridstring + + - subopt1: CONCERNED + + - name: Task with suboptions as string + module: + secret: MARLIN + subopt_dict: str_sub_opt1=FLICK diff --git a/test/integration/targets/no_log/no_log_suboptions_invalid.yml b/test/integration/targets/no_log/no_log_suboptions_invalid.yml new file mode 100644 index 0000000000..933a8a9bb2 --- /dev/null +++ b/test/integration/targets/no_log/no_log_suboptions_invalid.yml @@ -0,0 +1,45 @@ +- name: test no log with suboptions + hosts: testhost + gather_facts: no + ignore_errors: yes + + tasks: + - name: Task with suboptions and invalid parameter + module: + secret: SUPREME + invalid: param + subopt_dict: + str_sub_opt1: IDIOM + str_sub_opt2: otherstring + nested_subopt: + n_subopt1: MOCKUP + + subopt_list: + - subopt1: EDUCATED + subopt2: thridstring + - subopt1: FOOTREST + + - name: Task with suboptions as string with invalid parameter + module: + secret: FOOTREST + invalid: param + subopt_dict: str_sub_opt1=CRAFTY + + - name: Task with suboptions with dict instead of list + module: + secret: FELINE + subopt_dict: + str_sub_opt1: CRYSTAL + str_sub_opt2: otherstring + nested_subopt: + n_subopt1: EXPECTANT + subopt_list: + foo: bar + + - name: Task with suboptions with incorrect data type + module: + secret: AGROUND + subopt_dict: 9068.21361 + subopt_list: + - subopt1: GOLIATH + - subopt1: FREEFALL diff --git a/test/integration/targets/no_log/runme.sh b/test/integration/targets/no_log/runme.sh index 474e755e13..bb5c048fc9 100755 --- a/test/integration/targets/no_log/runme.sh +++ b/test/integration/targets/no_log/runme.sh @@ -13,3 +13,9 @@ set -eux # 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" ] |