Merge lp:~dpb/charms/trusty/apache2/avoid-regen-cert into lp:charms/trusty/apache2

[David Britton]

Don't regen a self-signed cert unless we need to

MP For Precise: https://code.launchpad.net/~davidpbritton/charms/precise/apache2/avoid-regen-cert/+merge/221102

Don't regen a self-signed cert unless we need to:

- pulled out cert generation code
- added test cases for it
- made regeneration dependent on necessity (hostname/ip changed, servername changed, cert is missing/invalid)
- Included small fix for lp:1302645 to install python-yaml

[lp:~dpb/charms/trusty/apache2/avoid-regen-cert]

56. By David Britton on 2014-06-28

merge up

57. By David Britton on 2014-06-28

apply same importerror check to trusty, keeping precise and trusty in sync.

[Chris Glass (tribaal)]

Looks good!

review: Approve

[Tim Van Steenburgh (tvansteenburgh)]

+1 LGTM.

review: Approve

[Charles Butler (lazypower)]

DPB, Thanks for the patch submission! I noticed this behavior myself and this is a most welcome fix. The tests are a big +1

review: Approve

=== modified file 'README.md'
--- README.md	2014-05-21 18:31:11 +0000
+++ README.md	2014-06-28 19:27:49 +0000
@@ -17,7 +17,8 @@
     sudo apt-get install python-software-properties
     sudo add-apt-repository ppa:chrisjohnston/flake8
     sudo apt-get update
-    sudo apt-get install flake8 python-nose python-coverage python-testtools
+    sudo apt-get install python-flake8 python-nose python-coverage \
+                         python-testtools python-pyasn1 python-pyasn1-modules
 
 To fetch additional source dependencies and run the tests:
 

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-06-23 19:40:44 +0000
+++ hooks/hooks.py	2014-06-28 19:27:49 +0000
@@ -49,14 +49,12 @@
 # Supporting functions
 ###############################################################################
 
-#------------------------------------------------------------------------------
-# apt_get_install( package ):  Installs a package
-#------------------------------------------------------------------------------
-def apt_get_install(packages=None):
-    if packages is None:
+def apt_get_install(package=None):
+    """Install a package."""
+    if package is None:
         return False
     cmd_line = ['apt-get', '-y', 'install', '-qq']
-    cmd_line.append(packages)
+    cmd_line.append(package)
     return subprocess.call(cmd_line)
 
 
@@ -180,6 +178,72 @@
     return True
 
 
+def gen_selfsigned_cert(config, cert_file, key_file):
+    """
+    Create a self-signed certificate.
+
+    @param config: charm data from config-get
+    @param cert_file: destination path of generated certificate
+    @param key_file: destination path of generated private key
+    """
+    os.environ['OPENSSL_CN'] = config['servername']
+    os.environ['OPENSSL_PUBLIC'] = unit_get("public-address")
+    os.environ['OPENSSL_PRIVATE'] = unit_get("private-address")
+    run(
+        ['openssl', 'req', '-new', '-x509', '-nodes', '-config',
+         os.path.join(os.environ['CHARM_DIR'], 'data', 'openssl.cnf'),
+         '-keyout', key_file, '-out', cert_file])
+
+
+def is_selfsigned_cert_stale(config, cert_file, key_file):
+    """
+    Do we need to generate a new self-signed cert?
+
+    @param config: charm data from config-get
+    @param cert_file: destination path of generated certificate
+    @param key_file: destination path of generated private key
+    """
+    # Basic Existence Checks
+    if not os.path.exists(cert_file):
+        return True
+    if not os.path.exists(key_file):
+        return True
+
+    # Common Name
+    from OpenSSL import crypto
+    cert = crypto.load_certificate(
+        crypto.FILETYPE_PEM, file(cert_file).read())
+    cn = cert.get_subject().commonName
+    if config['servername'] != cn:
+        return True
+
+    # Subject Alternate Name -- only trusty+ support this
+    try:
+        from pyasn1.codec.der import decoder
+        from pyasn1_modules import rfc2459
+    except ImportError:
+        log("Cannot check subjAltName on <= 12.04, skipping.")
+        return False
+    cert_addresses = set()
+    unit_addresses = set(
+        [unit_get("public-address"), unit_get("private-address")])
+    for i in range(0, cert.get_extension_count()):
+        extension = cert.get_extension(i)
+        try:
+            names = decoder.decode(
+                extension.get_data(), asn1Spec=rfc2459.SubjectAltName())[0]
+            for name in names:
+                cert_addresses.add(str(name.getComponent()))
+        except:
+            pass
+    if cert_addresses != unit_addresses:
+        log("subjAltName: Cert (%s) != Unit (%s), assuming stale" % (
+            cert_addresses, unit_addresses))
+        return True
+
+    return False
+
+
 def _get_key_file_location(config_data):
     """Look at the config, generate the key file location."""
     key_file = None
@@ -207,9 +271,6 @@
     return chain_file
 
 
-###############################################################################
-# Hook functions
-###############################################################################
 def config_get(scope=None):
     """
     Wrapper around charm helper's config_get to replace an empty servername
@@ -227,6 +288,9 @@
     if not os.path.exists(default_apache2_service_config_dir):
         os.mkdir(default_apache2_service_config_dir, 0600)
     apt_get_install("python-jinja2")
+    apt_get_install("python-pyasn1")
+    apt_get_install("python-pyasn1-modules")
+    apt_get_install("python-yaml")
     install_status = apt_get_install("apache2")
     if install_status == 0:
         ensure_package_status(service_affecting_packages,
@@ -518,12 +582,8 @@
     if cert_file is not None and key_file is not None:
         # ssl_cert is SELFSIGNED so generate self-signed certificate for use.
         if config_data['ssl_cert'] and config_data['ssl_cert'] == "SELFSIGNED":
-            os.environ['OPENSSL_CN'] = config_data['servername']
-            os.environ['OPENSSL_PUBLIC'] = unit_get("public-address")
-            os.environ['OPENSSL_PRIVATE'] = unit_get("private-address")
-            run(['openssl', 'req', '-new', '-x509', '-nodes', '-config',
-                 os.path.join(os.environ['CHARM_DIR'], 'data', 'openssl.cnf'),
-                 '-keyout', key_file, '-out', cert_file])
+            if is_selfsigned_cert_stale(config_data, cert_file, key_file):
+                gen_selfsigned_cert(config_data, cert_file, key_file)
 
         # Use SSL certificate and key provided either as a base64 string or
         # shipped out with the charm.

=== modified file 'hooks/tests/test_balancer_hook.py'
--- hooks/tests/test_balancer_hook.py	2014-01-16 12:33:28 +0000
+++ hooks/tests/test_balancer_hook.py	2014-06-28 19:27:49 +0000
@@ -515,6 +515,9 @@
                                  0600)
         apt_get_install.assert_has_calls([
             call('python-jinja2'),
+            call('python-pyasn1'),
+            call('python-pyasn1-modules'),
+            call('python-yaml'),
             call('apache2'),
         ])
 
@@ -534,6 +537,9 @@
         self.assertEqual(result, 'some result')
         apt_get_install.assert_has_calls([
             call('python-jinja2'),
+            call('python-pyasn1'),
+            call('python-pyasn1-modules'),
+            call('python-yaml'),
             call('apache2'),
             call('extra'),
         ])
@@ -556,6 +562,9 @@
         self.assertFalse(mkdir.called)
         apt_get_install.assert_has_calls([
             call('python-jinja2'),
+            call('python-pyasn1'),
+            call('python-pyasn1-modules'),
+            call('python-yaml'),
             call('apache2'),
         ])