From 10b600424efbabcdbe2727e1a94d3ba15778ae71 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 08:45:32 -0700 Subject: [PATCH 1/8] refactoring searching for cert --- lemur/sources/service.py | 47 +++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/lemur/sources/service.py b/lemur/sources/service.py index d5bd7426..070e1a47 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -124,40 +124,47 @@ def sync_endpoints(source): return new, updated +def find_cert(certificate): + updated_by_hash = 0 + exists = False + + if certificate.get("search", None): + conditions = certificate.pop("search") + exists = certificate_service.get_by_attributes(conditions) + + if not exists and certificate.get("name"): + result = certificate_service.get_by_name(certificate["name"]) + if result: + exists = [result] + + if not exists and certificate.get("serial"): + exists = certificate_service.get_by_serial(certificate["serial"]) + + if not exists: + cert = parse_certificate(certificate["body"]) + matching_serials = certificate_service.get_by_serial(serial(cert)) + exists = find_matching_certificates_by_hash(cert, matching_serials) + updated_by_hash += 1 + + exists = [x for x in exists if x] + return exists, updated_by_hash + # TODO this is very slow as we don't batch update certificates def sync_certificates(source, user): - new, updated = 0, 0 + new, updated, updated_by_hash = 0, 0, 0 current_app.logger.debug("Retrieving certificates from {0}".format(source.label)) s = plugins.get(source.plugin_name) certificates = s.get_certificates(source.options) for certificate in certificates: - exists = False - - if certificate.get("search", None): - conditions = certificate.pop("search") - exists = certificate_service.get_by_attributes(conditions) - - if not exists and certificate.get("name"): - result = certificate_service.get_by_name(certificate["name"]) - if result: - exists = [result] - - if not exists and certificate.get("serial"): - exists = certificate_service.get_by_serial(certificate["serial"]) - - if not exists: - cert = parse_certificate(certificate["body"]) - matching_serials = certificate_service.get_by_serial(serial(cert)) - exists = find_matching_certificates_by_hash(cert, matching_serials) + exists, updated_by_hash = find_cert(certificate) if not certificate.get("owner"): certificate["owner"] = user.email certificate["creator"] = user - exists = [x for x in exists if x] if not exists: certificate_create(certificate, source) From d43e859c34ca61caca375485a5c0a912655d5474 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 08:46:01 -0700 Subject: [PATCH 2/8] describing the cert for each endpoint, for better cert search --- lemur/plugins/lemur_aws/plugin.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lemur/plugins/lemur_aws/plugin.py b/lemur/plugins/lemur_aws/plugin.py index cf6c8643..a03e92a8 100644 --- a/lemur/plugins/lemur_aws/plugin.py +++ b/lemur/plugins/lemur_aws/plugin.py @@ -32,7 +32,9 @@ .. moduleauthor:: Mikhail Khodorovskiy .. moduleauthor:: Harm Weites """ +from acme.errors import ClientError from flask import current_app +from lemur.extensions import sentry, metrics from lemur.plugins import lemur_aws as aws from lemur.plugins.bases import DestinationPlugin, ExportDestinationPlugin, SourcePlugin @@ -110,6 +112,8 @@ def get_elb_endpoints(account_number, region, elb_dict): listener["Listener"]["SSLCertificateId"] ), ) + endpoint["certificate"] = get_elb_certificate_by_name(certificate_name=endpoint["certificate_name"], + account_number=account_number) if listener["PolicyNames"]: policy = elb.describe_load_balancer_policies( @@ -127,6 +131,28 @@ def get_elb_endpoints(account_number, region, elb_dict): return endpoints +def get_elb_certificate_by_name(certificate_name, account_number): + # certificate name may contain path, in which case we remove it + if "/" in certificate_name: + certificate_name = certificate_name.split('/')[1] + try: + cert = iam.get_certificate(certificate_name, account_number=account_number) + return dict( + body=cert["CertificateBody"], + chain=cert.get("CertificateChain"), + name=cert["ServerCertificateMetadata"]["ServerCertificateName"], + ) + except ClientError: + current_app.logger.warning( + "get_elb_certificate_failed: Unable to get certificate for {0}".format(certificate_name)) + sentry.captureException() + metrics.send( + "get_elb_certificate_failed", "counter", 1, + metric_tags={"certificate_name": certificate_name, "account_number": account_number} + ) + return None + + def get_elb_endpoints_v2(account_number, region, elb_dict): """ Retrieves endpoint information from elbv2 response data. @@ -153,6 +179,8 @@ def get_elb_endpoints_v2(account_number, region, elb_dict): port=listener["Port"], certificate_name=iam.get_name_from_arn(certificate["CertificateArn"]), ) + endpoint["certificate"] = get_elb_certificate_by_name(certificate_name=endpoint["certificate_name"], + account_number=account_number) if listener["SslPolicy"]: policy = elb.describe_ssl_policies_v2( From f075c5af3d7e6c8d5353186770b3b7bc05453b50 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 08:48:11 -0700 Subject: [PATCH 3/8] in case no cert match via name-search, search via the cert itself (serial number, hash comparison) --- lemur/sources/service.py | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/lemur/sources/service.py b/lemur/sources/service.py index 070e1a47..23f2af72 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -66,7 +66,7 @@ def sync_update_destination(certificate, source): def sync_endpoints(source): - new, updated = 0, 0 + new, updated, updated_by_hash = 0, 0, 0 current_app.logger.debug("Retrieving endpoints from {0}".format(source.label)) s = plugins.get(source.plugin_name) @@ -89,6 +89,29 @@ def sync_endpoints(source): endpoint["certificate"] = certificate_service.get_by_name(certificate_name) + # if get cert by name failed, we attempt a search via serial number and hash comparison + # and link the endpoint certificate to Lemur certificate + if not endpoint["certificate"]: + certificate_attached_to_endpoint = endpoint.pop("certificate") + if certificate_attached_to_endpoint: + lemur_matching_cert, updated_by_hash_tmp = find_cert(certificate_attached_to_endpoint) + updated_by_hash += updated_by_hash_tmp + + if lemur_matching_cert: + endpoint["certificate"] = lemur_matching_cert[0] + + if len(lemur_matching_cert) > 1: + current_app.logger.error( + "Too Many Certificates Found. Name: {0} Endpoint: {1}".format( + certificate_name, endpoint["name"] + ) + ) + metrics.send("endpoint.certificate.conflict", + "counter", 1, + metric_tags={"cert": certificate_name, "endpoint": endpoint["name"], + "acct": s.get_option("accountNumber", source.options)}) + + # this indicates the we were not able to describe the endpoint cert if not endpoint["certificate"]: current_app.logger.error( "Certificate Not Found. Name: {0} Endpoint: {1}".format( @@ -97,7 +120,8 @@ def sync_endpoints(source): ) metrics.send("endpoint.certificate.not.found", "counter", 1, - metric_tags={"cert": certificate_name, "endpoint": endpoint["name"], "acct": s.get_option("accountNumber", source.options)}) + metric_tags={"cert": certificate_name, "endpoint": endpoint["name"], + "acct": s.get_option("accountNumber", source.options)}) continue policy = endpoint.pop("policy") @@ -122,7 +146,8 @@ def sync_endpoints(source): endpoint_service.update(exists.id, **endpoint) updated += 1 - return new, updated + return new, updated, updated_by_hash + def find_cert(certificate): updated_by_hash = 0 @@ -159,7 +184,7 @@ def sync_certificates(source, user): certificates = s.get_certificates(source.options) for certificate in certificates: - exists, updated_by_hash = find_cert(certificate) + exists, updated_by_hash = find_cert(certificate) if not certificate.get("owner"): certificate["owner"] = user.email @@ -179,12 +204,12 @@ def sync_certificates(source, user): certificate_update(e, source) updated += 1 - return new, updated + return new, updated, updated_by_hash def sync(source, user): - new_certs, updated_certs = sync_certificates(source, user) - new_endpoints, updated_endpoints = sync_endpoints(source) + new_certs, updated_certs, updated_certs_by_hash = sync_certificates(source, user) + new_endpoints, updated_endpoints, updated_endpoints_by_hash = sync_endpoints(source) source.last_run = arrow.utcnow() database.update(source) From 8aea257e6abb3f2d940ebf230fa81075c2425547 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 09:24:49 -0700 Subject: [PATCH 4/8] optimizing the call to describe cert to only the few certs with the naming issue --- lemur/plugins/lemur_aws/plugin.py | 48 ++++++++++++++----------------- lemur/sources/service.py | 16 +++++++++-- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/lemur/plugins/lemur_aws/plugin.py b/lemur/plugins/lemur_aws/plugin.py index a03e92a8..46c65c4f 100644 --- a/lemur/plugins/lemur_aws/plugin.py +++ b/lemur/plugins/lemur_aws/plugin.py @@ -112,8 +112,6 @@ def get_elb_endpoints(account_number, region, elb_dict): listener["Listener"]["SSLCertificateId"] ), ) - endpoint["certificate"] = get_elb_certificate_by_name(certificate_name=endpoint["certificate_name"], - account_number=account_number) if listener["PolicyNames"]: policy = elb.describe_load_balancer_policies( @@ -131,28 +129,6 @@ def get_elb_endpoints(account_number, region, elb_dict): return endpoints -def get_elb_certificate_by_name(certificate_name, account_number): - # certificate name may contain path, in which case we remove it - if "/" in certificate_name: - certificate_name = certificate_name.split('/')[1] - try: - cert = iam.get_certificate(certificate_name, account_number=account_number) - return dict( - body=cert["CertificateBody"], - chain=cert.get("CertificateChain"), - name=cert["ServerCertificateMetadata"]["ServerCertificateName"], - ) - except ClientError: - current_app.logger.warning( - "get_elb_certificate_failed: Unable to get certificate for {0}".format(certificate_name)) - sentry.captureException() - metrics.send( - "get_elb_certificate_failed", "counter", 1, - metric_tags={"certificate_name": certificate_name, "account_number": account_number} - ) - return None - - def get_elb_endpoints_v2(account_number, region, elb_dict): """ Retrieves endpoint information from elbv2 response data. @@ -179,8 +155,6 @@ def get_elb_endpoints_v2(account_number, region, elb_dict): port=listener["Port"], certificate_name=iam.get_name_from_arn(certificate["CertificateArn"]), ) - endpoint["certificate"] = get_elb_certificate_by_name(certificate_name=endpoint["certificate_name"], - account_number=account_number) if listener["SslPolicy"]: policy = elb.describe_ssl_policies_v2( @@ -299,6 +273,28 @@ class AWSSourcePlugin(SourcePlugin): account_number = self.get_option("accountNumber", options) iam.delete_cert(certificate.name, account_number=account_number) + def get_certificate_by_name(self, certificate_name, options): + account_number = self.get_option("accountNumber", options) + # certificate name may contain path, in which case we remove it + if "/" in certificate_name: + certificate_name = certificate_name.split('/')[1] + try: + cert = iam.get_certificate(certificate_name, account_number=account_number) + return dict( + body=cert["CertificateBody"], + chain=cert.get("CertificateChain"), + name=cert["ServerCertificateMetadata"]["ServerCertificateName"], + ) + except ClientError: + current_app.logger.warning( + "get_elb_certificate_failed: Unable to get certificate for {0}".format(certificate_name)) + sentry.captureException() + metrics.send( + "get_elb_certificate_failed", "counter", 1, + metric_tags={"certificate_name": certificate_name, "account_number": account_number} + ) + return None + class AWSDestinationPlugin(DestinationPlugin): title = "AWS" diff --git a/lemur/sources/service.py b/lemur/sources/service.py index 23f2af72..498adfeb 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -15,7 +15,7 @@ from lemur.sources.models import Source from lemur.certificates.models import Certificate from lemur.certificates import service as certificate_service from lemur.endpoints import service as endpoint_service -from lemur.extensions import metrics +from lemur.extensions import metrics, sentry from lemur.destinations import service as destination_service from lemur.certificates.schemas import CertificateUploadInputSchema @@ -92,7 +92,18 @@ def sync_endpoints(source): # if get cert by name failed, we attempt a search via serial number and hash comparison # and link the endpoint certificate to Lemur certificate if not endpoint["certificate"]: - certificate_attached_to_endpoint = endpoint.pop("certificate") + certificate_attached_to_endpoint = None + try: + certificate_attached_to_endpoint = s.get_certificate_by_name(certificate_name, source.options) + except NotImplementedError: + current_app.logger.warning( + "Unable to describe server certificate for endpoints in source {0}:" + " plugin has not implemented 'get_certificate_by_name'".format( + source.label + ) + ) + sentry.captureException() + if certificate_attached_to_endpoint: lemur_matching_cert, updated_by_hash_tmp = find_cert(certificate_attached_to_endpoint) updated_by_hash += updated_by_hash_tmp @@ -111,7 +122,6 @@ def sync_endpoints(source): metric_tags={"cert": certificate_name, "endpoint": endpoint["name"], "acct": s.get_option("accountNumber", source.options)}) - # this indicates the we were not able to describe the endpoint cert if not endpoint["certificate"]: current_app.logger.error( "Certificate Not Found. Name: {0} Endpoint: {1}".format( From 1768aad9e2ee95ed28ecfa9837b7db3597ff8551 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 10:17:58 -0700 Subject: [PATCH 5/8] capturing no such entity exception. --- lemur/plugins/lemur_aws/iam.py | 10 ++++++---- lemur/plugins/lemur_aws/plugin.py | 11 ++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lemur/plugins/lemur_aws/iam.py b/lemur/plugins/lemur_aws/iam.py index 67c35262..13590ddd 100644 --- a/lemur/plugins/lemur_aws/iam.py +++ b/lemur/plugins/lemur_aws/iam.py @@ -10,7 +10,7 @@ import botocore from retrying import retry -from lemur.extensions import metrics +from lemur.extensions import metrics, sentry from lemur.plugins.lemur_aws.sts import sts_client @@ -122,9 +122,11 @@ def get_certificate(name, **kwargs): """ client = kwargs.pop("client") metrics.send("get_certificate", "counter", 1, metric_tags={"name": name}) - return client.get_server_certificate(ServerCertificateName=name)[ - "ServerCertificate" - ] + try: + return client.get_server_certificate(ServerCertificateName=name)["ServerCertificate"] + except client.exceptions.NoSuchEntityException: + sentry.captureException() + return None @sts_client("iam") diff --git a/lemur/plugins/lemur_aws/plugin.py b/lemur/plugins/lemur_aws/plugin.py index 46c65c4f..86cd7912 100644 --- a/lemur/plugins/lemur_aws/plugin.py +++ b/lemur/plugins/lemur_aws/plugin.py @@ -280,11 +280,12 @@ class AWSSourcePlugin(SourcePlugin): certificate_name = certificate_name.split('/')[1] try: cert = iam.get_certificate(certificate_name, account_number=account_number) - return dict( - body=cert["CertificateBody"], - chain=cert.get("CertificateChain"), - name=cert["ServerCertificateMetadata"]["ServerCertificateName"], - ) + if cert: + return dict( + body=cert["CertificateBody"], + chain=cert.get("CertificateChain"), + name=cert["ServerCertificateMetadata"]["ServerCertificateName"], + ) except ClientError: current_app.logger.warning( "get_elb_certificate_failed: Unable to get certificate for {0}".format(certificate_name)) From 9037f8843072ed3ab1695d4bca681d38e01f46de Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 11:02:41 -0700 Subject: [PATCH 6/8] just in case the path varies --- lemur/plugins/lemur_aws/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_aws/plugin.py b/lemur/plugins/lemur_aws/plugin.py index 86cd7912..98b01672 100644 --- a/lemur/plugins/lemur_aws/plugin.py +++ b/lemur/plugins/lemur_aws/plugin.py @@ -277,7 +277,7 @@ class AWSSourcePlugin(SourcePlugin): account_number = self.get_option("accountNumber", options) # certificate name may contain path, in which case we remove it if "/" in certificate_name: - certificate_name = certificate_name.split('/')[1] + certificate_name = certificate_name.split('/')[-1] try: cert = iam.get_certificate(certificate_name, account_number=account_number) if cert: From 14e13b512e70f1819f88964291744ab690417aff Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 11:03:28 -0700 Subject: [PATCH 7/8] providing a count for conflicts --- lemur/sources/service.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lemur/sources/service.py b/lemur/sources/service.py index 498adfeb..8ba4ea0d 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -113,12 +113,12 @@ def sync_endpoints(source): if len(lemur_matching_cert) > 1: current_app.logger.error( - "Too Many Certificates Found. Name: {0} Endpoint: {1}".format( - certificate_name, endpoint["name"] + "Too Many Certificates Found{0}. Name: {1} Endpoint: {2}".format( + len(lemur_matching_cert), certificate_name, endpoint["name"] ) ) metrics.send("endpoint.certificate.conflict", - "counter", 1, + "gauge", len(lemur_matching_cert), metric_tags={"cert": certificate_name, "endpoint": endpoint["name"], "acct": s.get_option("accountNumber", source.options)}) From 06f4aed6939f8b7081b30002f705a5be5d2cdc62 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 18 Oct 2019 11:20:52 -0700 Subject: [PATCH 8/8] keeping track of certs found by hash --- lemur/sources/service.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lemur/sources/service.py b/lemur/sources/service.py index 8ba4ea0d..f69f70f5 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -221,6 +221,14 @@ def sync(source, user): new_certs, updated_certs, updated_certs_by_hash = sync_certificates(source, user) new_endpoints, updated_endpoints, updated_endpoints_by_hash = sync_endpoints(source) + metrics.send("sync.updated_certs_by_hash", + "gauge", updated_certs_by_hash, + metric_tags={"source": source.label}) + + metrics.send("sync.updated_endpoints_by_hash", + "gauge", updated_endpoints_by_hash, + metric_tags={"source": source.label}) + source.last_run = arrow.utcnow() database.update(source)