From d1519343d13f1d87c3b52ef953a8507b294e4936 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 7 Aug 2019 17:54:10 -0700 Subject: [PATCH 1/6] improving check revoked by only considering authorities which do support revocation and also only including not expired certs --- lemur/certificates/cli.py | 6 ++++-- lemur/certificates/service.py | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index b57ff175..e5e1191b 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -33,7 +33,7 @@ from lemur.certificates.service import ( get_certificate_primitives, get_all_pending_reissue, get_by_name, - get_all_certs, + get_all_valid_certs, get, ) @@ -467,7 +467,9 @@ def check_revoked(): encounters an issue with verification it marks the certificate status as `unknown`. """ - for cert in get_all_certs(): + + certs = get_all_valid_certs(current_app.config.get("CHECK_REVOCATION_AUTHORITY_IDS", [])) + for cert in certs: try: if cert.chain: status = verify_string(cert.body, cert.chain) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index 5a65c383..bb714eb0 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -102,6 +102,25 @@ def get_all_certs(): return Certificate.query.all() +def get_all_valid_certs(authority_ids): + """ + Retrieves all valid (not expired) certificates within Lemur, for the given authority_ids + ignored if no authority_ids provided. + + :return: + """ + if authority_ids: + return ( + Certificate.query.filter(Certificate.not_after > arrow.now().format("YYYY-MM-DD")) + .filter(Certificate.authority_id.in_(authority_ids)).all() + ) + else: + return ( + Certificate.query.filter(Certificate.not_after > arrow.now().format("YYYY-MM-DD")).all() + ) + + + def get_all_pending_cleaning(source): """ Retrieves all certificates that are available for cleaning. From 8340e0653bad6f17e5522b83b1a12d7216cbb992 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 7 Aug 2019 18:04:28 -0700 Subject: [PATCH 2/6] making lint happy --- lemur/certificates/service.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index bb714eb0..d25a136a 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -120,7 +120,6 @@ def get_all_valid_certs(authority_ids): ) - def get_all_pending_cleaning(source): """ Retrieves all certificates that are available for cleaning. From 49c4a9c3b279a96ae31ddd350009f40a0b96e7f5 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 22 May 2020 17:29:30 -0700 Subject: [PATCH 3/6] making the revocation to be scoped based on the authority plugin name --- lemur/certificates/cli.py | 2 +- lemur/certificates/service.py | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index e5e1191b..5ebe7e0f 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -468,7 +468,7 @@ def check_revoked(): as `unknown`. """ - certs = get_all_valid_certs(current_app.config.get("CHECK_REVOCATION_AUTHORITY_IDS", [])) + certs = get_all_valid_certs(current_app.config.get("SUPPORTED_REVOCATION_AUTHORITY_PLUGINS", [])) for cert in certs: try: if cert.chain: diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index d25a136a..8566fdf7 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -102,17 +102,20 @@ def get_all_certs(): return Certificate.query.all() -def get_all_valid_certs(authority_ids): +def get_all_valid_certs(authority_plugin_name): """ - Retrieves all valid (not expired) certificates within Lemur, for the given authority_ids - ignored if no authority_ids provided. + Retrieves all valid (not expired) certificates within Lemur, for the given authority plugin names + ignored if no authority_plugin_name provided. + + Note that depending on the DB size retrieving all certificates might an expensive operation :return: """ - if authority_ids: + if authority_plugin_name: return ( - Certificate.query.filter(Certificate.not_after > arrow.now().format("YYYY-MM-DD")) - .filter(Certificate.authority_id.in_(authority_ids)).all() + Certificate.query.outerjoin(Authority, Authority.id == Certificate.authority_id).filter( + Certificate.not_after > arrow.now().format("YYYY-MM-DD")).filter( + Authority.plugin_name.in_(authority_plugin_name)).all() ) else: return ( From c9767b3172fba9ee6043f8b6a0f3dbf8551d5e60 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 22 May 2020 17:32:44 -0700 Subject: [PATCH 4/6] adding logging for revoked certs --- lemur/certificates/cli.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 5ebe7e0f..b08aa0ba 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -468,6 +468,11 @@ def check_revoked(): as `unknown`. """ + log_data = { + "function": f"{__name__}.{sys._getframe().f_code.co_name}", + "message": "Checking for revoked Certificates" + } + certs = get_all_valid_certs(current_app.config.get("SUPPORTED_REVOCATION_AUTHORITY_PLUGINS", [])) for cert in certs: try: @@ -478,6 +483,20 @@ def check_revoked(): cert.status = "valid" if status else "revoked" + if cert.status == "revoked": + log_data["valid"] = cert.status + log_data["certificate_name"] = cert.name + log_data["certificate_id"] = cert.id + metrics.send( + "certificate_revoked", + "counter", + 1, + metric_tags={"status": log_data["valid"], + "certificate_name": log_data["certificate_name"], + "certificate_id": log_data["certificate_id"]}, + ) + current_app.logger.info(log_data) + except Exception as e: sentry.captureException() current_app.logger.exception(e) From 49a8b80df24c7c9df2435fea06a6258eed1913a9 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 22 May 2020 17:36:34 -0700 Subject: [PATCH 5/6] better exception handling when OCSP or CRL or not implemented --- lemur/certificates/verify.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lemur/certificates/verify.py b/lemur/certificates/verify.py index 76c6b521..989a2317 100644 --- a/lemur/certificates/verify.py +++ b/lemur/certificates/verify.py @@ -8,6 +8,7 @@ import requests import subprocess from flask import current_app +from lemur.extensions import sentry from requests.exceptions import ConnectionError, InvalidSchema from cryptography import x509 from cryptography.hazmat.backends import default_backend @@ -152,10 +153,18 @@ def verify(cert_path, issuer_chain_path): # OCSP is our main source of truth, in a lot of cases CRLs # have been deprecated and are no longer updated - verify_result = ocsp_verify(cert, cert_path, issuer_chain_path) + try: + verify_result = ocsp_verify(cert, cert_path, issuer_chain_path) + except Exception as e: + sentry.captureException() + current_app.logger.exception(e) if verify_result is None: - verify_result = crl_verify(cert, cert_path) + try: + verify_result = crl_verify(cert, cert_path) + except Exception as e: + sentry.captureException() + current_app.logger.exception(e) if verify_result is None: current_app.logger.debug("Failed to verify {}".format(cert.serial_number)) From 10dfedee36f951d79401d3add16a03653b8b16cb Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 22 May 2020 18:33:43 -0700 Subject: [PATCH 6/6] making lint happy --- lemur/certificates/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index dde7f79d..7fdef165 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -660,7 +660,7 @@ def check_revoked(): "certificate_revoked", "counter", 1, - metric_tags={"status": log_data["valid"], + metric_tags={"status": log_data["valid"], "certificate_name": log_data["certificate_name"], "certificate_id": log_data["certificate_id"]}, )