upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-06-06 23:32:06 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Use certificate file path as key for the internal storage and remove revoked and deleted certificates from pool when met

Browse source
This commit is contained in:
Joona Hoikkala 2020-04-16 00:50:19 +03:00
parent 57cd0c7d81
commit 741278ef67
No known key found for this signature in database
GPG key ID: D5AA86BBF9B29A5C
2 changed files with 62 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
certbot-apache/certbot_apache/_internal/prefetch_ocsp.py
View file

@ -34,7 +34,7 @@ from datetime import datetime
import logging
import time
from acme.magic_typing import Dict # pylint: disable=unused-import, no-name-in-module
from acme.magic_typing import Dict, Union # pylint: disable=unused-import, no-name-in-module
from certbot import errors
from certbot import ocsp
@ -90,7 +90,7 @@ class OCSPPrefetchMixin(object):
"""OCSPPrefetchMixin implements OCSP response prefetching"""
def __init__(self, *args, **kwargs):
self._ocsp_prefetch = {} # type: Dict[str, str]
self._ocsp_prefetch = {} # type: Dict[str, Dict[str, Union[str, float]]]
# This is required because of python super() call chain.
# Additionally, mypy isn't able to figure the chain out and needs to be
# disabled for this line. See https://github.com/python/mypy/issues/5887
@ -121,16 +121,16 @@ class OCSPPrefetchMixin(object):
"Apache OCSP stapling cache database.")
raise errors.NotSupportedError(msg)
def _ocsp_refresh_needed(self, pf_obj):
def _ocsp_refresh_needed(self, lastupdate):
"""Refreshes OCSP response for a certificate if it's due
:param dict pf_obj: OCSP prefetch object from pluginstorage
:param int lastupdate: Last update timestamp from pluginstorage entry
:returns: If OCSP response was updated
:rtype: bool
"""
ttl = pf_obj["lastupdate"] + constants.OCSP_INTERNAL_TTL
ttl = lastupdate + constants.OCSP_INTERNAL_TTL
if ttl < time.time():
return True
return False
@ -150,6 +150,9 @@ class OCSPPrefetchMixin(object):
os.path.dirname(self._ocsp_work),
apache_util.certid_sha1_hex(cert_path))
handler = ocsp.RevocationChecker()
if not os.path.isfile(cert_path):
raise OCSPCertificateError("Certificate has been removed from the system.")
if not handler.ocsp_revoked_by_paths(cert_path, chain_path, ocsp_workfile):
# Guaranteed good response
cert_sha = apache_util.certid_sha1(cert_path)
@ -159,6 +162,7 @@ class OCSPPrefetchMixin(object):
else:
logger.warning("Encountered an issue while trying to prefetch OCSP "
"response for certificate: %s", cert_path)
raise OCSPCertificateError("Certificate has been revoked.")
finally:
try:
os.remove(ocsp_workfile)
@ -251,14 +255,21 @@ class OCSPPrefetchMixin(object):
"""
status = {
"lastupdate": time.time(),
"cert_path": cert_path,
"chain_path": chain_path
}
cert_id = apache_util.certid_sha1_hex(cert_path)
self._ocsp_prefetch[cert_id] = status
self._ocsp_prefetch[cert_path] = status
self.storage.put("ocsp_prefetch", self._ocsp_prefetch)
self.storage.save()
def _ocsp_prefetch_remove(self, cert_path):
"""Removes OCSP prefetch configuration from PluginStorage object for
a certificate.
:param str cert_path: Filesystem path to certificate
"""
self._ocsp_prefetch.pop(cert_path)
self.storage.put("ocsp_prefetch", self._ocsp_prefetch)
def _ocsp_prefetch_fetch_state(self):
"""
Populates the OCSP prefetch state from the pluginstorage.
@ -365,16 +376,24 @@ class OCSPPrefetchMixin(object):
# No OCSP prefetching enabled for any certificate
return
for pf in self._ocsp_prefetch.values():
if self._ocsp_refresh_needed(pf):
# make a copy of the list of dictionary keys as we might remove items mid-iteration
for cert_path in list(self._ocsp_prefetch):
pf = self._ocsp_prefetch[cert_path]
if self._ocsp_refresh_needed(pf["lastupdate"]):
try:
self._ocsp_refresh(pf["cert_path"], pf["chain_path"])
self._ocsp_refresh(cert_path, pf["chain_path"])
except OCSPCertificateError as err:
self._ocsp_prefetch_remove(cert_path)
msg = ("Error when trying to prefetch OCSP staple: {} " +
"OCSP prefetch functionality removed for the certificate").format(err)
logger.warning(msg)
continue
except errors.PluginError as err:
msg = "Encountered a issue when trying to renew OCSP staple: {}".format(err)
logger.warning(msg)
# Save the status to pluginstorage. Do this even if errored out to prevent
# spurious errors when handling multiple renewals.
self._ocsp_prefetch_save(pf["cert_path"], pf["chain_path"])
self._ocsp_prefetch_save(cert_path, pf["chain_path"])
def restart(self):
"""Reloads the Apache server. When restarting, Apache deletes
@ -404,4 +423,8 @@ class OCSPPrefetchMixin(object):
self._ocsp_prefetch_restore_db()
class OCSPCertificateError(errors.PluginError):
"""Error that prompts for removal of certificate from OCSP prefetch pool."""
OCSPPrefetchEnhancement.register(OCSPPrefetchMixin) # pylint: disable=no-member

53
certbot-apache/tests/ocsp_prefetch_test.py
View file

@ -4,6 +4,8 @@ from datetime import datetime
from datetime import timedelta
import json
import sys
import tempfile
import time
import unittest
import mock
@ -89,7 +91,8 @@ class OCSPPrefetchTest(util.ApacheTest):
self.config_path, self.vhost_path, self.config_dir, self.work_dir,
os_info="debian")
self.lineage = mock.MagicMock(cert_path="cert", chain_path="chain")
_, self.tmp_certfile = tempfile.mkstemp()
self.lineage = mock.MagicMock(cert_path=self.tmp_certfile, chain_path="chain")
self.config.parser.modules.add("headers_module")
self.config.parser.modules.add("mod_headers.c")
self.config.parser.modules.add("ssl_module")
@ -102,6 +105,9 @@ class OCSPPrefetchTest(util.ApacheTest):
self.config._ensure_ocsp_dirs()
self.db_path = os.path.join(self.work_dir, "ocsp", "ocsp_cache") + ".db"
def tearDown(self):
os.remove(self.tmp_certfile)
def _call_mocked(self, func, *args, **kwargs):
"""Helper method to call functins with mock stack"""
@ -154,7 +160,7 @@ class OCSPPrefetchTest(util.ApacheTest):
ref_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_refresh"
with mock.patch(ref_path):
self.call_mocked_py2(self.config.enable_ocsp_prefetch,
self.lineage, ["ocspvhost.com"])
self.lineage, ["ocspvhost.com"])
self.assertTrue(mock_enable.called)
self.assertEqual(len(self.config._ocsp_prefetch), 1)
@ -211,35 +217,27 @@ class OCSPPrefetchTest(util.ApacheTest):
self.call_mocked_py2(self.config.update_ocsp_prefetch, None)
self.assertTrue(mock_ocsp.called)
def test_ocsp_prefetch_refresh_noop(self):
def ocsp_req_mock(_cert, _chain, workfile):
"""Method to mock the OCSP request and write response to file"""
with open(workfile, 'w') as fh:
fh.write("MOCKRESPONSE")
return True
ocsp_path = "certbot.ocsp.RevocationChecker.ocsp_revoked_by_paths"
with mock.patch(ocsp_path, side_effect=ocsp_req_mock):
self.call_mocked_py2(self.config.enable_ocsp_prefetch,
self.lineage, ["ocspvhost.com"])
def test_ocsp_prefetch_refresh_cert_deleted(self):
self.config._ocsp_prefetch_save("nonexistent_cert_path", "irrelevant")
self.assertEqual(len(self.config._ocsp_prefetch), 1)
rn_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_refresh_needed"
with mock.patch(rn_path) as mock_needed:
mock_needed.return_value = True
with mock.patch("certbot_apache._internal.prefetch_ocsp.logger.warning") as mock_log:
self.call_mocked_py2(self.config.update_ocsp_prefetch, None)
self.assertTrue(mock_log.called)
self.assertTrue("has been removed" in mock_log.call_args[0][0])
self.assertEqual(len(self.config._ocsp_prefetch), 0)
def test_ocsp_prefetch_refresh_noop(self):
self.config._ocsp_prefetch_save(self.lineage.cert_path, "irrelevant")
refresh_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_refresh"
with mock.patch(refresh_path) as mock_refresh:
self.call_mocked_py2(self.config.update_ocsp_prefetch, None)
self.assertFalse(mock_refresh.called)
def test_ocsp_prefetch_refresh_error(self):
def ocsp_req_mock(_cert, _chain, workfile):
"""Method to mock the OCSP request and write response to file"""
with open(workfile, 'w') as fh:
fh.write("MOCKRESPONSE")
return True
ocsp_path = "certbot.ocsp.RevocationChecker.ocsp_revoked_by_paths"
with mock.patch(ocsp_path, side_effect=ocsp_req_mock):
self.call_mocked_py2(self.config.enable_ocsp_prefetch,
self.lineage, ["ocspvhost.com"])
self.assertEqual(len(self.config._ocsp_prefetch), 1)
self.config._ocsp_prefetch_save(self.lineage.cert_path, "irrelevant")
refresh_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_refresh"
rn_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_refresh_needed"
with mock.patch(refresh_path) as mock_refresh:
@ -293,8 +291,11 @@ class OCSPPrefetchTest(util.ApacheTest):
with mock.patch(ocsp_path) as mock_ocsp:
mock_ocsp.return_value = True
with mock.patch(log_path) as mock_log:
self.call_mocked_py2(self.config.enable_ocsp_prefetch,
self.lineage, ["ocspvhost.com"])
self.assertRaises(errors.PluginError,
self.call_mocked_py2,
self.config.enable_ocsp_prefetch,
self.lineage, ["ocspvhost.com"]
)
self.assertTrue(mock_log.called)
self.assertTrue(
"trying to prefetch OCSP" in mock_log.call_args[0][0])