Remove the params module option from ldap_attr and ldap_entry

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 @@
     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 C(params) parameter was removed in Ansible-2.10 due to circumventing Ansible's parameter
+     handling.  The C(params) parameter started disallowing setting the 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 @@
         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 @@
 #   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'''
@@ -156,6 +157,7 @@
 '''
 
 import traceback
+from distutils.version import LooseVersion
 
 from ansible.module_utils.basic import AnsibleModule, missing_required_lib
 from ansible.module_utils._text import to_native, to_bytes
@@ -255,11 +257,28 @@ 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 'params' in module.params and isinstance(module.params['params'], dict):
+    # For Ansible-2.9.x and below, allow the params module parameter with a warning
+    if LooseVersion(module.ansible_version) < LooseVersion('2.10'):
+        if module.params['params']:
+            module.deprecate("The `params` option to ldap_attr will be removed in Ansible 2.10"
+                             " since it circumvents Ansible's option handling", version='2.10')
+
+        # However, the bind_pw parameter contains a password so it **must** go through the normal
+        # argument parsing even though removing it breaks backwards compat.
+        if 'bind_pw' in module.params['params']:
+            module.fail_json("Using `bind_pw` with the `params` option has been disallowed since"
+                             " it is insecure.  Use the `bind_pw` option directly.  The `params`"
+                             " option will be removed in Ansible-2.10")
+
+        # Update module parameters with user's parameters if defined
         module.params.update(module.params['params'])
-        # Remove the params
+        # Remove params itself
         module.params.pop('params', None)
+    else:
+        # For Ansible 2.10 and above
+        if module.params['params']:
+            module.fail_json("The `params` option to ldap_attr was removed in Ansible-2.10 since"
+                             " it circumvents Ansible's option handling")
 
     # Instantiate the LdapAttr object
     ldap = LdapAttr(module)

plugins/modules/net_tools/ldap/ldap_entry.py

@@ -32,6 +32,9 @@
     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 C(params) parameter was removed in Ansible-2.10 due to circumventing Ansible's parameter
+     handling.  The C(params) parameter started disallowing setting the 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 @@
       - 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 @@
 #   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 }}"
 """
 
 
@@ -108,6 +108,7 @@
 """
 
 import traceback
+from distutils.version import LooseVersion
 
 from ansible.module_utils.basic import AnsibleModule, missing_required_lib
 from ansible.module_utils.six import string_types
@@ -205,6 +206,29 @@ def main():
         module.fail_json(msg=missing_required_lib('python-ldap'),
                          exception=LDAP_IMP_ERR)
 
+    # For Ansible-2.9.x and below, allow the params module parameter with a warning
+    if LooseVersion(module.ansible_version) < LooseVersion('2.10'):
+        if module.params['params']:
+            module.deprecate("The `params` option to ldap_attr will be removed in Ansible 2.10"
+                             " since it circumvents Ansible's option handling", version='2.10')
+
+        # However, the bind_pw parameter contains a password so it **must** go through the normal
+        # argument parsing even though removing it breaks backwards compat.
+        if 'bind_pw' in module.params['params']:
+            module.fail_json("Using `bind_pw` with the `params` option has been disallowed since"
+                             " it is insecure.  Use the `bind_pw` option directly.  The `params`"
+                             " option will be removed in Ansible-2.10")
+
+        # Update module parameters with user's parameters if defined
+        module.params.update(module.params['params'])
+        # Remove params itself
+        module.params.pop('params', None)
+    else:
+        # For Ansible 2.10 and above
+        if module.params['params']:
+            module.fail_json("The `params` option to ldap_attr was removed in Ansible-2.10 since"
+                             " it circumvents Ansible's option handling")
+
     state = module.params['state']
 
     # Check if objectClass is present when needed
@@ -218,17 +242,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