Handle new juju charm proxy settings and https keyserver URLs (#248)

Handle new juju charm proxy settings and https keyserver URLs

This change reworks proxy server handling for add-apt-repository and
key retrieval cases and introduces support for new juju-prefixed proxy
setting environment variables that do not modify the http(s) connections
made from hook execution environments by charm code or forked and
execve-ed applications that were given environment variables of a parent
process. The new proxy settings are available as of Juju 2.4.0 but are
properly applied as of Juju 2.4.2 (see lp:1782236).

add-apt-repository comes from the software-properties package and only
reacts to HTTP_PROXY and HTTPS_PROXY environment variables as of bionic
when it was switched to using curl (lp:1433761) and HTTPS-based Ubuntu
keyserver URLs. For Xenial and other releases older than Bionic it is
necessary to use charm options specifying GPG keys in the radix format
AND source URLs in the following formats to avoid triggering the
add-apt-repository behavior related to GPG keys for ppa shortcuts:

deb [arch=<arches-csv] uri distribution [component1] [component2] [...]

See lp:1433761 and https://git.launchpad.net/ubuntu/+source/software-properties/commit/?id=f57935235ca0f52b32da7efe2a24cb26c7fc4573

A manpage for apt-key mentions the following in a section about the
"add" command:
"Note: Instead of using this command a keyring should be placed directly
in the /etc/apt/trusted.gpg.d/ directory with a descriptive name and
either "gpg" or "asc" as file extension."

The support for /etc/apt/trusted.gpg.d/ goes back to 2010:
https://salsa.debian.org/nathanruiz-guest/apt/commit/c24f6ce22cd6720004addad2e3382b3caa6b1b7c

Using "asc" in this directory is only supported as of apt 1.4.

https://salsa.debian.org/nathanruiz-guest/apt/commit/f77ea8235cafb258d1cb0b2b90e95aa36e5c4650
https://salsa.debian.org/nathanruiz-guest/apt/commit/2906182db398419a9c59a928b7ae73cf7c7aa307

Binary GPG format is used in this change given that trusty uses 1.0.1,
xenial uses 1.2.x and only bionic has 1.6.x. This requires de-armoring
of ASCII armor-formatted GPG keys downloaded from the Ubuntu keyserver.

apt-key usage is completely removed by this change.

HTTPS is used for key retrieval with this change which is a functional
change to a more secure way of retrieving GPG keys. A subset of charms
using PPAs will be affected by that.

Since HTTPS is used, if SSLBump-like HTTPS proxies are in place,
they will impersonate keyserver.ubuntu.com and generate a certificate
with keyserver.ubuntu.com in the CN field or in SubjAltName fields of a
certificate. If such proxy behavior is expected it is necessary to add
the CA certificate chain containing the intermediate CA of the SSLBump
proxy to every machine that this code runs on via ca-certs cloud-init
directive (via cloudinit-userdata model-config) or via other means
(such as through a custom charm option). curl relies on openssl provided
by the distribution which means it is affected by the system trusted
certificate store. Also note that DNS resolution for the hostname in a
URL is done at a proxy server - not at the client side.

charmhelpers/core/hookenv.py

@@ -50,6 +50,11 @@
 MARKER = object()
 SH_MAX_ARG = 131071
 
+
+RANGE_WARNING = ('Passing NO_PROXY string that includes a cidr. '
+                 'This may not be compatible with software you are '
+                 'running in your shell.')
+
 cache = {}
 
 
@@ -1414,3 +1419,72 @@ def unit_doomed(unit=None):
     # I don't think 'dead' units ever show up in the goal-state, but
     # check anyway in addition to 'dying'.
     return units[unit]['status'] in ('dying', 'dead')
+
+
+def env_proxy_settings(selected_settings=None):
+    """Get proxy settings from process environment variables.
+
+    Get charm proxy settings from environment variables that correspond to
+    juju-http-proxy, juju-https-proxy and juju-no-proxy (available as of 2.4.2,
+    see lp:1782236) in a format suitable for passing to an application that
+    reacts to proxy settings passed as environment variables. Some applications
+    support lowercase or uppercase notation (e.g. curl), some support only
+    lowercase (e.g. wget), there are also subjectively rare cases of only
+    uppercase notation support. no_proxy CIDR and wildcard support also varies
+    between runtimes and applications as there is no enforced standard.
+
+    Some applications may connect to multiple destinations and expose config
+    options that would affect only proxy settings for a specific destination
+    these should be handled in charms in an application-specific manner.
+
+    :param selected_settings: format only a subset of possible settings
+    :type selected_settings: list
+    :rtype: Option(None, dict[str, str])
+    """
+    SUPPORTED_SETTINGS = {
+        'http': 'HTTP_PROXY',
+        'https': 'HTTPS_PROXY',
+        'no_proxy': 'NO_PROXY',
+        'ftp': 'FTP_PROXY'
+    }
+    if selected_settings is None:
+        selected_settings = SUPPORTED_SETTINGS
+
+    selected_vars = [v for k, v in SUPPORTED_SETTINGS.items()
+                     if k in selected_settings]
+    proxy_settings = {}
+    for var in selected_vars:
+        var_val = os.getenv(var)
+        if var_val:
+            proxy_settings[var] = var_val
+            proxy_settings[var.lower()] = var_val
+        # Now handle juju-prefixed environment variables. The legacy vs new
+        # environment variable usage is mutually exclusive
+        charm_var_val = os.getenv('JUJU_CHARM_{}'.format(var))
+        if charm_var_val:
+            proxy_settings[var] = charm_var_val
+            proxy_settings[var.lower()] = charm_var_val
+    if 'no_proxy' in proxy_settings:
+        if _contains_range(proxy_settings['no_proxy']):
+            log(RANGE_WARNING, level=WARNING)
+    return proxy_settings if proxy_settings else None
+
+
+def _contains_range(addresses):
+    """Check for cidr or wildcard domain in a string.
+
+    Given a string comprising a comma seperated list of ip addresses
+    and domain names, determine whether the string contains IP ranges
+    or wildcard domains.
+
+    :param addresses: comma seperated list of domains and ip addresses.
+    :type addresses: str
+    """
+    return (
+        # Test for cidr (e.g. 10.20.20.0/24)
+        "/" in addresses or
+        # Test for wildcard domains (*.foo.com or .foo.com)
+        "*" in addresses or
+        addresses.startswith(".") or
+        ",." in addresses or
+        " ." in addresses)

charmhelpers/fetch/ubuntu.py

@@ -19,15 +19,16 @@
 import six
 import time
 import subprocess
-from tempfile import NamedTemporaryFile
 
 from charmhelpers.core.host import (
-    lsb_release
+    get_distrib_codename,
+    CompareHostReleases,
 )
 from charmhelpers.core.hookenv import (
     log,
     DEBUG,
     WARNING,
+    env_proxy_settings,
 )
 from charmhelpers.fetch import SourceConfigError, GPGKeyError
 
@@ -303,12 +304,17 @@ def import_key(key):
     """Import an ASCII Armor key.
 
     A Radix64 format keyid is also supported for backwards
-    compatibility, but should never be used; the key retrieval
-    mechanism is insecure and subject to man-in-the-middle attacks
-    voiding all signature checks using that key.
-
-    :param keyid: The key in ASCII armor format,
-                  including BEGIN and END markers.
+    compatibility. In this case Ubuntu keyserver will be
+    queried for a key via HTTPS by its keyid. This method
+    is less preferrable because https proxy servers may
+    require traffic decryption which is equivalent to a
+    man-in-the-middle attack (a proxy server impersonates
+    keyserver TLS certificates and has to be explicitly
+    trusted by the system).
+
+    :param key: A GPG key in ASCII armor format,
+                  including BEGIN and END markers or a keyid.
+    :type key: (bytes, str)
     :raises: GPGKeyError if the key could not be imported
     """
     key = key.strip()
@@ -319,35 +325,137 @@ def import_key(key):
         log("PGP key found (looks like ASCII Armor format)", level=DEBUG)
         if ('-----BEGIN PGP PUBLIC KEY BLOCK-----' in key and
                 '-----END PGP PUBLIC KEY BLOCK-----' in key):
-            log("Importing ASCII Armor PGP key", level=DEBUG)
-            with NamedTemporaryFile() as keyfile:
-                with open(keyfile.name, 'w') as fd:
-                    fd.write(key)
-                    fd.write("\n")
-                cmd = ['apt-key', 'add', keyfile.name]
-                try:
-                    subprocess.check_call(cmd)
-                except subprocess.CalledProcessError:
-                    error = "Error importing PGP key '{}'".format(key)
-                    log(error)
-                    raise GPGKeyError(error)
+            log("Writing provided PGP key in the binary format", level=DEBUG)
+            if six.PY3:
+                key_bytes = key.encode('utf-8')
+            else:
+                key_bytes = key
+            key_name = _get_keyid_by_gpg_key(key_bytes)
+            key_gpg = _dearmor_gpg_key(key_bytes)
+            _write_apt_gpg_keyfile(key_name=key_name, key_material=key_gpg)
         else:
             raise GPGKeyError("ASCII armor markers missing from GPG key")
     else:
-        # We should only send things obviously not a keyid offsite
-        # via this unsecured protocol, as it may be a secret or part
-        # of one.
         log("PGP key found (looks like Radix64 format)", level=WARNING)
-        log("INSECURLY importing PGP key from keyserver; "
+        log("SECURELY importing PGP key from keyserver; "
             "full key not provided.", level=WARNING)
-        cmd = ['apt-key', 'adv', '--keyserver',
-               'hkp://keyserver.ubuntu.com:80', '--recv-keys', key]
-        try:
-            _run_with_retries(cmd)
-        except subprocess.CalledProcessError:
-            error = "Error importing PGP key '{}'".format(key)
-            log(error)
-            raise GPGKeyError(error)
+        # as of bionic add-apt-repository uses curl with an HTTPS keyserver URL
+        # to retrieve GPG keys. `apt-key adv` command is deprecated as is
+        # apt-key in general as noted in its manpage. See lp:1433761 for more
+        # history. Instead, /etc/apt/trusted.gpg.d is used directly to drop
+        # gpg
+        key_asc = _get_key_by_keyid(key)
+        # write the key in GPG format so that apt-key list shows it
+        key_gpg = _dearmor_gpg_key(key_asc)
+        _write_apt_gpg_keyfile(key_name=key, key_material=key_gpg)
+
+
+def _get_keyid_by_gpg_key(key_material):
+    """Get a GPG key fingerprint by GPG key material.
+    Gets a GPG key fingerprint (40-digit, 160-bit) by the ASCII armor-encoded
+    or binary GPG key material. Can be used, for example, to generate file
+    names for keys passed via charm options.
+
+    :param key_material: ASCII armor-encoded or binary GPG key material
+    :type key_material: bytes
+    :raises: GPGKeyError if invalid key material has been provided
+    :returns: A GPG key fingerprint
+    :rtype: str
+    """
+    # trusty, xenial and bionic handling differs due to gpg 1.x to 2.x change
+    release = get_distrib_codename()
+    is_gpgv2_distro = CompareHostReleases(release) >= "bionic"
+    if is_gpgv2_distro:
+        # --import is mandatory, otherwise fingerprint is not printed
+        cmd = 'gpg --with-colons --import-options show-only --import --dry-run'
+    else:
+        cmd = 'gpg --with-colons --with-fingerprint'
+    ps = subprocess.Popen(cmd.split(),
+                          stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE,
+                          stdin=subprocess.PIPE)
+    out, err = ps.communicate(input=key_material)
+    if six.PY3:
+        out = out.decode('utf-8')
+        err = err.decode('utf-8')
+    if 'gpg: no valid OpenPGP data found.' in err:
+        raise GPGKeyError('Invalid GPG key material provided')
+    # from gnupg2 docs: fpr :: Fingerprint (fingerprint is in field 10)
+    return re.search(r"^fpr:{9}([0-9A-F]{40}):$", out, re.MULTILINE).group(1)
+
+
+def _get_key_by_keyid(keyid):
+    """Get a key via HTTPS from the Ubuntu keyserver.
+    Different key ID formats are supported by SKS keyservers (the longer ones
+    are more secure, see "dead beef attack" and https://evil32.com/). Since
+    HTTPS is used, if SSLBump-like HTTPS proxies are in place, they will
+    impersonate keyserver.ubuntu.com and generate a certificate with
+    keyserver.ubuntu.com in the CN field or in SubjAltName fields of a
+    certificate. If such proxy behavior is expected it is necessary to add the
+    CA certificate chain containing the intermediate CA of the SSLBump proxy to
+    every machine that this code runs on via ca-certs cloud-init directive (via
+    cloudinit-userdata model-config) or via other means (such as through a
+    custom charm option). Also note that DNS resolution for the hostname in a
+    URL is done at a proxy server - not at the client side.
+
+    8-digit (32 bit) key ID
+    https://keyserver.ubuntu.com/pks/lookup?search=0x4652B4E6
+    16-digit (64 bit) key ID
+    https://keyserver.ubuntu.com/pks/lookup?search=0x6E85A86E4652B4E6
+    40-digit key ID:
+    https://keyserver.ubuntu.com/pks/lookup?search=0x35F77D63B5CEC106C577ED856E85A86E4652B4E6
+
+    :param keyid: An 8, 16 or 40 hex digit keyid to find a key for
+    :type keyid: (bytes, str)
+    :returns: A key material for the specified GPG key id
+    :rtype: (str, bytes)
+    :raises: subprocess.CalledProcessError
+    """
+    # options=mr - machine-readable output (disables html wrappers)
+    keyserver_url = ('https://keyserver.ubuntu.com'
+                     '/pks/lookup?op=get&options=mr&exact=on&search=0x{}')
+    curl_cmd = ['curl', keyserver_url.format(keyid)]
+    # use proxy server settings in order to retrieve the key
+    return subprocess.check_output(curl_cmd,
+                                   env=env_proxy_settings(['https']))
+
+
+def _dearmor_gpg_key(key_asc):
+    """Converts a GPG key in the ASCII armor format to the binary format.
+
+    :param key_asc: A GPG key in ASCII armor format.
+    :type key_asc: (str, bytes)
+    :returns: A GPG key in binary format
+    :rtype: (str, bytes)
+    :raises: GPGKeyError
+    """
+    ps = subprocess.Popen(['gpg', '--dearmor'],
+                          stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE,
+                          stdin=subprocess.PIPE)
+    out, err = ps.communicate(input=key_asc)
+    # no need to decode output as it is binary (invalid utf-8), only error
+    if six.PY3:
+        err = err.decode('utf-8')
+    if 'gpg: no valid OpenPGP data found.' in err:
+        raise GPGKeyError('Invalid GPG key material. Check your network setup'
+                          ' (MTU, routing, DNS) and/or proxy server settings'
+                          ' as well as destination keyserver status.')
+    else:
+        return out
+
+
+def _write_apt_gpg_keyfile(key_name, key_material):
+    """Writes GPG key material into a file at a provided path.
+
+    :param key_name: A key name to use for a key file (could be a fingerprint)
+    :type key_name: str
+    :param key_material: A GPG key material (binary)
+    :type key_material: (str, bytes)
+    """
+    with open('/etc/apt/trusted.gpg.d/{}.gpg'.format(key_name),
+              'wb') as keyf:
+        keyf.write(key_material)
 
 
 def add_source(source, key=None, fail_invalid=False):
@@ -442,13 +550,13 @@ def add_source(source, key=None, fail_invalid=False):
 def _add_proposed():
     """Add the PROPOSED_POCKET as /etc/apt/source.list.d/proposed.list
 
-    Uses lsb_release()['DISTRIB_CODENAME'] to determine the correct staza for
+    Uses get_distrib_codename to determine the correct stanza for
     the deb line.
 
     For intel architecutres PROPOSED_POCKET is used for the release, but for
     other architectures PROPOSED_PORTS_POCKET is used for the release.
     """
-    release = lsb_release()['DISTRIB_CODENAME']
+    release = get_distrib_codename()
     arch = platform.machine()
     if arch not in six.iterkeys(ARCH_TO_PROPOSED_POCKET):
         raise SourceConfigError("Arch {} not supported for (distro-)proposed"
@@ -461,11 +569,16 @@ def _add_apt_repository(spec):
     """Add the spec using add_apt_repository
 
     :param spec: the parameter to pass to add_apt_repository
+    :type spec: str
     """
     if '{series}' in spec:
-        series = lsb_release()['DISTRIB_CODENAME']
+        series = get_distrib_codename()
         spec = spec.replace('{series}', series)
-    _run_with_retries(['add-apt-repository', '--yes', spec])
+    # software-properties package for bionic properly reacts to proxy settings
+    # passed as environment variables (See lp:1433761). This is not the case
+    # LTS and non-LTS releases below bionic.
+    _run_with_retries(['add-apt-repository', '--yes', spec],
+                      cmd_env=env_proxy_settings(['https']))
 
 
 def _add_cloud_pocket(pocket):
@@ -534,7 +647,7 @@ def _verify_is_ubuntu_rel(release, os_release):
     :raises: SourceConfigError if the release is not the same as the ubuntu
         release.
     """
-    ubuntu_rel = lsb_release()['DISTRIB_CODENAME']
+    ubuntu_rel = get_distrib_codename()
     if release != ubuntu_rel:
         raise SourceConfigError(
             'Invalid Cloud Archive release specified: {}-{} on this Ubuntu'