diff --git a/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py b/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py index 07d0f1088..6a3f04fd2 100644 --- a/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py +++ b/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py @@ -367,6 +367,24 @@ class OCSPPrefetchMixin(object): "enhancement: %s\nOCSP prefetch was not enabled.") raise errors.PluginError(msg % str(err)) + def deploy_ocsp_prefetch(self, lineage): + """When certificate gets renewed, ensure that we're able to serve an appropriate OCSP + staple after the restart that replaces the certificate.""" + + self._ocsp_prefetch_fetch_state() + if not self._ocsp_prefetch: + # No OCSP prefetching enabled for any certificate + return + + if lineage.cert_path in self._ocsp_prefetch: + pf = self._ocsp_prefetch[lineage.cert_path] + try: + self._ocsp_try_refresh(lineage.cert_path) + except OCSPCertificateError: + # This error was logged and handled already down the stack. Return to avoid save. + return + self._ocsp_prefetch_save(lineage.cert_path, pf["chain_path"]) + def update_ocsp_prefetch(self, _unused_lineage): """Checks all certificates that are managed by OCSP prefetch, and refreshes OCSP responses for them if required.""" @@ -381,20 +399,33 @@ class OCSPPrefetchMixin(object): pf = self._ocsp_prefetch[cert_path] if self._ocsp_refresh_needed(pf["lastupdate"]): try: - 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) + self._ocsp_try_refresh(cert_path) + except OCSPCertificateError: + # We want to skip saving in this case, as we just removed the + # certificate from prefetch pool. 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(cert_path, pf["chain_path"]) + def _ocsp_try_refresh(self, cert_path): + """Attempt to refresh OCSP staple for a certificate. + + :param str cert_path: Path to certificate + """ + + pf = self._ocsp_prefetch[cert_path] + + try: + 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) + raise + except errors.PluginError as err: + msg = "Encountered a issue when trying to renew OCSP staple: {}".format(err) + logger.warning(msg) + def restart(self): """Reloads the Apache server. When restarting, Apache deletes the DBM cache file used to store OCSP staples. In this override diff --git a/certbot-apache/tests/ocsp_prefetch_test.py b/certbot-apache/tests/ocsp_prefetch_test.py index 5d90916d7..df9cc877e 100644 --- a/certbot-apache/tests/ocsp_prefetch_test.py +++ b/certbot-apache/tests/ocsp_prefetch_test.py @@ -19,7 +19,7 @@ from certbot import errors from certbot.compat import os import util -from certbot_apache._internal.prefetch_ocsp import DBMHandler +from certbot_apache._internal.prefetch_ocsp import DBMHandler, OCSPCertificateError class MockDBM(object): @@ -186,6 +186,35 @@ class OCSPPrefetchTest(util.ApacheTest): self.lineage, ["domainnotfound"]) + def test_deploy_ocsp_prefetch_noop(self): + dumb_lineage = mock.MagicMock(cert_path="not_found") + refresh = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_try_refresh" + + with mock.patch(refresh) as mock_refresh: + self.config.deploy_ocsp_prefetch(dumb_lineage) + self.assertFalse(mock_refresh.called) + + @mock.patch("certbot_apache._internal.override_debian.DebianConfigurator._ocsp_try_refresh") + def test_deploy_ocsp_prefetch(self, mock_refresh): + self.config._ocsp_prefetch_save("artificial_path", "irrelevant") + mock_lineage = mock.MagicMock(cert_path="artificial_path") + s_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_prefetch_save" + with mock.patch(s_path) as mock_save: + self.config.deploy_ocsp_prefetch(mock_lineage) + self.assertTrue(mock_refresh.called) + self.assertTrue(mock_save.called) + + @mock.patch("certbot_apache._internal.override_debian.DebianConfigurator._ocsp_try_refresh") + def test_deploy_ocsp_prefetch_error(self, mock_refresh): + self.config._ocsp_prefetch_save("artificial_path", "irrelevant") + mock_lineage = mock.MagicMock(cert_path="artificial_path") + mock_refresh.side_effect = OCSPCertificateError("Error") + s_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_prefetch_save" + with mock.patch(s_path) as mock_save: + self.config.deploy_ocsp_prefetch(mock_lineage) + self.assertTrue(mock_refresh.called) + self.assertFalse(mock_save.called) + @mock.patch("certbot_apache._internal.constants.OCSP_INTERNAL_TTL", 0) def test_ocsp_prefetch_refresh(self): def ocsp_req_mock(_cert, _chain, workfile): diff --git a/certbot/certbot/plugins/enhancements.py b/certbot/certbot/plugins/enhancements.py index 037e37b56..dad44e075 100644 --- a/certbot/certbot/plugins/enhancements.py +++ b/certbot/certbot/plugins/enhancements.py @@ -234,7 +234,7 @@ _INDEX = [ "cli_action": "store_true", "class": OCSPPrefetchEnhancement, "updater_function": "update_ocsp_prefetch", - "deployer_function": None, + "deployer_function": "deploy_ocsp_prefetch", "enable_function": "enable_ocsp_prefetch" } ] # type: List[Dict[str, Any]]