From d1519343d13f1d87c3b52ef953a8507b294e4936 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 7 Aug 2019 17:54:10 -0700 Subject: [PATCH 1/8] 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/8] 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 4923bbf8a7fcd5ccd8d6e8825c5bd749ed0490e8 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 22 May 2020 16:22:12 -0700 Subject: [PATCH 3/8] adding json formatted logging --- lemur/certificates/cli.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index d007e458..54b92ec2 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -210,6 +210,10 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c status = FAILURE_METRIC_STATUS + log_data = { + "function": f"{__name__}.{sys._getframe().f_code.co_name}", + } + try: old_cert = validate_certificate(old_certificate_name) new_cert = validate_certificate(new_certificate_name) @@ -219,26 +223,44 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c print( f"[+] Rotating endpoint: {endpoint.name} to certificate {new_cert.name}" ) + log_data["message"] = "Rotating endpoint" + log_data["endpoint"] = endpoint.dnsname + log_data["certificate"] = new_cert.name request_rotation(endpoint, new_cert, message, commit) + current_app.logger.info(log_data) elif old_cert and new_cert: print(f"[+] Rotating all endpoints from {old_cert.name} to {new_cert.name}") + + log_data["message"] = "Rotating all endpoints" + log_data["certificate"] = new_cert.name + log_data["certificate_old"] = old_cert.name + log_data["message"] = "Rotating endpoint from old to new cert" for endpoint in old_cert.endpoints: print(f"[+] Rotating {endpoint.name}") + log_data["endpoint"] = endpoint.dnsname request_rotation(endpoint, new_cert, message, commit) + current_app.logger.info(log_data) else: print("[+] Rotating all endpoints that have new certificates available") + log_data["message"] = "Rotating all endpoints that have new certificates available" for endpoint in endpoint_service.get_all_pending_rotation(): + log_data["endpoint"] = endpoint.dnsname if len(endpoint.certificate.replaced) == 1: print( f"[+] Rotating {endpoint.name} to {endpoint.certificate.replaced[0].name}" ) + log_data["certificate"] = endpoint.certificate.replaced[0].name request_rotation( endpoint, endpoint.certificate.replaced[0], message, commit ) + current_app.logger.info(log_data) + else: + log_data["message"] = "Failed to rotate endpoint due to Multiple replacement certificates found" + print(log_data) metrics.send( "endpoint_rotation", "counter", From 49c4a9c3b279a96ae31ddd350009f40a0b96e7f5 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 22 May 2020 17:29:30 -0700 Subject: [PATCH 4/8] 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 5/8] 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 6/8] 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 7/8] 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"]}, ) From 4eeab91d7396106197b86c47670924c73081e884 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 22 May 2020 18:36:39 -0700 Subject: [PATCH 8/8] making lint happy --- lemur/certificates/cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index cdb5dbc4..88cfa134 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -232,7 +232,6 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c elif old_cert and new_cert: print(f"[+] Rotating all endpoints from {old_cert.name} to {new_cert.name}") - log_data["message"] = "Rotating all endpoints" log_data["certificate"] = new_cert.name log_data["certificate_old"] = old_cert.name