Skip to content

Commit

Permalink
OpenSSL checkend fix (#97)
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
tofu-rocketry authored May 21, 2019
1 parent 1a9f10c commit 6d3f00a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 10 deletions.
9 changes: 7 additions & 2 deletions ssm/crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
9 changes: 5 additions & 4 deletions ssm/ssm2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down
10 changes: 6 additions & 4 deletions test/test_ssm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'])

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6d3f00a

Please sign in to comment.