Merge lp:~jacekn/charm-helpers/charm-helpers-ssl into lp:charm-helpers

[Jacek Nykis]

Added contrib.ssl module which can create selfsigned SSL certificates.

[Matthew Wedgwood (mew)]

This looks quite helpful. Thanks.

I see a few issues here.

Running this as generate_selfsigned("mykey", "mycert") fails:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "charmhelpers/contrib/ssl/__init__.py", line 19, in generate_selfsigned
    subprocess.check_call(cmd)
UnboundLocalError: local variable 'cmd' referenced before assignment

There should be a test that catches this.

Furthermore, the "subject" argument is a little opaque. I somewhat expected to be able to pass it a hostname, but that's not at all what's needed. I'd recommend any combination of:

1. Adding a docstring with a little explanation
2. Exposing the subject components as function args (rather than as one dict)

As a bonus, it might be nice to allow users to specify the number of bits the for the cert as 1024 will inevitably fall out of vogue.
3. Building a simple "CertificateSubject" class to help users construct a valid subject
4. Providing reasonable defaults.

review: Needs Fixing

[lp:~jacekn/charm-helpers/charm-helpers-ssl]

75. By Jacek Nykis on 2013-08-27

Added docstring to the generate_selfsigned function. Improved the function to accept single cn argument and partial subject dict

[Matthew Wedgwood (mew)]

I'm able to get the new function succeed using the "cn" parameter, but it explodes[1] when I try to use "subject." I've tried that one with various permutations on the keys included in the dict. I haven't tested the "config" parameter.

Also, I notice you're returning False for failures, but when the function actually succeeds, it returns None. You might want an explicit "return True" at the end.

[1] http://pastebin.ubuntu.com/6034287/

review: Needs Fixing

[lp:~jacekn/charm-helpers/charm-helpers-ssl]

76. By Jacek Nykis on 2013-08-28

Added more error checking to contrib.ssl.generate_selfsigned function

[Jacek Nykis (jacekn)]

I added more error checking and return True statement.

Your test is failing as expected, country code should be 2 letter code. With my changes this will be picked up by the try/except block I added but there is no way I can return exact reason for openssl command failure.

[Matthew Wedgwood (mew)]

Ah, that makes sense. With these changes, LGTM.

review: Approve

=== added directory 'charmhelpers/contrib/ssl'
=== added file 'charmhelpers/contrib/ssl/__init__.py'
--- charmhelpers/contrib/ssl/__init__.py	1970-01-01 00:00:00 +0000
+++ charmhelpers/contrib/ssl/__init__.py	2013-08-28 09:12:15 +0000
@@ -0,0 +1,79 @@
+import subprocess
+from charmhelpers.core import hookenv
+
+
+def generate_selfsigned(keyfile, certfile, keysize="1024", config=None, subject=None, cn=None):
+    """Generate selfsigned SSL keypair
+
+    You must provide one of the 3 optional arguments:
+    config, subject or cn
+    If more than one is provided the leftmost will be used
+
+    Arguments:
+    keyfile -- (required) full path to the keyfile to be created
+    certfile -- (required) full path to the certfile to be created
+    keysize -- (optional) SSL key length
+    config -- (optional) openssl configuration file
+    subject -- (optional) dictionary with SSL subject variables
+    cn -- (optional) cerfificate common name
+
+    Required keys in subject dict:
+    cn -- Common name (eq. FQDN)
+
+    Optional keys in subject dict
+    country -- Country Name (2 letter code)
+    state -- State or Province Name (full name)
+    locality -- Locality Name (eg, city)
+    organization -- Organization Name (eg, company)
+    organizational_unit -- Organizational Unit Name (eg, section)
+    email -- Email Address
+    """
+
+    cmd = []
+    if config:
+        cmd = ["/usr/bin/openssl", "req", "-new", "-newkey",
+               "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509",
+               "-keyout", keyfile,
+               "-out",    certfile, "-config", config]
+    elif subject:
+        ssl_subject = ""
+        if "country" in subject:
+            ssl_subject = ssl_subject + "/C={}".format(subject["country"])
+        if "state" in subject:
+            ssl_subject = ssl_subject + "/ST={}".format(subject["state"])
+        if "locality" in subject:
+            ssl_subject = ssl_subject + "/L={}".format(subject["locality"])
+        if "organization" in subject:
+            ssl_subject = ssl_subject + "/O={}".format(subject["organization"])
+        if "organizational_unit" in subject:
+            ssl_subject = ssl_subject + "/OU={}".format(subject["organizational_unit"])
+        if "cn" in subject:
+            ssl_subject = ssl_subject + "/CN={}".format(subject["cn"])
+        else:
+            hookenv.log("When using \"subject\" argument you must " \
+                "provide \"cn\" field at very least")
+            return False
+        if "email" in subject:
+            ssl_subject = ssl_subject + "/emailAddress={}".format(subject["email"])
+
+        cmd = ["/usr/bin/openssl", "req", "-new", "-newkey",
+               "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509",
+               "-keyout", keyfile,
+               "-out",    certfile, "-subj", ssl_subject]
+    elif cn:
+        cmd = ["/usr/bin/openssl", "req", "-new", "-newkey",
+               "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509",
+               "-keyout", keyfile,
+               "-out",    certfile, "-subj", "/CN={}".format(cn)]
+
+    if not cmd:
+        hookenv.log("No config, subject or cn provided," \
+                "unable to generate self signed SSL certificates")
+        return False
+    try:
+        subprocess.check_call(cmd)
+        return True
+    except Exception as e:
+        print "Execution of openssl command failed:\n{}".format(e)
+        return False
+
 
=== added directory 'tests/contrib/ssl'
=== added file 'tests/contrib/ssl/__init__.py'
=== added file 'tests/contrib/ssl/test_ssl.py'
--- tests/contrib/ssl/test_ssl.py	1970-01-01 00:00:00 +0000
+++ tests/contrib/ssl/test_ssl.py	2013-08-28 09:12:15 +0000
@@ -0,0 +1,60 @@
+import subprocess
+
+from mock import patch, call, MagicMock
+from testtools import TestCase
+
+from charmhelpers.contrib import ssl
+
+
+class HelpersTest(TestCase):
+    @patch('subprocess.check_call')
+    def test_generate_selfsigned_dict(self, mock_call):
+        subject = {"country": "UK",
+                   "locality": "my_locality",
+                   "state": "my_state",
+                   "organization": "my_organization",
+                   "organizational_unit": "my_unit",
+                   "cn": "mysite.example.com",
+                   "email": "[e-mail]"
+                   }
+
+        ssl.generate_selfsigned("mykey.key", "mycert.crt", subject=subject)
+        mock_call.assert_called_with(['/usr/bin/openssl', 'req', '-new',
+                                      '-newkey', 'rsa:1024', '-days', '365',
+                                      '-nodes', '-x509', '-keyout',
+                                      'mykey.key', '-out', 'mycert.crt',
+                                      '-subj',
+                                      '/C=UK/ST=my_state/L=my_locality'
+                                      '/O=my_organization/OU=my_unit'
+                                      '/CN=mysite.example.com'
+                                      '/emailAddress=[e-mail]']
+                                     )
+
+    @patch('charmhelpers.core.hookenv.log')
+    def test_generate_selfsigned_failure(self, mock_log):
+        # This is NOT enough, functino requires cn key
+        subject = {"country": "UK",
+                   "locality": "my_locality"}
+
+        result = ssl.generate_selfsigned("mykey.key", "mycert.crt", subject=subject)
+        self.assertFalse(result)
+
+
+    @patch('subprocess.check_call')
+    def test_generate_selfsigned_file(self, mock_call):
+        ssl.generate_selfsigned("mykey.key", "mycert.crt", config="test.cnf")
+        mock_call.assert_called_with(['/usr/bin/openssl', 'req', '-new',
+                                      '-newkey', 'rsa:1024', '-days', '365',
+                                      '-nodes', '-x509', '-keyout',
+                                      'mykey.key', '-out', 'mycert.crt',
+                                      '-config', 'test.cnf'])
+
+
+    @patch('subprocess.check_call')
+    def test_generate_selfsigned_cn_key(self, mock_call):
+        ssl.generate_selfsigned("mykey.key", "mycert.crt", keysize="2048", cn="mysite.example.com")
+        mock_call.assert_called_with(['/usr/bin/openssl', 'req', '-new',
+                                      '-newkey', 'rsa:2048', '-days', '365',
+                                      '-nodes', '-x509', '-keyout',
+                                      'mykey.key', '-out', 'mycert.crt',
+                                      '-subj', '/CN=mysite.example.com'])