new file mode 100644

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

@@ -1665,6 +1665,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,

@@ -2032,7 +2060,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)

new file mode 100644

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

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

new file mode 100644

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

new file mode 100644

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

@@ -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" ]