From a233c7db6170f44a6084502625fa92a54bebb837 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Fri, 26 Mar 2021 16:53:30 +0000 Subject: [PATCH 1/3] Add additional tests for check_cert_key - Add tests to complete check of ordering of cert and key. - Add checks of invalid cert and key files. --- test/test_crypto.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/test_crypto.py b/test/test_crypto.py index 1f8adbc0..da85b4ad 100644 --- a/test/test_crypto.py +++ b/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, \ @@ -75,6 +76,19 @@ def test_check_cert_key(self): if check_cert_key(TEST_CERT_FILE, TEST_CERT_FILE): self.fail('Accepted certificate as key.') + # Check incorrect ordering of cert and key path arguments. + 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.') + + # 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.') + if not check_cert_key(TEST_CERT_FILE, TEST_KEY_FILE): self.fail('Cert and key match but function failed.') From 21da9db5da697293939ca56337a712768bbdbcb4 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Fri, 26 Mar 2021 17:55:02 +0000 Subject: [PATCH 2/3] Refactor check_cert_key tests to modern Python --- test/test_crypto.py | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/test/test_crypto.py b/test/test_crypto.py index da85b4ad..2f613ca7 100644 --- a/test/test_crypto.py +++ b/test/test_crypto.py @@ -60,38 +60,36 @@ 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.') - - # Check incorrect ordering of cert and key path arguments. + """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.') - # Check behaviour with an invalid cert or key file. + 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.') - if not check_cert_key(TEST_CERT_FILE, TEST_KEY_FILE): - self.fail('Cert and key match but function failed.') - def test_sign(self): ''' I haven't found a good way to test this yet. Each time you sign a From 014fc47f640d2709ea16f39ebf945acae8f2d3f6 Mon Sep 17 00:00:00 2001 From: Carlos A Date: Sat, 4 Apr 2020 15:43:29 +0200 Subject: [PATCH 3/3] Generalize certificate and a key matching Change to comparing the public keys of the cert and key as this allows for both RSA and EC keys. --- ssm/crypto.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ssm/crypto.py b/ssm/crypto.py index 4000e6d7..e9380396 100644 --- a/ssm/crypto.py +++ b/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.