upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-05-28 04:34:11 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Don't send OCSP requests for expired certificates (#7387)

Browse source 
Fixes #7152.

* don't check ocsp if cert is expired when getting cert information

* don't check ocsp if the cert is expired in ocsp_revoked

* update tests

* update changelog

* move pytz import to the top of the test file
This commit is contained in:
ohemorange 2019-09-23 20:20:11 -04:00 committed by Brad Warren
parent e402993c34
commit 18e6c6c2a8
4 changed files with 54 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -10,7 +10,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
### Changed
*
* Don't send OCSP requests for expired certificates
### Fixed

2
certbot/cert_manager.py
View file

@ -262,7 +262,7 @@ def human_readable_cert_info(config, cert, skip_filter_checks=False):
reasons.append('TEST_CERT')
if cert.target_expiry <= now:
reasons.append('EXPIRED')
if checker.ocsp_revoked(cert.cert, cert.chain):
elif checker.ocsp_revoked(cert):
reasons.append('REVOKED')
if reasons:

20
certbot/ocsp.py
View file

@ -16,11 +16,13 @@ from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import serialization
from cryptography.hazmat.primitives import hashes # type: ignore
from cryptography.exceptions import UnsupportedAlgorithm, InvalidSignature
import pytz
import requests
from acme.magic_typing import Optional, Tuple # pylint: disable=unused-import, no-name-in-module
from certbot import crypto_util
from certbot import errors
from certbot.storage import RenewableCert # pylint: disable=unused-import
from certbot import util
logger = logging.getLogger(__name__)
@ -48,21 +50,29 @@ class RevocationChecker(object):
else:
self.host_args = lambda host: ["Host", host]
def ocsp_revoked(self, cert_path, chain_path):
# type: (str, str) -> bool
def ocsp_revoked(self, cert):
# type: (RenewableCert) -> bool
"""Get revoked status for a particular cert version.
.. todo:: Make this a non-blocking call
:param str cert_path: Path to certificate
:param str chain_path: Path to intermediate cert
:returns: True if revoked; False if valid or the check failed
:param `.storage.RenewableCert` cert: Certificate object
:returns: True if revoked; False if valid or the check failed or cert is expired.
:rtype: bool
"""
cert_path, chain_path = cert.cert, cert.chain
if self.broken:
return False
# Let's Encrypt doesn't update OCSP for expired certificates,
# so don't check OCSP if the cert is expired.
# https://github.com/certbot/certbot/issues/7152
now = pytz.UTC.fromutc(datetime.utcnow())
if cert.target_expiry <= now:
return False
url, host = _determine_ocsp_server(cert_path)
if not host or not url:
return False

55
certbot/tests/ocsp_test.py
View file

@ -16,6 +16,7 @@ try:
except (ImportError, AttributeError): # pragma: no cover
ocsp_lib = None # type: ignore
import mock
import pytz
from certbot import errors
from certbot.tests import util as test_util
@ -72,21 +73,34 @@ class OCSPTestOpenSSL(unittest.TestCase):
@mock.patch('certbot.ocsp._determine_ocsp_server')
@mock.patch('certbot.util.run_script')
def test_ocsp_revoked(self, mock_run, mock_determine):
now = pytz.UTC.fromutc(datetime.utcnow())
cert_obj = mock.MagicMock()
cert_obj.cert = "x"
cert_obj.chain = "y"
cert_obj.target_expiry = now + timedelta(hours=2)
self.checker.broken = True
mock_determine.return_value = ("", "")
self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
self.assertEqual(self.checker.ocsp_revoked(cert_obj), False)
self.checker.broken = False
mock_run.return_value = tuple(openssl_happy[1:])
self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
self.assertEqual(self.checker.ocsp_revoked(cert_obj), False)
self.assertEqual(mock_run.call_count, 0)
mock_determine.return_value = ("http://x.co", "x.co")
self.assertEqual(self.checker.ocsp_revoked("blah.pem", "chain.pem"), False)
self.assertEqual(self.checker.ocsp_revoked(cert_obj), False)
mock_run.side_effect = errors.SubprocessError("Unable to load certificate launcher")
self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
self.assertEqual(self.checker.ocsp_revoked(cert_obj), False)
self.assertEqual(mock_run.call_count, 2)
# cert expired
cert_obj.target_expiry = now
mock_determine.return_value = ("", "")
count_before = mock_determine.call_count
self.assertEqual(self.checker.ocsp_revoked(cert_obj), False)
self.assertEqual(mock_determine.call_count, count_before)
def test_determine_ocsp_server(self):
cert_path = test_util.vector_path('ocsp_certificate.pem')
@ -131,18 +145,23 @@ class OSCPTestCryptography(unittest.TestCase):
self.checker = ocsp.RevocationChecker()
self.cert_path = test_util.vector_path('ocsp_certificate.pem')
self.chain_path = test_util.vector_path('ocsp_issuer_certificate.pem')
self.cert_obj = mock.MagicMock()
self.cert_obj.cert = self.cert_path
self.cert_obj.chain = self.chain_path
now = pytz.UTC.fromutc(datetime.utcnow())
self.cert_obj.target_expiry = now + timedelta(hours=2)
@mock.patch('certbot.ocsp._determine_ocsp_server')
@mock.patch('certbot.ocsp._check_ocsp_cryptography')
def test_ensure_cryptography_toggled(self, mock_revoke, mock_determine):
mock_determine.return_value = ('http://example.com', 'example.com')
self.checker.ocsp_revoked(self.cert_path, self.chain_path)
self.checker.ocsp_revoked(self.cert_obj)
mock_revoke.assert_called_once_with(self.cert_path, self.chain_path, 'http://example.com')
def test_revoke(self):
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertTrue(revoked)
def test_responder_is_issuer(self):
@ -152,7 +171,7 @@ class OSCPTestCryptography(unittest.TestCase):
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED,
ocsp_lib.OCSPResponseStatus.SUCCESSFUL) as mocks:
mocks['mock_response'].return_value.responder_name = issuer.subject
self.checker.ocsp_revoked(self.cert_path, self.chain_path)
self.checker.ocsp_revoked(self.cert_obj)
# Here responder and issuer are the same. So only the signature of the OCSP
# response is checked (using the issuer/responder public key).
self.assertEqual(mocks['mock_check'].call_count, 1)
@ -167,7 +186,7 @@ class OSCPTestCryptography(unittest.TestCase):
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED,
ocsp_lib.OCSPResponseStatus.SUCCESSFUL) as mocks:
self.checker.ocsp_revoked(self.cert_path, self.chain_path)
self.checker.ocsp_revoked(self.cert_obj)
# Here responder and issuer are not the same. Two signatures will be checked then,
# first to verify the responder cert (using the issuer public key), second to
# to verify the OCSP response itself (using the responder public key).
@ -181,17 +200,17 @@ class OSCPTestCryptography(unittest.TestCase):
# Server return an invalid HTTP response
with _ocsp_mock(ocsp_lib.OCSPCertStatus.UNKNOWN, ocsp_lib.OCSPResponseStatus.SUCCESSFUL,
http_status_code=400):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# OCSP response in invalid
with _ocsp_mock(ocsp_lib.OCSPCertStatus.UNKNOWN, ocsp_lib.OCSPResponseStatus.UNAUTHORIZED):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# OCSP response is valid, but certificate status is unknown
with _ocsp_mock(ocsp_lib.OCSPCertStatus.UNKNOWN, ocsp_lib.OCSPResponseStatus.SUCCESSFUL):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# The OCSP response says that the certificate is revoked, but certificate
@ -200,32 +219,32 @@ class OSCPTestCryptography(unittest.TestCase):
with mock.patch('cryptography.x509.Extensions.get_extension_for_class',
side_effect=x509.ExtensionNotFound(
'Not found', x509.AuthorityInformationAccessOID.OCSP)):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# OCSP response uses an unsupported signature.
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL,
check_signature_side_effect=UnsupportedAlgorithm('foo')):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# OSCP signature response is invalid.
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL,
check_signature_side_effect=InvalidSignature('foo')):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# Assertion error on OCSP response validity
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL,
check_signature_side_effect=AssertionError('foo')):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# No responder cert in OCSP response
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED,
ocsp_lib.OCSPResponseStatus.SUCCESSFUL) as mocks:
mocks['mock_response'].return_value.certificates = []
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
# Responder cert is not signed by certificate issuer
@ -234,7 +253,7 @@ class OSCPTestCryptography(unittest.TestCase):
cert = mocks['mock_response'].return_value.certificates[0]
mocks['mock_response'].return_value.certificates[0] = mock.Mock(
issuer='fake', subject=cert.subject)
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)
with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL):
@ -245,7 +264,7 @@ class OSCPTestCryptography(unittest.TestCase):
with mock.patch('cryptography.x509.Extensions.get_extension_for_class',
side_effect=x509.ExtensionNotFound(
'Not found', x509.AuthorityInformationAccessOID.OCSP)):
revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path)
revoked = self.checker.ocsp_revoked(self.cert_obj)
self.assertFalse(revoked)