Enable OCSP and revocation checking based on certificate and chain filepaths

This commit is contained in:
Joona Hoikkala 2020-01-26 15:42:01 +02:00
parent fe0a985228
commit 53f8ad88db
No known key found for this signature in database
GPG key ID: D5AA86BBF9B29A5C
3 changed files with 25 additions and 19 deletions

View file

@ -115,7 +115,7 @@ class OCSPPrefetchMixin(object):
self.config.work_dir, "ocsp",
apache_util.certid_sha1_hex(cert_path))
handler = ocsp.RevocationChecker()
if not handler.revoked(cert_path, chain_path, ocsp_workfile):
if not handler.ocsp_revoked_cert(cert_path, chain_path, ocsp_workfile):
# Guaranteed good response
cache_path = os.path.join(self.config.config_dir, "ocsp", "ocsp_cache")
# dbm.open automatically adds the file extension, it will be

View file

@ -65,21 +65,14 @@ class RevocationChecker(object):
.. todo:: Make this a non-blocking call
:param `.storage.RenewableCert` cert: Certificate object
:returns: True if revoked; False if valid or the check failed or cert is expired.
:rtype: bool
"""
cert_path, chain_path = cert.cert, cert.chain
return self.ocsp_revoked_cert(cert.cert, cert.chain)
# Let's Encrypt doesn't update OCSP for expired certificates,
# so don't check OCSP if the cert is expired.
# https://github.com/certbot/certbot/issues/7152
now = pytz.UTC.fromutc(datetime.utcnow())
if cert.target_expiry <= now:
return False
return self.revoked(cert_path, chain_path)
def revoked(self, cert_path, chain_path, response_file=None):
def ocsp_revoked_cert(self, cert_path, chain_path, response_file=None):
# type: (str, str, Optional[str]) -> bool
"""Performs the OCSP revocation check
@ -90,6 +83,13 @@ class RevocationChecker(object):
:rtype: bool
"""
# Let's Encrypt doesn't update OCSP for expired certificates,
# so don't check OCSP if the cert is expired.
# https://github.com/certbot/certbot/issues/7152
now = pytz.UTC.fromutc(datetime.utcnow())
if crypto_util.notAfter(cert_path) <= now:
return False
if self.broken:
return False

View file

@ -75,13 +75,14 @@ class OCSPTestOpenSSL(unittest.TestCase):
self.assertEqual(checker.broken, True)
@mock.patch('certbot._internal.ocsp._determine_ocsp_server')
@mock.patch('certbot._internal.ocsp.crypto_util.notAfter')
@mock.patch('certbot.util.run_script')
def test_ocsp_revoked(self, mock_run, mock_determine):
def test_ocsp_revoked(self, mock_run, mock_na, mock_determine):
now = pytz.UTC.fromutc(datetime.utcnow())
cert_obj = mock.MagicMock()
cert_obj.cert = "x"
cert_obj.chain = "y"
cert_obj.target_expiry = now + timedelta(hours=2)
mock_na.return_value = now + timedelta(hours=2)
self.checker.broken = True
mock_determine.return_value = ("", "")
@ -99,7 +100,7 @@ class OCSPTestOpenSSL(unittest.TestCase):
self.assertEqual(mock_run.call_count, 2)
# cert expired
cert_obj.target_expiry = now
mock_na.return_value = now
mock_determine.return_value = ("", "")
count_before = mock_determine.call_count
self.assertEqual(self.checker.ocsp_revoked(cert_obj), False)
@ -152,21 +153,26 @@ class OSCPTestCryptography(unittest.TestCase):
self.cert_obj = mock.MagicMock()
self.cert_obj.cert = self.cert_path
self.cert_obj.chain = self.chain_path
def _call_expirymock(self, func, *args, **kwargs):
"""Call function with mocked certificate expiry time"""
now = pytz.UTC.fromutc(datetime.utcnow())
self.cert_obj.target_expiry = now + timedelta(hours=2)
with mock.patch('certbot._internal.ocsp.crypto_util.notAfter') as mock_na:
mock_na.return_value = now + timedelta(hours=2)
return func(*args, **kwargs)
@mock.patch('certbot._internal.ocsp._determine_ocsp_server')
@mock.patch('certbot._internal.ocsp._check_ocsp_cryptography')
def test_ensure_cryptography_toggled(self, mock_revoke, mock_determine):
mock_determine.return_value = ('http://example.com', 'example.com')
self.checker.ocsp_revoked(self.cert_obj)
self._call_expirymock(self.checker.ocsp_revoked, self.cert_obj)
mock_revoke.assert_called_once_with(self.cert_path, self.chain_path,
'http://example.com', None)
def test_revoke(self):
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL):
revoked = self.checker.ocsp_revoked(self.cert_obj)
revoked = self._call_expirymock(self.checker.ocsp_revoked, self.cert_obj)
self.assertTrue(revoked)
def test_responder_is_issuer(self):
@ -176,7 +182,7 @@ class OSCPTestCryptography(unittest.TestCase):
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED,
ocsp_lib.OCSPResponseStatus.SUCCESSFUL) as mocks:
mocks['mock_response'].return_value.responder_name = issuer.subject
self.checker.ocsp_revoked(self.cert_obj)
self._call_expirymock(self.checker.ocsp_revoked, self.cert_obj)
# Here responder and issuer are the same. So only the signature of the OCSP
# response is checked (using the issuer/responder public key).
self.assertEqual(mocks['mock_check'].call_count, 1)
@ -191,7 +197,7 @@ class OSCPTestCryptography(unittest.TestCase):
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED,
ocsp_lib.OCSPResponseStatus.SUCCESSFUL) as mocks:
self.checker.ocsp_revoked(self.cert_obj)
self._call_expirymock(self.checker.ocsp_revoked, self.cert_obj)
# Here responder and issuer are not the same. Two signatures will be checked then,
# first to verify the responder cert (using the issuer public key), second to
# to verify the OCSP response itself (using the responder public key).