From b9b3ae2286d9486c2657e8f6e69cc7fb4395996d Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:38:59 -0800 Subject: [PATCH 01/13] better exception handling --- lemur/certificates/cli.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 4b4b22e9..8f4d9ca7 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -119,6 +119,11 @@ def request_rotation(endpoint, certificate, message, commit): status = SUCCESS_METRIC_STATUS except Exception as e: + sentry.captureException(extra={"certificate_name": str(certificate.name), + "endpoint": str(endpoint.dnsname)}) + current_app.logger.exception( + f"Error rotating certificate: {certificate.name}", exc_info=True + ) print( "[!] Failed to rotate endpoint {0} to certificate {1} reason: {2}".format( endpoint.name, certificate.name, e From c26db968d2c504eae8a1e8f2f674d8c8abb0768e Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:43:21 -0800 Subject: [PATCH 02/13] better logging --- lemur/certificates/cli.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 8f4d9ca7..2bf0a152 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -130,7 +130,9 @@ def request_rotation(endpoint, certificate, message, commit): ) ) - metrics.send("endpoint_rotation", "counter", 1, metric_tags={"status": status}) + metrics.send("endpoint_rotation", "counter", 1, metric_tags={"status": status, + "certificate_name": str(certificate.name), + "endpoint": str(endpoint.dnsname)}) def request_reissue(certificate, commit): From de762d2e535429d78194e97a233f76b746ee3932 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:43:59 -0800 Subject: [PATCH 03/13] better logging --- lemur/certificates/cli.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 2bf0a152..24f8f59a 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -231,7 +231,7 @@ 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["message"] = "Rotating one endpoint" log_data["endpoint"] = endpoint.dnsname log_data["certificate"] = new_cert.name request_rotation(endpoint, new_cert, message, commit) @@ -425,7 +425,6 @@ def rotate_region(endpoint_name, new_certificate_name, old_certificate_name, mes 1, metric_tags={ "region": region, - "old_certificate_name": str(old_cert), "new_certificate_name": str(endpoint.certificate.replaced[0].name), "endpoint_name": str(endpoint.dnsname), }, @@ -450,7 +449,6 @@ def rotate_region(endpoint_name, new_certificate_name, old_certificate_name, mes 1, metric_tags={ "status": FAILURE_METRIC_STATUS, - "old_certificate_name": str(old_cert), "new_certificate_name": str(endpoint.certificate.replaced[0].name), "endpoint_name": str(endpoint.dnsname), "message": str(message), From 92dabe5d43bcf0b57285de280ca98eaa9b4fed20 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:44:10 -0800 Subject: [PATCH 04/13] ineffective line --- lemur/certificates/cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 24f8f59a..b1b84492 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -239,8 +239,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 log_data["message"] = "Rotating endpoint from old to new cert" From 80f83efec2055dc037332a7e0eba22f9ba8f37c5 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:44:19 -0800 Subject: [PATCH 05/13] todo --- lemur/certificates/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index b1b84492..56dd9260 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -374,6 +374,7 @@ def rotate_region(endpoint_name, new_certificate_name, old_certificate_name, mes :param message: Send a rotation notification to the certificates owner. :param commit: Persist changes. :param region: Region in which to rotate the endpoint. + #todo: merge this method with rotate() """ if commit: print("[!] Running in COMMIT mode.") From 089796a84957961a36ef66e285f9eab90e97fea2 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:47:41 -0800 Subject: [PATCH 06/13] bug, intention was to skip a region --- lemur/certificates/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 56dd9260..85b62c41 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -428,6 +428,7 @@ def rotate_region(endpoint_name, new_certificate_name, old_certificate_name, mes "endpoint_name": str(endpoint.dnsname), }, ) + continue if len(endpoint.certificate.replaced) == 1: log_data["certificate"] = endpoint.certificate.replaced[0].name From e250a613444d5a9b7d3aa9f83ccb6ca6dc4b89d1 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:48:39 -0800 Subject: [PATCH 07/13] fixing the message bug --- 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 85b62c41..e3ce99f6 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -250,7 +250,6 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c 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: @@ -284,6 +283,7 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c f"[!] Failed to rotate endpoint {endpoint.name} reason: " "Multiple replacement certificates found." ) + log_data["message"] = "Rotating endpoint from old to new cert" status = SUCCESS_METRIC_STATUS print("[+] Done!") From 0d7e8d77e48bb48d76c28b869876a8073b84f348 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:49:00 -0800 Subject: [PATCH 08/13] comments --- lemur/certificates/cli.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index e3ce99f6..52bcae3a 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -249,6 +249,8 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c current_app.logger.info(log_data) else: + # no certificate name or endpoint was provided, so we will now fetch all endpoints, + # which have are attached to a certificate that has been replaced print("[+] Rotating all endpoints that have new certificates available") for endpoint in endpoint_service.get_all_pending_rotation(): log_data["endpoint"] = endpoint.dnsname From 63e9fdd0e19621788bf3581fa9902c7eb2964e00 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:50:02 -0800 Subject: [PATCH 09/13] rotate also in case of multiple certificates replacing the old one, just select the first one! --- lemur/certificates/cli.py | 43 +++++++++++---------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index 52bcae3a..a4c10808 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -253,39 +253,20 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c # which have are attached to a certificate that has been replaced print("[+] 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", - 1, - metric_tags={ - "status": FAILURE_METRIC_STATUS, - "old_certificate_name": str(old_cert), - "new_certificate_name": str( - endpoint.certificate.replaced[0].name - ), - "endpoint_name": str(endpoint.name), - "message": str(message), - }, - ) - print( - f"[!] Failed to rotate endpoint {endpoint.name} reason: " - "Multiple replacement certificates found." - ) log_data["message"] = "Rotating endpoint from old to new cert" + if len(endpoint.certificate.replaced) > 1: + log_data["message"] = f"Multiple replacement certificates found, going with the first one out of " \ + f"{len(endpoint.certificate.replaced)}" + + log_data["endpoint"] = endpoint.dnsname + log_data["certificate"] = endpoint.certificate.replaced[0].name + request_rotation(endpoint, endpoint.certificate.replaced[0], message, commit) + print(log_data) + print( + f"[+] Rotating {endpoint.name} to {endpoint.certificate.replaced[0].name}" + ) + current_app.logger.info(log_data) status = SUCCESS_METRIC_STATUS print("[+] Done!") From b80b6d0959a1652f80f34b86878d6a74a983fb2d Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:50:16 -0800 Subject: [PATCH 10/13] same change for rotate region --- lemur/certificates/cli.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index a4c10808..0fee8fc2 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -413,18 +413,15 @@ def rotate_region(endpoint_name, new_certificate_name, old_certificate_name, mes ) continue - if len(endpoint.certificate.replaced) == 1: - log_data["certificate"] = endpoint.certificate.replaced[0].name - log_data["message"] = "Rotating all endpoints in region" - print(log_data) - current_app.logger.info(log_data) - request_rotation(endpoint, endpoint.certificate.replaced[0], message, commit) - status = SUCCESS_METRIC_STATUS - else: - status = FAILURE_METRIC_STATUS - log_data["message"] = "Failed to rotate endpoint due to Multiple replacement certificates found" - print(log_data) - current_app.logger.info(log_data) + log_data["certificate"] = endpoint.certificate.replaced[0].name + log_data["message"] = "Rotating all endpoints in region" + if len(endpoint.certificate.replaced) > 1: + log_data["message"] = f"Multiple replacement certificates found, going with the first one out of " \ + f"{len(endpoint.certificate.replaced)}" + + request_rotation(endpoint, endpoint.certificate.replaced[0], message, commit) + print(log_data) + current_app.logger.info(log_data) metrics.send( "endpoint_rotation_region", From 895d5e6ec72604ca9daae68d7f8d05e0a07dab61 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 15:58:01 -0800 Subject: [PATCH 11/13] lint --- 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 0fee8fc2..dac7680d 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -131,7 +131,7 @@ def request_rotation(endpoint, certificate, message, commit): ) metrics.send("endpoint_rotation", "counter", 1, metric_tags={"status": status, - "certificate_name": str(certificate.name), + "certificate_name": str(certificate.name), "endpoint": str(endpoint.dnsname)}) From d5f678f70cc8976fc711034bf16ba8ffd9d5385b Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 29 Jan 2021 10:46:58 -0800 Subject: [PATCH 12/13] language --- lemur/certificates/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index dac7680d..f881b9b6 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -249,8 +249,8 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c current_app.logger.info(log_data) else: - # no certificate name or endpoint was provided, so we will now fetch all endpoints, - # which have are attached to a certificate that has been replaced + # No certificate name or endpoint is provided. We will now fetch all endpoints, + # which are associated with a certificate that has been replaced print("[+] Rotating all endpoints that have new certificates available") for endpoint in endpoint_service.get_all_pending_rotation(): From 6df1b2985c4e12b8c42280c527911e422e687a3f Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 29 Jan 2021 10:49:18 -0800 Subject: [PATCH 13/13] removing duplicate print --- lemur/certificates/cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index f881b9b6..e00f6ea7 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -262,7 +262,6 @@ def rotate(endpoint_name, new_certificate_name, old_certificate_name, message, c log_data["endpoint"] = endpoint.dnsname log_data["certificate"] = endpoint.certificate.replaced[0].name request_rotation(endpoint, endpoint.certificate.replaced[0], message, commit) - print(log_data) print( f"[+] Rotating {endpoint.name} to {endpoint.certificate.replaced[0].name}" ) @@ -420,7 +419,6 @@ def rotate_region(endpoint_name, new_certificate_name, old_certificate_name, mes f"{len(endpoint.certificate.replaced)}" request_rotation(endpoint, endpoint.certificate.replaced[0], message, commit) - print(log_data) current_app.logger.info(log_data) metrics.send(