ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
833 stars 1.53k forks source link

module java_cert ignores proxy authentication if http_proxy environment variable is set #4126

Open fmjaeschke opened 2 years ago

fmjaeschke commented 2 years ago

Summary

When the system environment "https_proxy" is set with the pattern http://USER:PASSWORD@HOST:PORT the authentication part is ignored. Only host and port will be set for keytool proxy settings.

Behaviour was mentioned before https://github.com/ansible/ansible/issues/54481

Issue Type

Bug Report

Component Name

java_cert

Ansible Version

$ ansible --version
ansible [core 2.12.1]
  config file = /home/lotte/.ansible.cfg
  configured module search path = ['/home/lotte/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.10/site-packages/ansible
  ansible collection location = /home/lotte/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.10.2 (main, Jan 15 2022, 19:56:27) [GCC 11.1.0]
  jinja version = 3.0.3
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general

# /usr/lib/python3.10/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 4.3.0  

Configuration

$ ansible-config dump --only-changed

OS / Environment

LSB Version: 1.4 Distributor ID: EndeavourOS Description: EndeavourOS Linux Release: rolling Codename: rolling

Steps to Reproduce

Expected Results

When environment variable "https_proxy" is set with the pattern http://USER:PASSWORD@HOST:PORT, keytool will not only be called with httpsProxyHost and httpsProxyPort settings, but with httpsProxyUser and httpsProxyPassword too.

Actual Results

Code of Conduct

ansibullbot commented 2 years ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 2 years ago

cc @absynth76 @haad click here for bot help

fmjaeschke commented 2 years ago

As my Python knowledge is vague at best and I don't know much about the unit test part . (tried to create patch against Git rev. 84124224aecd10086ff5b6db84aa6c0913b6d823 )

Found only integration tests for java_cert module, but whole proxy handling is absent there. https://github.com/ansible-collections/community.general/tree/main/tests/integration/targets/java_cert

--- a/plugins/modules/system/java_cert.py
+++ b/plugins/modules/system/java_cert.py
@@ -182,7 +182,7 @@ import re

 # import module snippets
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six.moves.urllib.parse import urlparse
+from ansible.module_utils.six.moves.urllib.parse import urlparse, unquote
 from ansible.module_utils.six.moves.urllib.request import getproxies

@@ -291,28 +291,39 @@ def _export_public_cert_from_pkcs12(module, executable, pkcs_file, alias, passwo

 def get_proxy_settings(scheme='https'):
-    """ Returns a tuple containing (proxy_host, proxy_port). (False, False) if no proxy is found """
+    """ Returns a tuple containing (proxy_host, proxy_port, proxy_user, proxy_pass). (False, False, False, False) if no proxy is found """
     proxy_url = getproxies().get(scheme, '')
     if not proxy_url:
-        return (False, False)
+        return (False, False, False, False)
     else:
         parsed_url = urlparse(proxy_url)
         if parsed_url.scheme:
             (proxy_host, proxy_port) = parsed_url.netloc.split(':')
         else:
             (proxy_host, proxy_port) = parsed_url.path.split(':')
-        return (proxy_host, proxy_port)
+
+        if parsed_url.user is not None and parsed_url.password is not None:
+            proxy_user = unquote(parsed_url.user)
+            proxy_pass = unquote(parsed_url.password)
+            return (proxy_host, proxy_port, proxy_user, proxy_pass)
+        else:
+            return (proxy_host, proxy_port, False, False)

 def build_proxy_options():
     """ Returns list of valid proxy options for keytool """
-    (proxy_host, proxy_port) = get_proxy_settings()
+    (proxy_host, proxy_port, proxy_user, proxy_pass) = get_proxy_settings()
     no_proxy = os.getenv("no_proxy")

     proxy_opts = []
     if proxy_host:
         proxy_opts.extend(["-J-Dhttps.proxyHost=%s" % proxy_host, "-J-Dhttps.proxyPort=%s" % proxy_port])

+        if proxy_user and proxy_pass:
+            proxy_opts.extend(["-J-Dhttps.proxyUser=%s" % proxy_user])
+            proxy_opts.extend(["-J-Dhttps.proxyPassword=%s" % proxy_pass])
+            proxy_opts.extend(["-J-Djdk.http.auth.tunneling.disabledSchemes="])
+
         if no_proxy is not None:
             # For Java's nonProxyHosts property, items are separated by '|',
             # and patterns have to start with "*".
felixfontein commented 2 years ago

I think this is more a feature than a bugfix, since the module never promised to handle user and password from https_proxy. But it's definitely worth to improve this.

ansibullbot commented 2 years ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help