From 447dc3752b1dfba2c90868f784acfbf6641bdd10 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 3 Mar 2020 08:45:00 -0800 Subject: [PATCH] add ocsp_revoked_by_paths --- certbot/certbot/_internal/storage.py | 4 +++- certbot/certbot/ocsp.py | 13 ++++++++++++- certbot/tests/storage_test.py | 6 +++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/certbot/certbot/_internal/storage.py b/certbot/certbot/_internal/storage.py index 5937d1d23..d926534b2 100644 --- a/certbot/certbot/_internal/storage.py +++ b/certbot/certbot/_internal/storage.py @@ -897,11 +897,13 @@ class RenewableCert(interfaces.RenewableCert): """ cert_path = self.version("cert", version) + chain_path = self.version("chain", version) # While the RevocationChecker should return False if it failed to # determine the OCSP status, let's ensure we don't crash Certbot by # catching all exceptions here. try: - return ocsp.RevocationChecker().ocsp_revoked(cert_path) + return ocsp.RevocationChecker().ocsp_revoked_by_paths(cert_path, + chain_path) except Exception as e: # pylint: disable=broad-except logger.warning( "An error occurred determining the OCSP staus of %s.", diff --git a/certbot/certbot/ocsp.py b/certbot/certbot/ocsp.py index 6a95f26fa..9799c675c 100644 --- a/certbot/certbot/ocsp.py +++ b/certbot/certbot/ocsp.py @@ -68,8 +68,19 @@ class RevocationChecker(object): :rtype: bool """ - cert_path, chain_path = cert.cert_path, cert.chain_path + return self.ocsp_revoked_by_paths(cert.cert_path, cert.chain_path) + def ocsp_revoked_by_paths(self, cert_path, chain_path): + # type: (str, str) -> bool + """Performs the OCSP revocation check + + :param str cert_path: Certificate filepath + :param str chain_path: Certificate chain filepath + + :returns: True if revoked; False if valid or the check failed or cert is expired. + :rtype: bool + + """ if self.broken: return False diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index 8d69a3c70..06c2b5c4c 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -672,29 +672,33 @@ class RenewableCertTests(BaseRenewableCertTest): errors.CertStorageError, self.test_rc._update_link_to, "elephant", 17) - @mock.patch("certbot.ocsp.RevocationChecker.ocsp_revoked") + @mock.patch("certbot.ocsp.RevocationChecker.ocsp_revoked_by_paths") def test_ocsp_revoked(self, mock_checker): # Write out test files for kind in ALL_FOUR: self._write_out_kind(kind, 1) version = self.test_rc.latest_common_version() expected_cert_path = self.test_rc.version("cert", version) + expected_chain_path = self.test_rc.version("chain", version) # Test with cert revoked mock_checker.return_value = True self.assertTrue(self.test_rc.ocsp_revoked(version)) self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) + self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) # Test with cert not revoked mock_checker.return_value = False self.assertFalse(self.test_rc.ocsp_revoked(version)) self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) + self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) # Test with error mock_checker.side_effect = ValueError with mock.patch("certbot._internal.storage.logger.warning") as logger: self.assertFalse(self.test_rc.ocsp_revoked(version)) self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) + self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) self.assertTrue(logger.called) def test_add_time_interval(self):