Remove the params module option from ldap_attr and ldap_entry (#113)

* Remove the params module option from ldap_attr and ldap_entry

Module options that circumvent Ansible's option handling were disallowed
in:
https://meetbot.fedoraproject.org/ansible-meeting/2017-09-28/ansible_dev_meeting.2017-09-28-15.00.log.html

Additionally, this particular usage can be insecure if bind_pw is set
this way as the password could end up in a logfile or displayed on
stdout.

Fixes CVE-2020-1746

* Remove checking the version of Ansible

Fix fail_json

* Apply suggestions from code review

Co-Authored-By: Felix Fontein <felix@fontein.de>

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
Co-authored-by: Felix Fontein <felix@fontein.de>

changelogs/fragments/ldap-params-removal.yml

@@ -0,0 +1,8 @@
+removed_features:
+  - "ldap_attr, ldap_entry - The ``params`` option has been removed in
+    Ansible-2.10 as it circumvents Ansible's option handling.  Setting
+    ``bind_pw`` with the ``params`` option was disallowed in Ansible-2.7, 2.8,
+    and 2.9 as it was insecure.  For information about this policy, see the
+discussion at:
+    https://meetbot.fedoraproject.org/ansible-meeting/2017-09-28/ansible_dev_meeting.2017-09-28-15.00.log.html
+    This fixes CVE-2020-1746"

plugins/modules/net_tools/ldap/ldap_attr.py

@@ -35,6 +35,9 @@ notes:
     rules. This should work out in most cases, but it is theoretically
     possible to see spurious changes when target and actual values are
     semantically identical but lexically distinct.
+  - "The I(params) parameter was removed due to circumventing Ansible's parameter
+     handling.  The I(params) parameter started disallowing setting the I(bind_pw) parameter in
+     Ansible-2.7 as it was insecure to set the parameter that way."
 deprecated:
   removed_in: '2.14'
   why: 'The current "ldap_attr" module does not support LDAP attribute insertions or deletions with objectClass dependencies.'
@@ -66,10 +69,6 @@ options:
         a list of strings (see examples).
     type: raw
     required: true
-  params:
-    description:
-    - Additional module parameters.
-    type: dict
 extends_documentation_fragment:
 - community.general.ldap.documentation
 
@@ -138,13 +137,15 @@ EXAMPLES = r'''
 #   server_uri: ldap://localhost/
 #   bind_dn: cn=admin,dc=example,dc=com
 #   bind_pw: password
+#
+# In the example below, 'args' is a task keyword, passed at the same level as the module
 - name: Get rid of an unneeded attribute
   ldap_attr:
     dn: uid=jdoe,ou=people,dc=example,dc=com
     name: shadowExpire
     values: []
     state: exact
-    params: "{{ ldap_auth }}"
+  args: "{{ ldap_auth }}"
 '''
 
 RETURN = r'''
@@ -255,11 +256,8 @@ def main():
         module.fail_json(msg=missing_required_lib('python-ldap'),
                          exception=LDAP_IMP_ERR)
 
-    # Update module parameters with user's parameters if defined
+    if module.params['params']:
-    if 'params' in module.params and isinstance(module.params['params'], dict):
+        module.fail_json(msg="The `params` option to ldap_attr was removed in since it circumvents Ansible's option handling")
-        module.params.update(module.params['params'])
-        # Remove the params
-        module.params.pop('params', None)
 
     # Instantiate the LdapAttr object
     ldap = LdapAttr(module)

plugins/modules/net_tools/ldap/ldap_entry.py

@@ -32,6 +32,9 @@ notes:
     rule allowing root to modify the server configuration. If you need to use
     a simple bind to access your server, pass the credentials in I(bind_dn)
     and I(bind_pw).
+  - "The I(params) parameter was removed due to circumventing Ansible's parameter
+     handling.  The I(params) parameter started disallowing setting the I(bind_pw) parameter in
+     Ansible-2.7 as it was insecure to set the parameter that way."
 author:
   - Jiri Tyr (@jtyr)
 requirements:
@@ -47,11 +50,6 @@ options:
       - If I(state=present), value or list of values to use when creating
         the entry. It can either be a string or an actual list of
         strings.
-  params:
-    description:
-      - List of options which allows to overwrite any of the task or the
-        I(attributes) options. To remove an option, set the value of the option
-        to C(null).
   state:
     description:
       - The target state of the entry.
@@ -95,11 +93,13 @@ EXAMPLES = """
 #   server_uri: ldap://localhost/
 #   bind_dn: cn=admin,dc=example,dc=com
 #   bind_pw: password
+#
+# In the example below, 'args' is a task keyword, passed at the same level as the module
 - name: Get rid of an old entry
   ldap_entry:
     dn: ou=stuff,dc=example,dc=com
     state: absent
-    params: "{{ ldap_auth }}"
+  args: "{{ ldap_auth }}"
 """
 
 
@@ -205,6 +205,9 @@ def main():
         module.fail_json(msg=missing_required_lib('python-ldap'),
                          exception=LDAP_IMP_ERR)
 
+    if module.params['params']:
+        module.fail_json(msg="The `params` option to ldap_attr was removed since it circumvents Ansible's option handling")
+
     state = module.params['state']
 
     # Check if objectClass is present when needed
@@ -218,17 +221,6 @@ def main():
                 isinstance(module.params['objectClass'], list))):
         module.fail_json(msg="objectClass must be either a string or a list.")
 
-    # Update module parameters with user's parameters if defined
-    if 'params' in module.params and isinstance(module.params['params'], dict):
-        for key, val in module.params['params'].items():
-            if key in module.argument_spec:
-                module.params[key] = val
-            else:
-                module.params['attributes'][key] = val
-
-        # Remove the params
-        module.params.pop('params', None)
-
     # Instantiate the LdapEntry object
     ldap = LdapEntry(module)
 

tests/sanity/ignore-2.10.txt

@@ -1100,8 +1100,11 @@ plugins/modules/net_tools/dnsmadeeasy.py validate-modules:parameter-type-not-in-
 plugins/modules/net_tools/ip_netns.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ipinfoio_facts.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ipinfoio_facts.py validate-modules:parameter-type-not-in-doc
+plugins/modules/net_tools/ldap/ldap_attr.py validate-modules:parameter-type-not-in-doc  # This triggers when a parameter is undocumented
+plugins/modules/net_tools/ldap/ldap_attr.py validate-modules:undocumented-parameter # Parameter removed but reason for removal is shown by custom code
 plugins/modules/net_tools/ldap/ldap_entry.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ldap/ldap_entry.py validate-modules:parameter-type-not-in-doc
+plugins/modules/net_tools/ldap/ldap_entry.py validate-modules:undocumented-parameter # Parameter removed but reason for removal is shown by custom code
 plugins/modules/net_tools/ldap/ldap_passwd.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ldap/ldap_passwd.py validate-modules:doc-required-mismatch
 plugins/modules/net_tools/netcup_dns.py validate-modules:doc-missing-type

tests/sanity/ignore-2.9.txt

@@ -1116,8 +1116,11 @@ plugins/modules/net_tools/dnsmadeeasy.py validate-modules:parameter-type-not-in-
 plugins/modules/net_tools/ip_netns.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ipinfoio_facts.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ipinfoio_facts.py validate-modules:parameter-type-not-in-doc
+plugins/modules/net_tools/ldap/ldap_attr.py validate-modules:parameter-type-not-in-doc  # This triggers when a parameter is undocumented
+plugins/modules/net_tools/ldap/ldap_attr.py validate-modules:undocumented-parameter # Parameter removed but reason for removal is shown by custom code
 plugins/modules/net_tools/ldap/ldap_entry.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ldap/ldap_entry.py validate-modules:parameter-type-not-in-doc
+plugins/modules/net_tools/ldap/ldap_entry.py validate-modules:undocumented-parameter # Parameter removed but reason for removal is shown by custom code
 plugins/modules/net_tools/ldap/ldap_passwd.py validate-modules:doc-missing-type
 plugins/modules/net_tools/ldap/ldap_passwd.py validate-modules:doc-required-mismatch
 plugins/modules/net_tools/netcup_dns.py validate-modules:doc-missing-type