Better error handling

certbot/cert_manager.py

@@ -172,12 +172,12 @@ def _report_human_readable(parsed_certs):
     for cert in parsed_certs:
         now = pytz.UTC.fromutc(datetime.datetime.utcnow())
         expiration_text = ""
+        revoked = ocsp.revoked_status(cert.cert, cert.chain)
         if cert.is_test_cert:
             expiration_text = "INVALID: TEST CERT"
         elif cert.target_expiry <= now:
             expiration_text = "INVALID: EXPIRED"
         else:
-            revoked = ocsp.revoked_status(cert.cert, cert.chain)
             if revoked:
                 expiration_text = "INVALID: " + revoked
 

certbot/ocsp.py

@@ -24,50 +24,69 @@ def revoked_status(cert_path, chain_path):
 
     """
 
-    url, _ = util.run_script(
-        ["openssl", "x509", "-in", cert_path, "-noout", "-ocsp_uri"])
+    if revoked_status.broken:
+        return False
+
+    if not util.exe_exists("openssl"):
+        logging.info("openssl not installed, can't check revocation")
+        revoked_status.broken = True
+        return False
+
+    try:
+        url, err = util.run_script(
+            ["openssl", "x509", "-in", cert_path, "-noout", "-ocsp_uri"],
+            log=logging.debug)
+    except errors.SubprocessError:
+        logger.info("Cannot extract OCSP URI from %s", cert_path)
+        return False
 
     url = url.rstrip()
     host = url.partition("://")[2].rstrip("/")
     if not host:
-        raise errors.Error(
-            "Unable to get OCSP host from cert, url - %s", url)
+        logger.info("Cannot process OCSP host from URL (%s) in cert at %s", url, cert_path)
+        return False
 
     # New versions of openssl want -header var=val, old ones want -header var val
     test_host_format = Popen(["openssl", "ocsp", "-header", "var", "val"],
                              stdout=PIPE, stderr=PIPE)
     _out, err = test_host_format.communicate()
     if "Missing =" in err:
-        host_arg = ["Host=" + host]
+        host_args = ["Host=" + host]
     else:
-        host_arg = ["Host", host]
+        host_args = ["Host", host]
 
     # jdkasten thanks "Bulletproof SSL and TLS - Ivan Ristic" for documenting this!
     try:
-        output, _ = util.run_script(
-            ["openssl", "ocsp",
-            "-no_nonce",
-            "-header"] + host_arg + [
-            "-issuer", chain_path,
-            "-cert", cert_path,
-            "-url", url,
-            "-CAfile", chain_path,
-            "-verify_other", chain_path])
+        cmd = ["openssl", "ocsp",
+               "-no_nonce",
+               "-issuer", chain_path,
+               "-cert", cert_path,
+               "-url", url,
+               "-CAfile", chain_path,
+               "-verify_other", chain_path,
+               "-header"] + host_args
+        output, err = util.run_script(cmd, log=logging.debug)
     except errors.SubprocessError:
-        return "OCSP Failure"
+        logger.info("OCSP querying seems to be broken, assuming nothing is revoked...")
+        logger.debug("Command was:\n%s\nError was:\n%s", " ".join(cmd), err)
+        revoked_status.broken = True
+        return False
 
-    return _translate_ocsp_query(cert_path, output)
+    return _translate_ocsp_query(cert_path, output, err)
+revoked_status.broken = False
 
 
-def _translate_ocsp_query(cert_path, ocsp_output):
+def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors):
     """Returns a label string out of the query."""
     if not "Response verify OK":
-        return "Revocation Unknown"
+        logger.info("Revocation status for %s is unknown", cert_path)
+        logger.debug("Uncertain ouput:\n%s\nstderr:\n%s", ocsp_output, ocsp_errors)
+        return ""
     if cert_path + ": good" in ocsp_output:
         return ""
     elif cert_path + ": revoked" in ocsp_output:
         return REV_LABEL
     else:
-        raise errors.Error(
-            "Unable to properly parse OCSP output: %s", ocsp_output)
+        logger.warn("Unable to properly parse OCSP output: %s", ocsp_output)
+        return ""
 

certbot/util.py

@@ -38,10 +38,11 @@ ANSI_SGR_RED = "\033[31m"
 ANSI_SGR_RESET = "\033[0m"
 
 
-def run_script(params):
+def run_script(params, log=logger.error):
     """Run the script with the given params.
 
     :param list params: List of parameters to pass to Popen
+    :param logging.Logger log: Logger to use for errors
 
     """
     try:
@@ -51,7 +52,7 @@ def run_script(params):
 
     except (OSError, ValueError):
         msg = "Unable to run the command: %s" % " ".join(params)
-        logger.error(msg)
+        log(msg)
         raise errors.SubprocessError(msg)
 
     stdout, stderr = proc.communicate()
@@ -60,7 +61,7 @@ def run_script(params):
         msg = "Error while running %s.\n%s\n%s" % (
             " ".join(params), stdout, stderr)
         # Enter recovery routine...
-        logger.error(msg)
+        log(msg)
         raise errors.SubprocessError(msg)
 
     return stdout, stderr