From 92eec5cc9c014aae35dc65ba0bc145f47b0d0acd Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 21 Oct 2020 18:52:55 -0700 Subject: [PATCH 1/8] revocation should only check for not expired and not revoked certs --- lemur/certificates/service.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index 6d1bd2ac..6daaa641 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -105,7 +105,7 @@ def get_all_certs(): def get_all_valid_certs(authority_plugin_name): """ - Retrieves all valid (not expired) certificates within Lemur, for the given authority plugin names + Retrieves all valid (not expired & not revoked) 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 @@ -116,11 +116,12 @@ def get_all_valid_certs(authority_plugin_name): return ( 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() + Authority.plugin_name.in_(authority_plugin_name)).filter(Certificate.revoked.is_(False)).all() ) else: return ( - Certificate.query.filter(Certificate.not_after > arrow.now().format("YYYY-MM-DD")).all() + Certificate.query.filter(Certificate.not_after > arrow.now().format("YYYY-MM-DD")).filter( + Certificate.revoked.is_(False)).all() ) From 906b3b2337c56486584a4751a0a3b77270c0ebcf Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 21 Oct 2020 19:52:25 -0700 Subject: [PATCH 2/8] better handling of status code --- lemur/plugins/lemur_entrust/plugin.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lemur/plugins/lemur_entrust/plugin.py b/lemur/plugins/lemur_entrust/plugin.py index 515e2400..d3f2c202 100644 --- a/lemur/plugins/lemur_entrust/plugin.py +++ b/lemur/plugins/lemur_entrust/plugin.py @@ -109,7 +109,12 @@ def handle_response(my_response): "response": d } current_app.logger.info(log_data) - return d + if d == {'response': 'No detailed message'}: + # status if no data + return s + else: + # return data from the response + return d class EntrustIssuerPlugin(IssuerPlugin): @@ -211,7 +216,7 @@ class EntrustIssuerPlugin(IssuerPlugin): deactivate_url = f"{base_url}/certificates/{certificate.external_id}/deactivations" response = self.session.post(deactivate_url) metrics.send("entrust_deactivate_certificate", "counter", 1) - return handle_response(response) + return response.status_code @staticmethod def create_authority(options): From a4dba0cb35a02960e6d63d6aa7bc7291673a4943 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 21 Oct 2020 19:52:51 -0700 Subject: [PATCH 3/8] creating a cli to handle entrust deactivation --- lemur/certificates/cli.py | 41 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index b883dee0..224f02a2 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -735,3 +735,44 @@ def automatically_enable_autorotate(): }) cert.rotation = True database.update(cert) + + +@manager.command +def deactivate_entrust_certificates(): + """ + Attempt to deactivate test certificates issued by Entrust + """ + + log_data = { + "function": f"{__name__}.{sys._getframe().f_code.co_name}", + "message": "Deactivating Entrust certificates" + } + + certificates = get_all_valid_certs(['entrust-issuer']) + entrust_plugin = plugins.get('entrust-issuer') + for cert in certificates: + try: + response = entrust_plugin.deactivate_certificate(cert) + if response == 200: + cert.status = "revoked" + else: + cert.status = "unknown" + + log_data["valid"] = cert.status + log_data["certificate_name"] = cert.name + log_data["certificate_id"] = cert.id + metrics.send( + "certificate_deactivate", + "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) + + database.update(cert) + + except Exception as e: + sentry.captureException() + current_app.logger.exception(e) From 2cc03088cdba41bfaffb5ebacd28379a521f235b Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 21 Oct 2020 19:53:08 -0700 Subject: [PATCH 4/8] creating a celery task --- lemur/common/celery.py | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/lemur/common/celery.py b/lemur/common/celery.py index a490b13b..f72fd207 100644 --- a/lemur/common/celery.py +++ b/lemur/common/celery.py @@ -759,7 +759,7 @@ def check_revoked(): log_data = { "function": function, - "message": "check if any certificates are revoked revoked", + "message": "check if any valid certificate is revoked", "task_id": task_id, } @@ -842,3 +842,39 @@ def enable_autorotate_for_certs_attached_to_endpoint(): cli_certificate.automatically_enable_autorotate() metrics.send(f"{function}.success", "counter", 1) return log_data + + +@celery.task(soft_time_limit=3600) +def deactivate_entrust(): + """ + This celery task attempts to deactivate all not yet deactivated Entrust certificates, and should only run in TEST + :return: + """ + function = f"{__name__}.{sys._getframe().f_code.co_name}" + task_id = None + if celery.current_task: + task_id = celery.current_task.request.id + + log_data = { + "function": function, + "message": "deactivate entrust certificates", + "task_id": task_id, + } + + if task_id and is_task_active(function, task_id, None): + log_data["message"] = "Skipping task: Task is already active" + current_app.logger.debug(log_data) + return + + current_app.logger.debug(log_data) + try: + cli_certificate.deactivate_entrust_certificates() + except SoftTimeLimitExceeded: + log_data["message"] = "Time limit exceeded." + current_app.logger.error(log_data) + sentry.captureException() + metrics.send("celery.timeout", "counter", 1, metric_tags={"function": function}) + return + + metrics.send(f"{function}.success", "counter", 1) + return log_data From c40ecd12cbe8df3913897df5fd2fb95ce6e559d6 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 22 Oct 2020 10:58:16 -0700 Subject: [PATCH 5/8] improved naming --- lemur/common/celery.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/common/celery.py b/lemur/common/celery.py index f72fd207..f9d58bd9 100644 --- a/lemur/common/celery.py +++ b/lemur/common/celery.py @@ -845,7 +845,7 @@ def enable_autorotate_for_certs_attached_to_endpoint(): @celery.task(soft_time_limit=3600) -def deactivate_entrust(): +def deactivate_entrust_test_certificates(): """ This celery task attempts to deactivate all not yet deactivated Entrust certificates, and should only run in TEST :return: From 2e7e3a82fa0b909ea83b0989e2f32dee084a1bce Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 22 Oct 2020 11:57:54 -0700 Subject: [PATCH 6/8] Update cli.py logging in exception --- lemur/certificates/cli.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 224f02a2..cf2ff367 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -774,5 +774,7 @@ def deactivate_entrust_certificates(): database.update(cert) except Exception as e: + current_app.logger.info(log_data) sentry.captureException() current_app.logger.exception(e) + From cf87e178c8f70d527478544f6b37a50a901c62cf Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 22 Oct 2020 17:33:02 -0700 Subject: [PATCH 7/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 cf2ff367..f23948be 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -777,4 +777,3 @@ def deactivate_entrust_certificates(): current_app.logger.info(log_data) sentry.captureException() current_app.logger.exception(e) - From 9ce0010bf1a76a1057a9d3b88f1a51966d552568 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 22 Oct 2020 17:33:39 -0700 Subject: [PATCH 8/8] handle_respone can also handle the no data response --- lemur/plugins/lemur_entrust/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_entrust/plugin.py b/lemur/plugins/lemur_entrust/plugin.py index d3f2c202..0e9f6b7f 100644 --- a/lemur/plugins/lemur_entrust/plugin.py +++ b/lemur/plugins/lemur_entrust/plugin.py @@ -216,7 +216,7 @@ class EntrustIssuerPlugin(IssuerPlugin): deactivate_url = f"{base_url}/certificates/{certificate.external_id}/deactivations" response = self.session.post(deactivate_url) metrics.send("entrust_deactivate_certificate", "counter", 1) - return response.status_code + return handle_response(response) @staticmethod def create_authority(options):