Add a 10-second timeout to OCSP queries. (#7860)

* Add a 10-second timeout to OCSP queries.

Closes #7859

* Update CHANGELOG

* Fix test

certbot/CHANGELOG.md

@@ -25,6 +25,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
   `resource` field in any requests or the `type` field when responding to challenges.
 * Fix nginx plugin crash when non-ASCII configuration file is being read (instead,
   the user will be warned that UTF-8 must be used).
+* Fix hanging OCSP queries during revocation checking - added a 10 second timeout.
 
 More details about these changes can be found on our GitHub repo.
 

certbot/certbot/ocsp.py

@@ -70,12 +70,13 @@ class RevocationChecker(object):
         """
         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
+    def ocsp_revoked_by_paths(self, cert_path, chain_path, timeout=10):
+        # type: (str, str, int) -> bool
         """Performs the OCSP revocation check
 
         :param str cert_path: Certificate filepath
-        :param str chain_path: Certificate chain filepath
+        :param str chain_path: Certificate chain
+        :param int timeout: Timeout (in seconds) for the OCSP query
 
         :returns: True if revoked; False if valid or the check failed or cert is expired.
         :rtype: bool
@@ -96,11 +97,11 @@ class RevocationChecker(object):
             return False
 
         if self.use_openssl_binary:
-            return self._check_ocsp_openssl_bin(cert_path, chain_path, host, url)
-        return _check_ocsp_cryptography(cert_path, chain_path, url)
+            return self._check_ocsp_openssl_bin(cert_path, chain_path, host, url, timeout)
+        return _check_ocsp_cryptography(cert_path, chain_path, url, timeout)
 
-    def _check_ocsp_openssl_bin(self, cert_path, chain_path, host, url):
-        # type: (str, str, str, str) -> bool
+    def _check_ocsp_openssl_bin(self, cert_path, chain_path, host, url, timeout):
+        # type: (str, str, str, str, int) -> bool
         # jdkasten thanks "Bulletproof SSL and TLS - Ivan Ristic" for documenting this!
         cmd = ["openssl", "ocsp",
                "-no_nonce",
@@ -110,6 +111,7 @@ class RevocationChecker(object):
                "-CAfile", chain_path,
                "-verify_other", chain_path,
                "-trust_other",
+               "-timeout", str(timeout),
                "-header"] + self.host_args(host)
         logger.debug("Querying OCSP for %s", cert_path)
         logger.debug(" ".join(cmd))
@@ -152,8 +154,8 @@ def _determine_ocsp_server(cert_path):
     return None, None
 
 
-def _check_ocsp_cryptography(cert_path, chain_path, url):
-    # type: (str, str, str) -> bool
+def _check_ocsp_cryptography(cert_path, chain_path, url, timeout):
+    # type: (str, str, str, int) -> bool
     # Retrieve OCSP response
     with open(chain_path, 'rb') as file_handler:
         issuer = x509.load_pem_x509_certificate(file_handler.read(), default_backend())
@@ -165,7 +167,8 @@ def _check_ocsp_cryptography(cert_path, chain_path, url):
     request_binary = request.public_bytes(serialization.Encoding.DER)
     try:
         response = requests.post(url, data=request_binary,
-                                 headers={'Content-Type': 'application/ocsp-request'})
+                                 headers={'Content-Type': 'application/ocsp-request'},
+                                 timeout=timeout)
     except requests.exceptions.RequestException:
         logger.info("OCSP check failed for %s (are we offline?)", cert_path, exc_info=True)
         return False

certbot/tests/ocsp_test.py

@@ -162,11 +162,11 @@ class OSCPTestCryptography(unittest.TestCase):
 
     @mock.patch('certbot.ocsp._determine_ocsp_server')
     @mock.patch('certbot.ocsp._check_ocsp_cryptography')
-    def test_ensure_cryptography_toggled(self, mock_revoke, mock_determine):
+    def test_ensure_cryptography_toggled(self, mock_check, mock_determine):
         mock_determine.return_value = ('http://example.com', 'example.com')
         self.checker.ocsp_revoked(self.cert_obj)
 
-        mock_revoke.assert_called_once_with(self.cert_path, self.chain_path, 'http://example.com')
+        mock_check.assert_called_once_with(self.cert_path, self.chain_path, 'http://example.com', 10)
 
     def test_revoke(self):
         with _ocsp_mock(ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL):