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

Address review comments

Browse source
This commit is contained in:
Joona Hoikkala 2020-01-31 20:06:52 +02:00
parent a8a106c325
commit b6ea34c61d
No known key found for this signature in database
GPG key ID: D5AA86BBF9B29A5C
7 changed files with 62 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -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

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

@ -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

13
certbot/certbot/_internal/main.py
View file

@ -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)

4
certbot/certbot/_internal/ocsp.py
View file

@ -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

9
certbot/certbot/plugins/enhancements.py
View file

@ -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",

5
certbot/certbot/util.py
View file

@ -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

27
certbot/tests/ocsp_test.py
View file

@ -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