From 1207de8925509a43d327ab2a9af28062591d7ae3 Mon Sep 17 00:00:00 2001 From: sayali Date: Mon, 23 Nov 2020 15:24:11 -0800 Subject: [PATCH 1/9] Remove certificate from AWS and cleanup after cert revoke --- lemur/certificates/service.py | 59 ++++++++++ lemur/certificates/views.py | 41 +++++-- lemur/plugins/lemur_aws/elb.py | 32 ++++++ lemur/plugins/lemur_aws/plugin.py | 39 +++++++ lemur/plugins/lemur_aws/tests/test_elb.py | 106 +++++++++++++++++- .../certificates/certificate/revoke.tpl.html | 5 +- 6 files changed, 270 insertions(+), 12 deletions(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index ac844120..1aabec48 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -21,6 +21,7 @@ from lemur.certificates.schemas import CertificateOutputSchema, CertificateInput from lemur.common.utils import generate_private_key, truthiness from lemur.destinations.models import Destination from lemur.domains.models import Domain +from lemur.endpoints import service as endpoint_service from lemur.extensions import metrics, sentry, signals from lemur.models import certificate_associations from lemur.notifications.models import Notification @@ -797,3 +798,61 @@ def reissue_certificate(certificate, replace=None, user=None): new_cert = create(**primitives) return new_cert + + +def is_attached_to_endpoint(certificate_name, endpoint_name): + """ + Find if given certificate is attached to the endpoint. Both, certificate and endpoint, are identified by name. + This method talks to elb and finds the real time information. + :param certificate_name: + :param endpoint_name: + :return: True if certificate is attached to the given endpoint, False otherwise + """ + endpoint = endpoint_service.get_by_name(endpoint_name) + attached_certificates = endpoint.source.plugin.get_endpoint_certificate_names(endpoint) + return certificate_name in attached_certificates + + +def remove_from_destination(certificate, destination): + """ + Remove the certificate from given destination if clean() is implemented + :param certificate: + :param destination: + :return: + """ + plugin = plugins.get(destination.plugin_name) + if not hasattr(plugin, "clean"): + info_text = f"Cannot clean certificate {certificate.name}, {destination.plugin_name} plugin does not implement 'clean()'" + current_app.logger.warning(info_text) + else: + plugin.clean(certificate=certificate, options=destination.options) + + +def cleanup_after_revoke(certificate): + """ + Perform the needed cleanup for a revoked certificate. This includes - + 1. Disabling notification + 2. Disabling auto-rotation + 3. Update certificate status to 'revoked' + 4. Remove from AWS + :param certificate: Certificate object to modify and update in DB + :return: None + """ + certificate.notify = False + certificate.rotation = False + certificate.status = 'revoked' + + error_message = "" + + for destination in list(certificate.destinations): + try: + remove_from_destination(certificate, destination) + certificate.destinations.remove(destination) + except Exception as e: + # This cleanup is the best-effort since certificate is already revoked at this point. + # We will capture the exception and move on to the next destination + sentry.captureException() + error_message = error_message + f"Failed to remove destination: {destination.label}. {str(e)}. " + + database.update(certificate) + return error_message diff --git a/lemur/certificates/views.py b/lemur/certificates/views.py index a066f20f..8b8b36c3 100644 --- a/lemur/certificates/views.py +++ b/lemur/certificates/views.py @@ -19,6 +19,7 @@ from lemur.auth.permissions import AuthorityPermission, CertificatePermission from lemur.certificates import service from lemur.certificates.models import Certificate +from lemur.extensions import sentry from lemur.plugins.base import plugins from lemur.certificates.schemas import ( certificate_input_schema, @@ -888,8 +889,24 @@ class Certificates(AuthenticatedResource): if cert.owner != data["owner"]: service.cleanup_owner_roles_notification(cert.owner, data) + error_message = "" + # if destination is removed, cleanup the certificate from AWS + for destination in cert.destinations: + if destination not in data["destinations"]: + try: + service.remove_from_destination(cert, destination) + except Exception as e: + sentry.captureException() + # Add the removed destination back + data["destinations"].append(destination) + error_message = error_message + f"Failed to remove destination: {destination.label}. {str(e)}. " + + # go ahead with DB update cert = service.update(certificate_id, **data) log_service.create(g.current_user, "update_cert", certificate=cert) + + if error_message: + return dict(message=f"Edit Successful except -\n\n {error_message}"), 400 return cert @validate_schema(certificate_edit_input_schema, certificate_output_schema) @@ -1429,20 +1446,28 @@ class CertificateRevoke(AuthenticatedResource): 403, ) - if not cert.external_id: - return dict(message="Cannot revoke certificate. No external id found."), 400 + # if not cert.external_id: + # return dict(message="Cannot revoke certificate. No external id found."), 400 if cert.endpoints: - return ( - dict( - message="Cannot revoke certificate. Endpoints are deployed with the given certificate." - ), - 403, - ) + for endpoint in cert.endpoints: + if service.is_attached_to_endpoint(cert.name, endpoint.name): + return ( + dict( + message="Cannot revoke certificate. Endpoints are deployed with the given certificate." + ), + 403, + ) plugin = plugins.get(cert.authority.plugin_name) plugin.revoke_certificate(cert, data) + log_service.create(g.current_user, "revoke_cert", certificate=cert) + + # Perform cleanup after revoke + error_message = service.cleanup_after_revoke(cert) + if error_message: + return dict(message=f"Certificate (id:{cert.id}) is revoked - {error_message}"), 400 return dict(id=cert.id) diff --git a/lemur/plugins/lemur_aws/elb.py b/lemur/plugins/lemur_aws/elb.py index 595a3826..1da29276 100644 --- a/lemur/plugins/lemur_aws/elb.py +++ b/lemur/plugins/lemur_aws/elb.py @@ -149,6 +149,38 @@ def get_listener_arn_from_endpoint(endpoint_name, endpoint_port, **kwargs): raise +@sts_client("elbv2") +@retry(retry_on_exception=retry_throttled, wait_fixed=2000, stop_max_attempt_number=5) +def get_load_balancer_arn_from_endpoint(endpoint_name, **kwargs): + """ + Get a load balancer ARN from an endpoint. + :param endpoint_name: + :return: + """ + try: + client = kwargs.pop("client") + elbs = client.describe_load_balancers(Names=[endpoint_name]) + for elb in elbs["LoadBalancers"]: + return elb["LoadBalancerArn"] + + except Exception as e: # noqa + metrics.send( + "get_load_balancer_arn_from_endpoint", + "counter", + 1, + metric_tags={ + "error": str(e), + "endpoint_name": endpoint_name, + }, + ) + sentry.captureException( + extra={ + "endpoint_name": str(endpoint_name), + } + ) + raise + + @sts_client("elb") @retry(retry_on_exception=retry_throttled, wait_fixed=2000, stop_max_attempt_number=20) def get_elbs(**kwargs): diff --git a/lemur/plugins/lemur_aws/plugin.py b/lemur/plugins/lemur_aws/plugin.py index fcc2e0cf..efcce4d0 100644 --- a/lemur/plugins/lemur_aws/plugin.py +++ b/lemur/plugins/lemur_aws/plugin.py @@ -300,6 +300,41 @@ class AWSSourcePlugin(SourcePlugin): ) return None + def get_endpoint_certificate_names(self, endpoint): + options = endpoint.source.options + account_number = self.get_option("accountNumber", options) + region = get_region_from_dns(endpoint.dnsname) + certificate_names = [] + + if endpoint.type == "elb": + elb_details = elb.get_elbs(account_number=account_number, + region=region, + LoadBalancerNames=[endpoint.name],) + + for lb_description in elb_details["LoadBalancerDescriptions"]: + for listener_description in lb_description["ListenerDescriptions"]: + listener = listener_description.get("Listener") + if not listener.get("SSLCertificateId"): + continue + + certificate_names.append(iam.get_name_from_arn(listener.get("SSLCertificateId"))) + elif endpoint.type == "elbv2": + listeners = elb.describe_listeners_v2( + account_number=account_number, + region=region, + LoadBalancerArn=elb.get_load_balancer_arn_from_endpoint(endpoint.name, + account_number=account_number, + region=region), + ) + for listener in listeners["Listeners"]: + if not listener.get("Certificates"): + continue + + for certificate in listener["Certificates"]: + certificate_names.append(iam.get_name_from_arn(certificate["CertificateArn"])) + + return certificate_names + class AWSDestinationPlugin(DestinationPlugin): title = "AWS" @@ -344,6 +379,10 @@ class AWSDestinationPlugin(DestinationPlugin): def deploy(self, elb_name, account, region, certificate): pass + def clean(self, certificate, options, **kwargs): + account_number = self.get_option("accountNumber", options) + iam.delete_cert(certificate.name, account_number=account_number) + class S3DestinationPlugin(ExportDestinationPlugin): title = "AWS-S3" diff --git a/lemur/plugins/lemur_aws/tests/test_elb.py b/lemur/plugins/lemur_aws/tests/test_elb.py index 4571b87a..2b56a7c5 100644 --- a/lemur/plugins/lemur_aws/tests/test_elb.py +++ b/lemur/plugins/lemur_aws/tests/test_elb.py @@ -1,5 +1,5 @@ import boto3 -from moto import mock_sts, mock_elb +from moto import mock_sts, mock_ec2, mock_elb, mock_elbv2, mock_iam @mock_sts() @@ -27,3 +27,107 @@ def test_get_all_elbs(app, aws_credentials): elbs = get_all_elbs(account_number="123456789012", region="us-east-1") assert elbs + + +@mock_sts() +@mock_ec2 +@mock_elbv2() +@mock_iam +def test_create_elb_with_https_listener_miscellaneous(app, aws_credentials): + from lemur.plugins.lemur_aws import iam, elb + endpoint_name = "example-lbv2" + account_number = "123456789012" + region_ue1 = "us-east-1" + + client = boto3.client("elbv2", region_name="us-east-1") + ec2 = boto3.resource("ec2", region_name="us-east-1") + + # Create VPC + vpc = ec2.create_vpc(CidrBlock="172.28.7.0/24") + + # Create LB (elbv2) in above VPC + assert create_load_balancer(client, ec2, vpc.id, endpoint_name) + # Create target group + target_group_arn = create_target_group(client, vpc.id) + assert target_group_arn + + # Test get_load_balancer_arn_from_endpoint + lb_arn = elb.get_load_balancer_arn_from_endpoint(endpoint_name, + account_number=account_number, + region=region_ue1) + assert lb_arn + + # Test describe_listeners_v2 + listeners = elb.describe_listeners_v2(account_number=account_number, + region=region_ue1, + LoadBalancerArn=lb_arn) + assert listeners + assert not listeners["Listeners"] + + # Upload cert + response = iam.upload_cert("LemurTestCert", "testCert", "cert1", "cert2", + account_number=account_number) + assert response + cert_arn = response["ServerCertificateMetadata"]["Arn"] + assert cert_arn + + # Create https listener using above cert + listeners = client.create_listener( + LoadBalancerArn=lb_arn, + Protocol="HTTPS", + Port=443, + Certificates=[{"CertificateArn": cert_arn}], + DefaultActions=[{"Type": "forward", "TargetGroupArn": target_group_arn}], + ) + assert listeners + listener_arn = listeners["Listeners"][0]["ListenerArn"] + assert listener_arn + + assert listeners["Listeners"] + for listener in listeners["Listeners"]: + if listener["Port"] == 443: + assert listener["Certificates"] + assert cert_arn == listener["Certificates"][0]["CertificateArn"] + + # Test get_listener_arn_from_endpoint + assert listener_arn == elb.get_listener_arn_from_endpoint( + endpoint_name, + 443, + account_number=account_number, + region=region_ue1, + ) + + +@mock_sts() +@mock_elb() +def test_get_all_elbs_v2(): + from lemur.plugins.lemur_aws.elb import get_all_elbs_v2 + + elbs = get_all_elbs_v2(account_number="123456789012", + region="us-east-1") + assert elbs + + +def create_load_balancer(client, ec2, vpc_id, endpoint_name): + subnet1 = ec2.create_subnet( + VpcId=vpc_id, + CidrBlock="172.28.7.192/26", + AvailabilityZone="us-east-1a" + ) + + return client.create_load_balancer( + Name=endpoint_name, + Subnets=[ + subnet1.id, + ], + ) + + +def create_target_group(client, vpc_id): + response = client.create_target_group( + Name="a-target", + Protocol="HTTPS", + Port=443, + VpcId=vpc_id, + ) + return response.get("TargetGroups")[0]["TargetGroupArn"] diff --git a/lemur/static/app/angular/certificates/certificate/revoke.tpl.html b/lemur/static/app/angular/certificates/certificate/revoke.tpl.html index d91c7989..779d2ffd 100644 --- a/lemur/static/app/angular/certificates/certificate/revoke.tpl.html +++ b/lemur/static/app/angular/certificates/certificate/revoke.tpl.html @@ -26,9 +26,8 @@
-

Certificate cannot be revoked, it is associated with the following endpoints. Disassociate this - certificate - before revoking.

+

Certificate might be associated with the following endpoints. Disassociate this + certificate before revoking or continue if you've already done so.

From d2abe59e6ecf8d5aebd801dfb0fe95abd0058d34 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Wed, 2 Dec 2020 13:44:25 +0700 Subject: [PATCH 9/9] Use --no-emit-index-url instead of deprecated --no-index in pip-compile --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 3312a41d..e8d900b1 100644 --- a/Makefile +++ b/Makefile @@ -115,10 +115,10 @@ endif @echo "--> Updating Python requirements" pip install --upgrade pip pip install --upgrade pip-tools - pip-compile --output-file requirements.txt requirements.in -U --no-index - pip-compile --output-file requirements-docs.txt requirements-docs.in -U --no-index - pip-compile --output-file requirements-dev.txt requirements-dev.in -U --no-index - pip-compile --output-file requirements-tests.txt requirements-tests.in -U --no-index + pip-compile --output-file requirements.txt requirements.in -U --no-emit-index-url + pip-compile --output-file requirements-docs.txt requirements-docs.in -U --no-emit-index-url + pip-compile --output-file requirements-dev.txt requirements-dev.in -U --no-emit-index-url + pip-compile --output-file requirements-tests.txt requirements-tests.in -U --no-emit-index-url @echo "--> Done updating Python requirements" @echo "--> Removing python-ldap from requirements-docs.txt" grep -v "python-ldap" requirements-docs.txt > tempreqs && mv tempreqs requirements-docs.txt