Simplify the ocsp_revoked() return type

- we weren't reacting to None, so call it False instead
This commit is contained in:
Peter Eckersley 2016-12-22 15:35:29 -08:00
parent 19143d8303
commit 9aa93c05c1
2 changed files with 11 additions and 11 deletions

View file

@ -37,17 +37,17 @@ class RevocationChecker(object):
:param str cert_path: Path to certificate
:param str chain_path: Path to intermediate cert
:rtype bool or None:
:returns: False if valid; True if revoked; None if check itself failed
:returns: True if revoked; False if valid or the check failed
"""
if self.broken:
return None
return False
logger.debug("Querying OCSP for %s", cert_path)
url, host = self.determine_ocsp_server(cert_path)
if not host:
return None
return False
# jdkasten thanks "Bulletproof SSL and TLS - Ivan Ristic" for documenting this!
cmd = ["openssl", "ocsp",
"-no_nonce",
@ -62,7 +62,7 @@ class RevocationChecker(object):
except errors.SubprocessError as e:
logger.info("OCSP check failed for %s (are we offline?)", cert_path)
logger.debug("Command was:\n%s\nError was:\n%s", " ".join(cmd), e)
return None
return False
return _translate_ocsp_query(cert_path, output, err)
@ -98,12 +98,12 @@ def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors):
if not "Response verify OK" in ocsp_errors:
logger.info("Revocation status for %s is unknown", cert_path)
logger.debug("Uncertain ouput:\n%s\nstderr:\n%s", ocsp_output, ocsp_errors)
return None
return False
if cert_path + ": good" in ocsp_output:
return False
elif cert_path + ": revoked" in ocsp_output:
return True
else:
logger.warn("Unable to properly parse OCSP output: %s", ocsp_output)
return None
return False

View file

@ -59,17 +59,17 @@ class OCSPTest(unittest.TestCase):
def test_ocsp_revoked(self, mock_run, mock_determine):
self.checker.broken = True
mock_determine.return_value = ("", "")
self.assertEqual(self.checker.ocsp_revoked("x", "y"), None)
self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
self.checker.broken = False
mock_run.return_value = tuple(openssl_happy[1:])
self.assertEqual(self.checker.ocsp_revoked("x", "y"), None)
self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
self.assertEqual(mock_run.call_count, 0)
mock_determine.return_value = ("http://x.co", "x.co")
self.assertEqual(self.checker.ocsp_revoked("blah.pem", "chain.pem"), False)
mock_run.side_effect = errors.SubprocessError("Unable to load certificate launcher")
self.assertEqual(self.checker.ocsp_revoked("x", "y"), None)
self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
self.assertEqual(mock_run.call_count, 2)
@ -97,10 +97,10 @@ class OCSPTest(unittest.TestCase):
mock_run.return_value = openssl_confused
from certbot import ocsp
self.assertEqual(ocsp._translate_ocsp_query(*openssl_happy), False)
self.assertEqual(ocsp._translate_ocsp_query(*openssl_confused), None)
self.assertEqual(ocsp._translate_ocsp_query(*openssl_confused), False)
self.assertEqual(mock_log.debug.call_count, 1)
self.assertEqual(mock_log.warn.call_count, 0)
self.assertEqual(ocsp._translate_ocsp_query(*openssl_broken), None)
self.assertEqual(ocsp._translate_ocsp_query(*openssl_broken), False)
self.assertEqual(mock_log.warn.call_count, 1)
self.assertEqual(ocsp._translate_ocsp_query(*openssl_revoked), True)