Merge ~xavpaice/charm-nrpe:lp1712977 into ~nrpe-charmers/charm-nrpe:master

[Xav Paice]

[James Hebden (ec0)]

LGTM, thanks

review: Approve

diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py
index 12f37b2..899722f 100644
--- a/hooks/charmhelpers/core/hookenv.py
+++ b/hooks/charmhelpers/core/hookenv.py
@@ -218,6 +218,8 @@ def principal_unit():
         for rid in relation_ids(reltype):
             for unit in related_units(rid):
                 md = _metadata_unit(unit)
+                if not md:
+                    continue
                 subordinate = md.pop('subordinate', None)
                 if not subordinate:
                     return unit
@@ -511,7 +513,10 @@ def _metadata_unit(unit):
     """
     basedir = os.sep.join(charm_dir().split(os.sep)[:-2])
     unitdir = 'unit-{}'.format(unit.replace(os.sep, '-'))
-    with open(os.path.join(basedir, unitdir, 'charm', 'metadata.yaml')) as md:
+    joineddir = os.path.join(basedir, unitdir, 'charm', 'metadata.yaml')
+    if not os.path.exists(joineddir):
+        return None
+    with open(joineddir) as md:
         return yaml.safe_load(md)
 
 
diff --git a/hooks/charmhelpers/fetch/ubuntu.py b/hooks/charmhelpers/fetch/ubuntu.py
index 545348f..40e1cb5 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: