Sync charm-helpers

Change-Id: I0d49f664d7c9369a6570a249e0679606821e7061

hooks/charmhelpers/contrib/hardening/apache/checks/config.py

@@ -46,8 +46,9 @@ def get_audits():
     context = ApacheConfContext()
     settings = utils.get_settings('apache')
     audits = [
-        FilePermissionAudit(paths='/etc/apache2/apache2.conf', user='root',
+        FilePermissionAudit(paths=os.path.join(
-                            group='root', mode=0o0640),
+                            settings['common']['apache_dir'], 'apache2.conf'),
+                            user='root', group='root', mode=0o0640),
 
         TemplatedFile(os.path.join(settings['common']['apache_dir'],
                                    'mods-available/alias.conf'),

hooks/charmhelpers/contrib/openstack/context.py

@@ -41,9 +41,9 @@ from charmhelpers.core.hookenv import (
     charm_name,
     DEBUG,
     INFO,
-    WARNING,
     ERROR,
     status_set,
+    network_get_primary_address
 )
 
 from charmhelpers.core.sysctl import create as sysctl_create
@@ -80,6 +80,9 @@ from charmhelpers.contrib.openstack.neutron import (
 from charmhelpers.contrib.openstack.ip import (
     resolve_address,
     INTERNAL,
+    ADMIN,
+    PUBLIC,
+    ADDRESS_MAP,
 )
 from charmhelpers.contrib.network.ip import (
     get_address_in_network,
@@ -87,7 +90,6 @@ from charmhelpers.contrib.network.ip import (
     get_ipv6_addr,
     get_netmask_for_address,
     format_ipv6_addr,
-    is_address_in_network,
     is_bridge_member,
     is_ipv6_disabled,
 )
@@ -620,7 +622,6 @@ class HAProxyContext(OSContextGenerator):
             ctxt['haproxy_connect_timeout'] = config('haproxy-connect-timeout')
 
         if config('prefer-ipv6'):
-            ctxt['ipv6'] = True
             ctxt['local_host'] = 'ip6-localhost'
             ctxt['haproxy_host'] = '::'
         else:
@@ -736,11 +737,17 @@ class ApacheSSLContext(OSContextGenerator):
         return sorted(list(set(cns)))
 
     def get_network_addresses(self):
-        """For each network configured, return corresponding address and vip
+        """For each network configured, return corresponding address and
-           (if available).
+           hostnamr or vip (if available).
 
         Returns a list of tuples of the form:
 
+            [(address_in_net_a, hostname_in_net_a),
+             (address_in_net_b, hostname_in_net_b),
+             ...]
+
+            or, if no hostnames(s) available:
+
             [(address_in_net_a, vip_in_net_a),
              (address_in_net_b, vip_in_net_b),
              ...]
@@ -752,32 +759,27 @@ class ApacheSSLContext(OSContextGenerator):
              ...]
         """
         addresses = []
-        if config('vip'):
+        for net_type in [INTERNAL, ADMIN, PUBLIC]:
-            vips = config('vip').split()
+            net_config = config(ADDRESS_MAP[net_type]['config'])
-        else:
+            # NOTE(jamespage): Fallback must always be private address
-            vips = []
+            #                  as this is used to bind services on the
-
+            #                  local unit.
-        for net_type in ['os-internal-network', 'os-admin-network',
+            fallback = unit_get("private-address")
-                         'os-public-network']:
+            if net_config:
-            addr = get_address_in_network(config(net_type),
+                addr = get_address_in_network(net_config,
-                                          unit_get('private-address'))
+                                              fallback)
-            if len(vips) > 1 and is_clustered():
-                if not config(net_type):
-                    log("Multiple networks configured but net_type "
-                        "is None (%s)." % net_type, level=WARNING)
-                    continue
-
-                for vip in vips:
-                    if is_address_in_network(config(net_type), vip):
-                        addresses.append((addr, vip))
-                        break
-
-            elif is_clustered() and config('vip'):
-                addresses.append((addr, config('vip')))
             else:
-                addresses.append((addr, addr))
+                try:
+                    addr = network_get_primary_address(
+                        ADDRESS_MAP[net_type]['binding']
+                    )
+                except NotImplementedError:
+                    addr = fallback
 
-        return sorted(addresses)
+            endpoint = resolve_address(net_type)
+            addresses.append((addr, endpoint))
+
+        return sorted(set(addresses))
 
     def __call__(self):
         if isinstance(self.external_ports, six.string_types):
@@ -804,7 +806,7 @@ class ApacheSSLContext(OSContextGenerator):
             self.configure_cert(cn)
 
         addresses = self.get_network_addresses()
-        for address, endpoint in sorted(set(addresses)):
+        for address, endpoint in addresses:
             for api_port in self.external_ports:
                 ext_port = determine_apache_port(api_port,
                                                  singlenode_mode=True)
@@ -1419,14 +1421,26 @@ class NeutronAPIContext(OSContextGenerator):
                 'rel_key': 'report-interval',
                 'default': 30,
             },
+            'enable_qos': {
+                'rel_key': 'enable-qos',
+                'default': False,
+            },
         }
         ctxt = self.get_neutron_options({})
         for rid in relation_ids('neutron-plugin-api'):
             for unit in related_units(rid):
                 rdata = relation_get(rid=rid, unit=unit)
+                # The l2-population key is used by the context as a way of
+                # checking if the api service on the other end is sending data
+                # in a recent format.
                 if 'l2-population' in rdata:
                     ctxt.update(self.get_neutron_options(rdata))
 
+        if ctxt['enable_qos']:
+            ctxt['extension_drivers'] = 'qos'
+        else:
+            ctxt['extension_drivers'] = ''
+
         return ctxt
 
     def get_neutron_options(self, rdata):

hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg

@@ -48,9 +48,7 @@ listen stats
 {% for service, ports in service_ports.items() -%}
 frontend tcp-in_{{ service }}
     bind *:{{ ports[0] }}
-    {% if ipv6 -%}
     bind :::{{ ports[0] }}
-    {% endif -%}
     {% for frontend in frontends -%}
     acl net_{{ frontend }} dst {{ frontends[frontend]['network'] }}
     use_backend {{ service }}_{{ frontend }} if net_{{ frontend }}

hooks/charmhelpers/contrib/openstack/templating.py

@@ -20,7 +20,8 @@ from charmhelpers.fetch import apt_install, apt_update
 from charmhelpers.core.hookenv import (
     log,
     ERROR,
-    INFO
+    INFO,
+    TRACE
 )
 from charmhelpers.contrib.openstack.utils import OPENSTACK_CODENAMES
 
@@ -80,8 +81,10 @@ def get_loader(templates_dir, os_release):
             loaders.insert(0, FileSystemLoader(tmpl_dir))
         if rel == os_release:
             break
+    # demote this log to the lowest level; we don't really need to see these
+    # lots in production even when debugging.
     log('Creating choice loader with dirs: %s' %
-        [l.searchpath for l in loaders], level=INFO)
+        [l.searchpath for l in loaders], level=TRACE)
     return ChoiceLoader(loaders)
 
 

hooks/charmhelpers/contrib/openstack/utils.py

@@ -186,7 +186,7 @@ SWIFT_CODENAMES = OrderedDict([
     ('ocata',
         ['2.11.0', '2.12.0', '2.13.0']),
     ('pike',
-        ['2.13.0']),
+        ['2.13.0', '2.15.0']),
 ])
 
 # >= Liberty version->codename mapping

hooks/charmhelpers/core/hookenv.py

@@ -43,6 +43,7 @@ ERROR = "ERROR"
 WARNING = "WARNING"
 INFO = "INFO"
 DEBUG = "DEBUG"
+TRACE = "TRACE"
 MARKER = object()
 
 cache = {}
@@ -789,6 +790,9 @@ class Hooks(object):
 
 def charm_dir():
     """Return the root directory of the current charm"""
+    d = os.environ.get('JUJU_CHARM_DIR')
+    if d is not None:
+        return d
     return os.environ.get('CHARM_DIR')
 
 

hooks/charmhelpers/core/host.py

@@ -34,7 +34,7 @@ import six
 
 from contextlib import contextmanager
 from collections import OrderedDict
-from .hookenv import log
+from .hookenv import log, DEBUG
 from .fstab import Fstab
 from charmhelpers.osplatform import get_platform
 
@@ -487,13 +487,37 @@ def mkdir(path, owner='root', group='root', perms=0o555, force=False):
 
 def write_file(path, content, owner='root', group='root', perms=0o444):
     """Create or overwrite a file with the contents of a byte string."""
-    log("Writing file {} {}:{} {:o}".format(path, owner, group, perms))
     uid = pwd.getpwnam(owner).pw_uid
     gid = grp.getgrnam(group).gr_gid
-    with open(path, 'wb') as target:
+    # lets see if we can grab the file and compare the context, to avoid doing
-        os.fchown(target.fileno(), uid, gid)
+    # a write.
-        os.fchmod(target.fileno(), perms)
+    existing_content = None
-        target.write(content)
+    existing_uid, existing_gid = None, None
+    try:
+        with open(path, 'rb') as target:
+            existing_content = target.read()
+        stat = os.stat(path)
+        existing_uid, existing_gid = stat.st_uid, stat.st_gid
+    except:
+        pass
+    if content != existing_content:
+        log("Writing file {} {}:{} {:o}".format(path, owner, group, perms),
+            level=DEBUG)
+        with open(path, 'wb') as target:
+            os.fchown(target.fileno(), uid, gid)
+            os.fchmod(target.fileno(), perms)
+            target.write(content)
+        return
+    # the contents were the same, but we might still need to change the
+    # ownership.
+    if existing_uid != uid:
+        log("Changing uid on already existing content: {} -> {}"
+            .format(existing_uid, uid), level=DEBUG)
+        os.chown(path, uid, -1)
+    if existing_gid != gid:
+        log("Changing gid on already existing content: {} -> {}"
+            .format(existing_gid, gid), level=DEBUG)
+        os.chown(path, -1, gid)
 
 
 def fstab_remove(mp):

hooks/charmhelpers/fetch/ubuntu.py

@@ -27,6 +27,7 @@ from charmhelpers.core.host import (
 from charmhelpers.core.hookenv import (
     log,
     DEBUG,
+    WARNING,
 )
 from charmhelpers.fetch import SourceConfigError, GPGKeyError
 
@@ -261,34 +262,47 @@ def apt_unhold(packages, fatal=False):
     return apt_mark(packages, 'unhold', fatal=fatal)
 
 
-def import_key(keyid):
+def import_key(key):
-    """Import a key in either ASCII Armor or Radix64 format.
+    """Import an ASCII Armor key.
 
-    `keyid` is either the keyid to fetch from a PGP server, or
+    /!\ A Radix64 format keyid is also supported for backwards
-    the key in ASCII armor foramt.
+    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: String of key (or key id).
+    :param keyid: The key in ASCII armor format,
+                  including BEGIN and END markers.
     :raises: GPGKeyError if the key could not be imported
     """
-    key = keyid.strip()
+    key = key.strip()
-    if (key.startswith('-----BEGIN PGP PUBLIC KEY BLOCK-----') and
+    if '-' in key or '\n' in key:
-            key.endswith('-----END PGP PUBLIC KEY BLOCK-----')):
+        # Send everything not obviously a keyid to GPG to import, as
+        # we trust its validation better than our own. eg. handling
+        # comments before the key.
         log("PGP key found (looks like ASCII Armor format)", level=DEBUG)
-        log("Importing ASCII Armor PGP key", level=DEBUG)
+        if ('-----BEGIN PGP PUBLIC KEY BLOCK-----' in key and
-        with NamedTemporaryFile() as keyfile:
+                '-----END PGP PUBLIC KEY BLOCK-----' in key):
-            with open(keyfile.name, 'w') as fd:
+            log("Importing ASCII Armor PGP key", level=DEBUG)
-                fd.write(key)
+            with NamedTemporaryFile() as keyfile:
-                fd.write("\n")
+                with open(keyfile.name, 'w') as fd:
-            cmd = ['apt-key', 'add', keyfile.name]
+                    fd.write(key)
-            try:
+                    fd.write("\n")
-                subprocess.check_call(cmd)
+                cmd = ['apt-key', 'add', keyfile.name]
-            except subprocess.CalledProcessError:
+                try:
-                error = "Error importing PGP key '{}'".format(key)
+                    subprocess.check_call(cmd)
-                log(error)
+                except subprocess.CalledProcessError:
-                raise GPGKeyError(error)
+                    error = "Error importing PGP key '{}'".format(key)
+                    log(error)
+                    raise GPGKeyError(error)
+        else:
+            raise GPGKeyError("ASCII armor markers missing from GPG key")
     else:
-        log("PGP key found (looks like Radix64 format)", level=DEBUG)
+        # We should only send things obviously not a keyid offsite
-        log("Importing PGP key from keyserver", level=DEBUG)
+        # 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; "
+            "full key not provided.", level=WARNING)
         cmd = ['apt-key', 'adv', '--keyserver',
                'hkp://keyserver.ubuntu.com:80', '--recv-keys', key]
         try:

tests/charmhelpers/contrib/openstack/utils.py

@@ -186,7 +186,7 @@ SWIFT_CODENAMES = OrderedDict([
     ('ocata',
         ['2.11.0', '2.12.0', '2.13.0']),
     ('pike',
-        ['2.13.0']),
+        ['2.13.0', '2.15.0']),
 ])
 
 # >= Liberty version->codename mapping

tests/charmhelpers/core/hookenv.py

@@ -43,6 +43,7 @@ ERROR = "ERROR"
 WARNING = "WARNING"
 INFO = "INFO"
 DEBUG = "DEBUG"
+TRACE = "TRACE"
 MARKER = object()
 
 cache = {}
@@ -789,6 +790,9 @@ class Hooks(object):
 
 def charm_dir():
     """Return the root directory of the current charm"""
+    d = os.environ.get('JUJU_CHARM_DIR')
+    if d is not None:
+        return d
     return os.environ.get('CHARM_DIR')
 
 

tests/charmhelpers/core/host.py

@@ -34,7 +34,7 @@ import six
 
 from contextlib import contextmanager
 from collections import OrderedDict
-from .hookenv import log
+from .hookenv import log, DEBUG
 from .fstab import Fstab
 from charmhelpers.osplatform import get_platform
 
@@ -487,13 +487,37 @@ def mkdir(path, owner='root', group='root', perms=0o555, force=False):
 
 def write_file(path, content, owner='root', group='root', perms=0o444):
     """Create or overwrite a file with the contents of a byte string."""
-    log("Writing file {} {}:{} {:o}".format(path, owner, group, perms))
     uid = pwd.getpwnam(owner).pw_uid
     gid = grp.getgrnam(group).gr_gid
-    with open(path, 'wb') as target:
+    # lets see if we can grab the file and compare the context, to avoid doing
-        os.fchown(target.fileno(), uid, gid)
+    # a write.
-        os.fchmod(target.fileno(), perms)
+    existing_content = None
-        target.write(content)
+    existing_uid, existing_gid = None, None
+    try:
+        with open(path, 'rb') as target:
+            existing_content = target.read()
+        stat = os.stat(path)
+        existing_uid, existing_gid = stat.st_uid, stat.st_gid
+    except:
+        pass
+    if content != existing_content:
+        log("Writing file {} {}:{} {:o}".format(path, owner, group, perms),
+            level=DEBUG)
+        with open(path, 'wb') as target:
+            os.fchown(target.fileno(), uid, gid)
+            os.fchmod(target.fileno(), perms)
+            target.write(content)
+        return
+    # the contents were the same, but we might still need to change the
+    # ownership.
+    if existing_uid != uid:
+        log("Changing uid on already existing content: {} -> {}"
+            .format(existing_uid, uid), level=DEBUG)
+        os.chown(path, uid, -1)
+    if existing_gid != gid:
+        log("Changing gid on already existing content: {} -> {}"
+            .format(existing_gid, gid), level=DEBUG)
+        os.chown(path, -1, gid)
 
 
 def fstab_remove(mp):

tests/charmhelpers/fetch/ubuntu.py

@@ -27,6 +27,7 @@ from charmhelpers.core.host import (
 from charmhelpers.core.hookenv import (
     log,
     DEBUG,
+    WARNING,
 )
 from charmhelpers.fetch import SourceConfigError, GPGKeyError
 
@@ -261,34 +262,47 @@ def apt_unhold(packages, fatal=False):
     return apt_mark(packages, 'unhold', fatal=fatal)
 
 
-def import_key(keyid):
+def import_key(key):
-    """Import a key in either ASCII Armor or Radix64 format.
+    """Import an ASCII Armor key.
 
-    `keyid` is either the keyid to fetch from a PGP server, or
+    /!\ A Radix64 format keyid is also supported for backwards
-    the key in ASCII armor foramt.
+    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: String of key (or key id).
+    :param keyid: The key in ASCII armor format,
+                  including BEGIN and END markers.
     :raises: GPGKeyError if the key could not be imported
     """
-    key = keyid.strip()
+    key = key.strip()
-    if (key.startswith('-----BEGIN PGP PUBLIC KEY BLOCK-----') and
+    if '-' in key or '\n' in key:
-            key.endswith('-----END PGP PUBLIC KEY BLOCK-----')):
+        # Send everything not obviously a keyid to GPG to import, as
+        # we trust its validation better than our own. eg. handling
+        # comments before the key.
         log("PGP key found (looks like ASCII Armor format)", level=DEBUG)
-        log("Importing ASCII Armor PGP key", level=DEBUG)
+        if ('-----BEGIN PGP PUBLIC KEY BLOCK-----' in key and
-        with NamedTemporaryFile() as keyfile:
+                '-----END PGP PUBLIC KEY BLOCK-----' in key):
-            with open(keyfile.name, 'w') as fd:
+            log("Importing ASCII Armor PGP key", level=DEBUG)
-                fd.write(key)
+            with NamedTemporaryFile() as keyfile:
-                fd.write("\n")
+                with open(keyfile.name, 'w') as fd:
-            cmd = ['apt-key', 'add', keyfile.name]
+                    fd.write(key)
-            try:
+                    fd.write("\n")
-                subprocess.check_call(cmd)
+                cmd = ['apt-key', 'add', keyfile.name]
-            except subprocess.CalledProcessError:
+                try:
-                error = "Error importing PGP key '{}'".format(key)
+                    subprocess.check_call(cmd)
-                log(error)
+                except subprocess.CalledProcessError:
-                raise GPGKeyError(error)
+                    error = "Error importing PGP key '{}'".format(key)
+                    log(error)
+                    raise GPGKeyError(error)
+        else:
+            raise GPGKeyError("ASCII armor markers missing from GPG key")
     else:
-        log("PGP key found (looks like Radix64 format)", level=DEBUG)
+        # We should only send things obviously not a keyid offsite
-        log("Importing PGP key from keyserver", level=DEBUG)
+        # 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; "
+            "full key not provided.", level=WARNING)
         cmd = ['apt-key', 'adv', '--keyserver',
                'hkp://keyserver.ubuntu.com:80', '--recv-keys', key]
         try: