Merge pull request #125 from dealfonso/ec-certs

Generalize certificate and a key matching

ssm/crypto.py

@@ -63,24 +63,23 @@ def check_cert_key(certpath, keypath):
     if cert == key:
         return False
 
-    p1 = Popen(['openssl', 'x509', '-noout', '-modulus'],
+    p1 = Popen(['openssl', 'x509', '-pubkey', '-noout'],
                stdin=PIPE, stdout=PIPE, stderr=PIPE, universal_newlines=True)
-    modulus1, error = p1.communicate(cert)
+    pubkey1, error = p1.communicate(cert)
 
     if error != '':
         log.error(error)
         return False
 
-    p2 = Popen(['openssl', 'rsa', '-noout', '-modulus'],
+    p2 = Popen(['openssl', 'pkey', '-pubout'],
                stdin=PIPE, stdout=PIPE, stderr=PIPE, universal_newlines=True)
-    modulus2, error = p2.communicate(key)
+    pubkey2, error = p2.communicate(key)
 
     if error != '':
         log.error(error)
         return False
 
-    return modulus1.strip() == modulus2.strip()
-
+    return pubkey1.strip() == pubkey2.strip()
 
 def sign(text, certpath, keypath):
     """Sign the message using the certificate and key in the files specified.

test/test_crypto.py

@@ -4,6 +4,7 @@
 import logging
 import os
 from subprocess import call, Popen, PIPE
+import tempfile
 import quopri
 
 from ssm.crypto import check_cert_key, \
@@ -59,24 +60,35 @@ def tearDown(self):
         os.remove(self.ca_certpath)
 
     def test_check_cert_key(self):
-        '''
-        This will print an error log message for the tests that are
-        supposed to fail; you can ignore it.
-        '''
-
-        # One version of the method would have passed this, because of the
-        # way it checked for validity.
-        try:
-            if check_cert_key('hello', 'hello'):
-                self.fail('Accepted non-existent cert and key.')
-        except CryptoException:
-            pass
-
-        if check_cert_key(TEST_CERT_FILE, TEST_CERT_FILE):
-            self.fail('Accepted certificate as key.')
-
-        if not check_cert_key(TEST_CERT_FILE, TEST_KEY_FILE):
-            self.fail('Cert and key match but function failed.')
+        """Check that valid cert and key works."""
+        self.assertTrue(check_cert_key(TEST_CERT_FILE, TEST_KEY_FILE),
+                        'Cert and key match but function failed.')
+
+    def test_check_cert_key_invalid_paths(self):
+        """Check invalid file paths don't return True."""
+        self.assertFalse(check_cert_key('hello', 'hello'),
+                         'Accepted invalid file paths.')
+        self.assertFalse(check_cert_key(TEST_CERT_FILE, 'k'),
+                         'Accepted invalid key path.')
+        self.assertFalse(check_cert_key('c', TEST_KEY_FILE),
+                         'Accepted invalid cert path.')
+
+    def test_check_cert_key_arg_order(self):
+        """Check incorrect order of cert and key path args doesn't succeed."""
+        self.assertFalse(check_cert_key(TEST_CERT_FILE, TEST_CERT_FILE),
+                         'Accepted certificate as key.')
+        self.assertFalse(check_cert_key(TEST_KEY_FILE, TEST_KEY_FILE),
+                         'Accepted key as cert.')
+        self.assertFalse(check_cert_key(TEST_KEY_FILE, TEST_CERT_FILE),
+                         'Accepted key and cert wrong way round.')
+
+    def test_check_cert_key_invalid_files(self):
+        """Check behaviour with an invalid cert or key file."""
+        with tempfile.NamedTemporaryFile() as tmp:
+            self.assertFalse(check_cert_key(tmp.name, TEST_KEY_FILE),
+                             'Accepted invalid cert file.')
+            self.assertFalse(check_cert_key(TEST_CERT_FILE, tmp.name),
+                             'Accepted invalid key file.')
 
     def test_sign(self):
         '''