From 4138259c514dc3c4ad8deb3f3824d4263fbe18d1 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 27 Jan 2020 15:10:04 +0200 Subject: [PATCH] Add certbot-apache tests and mypy type hints --- .../certbot_apache/_internal/prefetch_ocsp.py | 2 +- certbot-apache/tests/ocsp_prefetch_test.py | 36 ++++++++++++++----- certbot/certbot/_internal/ocsp.py | 12 +++++-- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py b/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py index 845365ce6..00219f052 100644 --- a/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py +++ b/certbot-apache/certbot_apache/_internal/prefetch_ocsp.py @@ -145,7 +145,7 @@ class OCSPPrefetchMixin(object): if next_update: now = time.time() - res_ttl = int(next_update.total_seconds() - now) + res_ttl = int(time.mktime(next_update.timetuple()) - now) if res_ttl > 0: return res_ttl/2 return constants.OCSP_APACHE_TTL diff --git a/certbot-apache/tests/ocsp_prefetch_test.py b/certbot-apache/tests/ocsp_prefetch_test.py index b86225a55..a2a876e35 100644 --- a/certbot-apache/tests/ocsp_prefetch_test.py +++ b/certbot-apache/tests/ocsp_prefetch_test.py @@ -1,5 +1,7 @@ """Test for certbot_apache._internal.configurator OCSP Prefetching functionality""" import base64 +from datetime import datetime +from datetime import timedelta import json import unittest import sys @@ -16,7 +18,6 @@ from certbot.compat import os import util - class MockDBM(object): # pylint: disable=missing-docstring """Main mock DBM class for Py3 dbm module""" @@ -178,10 +179,15 @@ class OCSPPrefetchTest(util.ApacheTest): fh.write("MOCKRESPONSE") return False - ocsp_path = "certbot._internal.ocsp.RevocationChecker.revoked" + ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_cert" with mock.patch(ocsp_path, side_effect=ocsp_req_mock): - self.call_mocked_py2(self.config.enable_ocsp_prefetch, - self.lineage, ["ocspvhost.com"]) + with mock.patch('certbot._internal.ocsp.RevocationChecker.ocsp_times') as mock_times: + produced_at = datetime.today() - timedelta(days=1) + this_update = datetime.today() - timedelta(days=2) + next_update = datetime.today() + timedelta(days=2) + mock_times.return_value = produced_at, this_update, next_update + self.call_mocked_py2(self.config.enable_ocsp_prefetch, + self.lineage, ["ocspvhost.com"]) odbm = self.config._ocsp_dbm_open(self.db_path) self.assertEqual(len(odbm.keys()), 1) # The actual response data is prepended by Apache timestamp @@ -189,8 +195,13 @@ class OCSPPrefetchTest(util.ApacheTest): self.config._ocsp_dbm_close(odbm) with mock.patch(ocsp_path, side_effect=ocsp_req_mock) as mock_ocsp: - self.call_mocked_py2(self.config.update_ocsp_prefetch, None) - self.assertTrue(mock_ocsp.called) + with mock.patch('certbot._internal.ocsp.RevocationChecker.ocsp_times') as mock_times: + produced_at = datetime.today() - timedelta(days=1) + this_update = datetime.today() - timedelta(days=2) + next_update = datetime.today() + timedelta(days=2) + mock_times.return_value = produced_at, this_update, next_update + 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): @@ -199,7 +210,7 @@ class OCSPPrefetchTest(util.ApacheTest): fh.write("MOCKRESPONSE") return True - ocsp_path = "certbot._internal.ocsp.RevocationChecker.revoked" + ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_cert" with mock.patch(ocsp_path, side_effect=ocsp_req_mock): self.call_mocked_py2(self.config.enable_ocsp_prefetch, self.lineage, ["ocspvhost.com"]) @@ -250,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.revoked" + ocsp_path = "certbot._internal.ocsp.RevocationChecker.ocsp_revoked_cert" log_path = "certbot_apache._internal.prefetch_ocsp.logger.warning" with mock.patch(ocsp_path) as mock_ocsp: mock_ocsp.return_value = True @@ -328,6 +339,15 @@ class OCSPPrefetchTest(util.ApacheTest): db2 = self.call_mocked_py3(self.config._ocsp_dbm_open, self.db_path) self.assertEqual(db2[b'key'], expected_val) + @mock.patch("certbot_apache._internal.constants.OCSP_APACHE_TTL", 1234) + def test_ttl(self): + self.assertEqual(self.config._ocsp_ttl(None), 1234) + next_update = datetime.today() + timedelta(days=6) + ttl = self.config._ocsp_ttl(next_update) + # ttl should be roughly 3 days + self.assertTrue(ttl > 86400*2) + self.assertTrue(ttl < 86400*4) + @mock.patch("certbot_apache._internal.prefetch_ocsp.OCSPPrefetchMixin._ocsp_prefetch_fetch_state") @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.config_test") @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator._reload") diff --git a/certbot/certbot/_internal/ocsp.py b/certbot/certbot/_internal/ocsp.py index 300639b19..10789776c 100644 --- a/certbot/certbot/_internal/ocsp.py +++ b/certbot/certbot/_internal/ocsp.py @@ -21,6 +21,7 @@ from acme.magic_typing import Tuple # pylint: disable=unused-import, no-name-in from certbot import crypto_util from certbot import errors from certbot import util +from certbot._internal.storage import RenewableCert # pylint: disable=unused-import try: # Only cryptography>=2.5 has ocsp module @@ -99,14 +100,16 @@ class RevocationChecker(object): return _check_ocsp_cryptography(cert_path, chain_path, url, response_file) def ocsp_times(self, response_file): + # type: (str) -> Tuple[Optional[datetime], Optional[datetime], Optional[datetime]] """ Reads OCSP response file and returns producedAt, thisUpdate - and nextUpdate values in datetime format. + and nextUpdate values in datetime format, or None if an error + occures. :param str response_file: File path to OCSP response :returns: tuple of producedAt, thisUpdate and nextUpdate values - :rtype: tuple of datetime + :rtype: tuple of datetime or None """ if self.use_openssl_binary: @@ -169,6 +172,7 @@ def _determine_ocsp_server(cert_path): def _ocsp_times_openssl_bin(response_file): + # type: (str) -> Tuple[Optional[datetime], Optional[datetime], Optional[datetime]] """ Reads OCSP response file using OpenSSL binary and returns producedAt, thisUpdate and nextUpdate values in datetime format. @@ -182,7 +186,7 @@ def _ocsp_times_openssl_bin(response_file): logger.debug("Reading OCSP response from temp file: %s", response_file) logger.debug(" ".join(cmd)) try: - output, err = util.run_script(cmd, log=logger.debug) + output, _ = util.run_script(cmd, log=logger.debug) except errors.SubprocessError: logger.info("Reading OCSP response from file failed.") return None, None, None @@ -195,6 +199,7 @@ def _ocsp_times_openssl_bin(response_file): def _ocsp_times_cryptography(response_file): + # type: (str) -> Tuple[Optional[datetime], Optional[datetime], Optional[datetime]] """ Reads OCSP response using cryptography and returns producedAt, thisUpdate and nextUpdate values in datetime format, or None @@ -379,6 +384,7 @@ def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors): def _translate_ocsp_response_times(response): + # type: (str) -> Tuple[str, str, str] """ Parse openssl OCSP response output and return producedAt, thisUpdate and nextUpdate values.