From 11ef03e9dd1dc05fd80bbe74eafd89b3645965cf Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Mon, 6 Apr 2020 12:13:04 -0400 Subject: [PATCH] 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 Co-authored-by: Toshio Kuratomi Co-authored-by: Felix Fontein --- changelogs/fragments/ldap-params-removal.yml | 8 ++++++ plugins/modules/net_tools/ldap/ldap_attr.py | 18 ++++++-------- plugins/modules/net_tools/ldap/ldap_entry.py | 26 +++++++------------- tests/sanity/ignore-2.10.txt | 3 +++ tests/sanity/ignore-2.9.txt | 3 +++ 5 files changed, 31 insertions(+), 27 deletions(-) create mode 100644 changelogs/fragments/ldap-params-removal.yml diff --git a/changelogs/fragments/ldap-params-removal.yml b/changelogs/fragments/ldap-params-removal.yml new file mode 100644 index 0000000000..8e28b6aa37 --- /dev/null +++ b/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" diff --git a/plugins/modules/net_tools/ldap/ldap_attr.py b/plugins/modules/net_tools/ldap/ldap_attr.py index 80dde691b8..217c3ef286 100644 --- a/plugins/modules/net_tools/ldap/ldap_attr.py +++ b/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 'params' in module.params and isinstance(module.params['params'], dict): - module.params.update(module.params['params']) - # Remove the params - module.params.pop('params', None) + if module.params['params']: + module.fail_json(msg="The `params` option to ldap_attr was removed in since it circumvents Ansible's option handling") # Instantiate the LdapAttr object ldap = LdapAttr(module) diff --git a/plugins/modules/net_tools/ldap/ldap_entry.py b/plugins/modules/net_tools/ldap/ldap_entry.py index 6466a40263..8367b19e1b 100644 --- a/plugins/modules/net_tools/ldap/ldap_entry.py +++ b/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) diff --git a/tests/sanity/ignore-2.10.txt b/tests/sanity/ignore-2.10.txt index 3980225ac2..fcd6783239 100644 --- a/tests/sanity/ignore-2.10.txt +++ b/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 diff --git a/tests/sanity/ignore-2.9.txt b/tests/sanity/ignore-2.9.txt index 4837c7b3dc..1b022de009 100644 --- a/tests/sanity/ignore-2.9.txt +++ b/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