diff --git a/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py b/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py index f25ffc2ca..8b8d0bbe7 100644 --- a/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py +++ b/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py @@ -16,6 +16,7 @@ from certbot_apache._internal import constants logger = logging.getLogger(__name__) + class OCSPPrefetchMixin(object): """OCSPPrefetchMixin implements OCSP response prefetching""" @@ -115,7 +116,7 @@ class OCSPPrefetchMixin(object): self.config.work_dir, "ocsp", apache_util.certid_sha1_hex(cert_path)) handler = ocsp.RevocationChecker() - if not handler.ocsp_revoked_cert(cert_path, chain_path, ocsp_workfile): + if not handler.ocsp_revoked_by_paths(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 @@ -145,7 +146,7 @@ class OCSPPrefetchMixin(object): if next_update is not None: now = time.time() - res_ttl = int(time.mktime(next_update.timetuple()) - now) + res_ttl = int(next_update.timestamp() - now) if res_ttl > 0: return res_ttl/2 return constants.OCSP_APACHE_TTL @@ -279,8 +280,6 @@ class OCSPPrefetchMixin(object): or reload fails. """ - self.config_test() - if not self._ocsp_prefetch: # Try to populate OCSP prefetch structure from pluginstorage self._ocsp_prefetch_fetch_state() @@ -289,16 +288,13 @@ class OCSPPrefetchMixin(object): self._ocsp_prefetch_backup_db() try: - # Ignored because of issues with multiple class inheritance method - # resolution https://github.com/python/mypy/issues/4335 + # Ignored because mypy doesn't know that this class is used as + # a mixin and fails because object has no restart method. super(OCSPPrefetchMixin, self).restart() # type: ignore - except errors.MisconfigurationError: - self._ocsp_prefetch_restore_db() - raise - - if self._ocsp_prefetch: - # Restore the backed up dbm database - self._ocsp_prefetch_restore_db() + finally: + if self._ocsp_prefetch: + # Restore the backed up dbm database + self._ocsp_prefetch_restore_db() OCSPPrefetchEnhancement.register(OCSPPrefetchMixin) # pylint: disable=no-member diff --git a/certbot-apache/tests/ocsp_prefetch_test.py b/certbot-apache/tests/ocsp_prefetch_test.py index a2a876e35..9d4f3072b 100644 --- a/certbot-apache/tests/ocsp_prefetch_test.py +++ b/certbot-apache/tests/ocsp_prefetch_test.py @@ -179,7 +179,7 @@ class OCSPPrefetchTest(util.ApacheTest): fh.write("MOCKRESPONSE") return False - ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_cert" + ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_by_paths" with mock.patch(ocsp_path, side_effect=ocsp_req_mock): with mock.patch('certbot._internal.ocsp.RevocationChecker.ocsp_times') as mock_times: produced_at = datetime.today() - timedelta(days=1) @@ -210,7 +210,7 @@ class OCSPPrefetchTest(util.ApacheTest): fh.write("MOCKRESPONSE") return True - ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_cert" + ocsp_path = "certbot._internal.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"]) @@ -261,7 +261,7 @@ class OCSPPrefetchTest(util.ApacheTest): @mock.patch("certbot_apache._internal.prefetch_ocsp.OCSPPrefetchMixin.restart") def test_ocsp_prefetch_refresh_fail(self, _mock_restart): - ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_cert" + ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_by_paths" log_path = "certbot_apache._internal.prefetch_ocsp.logger.warning" with mock.patch(ocsp_path) as mock_ocsp: mock_ocsp.return_value = True diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 0fe7468c3..b26359ad2 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -805,6 +805,11 @@ def install(config, plugins): "If your certificate is managed by Certbot, please use --cert-name " "to define which certificate you would like to install.") + # It's important that the old style enhancements get enabled before + # the new style ones, as some of the new enhancements can modify the + # same configuration directives. _install_cert() in the above block + # handles the old style enhancements here. + if enhancements.are_requested(config): # In the case where we don't have certname, we have errored out already lineage = cert_manager.lineage_for_certname(config, config.certname) @@ -930,6 +935,9 @@ def enhance(config, plugins): if oldstyle_enh: le_client = _init_le_client(config, authenticator=None, installer=installer) le_client.enhance_config(domains, config.chain_path, ask_redirect=False) + # It's important that the old style enhancements get enabled before + # the new style ones, as some of the new enhancements can modify the + # same configuration directives. if enhancements.are_requested(config): enhancements.enable(lineage, domains, installer, config) @@ -1108,7 +1116,10 @@ def run(config, plugins): _report_new_cert(config, cert_path, fullchain_path, key_path) _install_cert(config, le_client, domains, new_lineage) - + # It's important that the old style enhancements get enabled before + # the new style ones, as some of the new enhancements can modify the + # same configuration directives. _install_cert() handles the old + # style enhancements here. if enhancements.are_requested(config) and new_lineage: enhancements.enable(new_lineage, domains, installer, config) diff --git a/certbot/certbot/_internal/ocsp.py b/certbot/certbot/_internal/ocsp.py index c70692f19..9c80baba3 100644 --- a/certbot/certbot/_internal/ocsp.py +++ b/certbot/certbot/_internal/ocsp.py @@ -69,9 +69,9 @@ class RevocationChecker(object): :rtype: bool """ - return self.ocsp_revoked_cert(cert.cert, cert.chain) + return self.ocsp_revoked_by_paths(cert.cert, cert.chain) - def ocsp_revoked_cert(self, cert_path, chain_path, response_file=None): + def ocsp_revoked_by_paths(self, cert_path, chain_path, response_file=None): # type: (str, str, Optional[str]) -> bool """Performs the OCSP revocation check diff --git a/certbot/certbot/plugins/enhancements.py b/certbot/certbot/plugins/enhancements.py index e9e98d16d..3b85813b6 100644 --- a/certbot/certbot/plugins/enhancements.py +++ b/certbot/certbot/plugins/enhancements.py @@ -180,10 +180,12 @@ class OCSPPrefetchEnhancement(object): def update_ocsp_prefetch(self, lineage, *args, **kwargs): """ Gets called for each lineage every time Certbot is run with 'renew' verb. - Implementation of this method should fetch a fresh OCSP response and if - valid, store it to be served for connecting clients. + Implementation of this method should fetch a fresh OCSP response if it's + needed and if valid, store it to be served for connecting clients. + :param lineage: Certificate lineage object :type lineage: certbot.storage.RenewableCert + .. note:: prepare() method inherited from `interfaces.IPlugin` might need to be called manually within implementation of this interface method to finalize the plugin initialization. @@ -195,6 +197,7 @@ class OCSPPrefetchEnhancement(object): Enables the OCSP enhancement, enabling OCSP Stapling functionality for the controlled software, and sets it up for prefetching the responses over the subsequent runs of Certbot renew. + :param lineage: Certificate lineage object :type lineage: certbot.storage.RenewableCert :param domains: List of domains in certificate to enhance @@ -224,7 +227,7 @@ _INDEX = [ "name": "OCSPPrefetch", "cli_help": "Prefetch OCSP responses for certificates in order to be " + "able to serve connecting clients fresh staple immediately", - "cli_flag": "--ocsp-prefetch", + "cli_flag": "--prefetch-ocsp", "cli_flag_default": constants.CLI_DEFAULTS["ocsp_prefetch"], "cli_groups": ["security", "enhance"], "cli_dest": "ocsp_prefetch", diff --git a/certbot/certbot/util.py b/certbot/certbot/util.py index 7fe3013e0..c104f6c9f 100644 --- a/certbot/certbot/util.py +++ b/certbot/certbot/util.py @@ -5,6 +5,7 @@ import argparse import atexit import collections from collections import OrderedDict +from datetime import datetime import distutils.version # pylint: disable=import-error,no-name-in-module import errno import logging @@ -15,7 +16,6 @@ import subprocess import sys import configargparse -from dateutil import parser import six from acme.magic_typing import Tuple # pylint: disable=unused-import, no-name-in-module @@ -611,6 +611,7 @@ def parse_datetime(dt_string): :rtype: datetime.datetime or None """ try: - return parser.parse(dt_string, ignoretz=True) + dateformat = "%b %d %H:%M:%S %Y %Z" + return datetime.strptime(dt_string, dateformat) except ValueError: return None diff --git a/certbot/tests/ocsp_test.py b/certbot/tests/ocsp_test.py index 9f298a365..051fa970e 100644 --- a/certbot/tests/ocsp_test.py +++ b/certbot/tests/ocsp_test.py @@ -145,6 +145,14 @@ class OCSPTestOpenSSL(unittest.TestCase): self.assertEqual(thisUpdate, datetime(2020, 1, 24, 11, 0)) self.assertEqual(nextUpdate, datetime(2020, 1, 31, 11, 0)) + @mock.patch('certbot.util.run_script') + def test_ocsp_response_get_times_no_nextupdate(self, mock_run): + mock_run.return_value = ocsp_times_example_nonext + producedAt, thisUpdate, nextUpdate = self.checker.ocsp_times("mocked") + self.assertEqual(producedAt, datetime(2020, 1, 24, 11, 10)) + self.assertEqual(thisUpdate, datetime(2020, 1, 24, 11, 0)) + self.assertEqual(nextUpdate, None) + @mock.patch('certbot.util.run_script') def test_ocsp_response_get_times_badoutput(self, mock_run): mock_run.return_value = ("Something unparsable", "") @@ -318,6 +326,20 @@ class OSCPTestCryptography(unittest.TestCase): self.assertEqual(this_update, datetime(2020, 1, 2, 8, 8)) self.assertEqual(next_update, datetime(2020, 1, 3, 4, 4)) + def test_ocsp_times_cryptography_no_nextupdate(self): + with mock.patch('certbot._internal.ocsp.open', mock.mock_open(read_data="")): + with mock.patch('cryptography.x509.ocsp.load_der_ocsp_response') as mock_load: + resp = mock.MagicMock() + resp.produced_at = datetime(2020, 1, 2, 9, 9) + resp.this_update = datetime(2020, 1, 2, 8, 8) + resp.next_update = None + mock_load.return_value = resp + + produced_at, this_update, next_update = self.checker.ocsp_times("mocked") + self.assertEqual(produced_at, datetime(2020, 1, 2, 9, 9)) + self.assertEqual(this_update, datetime(2020, 1, 2, 8, 8)) + self.assertEqual(next_update, None) + def test_ocsp_times_cryptography_error(self): with mock.patch('certbot._internal.ocsp.open', mock.mock_open(read_data="")) as mock_open: mock_open.side_effect = OSError @@ -429,6 +451,11 @@ ocsp_times_example = (""" Next Update: Jan 31 11:00:00 2020 GMT """, "") +ocsp_times_example_nonext = (""" + Produced At: Jan 24 11:10:00 2020 GMT + This Update: Jan 24 11:00:00 2020 GMT +""", "") + if __name__ == '__main__': unittest.main() # pragma: no cover