From 1ec1269a1c8d7b6dfe75b72eb281c7cf80b34e23 Mon Sep 17 00:00:00 2001 From: Ryan Beisner Date: Thu, 24 Aug 2017 17:15:56 -0500 Subject: [PATCH] Sync charm-helpers Change-Id: I3951f1323301ff17247f7a4f7c5c7fba4bf7840d --- .../contrib/hardening/apache/checks/config.py | 5 +- .../charmhelpers/contrib/openstack/context.py | 1 - .../contrib/openstack/templates/haproxy.cfg | 2 - .../contrib/openstack/templating.py | 7 ++- hooks/charmhelpers/core/hookenv.py | 1 + hooks/charmhelpers/core/host.py | 36 ++++++++++-- hooks/charmhelpers/fetch/ubuntu.py | 58 ++++++++++++------- tests/charmhelpers/core/hookenv.py | 1 + tests/charmhelpers/core/host.py | 36 ++++++++++-- tests/charmhelpers/fetch/ubuntu.py | 58 ++++++++++++------- 10 files changed, 142 insertions(+), 63 deletions(-) diff --git a/hooks/charmhelpers/contrib/hardening/apache/checks/config.py b/hooks/charmhelpers/contrib/hardening/apache/checks/config.py index b18b263d..06482aac 100644 --- a/hooks/charmhelpers/contrib/hardening/apache/checks/config.py +++ b/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', - group='root', mode=0o0640), + FilePermissionAudit(paths=os.path.join( + settings['common']['apache_dir'], 'apache2.conf'), + user='root', group='root', mode=0o0640), TemplatedFile(os.path.join(settings['common']['apache_dir'], 'mods-available/alias.conf'), diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index 4194e2c3..f67f3265 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -622,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: diff --git a/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg b/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg index edae7a0c..2e660450 100644 --- a/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg +++ b/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 }} diff --git a/hooks/charmhelpers/contrib/openstack/templating.py b/hooks/charmhelpers/contrib/openstack/templating.py index 934baf5d..d8c1fc7f 100644 --- a/hooks/charmhelpers/contrib/openstack/templating.py +++ b/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) diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index 814a9354..12f37b28 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -43,6 +43,7 @@ ERROR = "ERROR" WARNING = "WARNING" INFO = "INFO" DEBUG = "DEBUG" +TRACE = "TRACE" MARKER = object() cache = {} diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index b0043cbe..5656e2f5 100644 --- a/hooks/charmhelpers/core/host.py +++ b/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: - os.fchown(target.fileno(), uid, gid) - os.fchmod(target.fileno(), perms) - target.write(content) + # lets see if we can grab the file and compare the context, to avoid doing + # a write. + existing_content = None + 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): diff --git a/hooks/charmhelpers/fetch/ubuntu.py b/hooks/charmhelpers/fetch/ubuntu.py index 545348ff..40e1cb5b 100644 --- a/hooks/charmhelpers/fetch/ubuntu.py +++ b/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): - """Import a key in either ASCII Armor or Radix64 format. +def import_key(key): + """Import an ASCII Armor key. - `keyid` is either the keyid to fetch from a PGP server, or - the key in ASCII armor foramt. + /!\ 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: 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() - if (key.startswith('-----BEGIN PGP PUBLIC KEY BLOCK-----') and - key.endswith('-----END PGP PUBLIC KEY BLOCK-----')): + key = key.strip() + if '-' in key or '\n' in key: + # 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) - 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) + 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) + else: + raise GPGKeyError("ASCII armor markers missing from GPG key") else: - log("PGP key found (looks like Radix64 format)", level=DEBUG) - log("Importing PGP key from keyserver", level=DEBUG) + # 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; " + "full key not provided.", level=WARNING) cmd = ['apt-key', 'adv', '--keyserver', 'hkp://keyserver.ubuntu.com:80', '--recv-keys', key] try: diff --git a/tests/charmhelpers/core/hookenv.py b/tests/charmhelpers/core/hookenv.py index 814a9354..12f37b28 100644 --- a/tests/charmhelpers/core/hookenv.py +++ b/tests/charmhelpers/core/hookenv.py @@ -43,6 +43,7 @@ ERROR = "ERROR" WARNING = "WARNING" INFO = "INFO" DEBUG = "DEBUG" +TRACE = "TRACE" MARKER = object() cache = {} diff --git a/tests/charmhelpers/core/host.py b/tests/charmhelpers/core/host.py index b0043cbe..5656e2f5 100644 --- a/tests/charmhelpers/core/host.py +++ b/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: - os.fchown(target.fileno(), uid, gid) - os.fchmod(target.fileno(), perms) - target.write(content) + # lets see if we can grab the file and compare the context, to avoid doing + # a write. + existing_content = None + 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): diff --git a/tests/charmhelpers/fetch/ubuntu.py b/tests/charmhelpers/fetch/ubuntu.py index 545348ff..40e1cb5b 100644 --- a/tests/charmhelpers/fetch/ubuntu.py +++ b/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): - """Import a key in either ASCII Armor or Radix64 format. +def import_key(key): + """Import an ASCII Armor key. - `keyid` is either the keyid to fetch from a PGP server, or - the key in ASCII armor foramt. + /!\ 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: 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() - if (key.startswith('-----BEGIN PGP PUBLIC KEY BLOCK-----') and - key.endswith('-----END PGP PUBLIC KEY BLOCK-----')): + key = key.strip() + if '-' in key or '\n' in key: + # 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) - 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) + 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) + else: + raise GPGKeyError("ASCII armor markers missing from GPG key") else: - log("PGP key found (looks like Radix64 format)", level=DEBUG) - log("Importing PGP key from keyserver", level=DEBUG) + # 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; " + "full key not provided.", level=WARNING) cmd = ['apt-key', 'adv', '--keyserver', 'hkp://keyserver.ubuntu.com:80', '--recv-keys', key] try: