From 6d3f00a38deb33f39d977c8a432dd047ad44326c Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Tue, 21 May 2019 14:25:23 +0100 Subject: [PATCH] OpenSSL checkend fix (#97) - Add missing seconds argument after '-checkend' as it should be followed by a number that defines how many seconds ahead openssl x509 should check for cert expiry, otherwise later versions of OpenSSL raise syntax errors (treating next arg as the number). Set it to a day as certs should really be replaced before that considering the amount of time it can take a message to traverse the messaging system. - Update error messages to match verify_cert_date as verify_cert_date has been updated to check that the cert won't expire within the next day, the error messages should explain this if there's a verification failure - Increase the temp cert validity to 2 days so that it's comfortably longer than the period that verify_cert_date checks for (1 day). - Update expected error for expired cert test to match updated function. --- ssm/crypto.py | 9 +++++++-- ssm/ssm2.py | 9 +++++---- test/test_ssm.py | 10 ++++++---- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ssm/crypto.py b/ssm/crypto.py index 7fe6f176..f3b88ac0 100644 --- a/ssm/crypto.py +++ b/ssm/crypto.py @@ -214,11 +214,16 @@ def decrypt(encrypted_text, certpath, keypath): def verify_cert_date(certpath): - """Return True if certifcate is 'in date', otherwise return False.""" + """Check that certificate hasn't expired and won't expire within 24 hours. + + Return True if certifcate is 'in date', otherwise return False. + """ if certpath is None: raise CryptoException('Invalid None argument to verify_cert_date().') - args = ['openssl', 'x509', '-checkend', '-noout', '-in', certpath] + # Check if the certificate expires within the next 86400 seconds and exit + # non-zero if yes, it will expire, or zero if not. + args = ['openssl', 'x509', '-checkend', '86400', '-noout', '-in', certpath] p1 = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE) diff --git a/ssm/ssm2.py b/ssm/ssm2.py index c9b6d344..28e9762b 100644 --- a/ssm/ssm2.py +++ b/ssm/ssm2.py @@ -148,7 +148,8 @@ def __init__(self, hosts_and_ports, qpath, cert, key, dest=None, listen=None, # Check that the certificate has not expired. if not crypto.verify_cert_date(self._cert): - raise Ssm2Exception('Certificate %s has expired.' % self._cert) + raise Ssm2Exception('Certificate %s has expired or will expire ' + 'within a day.' % self._cert) # check the server certificate provided if enc_cert is not None: @@ -158,9 +159,9 @@ def __init__(self, hosts_and_ports, qpath, cert, key, dest=None, listen=None, # Check that the encyption certificate has not expired. if not crypto.verify_cert_date(enc_cert): raise Ssm2Exception( - 'Encryption certificate %s has expired. Please obtain the ' - 'new one from the final server receiving your messages.' % - enc_cert + 'Encryption certificate %s has expired or will expire ' + 'within a day. Please obtain the new one from the final ' + 'server receiving your messages.' % enc_cert ) if verify_enc_cert: if not crypto.verify_cert_path(self._enc_cert, self._capath, self._check_crls): diff --git a/test/test_ssm.py b/test/test_ssm.py index 79d1c2a3..896adba6 100644 --- a/test/test_ssm.py +++ b/test/test_ssm.py @@ -44,7 +44,8 @@ def setUp(self): # The subject has been hardcoded so that the generated # certificate subject matches the subject of the hardcoded, # expired, certificate at the bottom of this file. - call(['openssl', 'req', '-x509', '-nodes', '-days', '1', '-new', + # 2 days used so that verify_cert_date doesn't think it expires soon. + call(['openssl', 'req', '-x509', '-nodes', '-days', '2', '-new', '-key', self._key_path, '-out', TEST_CERT_FILE, '-subj', '/C=UK/O=STFC/OU=SC/CN=Test Cert']) @@ -66,7 +67,7 @@ def tearDown(self): except OSError, e: print 'Error removing temporary directory %s' % self._tmp_dir print e - + def test_on_message(self): ''' This is quite a complicated method, so it would take a long time @@ -95,8 +96,8 @@ def test_on_message(self): def test_init_expired_cert(self): """Test right exception is thrown creating an SSM with expired cert.""" - expected_error = ('Certificate %s has expired.' - % self._expired_cert_path) + expected_error = ('Certificate %s has expired or will expire ' + 'within a day.' % self._expired_cert_path) try: # Indirectly test crypto.verify_cert_date Ssm2(self._brokers, self._msgdir, self._expired_cert_path, @@ -134,6 +135,7 @@ def test_ssm_init_non_dirq(self): # Assert the outbound queue is of the expected type. self.assertTrue(isinstance(ssm._outq, MessageDirectory)) + TEST_CERT_FILE = '/tmp/test.crt' # As we want the expired certifcate to match the key used, we can't