Fixing an IAM syncing issue. Were duplicates were not properly sync'd… (#638)

* 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

@@ -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
 
 

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
+    client = kwargs.pop('client')
-    cert = response['get_server_certificate_response']['get_server_certificate_result']['server_certificate']
+    try:
-    body = cert['certificate_body']
+        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']))

lemur/plugins/lemur_aws/plugin.py

@@ -33,7 +33,6 @@
 .. moduleauthor:: Harm Weites <harm@weites.com>
 """
 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(name, body, private_key,
-            iam.upload_cert(self.get_option('accountNumber', options), name, body, private_key,
+                        cert_chain=cert_chain,
-                            cert_chain=cert_chain)
+                        account_number=self.get_option('accountNumber', options))
-        except BotoServerError as e:
-            if e.error_code != 'EntityAlreadyExists':
-                raise Exception(e)
 
     def deploy(self, elb_name, account, region, certificate):
         pass
@@ -208,18 +204,8 @@ class AWSSourcePlugin(SourcePlugin):
     ]
 
     def get_certificates(self, options, **kwargs):
-        certs = []
+        cert_data = iam.get_all_certificates(account_number=self.get_option('accountNumber', options))
-        arns = iam.get_all_server_certs(self.get_option('accountNumber', options))
+        return [dict(body=c['CertificateBody'], chain=c.get('CertificateChain'), name=c['ServerCertificateMetadata']['ServerCertificateName']) for c in cert_data]
-        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
 
     def get_endpoints(self, options, **kwargs):
         endpoints = []

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', '')

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)
+        cert = certificate_service.get_by_name(endpoint['certificate_name'])
-        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)
 
         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(
+            current_app.logger.debug("Updating Certificate. Name: {name}".format(name=certificate['name']))
-                "Multiple certificates found, attempt to deduplicate the following certificates: {0}".format(
+            certificate_update(exists, source)
-                    ",".join([x.name for x in exists])
+            updated += 1
-                )
+
-            )
+    assert len(certificates) == new + updated
 
     return new, updated
 

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)