From 7aa5ba9c6bf2dc05c34308b48e3d87bd91bcc00b Mon Sep 17 00:00:00 2001 From: kevgliss Date: Wed, 4 Jan 2017 17:46:47 -0800 Subject: [PATCH] =?UTF-8?q?Fixing=20an=20IAM=20syncing=20issue.=20Were=20d?= =?UTF-8?q?uplicates=20were=20not=20properly=20sync'd=E2=80=A6=20(#638)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fixing an IAM syncing issue. Were duplicates were not properly sync'd with Lemur. This resulted in a visibility gap. Even 'duplicates' need to sync'd to Lemur such that we can track rotation correctly. Failing on duplicates lead to missing those certificates and the endpoints onto which they were deployed. This commit removes the duplicate handling altogether. * Fixing tests. --- lemur/plugins/lemur_aws/elb.py | 6 +- lemur/plugins/lemur_aws/iam.py | 159 ++++++++++++---------- lemur/plugins/lemur_aws/plugin.py | 26 +--- lemur/plugins/lemur_aws/tests/test_iam.py | 13 +- lemur/sources/service.py | 32 ++--- lemur/tests/test_certificates.py | 2 +- 6 files changed, 110 insertions(+), 128 deletions(-) diff --git a/lemur/plugins/lemur_aws/elb.py b/lemur/plugins/lemur_aws/elb.py index 473c5edb..6dba80e0 100644 --- a/lemur/plugins/lemur_aws/elb.py +++ b/lemur/plugins/lemur_aws/elb.py @@ -10,6 +10,7 @@ from flask import current_app from retrying import retry +from lemur.extensions import metrics from lemur.exceptions import InvalidListener from lemur.plugins.lemur_aws.sts import sts_client @@ -22,11 +23,12 @@ def retry_throttled(exception): """ if isinstance(exception, botocore.exceptions.ClientError): if exception.response['Error']['Code'] == 'LoadBalancerNotFound': - return + return False if exception.response['Error']['Code'] == 'CertificateNotFound': - return + return False + metrics.send('ec2_retry', 'counter', 1) return True diff --git a/lemur/plugins/lemur_aws/iam.py b/lemur/plugins/lemur_aws/iam.py index 98433f89..429403b5 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.plugins.lemur_aws.sts import assume_service +from lemur.extensions import metrics from lemur.plugins.lemur_aws.sts import sts_client @@ -23,6 +23,8 @@ def retry_throttled(exception): if isinstance(exception, botocore.exceptions.ClientError): if exception.response['Error']['Code'] == 'NoSuchEntity': return False + + metrics.send('iam_retry', 'counter', 1) return True @@ -36,69 +38,6 @@ def get_name_from_arn(arn): return arn.split("/", 1)[1] -def upload_cert(account_number, name, body, private_key, cert_chain=None): - """ - Upload a certificate to AWS - - :param account_number: - :param name: - :param private_key: - :param cert_chain: - :return: - """ - return assume_service(account_number, 'iam').upload_server_cert(name, str(body), str(private_key), - cert_chain=str(cert_chain)) - - -@sts_client('iam') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) -def delete_cert(cert_name, **kwargs): - """ - Delete a certificate from AWS - - :param cert_name: - :return: - """ - client = kwargs.pop('client') - client.delete_server_certificate(ServerCertificateName=cert_name) - - -def get_all_server_certs(account_number): - """ - Use STS to fetch all of the SSL certificates from a given account - - :param account_number: - """ - marker = None - certs = [] - while True: - response = assume_service(account_number, 'iam').get_all_server_certs(marker=marker) - result = response['list_server_certificates_response']['list_server_certificates_result'] - - for cert in result['server_certificate_metadata_list']: - certs.append(cert['arn']) - - if result['is_truncated'] == 'true': - marker = result['marker'] - else: - return certs - - -def get_cert_from_arn(arn): - """ - Retrieves an SSL certificate from a given ARN. - - :param arn: - :return: - """ - name = get_name_from_arn(arn) - account_number = arn.split(":")[4] - name = name.split("/")[-1] - - response = assume_service(account_number, 'iam').get_server_certificate(name.strip()) - return digest_aws_cert_response(response) - - def create_arn_from_cert(account_number, region, certificate_name): """ Create an ARN from a certificate. @@ -112,18 +51,92 @@ def create_arn_from_cert(account_number, region, certificate_name): certificate_name=certificate_name) -def digest_aws_cert_response(response): +@sts_client('iam') +@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +def upload_cert(name, body, private_key, cert_chain=None, **kwargs): """ - Processes an AWS certifcate response and retrieves the certificate body and chain. + Upload a certificate to AWS - :param response: + :param name: + :param body: + :param private_key: + :param cert_chain: :return: """ - chain = None - cert = response['get_server_certificate_response']['get_server_certificate_result']['server_certificate'] - body = cert['certificate_body'] + client = kwargs.pop('client') + try: + if cert_chain: + return client.upload_server_certificate( + ServerCertificateName=name, + CertificateBody=str(body), + PrivateKey=str(private_key), + CertificateChain=str(cert_chain) + ) + else: + return client.upload_server_certificate( + ServerCertificateName=name, + CertificateBody=str(body), + PrivateKey=str(private_key) + ) + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] != 'EntityAlreadyExists': + raise e - if 'certificate_chain' in cert: - chain = cert['certificate_chain'] - return body, chain +@sts_client('iam') +@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +def delete_cert(cert_name, **kwargs): + """ + Delete a certificate from AWS + + :param cert_name: + :return: + """ + client = kwargs.pop('client') + client.delete_server_certificate(ServerCertificateName=cert_name) + + +@sts_client('iam') +@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +def get_certificate(name, **kwargs): + """ + Retrieves an SSL certificate. + + :return: + """ + client = kwargs.pop('client') + return client.get_server_certificate( + ServerCertificateName=name + )['ServerCertificate'] + + +@sts_client('iam') +@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +def get_certificates(**kwargs): + """ + Fetches one page of certificate objects for a given account. + :param kwargs: + :return: + """ + client = kwargs.pop('client') + return client.list_server_certificates(**kwargs) + + +def get_all_certificates(**kwargs): + """ + Use STS to fetch all of the SSL certificates from a given account + """ + certificates = [] + account_number = kwargs.get('account_number') + + while True: + response = get_certificates(**kwargs) + metadata = response['ServerCertificateMetadataList'] + + for m in metadata: + certificates.append(get_certificate(m['ServerCertificateName'], account_number=account_number)) + + if not response.get('Marker'): + return certificates + else: + kwargs.update(dict(Marker=response['Marker'])) diff --git a/lemur/plugins/lemur_aws/plugin.py b/lemur/plugins/lemur_aws/plugin.py index 96652945..26aeae19 100644 --- a/lemur/plugins/lemur_aws/plugin.py +++ b/lemur/plugins/lemur_aws/plugin.py @@ -33,7 +33,6 @@ .. moduleauthor:: Harm Weites """ from flask import current_app -from boto.exception import BotoServerError from lemur.plugins.bases import DestinationPlugin, SourcePlugin from lemur.plugins.lemur_aws import iam, s3, elb, ec2 @@ -125,7 +124,7 @@ def get_elb_endpoints_v2(account_number, region, elb_dict): endpoints = [] listeners = elb.describe_listeners_v2(account_number=account_number, region=region, LoadBalancerArn=elb_dict['LoadBalancerArn']) for listener in listeners['Listeners']: - if not listener['Certificates']: + if not listener.get('Certificates'): continue for certificate in listener['Certificates']: @@ -172,12 +171,9 @@ class AWSDestinationPlugin(DestinationPlugin): # } def upload(self, name, body, private_key, cert_chain, options, **kwargs): - try: - iam.upload_cert(self.get_option('accountNumber', options), name, body, private_key, - cert_chain=cert_chain) - except BotoServerError as e: - if e.error_code != 'EntityAlreadyExists': - raise Exception(e) + iam.upload_cert(name, body, private_key, + cert_chain=cert_chain, + account_number=self.get_option('accountNumber', options)) def deploy(self, elb_name, account, region, certificate): pass @@ -208,18 +204,8 @@ class AWSSourcePlugin(SourcePlugin): ] def get_certificates(self, options, **kwargs): - certs = [] - arns = iam.get_all_server_certs(self.get_option('accountNumber', options)) - for arn in arns: - cert_body, cert_chain = iam.get_cert_from_arn(arn) - cert_name = iam.get_name_from_arn(arn) - cert = dict( - body=cert_body, - chain=cert_chain, - name=cert_name - ) - certs.append(cert) - return certs + cert_data = iam.get_all_certificates(account_number=self.get_option('accountNumber', options)) + return [dict(body=c['CertificateBody'], chain=c.get('CertificateChain'), name=c['ServerCertificateMetadata']['ServerCertificateName']) for c in cert_data] def get_endpoints(self, options, **kwargs): endpoints = [] diff --git a/lemur/plugins/lemur_aws/tests/test_iam.py b/lemur/plugins/lemur_aws/tests/test_iam.py index eee0e236..4561a253 100644 --- a/lemur/plugins/lemur_aws/tests/test_iam.py +++ b/lemur/plugins/lemur_aws/tests/test_iam.py @@ -14,16 +14,7 @@ def test_get_name_from_arn(): @mock_sts() @mock_iam() def test_get_all_server_certs(app): - from lemur.plugins.lemur_aws.iam import upload_cert, get_all_server_certs + from lemur.plugins.lemur_aws.iam import upload_cert, get_all_certificates upload_cert('123456789012', 'testCert', EXTERNAL_VALID_STR, PRIVATE_KEY_STR) - certs = get_all_server_certs('123456789012') + certs = get_all_certificates('123456789012') assert len(certs) == 1 - - -@mock_sts() -@mock_iam() -def test_get_cert_from_arn(app): - from lemur.plugins.lemur_aws.iam import upload_cert, get_cert_from_arn - upload_cert('123456789012', 'testCert', EXTERNAL_VALID_STR, PRIVATE_KEY_STR) - body, chain = get_cert_from_arn('arn:aws:iam::123456789012:server-certificate/testCert') - assert body.replace('\n', '') == EXTERNAL_VALID_STR.replace('\n', '') diff --git a/lemur/sources/service.py b/lemur/sources/service.py index 6f1f4dbe..ab6ad508 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -72,20 +72,11 @@ def sync_endpoints(source): for endpoint in endpoints: exists = endpoint_service.get_by_dnsname(endpoint['dnsname']) - certificate_name = endpoint.pop('certificate_name', None) - certificate = endpoint.pop('certificate', None) - - if certificate_name: - cert = certificate_service.get_by_name(certificate_name) - - elif certificate: - cert = certificate_service.find_duplicates(certificate) - if not cert: - cert = certificate_service.import_certificate(**certificate) + cert = certificate_service.get_by_name(endpoint['certificate_name']) if not cert: current_app.logger.error( - "Unable to find associated certificate, be sure that certificates are sync'ed before endpoints") + "Certificate Not Found. Name: {0} Endpoint: {1}".format(endpoint['certificate_name'], endpoint['name'])) continue endpoint['certificate'] = cert @@ -101,10 +92,12 @@ def sync_endpoints(source): endpoint['source'] = source if not exists: + current_app.logger.debug("Endpoint Created: Name: {name}".format(name=endpoint['name'])) endpoint_service.create(**endpoint) new += 1 else: + current_app.logger.debug("Endpoint Updated: Name: {name}".format(name=endpoint['name'])) endpoint_service.update(exists.id, **endpoint) updated += 1 @@ -119,25 +112,22 @@ def sync_certificates(source, user): certificates = s.get_certificates(source.options) for certificate in certificates: - exists = certificate_service.find_duplicates(certificate) + exists = certificate_service.get_by_name(certificate['name']) certificate['owner'] = user.email certificate['creator'] = user if not exists: + current_app.logger.debug("Creating Certificate. Name: {name}".format(name=certificate['name'])) certificate_create(certificate, source) new += 1 - # check to make sure that existing certificates have the current source associated with it - elif len(exists) == 1: - certificate_update(exists[0], source) - updated += 1 else: - current_app.logger.warning( - "Multiple certificates found, attempt to deduplicate the following certificates: {0}".format( - ",".join([x.name for x in exists]) - ) - ) + current_app.logger.debug("Updating Certificate. Name: {name}".format(name=certificate['name'])) + certificate_update(exists, source) + updated += 1 + + assert len(certificates) == new + updated return new, updated diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index 278ffbc8..af6f5938 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -218,7 +218,7 @@ def test_certificate_valid_years(client, authority): 'owner': 'jim@example.com', 'authority': {'id': authority.id}, 'description': 'testtestest', - 'validityYears': 3 + 'validityYears': 2 } data, errors = CertificateInputSchema().load(input_data)