From a141b8c5ea6c584bd767833f5ae8daebc0ef15c3 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Tue, 19 Jun 2018 16:27:58 -0700 Subject: [PATCH 1/5] Support concurrent issuance in Route53 for LetsEncrypt --- lemur/plugins/lemur_acme/plugin.py | 49 ++++++++++++++++++++++------- lemur/plugins/lemur_acme/route53.py | 29 ++++++++++++++--- 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index a3f9af00..4d150e76 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -54,15 +54,24 @@ def maybe_remove_wildcard(host): return host.replace("*.", "") -def start_dns_challenge(acme_client, account_number, host, dns_provider, order): +def maybe_add_extension(host, dns_provider_options): + if dns_provider_options and dns_provider_options.get("acme_challenge_extension"): + host = host + dns_provider_options.get("acme_challenge_extension") + return host + + +def start_dns_challenge(acme_client, account_number, host, dns_provider, order, dns_provider_options): current_app.logger.debug("Starting DNS challenge for {0}".format(host)) dns_challenges = find_dns_challenge(order.authorizations) change_ids = [] + host_to_validate = maybe_remove_wildcard(host) + host_to_validate = maybe_add_extension(host_to_validate, dns_provider_options) + for dns_challenge in find_dns_challenge(order.authorizations): change_id = dns_provider.create_txt_record( - dns_challenge.validation_domain_name(maybe_remove_wildcard(host)), + dns_challenge.validation_domain_name(host_to_validate), dns_challenge.validation(acme_client.client.net.key), account_number ) @@ -104,7 +113,13 @@ def request_certificate(acme_client, authorizations, csr, order): authorization_resource, _ = acme_client.poll(authz) deadline = datetime.datetime.now() + datetime.timedelta(seconds=90) - orderr = acme_client.finalize_order(order, deadline) + + try: + orderr = acme_client.finalize_order(order, deadline) + except: + current_app.logger.error("Unable to finalize ACME order: {}".format(order), exc_info=True) + raise + pem_certificate = OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, orderr.fullchain_pem)).decode() @@ -154,24 +169,27 @@ def get_domains(options): return domains -def get_authorizations(acme_client, order, order_info, dns_provider): +def get_authorizations(acme_client, order, order_info, dns_provider, dns_provider_options): authorizations = [] for domain in order_info.domains: - authz_record = start_dns_challenge(acme_client, order_info.account_number, domain, dns_provider, order) + authz_record = start_dns_challenge(acme_client, order_info.account_number, domain, dns_provider, order, + dns_provider_options) authorizations.append(authz_record) return authorizations -def finalize_authorizations(acme_client, account_number, dns_provider, authorizations): +def finalize_authorizations(acme_client, account_number, dns_provider, authorizations, dns_provider_options): for authz_record in authorizations: complete_dns_challenge(acme_client, account_number, authz_record, dns_provider) for authz_record in authorizations: dns_challenges = authz_record.dns_challenge + host_to_validate = maybe_remove_wildcard(authz_record.host) + host_to_validate = maybe_add_extension(host_to_validate, dns_provider_options) for dns_challenge in dns_challenges: dns_provider.delete_txt_record( authz_record.change_id, account_number, - dns_challenge.validation_domain_name(maybe_remove_wildcard(authz_record.host)), + dns_challenge.validation_domain_name(host_to_validate), dns_challenge.validation(acme_client.client.net.key) ) @@ -235,16 +253,17 @@ class ACMEIssuerPlugin(IssuerPlugin): acme_client, registration = setup_acme_client(pending_cert.authority) order_info = authorization_service.get(pending_cert.external_id) dns_provider = dns_provider_service.get(pending_cert.dns_provider_id) + dns_provider_options = dns_provider.options dns_provider_type = self.get_dns_provider(dns_provider.provider_type) try: authorizations = get_authorizations( - acme_client, order_info.account_number, order_info.domains, dns_provider_type) + acme_client, order_info.account_number, order_info.domains, dns_provider_type, dns_provider_options) except ClientError: current_app.logger.error("Unable to resolve pending cert: {}".format(pending_cert.name), exc_info=True) return False authorizations = finalize_authorizations( - acme_client, order_info.account_number, dns_provider_type, authorizations) + acme_client, order_info.account_number, dns_provider_type, authorizations, dns_provider_options) pem_certificate, pem_certificate_chain = request_certificate(acme_client, authorizations, pending_cert.csr) cert = { 'body': "\n".join(str(pem_certificate).splitlines()), @@ -261,6 +280,7 @@ class ACMEIssuerPlugin(IssuerPlugin): acme_client, registration = setup_acme_client(pending_cert.authority) order_info = authorization_service.get(pending_cert.external_id) dns_provider = dns_provider_service.get(pending_cert.dns_provider_id) + dns_provider_options = dns_provider.options dns_provider_type = self.get_dns_provider(dns_provider.provider_type) try: order = acme_client.new_order(pending_cert.csr) @@ -268,7 +288,8 @@ class ACMEIssuerPlugin(IssuerPlugin): raise Exception("The currently selected ACME CA endpoint does" " not support issuing wildcard certificates.") - authorizations = get_authorizations(acme_client, order, order_info, dns_provider_type) + authorizations = get_authorizations(acme_client, order, order_info, dns_provider_type, + dns_provider_options) pending.append({ "acme_client": acme_client, @@ -277,6 +298,7 @@ class ACMEIssuerPlugin(IssuerPlugin): "authorizations": authorizations, "pending_cert": pending_cert, "order": order, + "dns_provider_options": dns_provider_options, }) except (ClientError, ValueError, Exception): current_app.logger.error("Unable to resolve pending cert: {}".format(pending_cert), exc_info=True) @@ -292,6 +314,7 @@ class ACMEIssuerPlugin(IssuerPlugin): entry["account_number"], entry["dns_provider_type"], entry["authorizations"], + entry["dns_provider_options"], ) pem_certificate, pem_certificate_chain = request_certificate( entry["acme_client"], @@ -329,6 +352,7 @@ class ACMEIssuerPlugin(IssuerPlugin): create_immediately = issuer_options.get('create_immediately', False) acme_client, registration = setup_acme_client(authority) dns_provider = issuer_options.get('dns_provider') + dns_provider_options = dns_provider.options if not dns_provider: raise InvalidConfiguration("DNS Provider setting is required for ACME certificates.") credentials = json.loads(dns_provider.credentials) @@ -354,8 +378,9 @@ class ACMEIssuerPlugin(IssuerPlugin): # Return id of the DNS Authorization return None, None, dns_authorization.id - authorizations = get_authorizations(acme_client, account_number, domains, dns_provider_type) - finalize_authorizations(acme_client, account_number, dns_provider_type, authorizations) + authorizations = get_authorizations(acme_client, account_number, domains, dns_provider_type, + dns_provider_options) + finalize_authorizations(acme_client, account_number, dns_provider_type, authorizations, dns_provider_options) pem_certificate, pem_certificate_chain = request_certificate(acme_client, authorizations, csr) # TODO add external ID (if possible) return pem_certificate, pem_certificate_chain, None diff --git a/lemur/plugins/lemur_acme/route53.py b/lemur/plugins/lemur_acme/route53.py index b55d215b..989e8dd2 100644 --- a/lemur/plugins/lemur_acme/route53.py +++ b/lemur/plugins/lemur_acme/route53.py @@ -33,6 +33,29 @@ def find_zone_id(domain, client=None): @sts_client('route53') def change_txt_record(action, zone_id, domain, value, client=None): + current_txt_records = [] + try: + current_txt_records = client.list_resource_record_sets( + HostedZoneId=zone_id, + StartRecordName=domain, + StartRecordType='TXT', + MaxItems="1")["ResourceRecordSets"][0]["ResourceRecords"] + except Exception as e: + # Current Resource Record does not exist + if "NoSuchHostedZone" not in str(type(e)): + raise + # For some reason TXT records need to be + # manually quoted. + current_txt_records.append({"Value": '"{}"'.format(value)}) + + if action == "DELETE" and len(current_txt_records) > 1: + # If we want to delete one record out of many, we'll update the record to not include the deleted value instead. + # This allows us to support concurrent issuance. + current_txt_records = [ + record for record in current_txt_records if not (record.get('Value') == '"{}"'.format(value)) + ] + action = "UPSERT" + response = client.change_resource_record_sets( HostedZoneId=zone_id, ChangeBatch={ @@ -43,11 +66,7 @@ def change_txt_record(action, zone_id, domain, value, client=None): "Name": domain, "Type": "TXT", "TTL": 300, - "ResourceRecords": [ - # For some reason TXT records need to be - # manually quoted. - {"Value": '"{}"'.format(value)} - ], + "ResourceRecords": current_txt_records, } } ] From 665a0bcffe7f1ce29e6cdc2ec5f92570f17857f5 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Tue, 19 Jun 2018 16:38:05 -0700 Subject: [PATCH 2/5] Update requirements --- requirements-dev.txt | 14 +++++++------- requirements-docs.txt | 8 ++++---- requirements-tests.txt | 18 +++++++++--------- requirements.txt | 21 +++++++++++---------- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 0fd765d0..4f6d3603 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -5,26 +5,26 @@ # pip-compile --no-index --output-file requirements-dev.txt requirements-dev.in # aspy.yaml==1.1.1 # via pre-commit -cached-property==1.4.2 # via pre-commit +cached-property==1.4.3 # via pre-commit certifi==2018.4.16 # via requests cfgv==1.1.0 # via pre-commit chardet==3.0.4 # via requests flake8==3.5.0 -identify==1.0.18 # via pre-commit -idna==2.6 # via requests +identify==1.1.0 # via pre-commit +idna==2.7 # via requests invoke==1.0.0 mccabe==0.6.1 # via flake8 -nodeenv==1.3.0 +nodeenv==1.3.1 pkginfo==1.4.2 # via twine -pre-commit==1.10.1 +pre-commit==1.10.2 pycodestyle==2.3.1 # via flake8 pyflakes==1.6.0 # via flake8 pyyaml==3.12 # via aspy.yaml, pre-commit requests-toolbelt==0.8.0 # via twine -requests==2.18.4 # via requests-toolbelt, twine +requests==2.19.1 # via requests-toolbelt, twine six==1.11.0 # via cfgv, pre-commit toml==0.9.4 # via pre-commit tqdm==4.23.4 # via twine twine==1.11.0 -urllib3==1.22 # via requests +urllib3==1.23 # via requests virtualenv==16.0.0 # via pre-commit diff --git a/requirements-docs.txt b/requirements-docs.txt index 4e96643b..b4c12d4d 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -5,7 +5,7 @@ # pip-compile --no-index --output-file requirements-docs.txt requirements-docs.in # acme==0.24.0 -alabaster==0.7.10 # via sphinx +alabaster==0.7.11 # via sphinx alembic-autogenerate-enums==0.0.2 alembic==0.9.9 aniso8601==3.0.0 @@ -15,8 +15,8 @@ asyncpool==1.0 babel==2.6.0 # via sphinx bcrypt==3.1.4 blinker==1.4 -boto3==1.7.31 -botocore==1.10.31 +boto3==1.7.32 +botocore==1.10.32 certifi==2018.4.16 cffi==1.11.5 click==6.7 @@ -76,7 +76,7 @@ retrying==1.3.3 s3transfer==0.1.13 six==1.11.0 snowballstemmer==1.2.1 # via sphinx -sphinx-rtd-theme==0.3.1 +sphinx-rtd-theme==0.4.0 sphinx==1.7.5 sphinxcontrib-httpdomain==1.6.1 sphinxcontrib-websupport==1.1.0 # via sphinx diff --git a/requirements-tests.txt b/requirements-tests.txt index bca2a199..4e19f638 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -8,9 +8,9 @@ asn1crypto==0.24.0 # via cryptography atomicwrites==1.1.5 # via pytest attrs==18.1.0 # via pytest aws-xray-sdk==0.95 # via moto -boto3==1.7.32 # via moto +boto3==1.7.41 # via moto boto==2.48.0 # via moto -botocore==1.10.32 # via boto3, moto, s3transfer +botocore==1.10.41 # via boto3, moto, s3transfer certifi==2018.4.16 # via requests cffi==1.11.5 # via cryptography chardet==3.0.4 # via requests @@ -18,14 +18,14 @@ click==6.7 # via flask cookies==2.2.1 # via moto, responses coverage==4.5.1 cryptography==2.2.2 # via moto -docker-pycreds==0.2.3 # via docker -docker==3.3.0 # via moto +docker-pycreds==0.3.0 # via docker +docker==3.4.0 # via moto docutils==0.14 # via botocore factory-boy==2.11.1 -faker==0.8.15 +faker==0.8.16 flask==1.0.2 # via pytest-flask freezegun==0.3.10 -idna==2.6 # via cryptography, requests +idna==2.7 # via cryptography, requests itsdangerous==0.24 # via flask jinja2==2.10 # via flask, moto jmespath==0.9.3 # via boto3, botocore @@ -36,7 +36,7 @@ mock==2.0.0 # via moto more-itertools==4.2.0 # via pytest moto==1.3.3 nose==1.3.7 -pbr==4.0.3 # via mock +pbr==4.0.4 # via mock pluggy==0.6.0 # via pytest py==1.5.3 # via pytest pyaml==17.12.1 # via moto @@ -49,12 +49,12 @@ python-dateutil==2.6.1 # via botocore, faker, freezegun, moto pytz==2018.4 # via moto pyyaml==3.12 # via pyaml requests-mock==1.5.0 -requests==2.18.4 # via aws-xray-sdk, docker, moto, requests-mock, responses +requests==2.19.1 # via aws-xray-sdk, docker, moto, requests-mock, responses responses==0.9.0 # via moto s3transfer==0.1.13 # via boto3 six==1.11.0 # via cryptography, docker, docker-pycreds, faker, freezegun, mock, more-itertools, moto, pytest, python-dateutil, requests-mock, responses, websocket-client text-unidecode==1.2 # via faker -urllib3==1.22 # via requests +urllib3==1.23 # via requests websocket-client==0.48.0 # via docker werkzeug==0.14.1 # via flask, moto, pytest-flask wrapt==1.10.11 # via aws-xray-sdk diff --git a/requirements.txt b/requirements.txt index 9e8ec438..46839bda 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,17 +4,17 @@ # # pip-compile --no-index --output-file requirements.txt requirements.in # -acme==0.24.0 +acme==0.25.1 alembic-autogenerate-enums==0.0.2 alembic==0.9.9 # via flask-migrate -aniso8601==3.0.0 # via flask-restful +aniso8601==3.0.2 # via flask-restful arrow==0.12.1 asn1crypto==0.24.0 # via cryptography asyncpool==1.0 bcrypt==3.1.4 # via flask-bcrypt, paramiko blinker==1.4 # via flask-mail, flask-principal, raven -boto3==1.7.32 -botocore==1.10.32 # via boto3, s3transfer +boto3==1.7.41 +botocore==1.10.41 # via boto3, s3transfer certifi==2018.4.16 cffi==1.11.5 # via bcrypt, cryptography, pynacl click==6.7 # via flask @@ -25,7 +25,7 @@ dnspython==1.15.0 # via dnspython3 docutils==0.14 # via botocore dyn==1.8.1 flask-bcrypt==0.7.1 -flask-cors==3.0.4 +flask-cors==3.0.6 flask-mail==0.9.1 flask-migrate==2.1.1 flask-principal==0.4.0 @@ -35,7 +35,7 @@ flask-sqlalchemy==2.3.2 flask==0.12 future==0.16.0 gunicorn==19.8.1 -idna==2.6 # via cryptography +idna==2.7 # via cryptography inflection==0.3.1 itsdangerous==0.24 # via flask jinja2==2.10 @@ -50,22 +50,23 @@ marshmallow==2.15.3 mock==2.0.0 # via acme ndg-httpsclient==0.5.0 paramiko==2.4.1 -pbr==4.0.3 # via mock +pbr==4.0.4 # via mock pem==17.1.0 -psycopg2==2.7.4 +psycopg2==2.7.5 pyasn1-modules==0.2.1 # via python-ldap pyasn1==0.4.3 # via ndg-httpsclient, paramiko, pyasn1-modules, python-ldap, requests pycparser==2.18 # via cffi pyjwt==1.6.4 pynacl==1.2.1 # via paramiko pyopenssl==17.2.0 -pyrfc3339==1.0 # via acme +pyrfc3339==1.1 # via acme python-dateutil==2.7.3 # via alembic, arrow, botocore python-editor==1.0.3 # via alembic python-ldap==3.1.0 pytz==2018.4 # via acme, flask-restful, pyrfc3339 pyyaml==3.12 # via cloudflare raven[flask]==6.9.0 +requests-toolbelt==0.8.0 # via acme requests[security]==2.11.1 retrying==1.3.3 s3transfer==0.1.13 # via boto3 @@ -73,6 +74,6 @@ six==1.11.0 sqlalchemy-utils==0.33.3 sqlalchemy==1.2.8 # via alembic, flask-sqlalchemy, marshmallow-sqlalchemy, sqlalchemy-utils tabulate==0.8.2 -tld==0.7.10 +tld==0.9 werkzeug==0.14.1 # via flask xmltodict==0.11.0 From 2d33d3e2b8b335c2343d7ee224c97cf790783147 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Tue, 19 Jun 2018 20:32:11 -0700 Subject: [PATCH 3/5] lint --- lemur/plugins/lemur_acme/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index 43f5244b..977b6ad8 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -130,7 +130,7 @@ def request_certificate(acme_client, authorizations, csr, order): def setup_acme_client(authority): - if not authority.options: + if not authority.options: raise InvalidAuthority("Invalid authority. Options not set") options = {} From dda7f54a1679aa82724bae2bfe9dc2ff945475d2 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Tue, 19 Jun 2018 20:58:00 -0700 Subject: [PATCH 4/5] lint --- lemur/plugins/lemur_acme/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index 977b6ad8..43f5244b 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -130,7 +130,7 @@ def request_certificate(acme_client, authorizations, csr, order): def setup_acme_client(authority): - if not authority.options: + if not authority.options: raise InvalidAuthority("Invalid authority. Options not set") options = {} From 3efc709e030e433e4d54392c395bb353b0525cca Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Tue, 19 Jun 2018 21:16:35 -0700 Subject: [PATCH 5/5] tests --- lemur/plugins/lemur_acme/tests/test_acme.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lemur/plugins/lemur_acme/tests/test_acme.py b/lemur/plugins/lemur_acme/tests/test_acme.py index c80079af..f8b58f46 100644 --- a/lemur/plugins/lemur_acme/tests/test_acme.py +++ b/lemur/plugins/lemur_acme/tests/test_acme.py @@ -52,7 +52,7 @@ class TestAcme(unittest.TestCase): iterable = mock_find_dns_challenge.return_value iterator = iter(values) iterable.__iter__.return_value = iterator - result = plugin.start_dns_challenge(mock_acme, "accountid", "host", mock_dns_provider, mock_order) + result = plugin.start_dns_challenge(mock_acme, "accountid", "host", mock_dns_provider, mock_order, {}) self.assertEqual(type(result), plugin.AuthorizationRecord) @patch('acme.client.Client') @@ -171,7 +171,7 @@ class TestAcme(unittest.TestCase): mock_order_info = Mock() mock_order_info.account_number = 1 mock_order_info.domains = ["test.fakedomain.net"] - result = plugin.get_authorizations("acme_client", mock_order, mock_order_info, "dns_provider") + result = plugin.get_authorizations("acme_client", mock_order, mock_order_info, "dns_provider", {}) self.assertEqual(result, ["test"]) @patch('lemur.plugins.lemur_acme.plugin.complete_dns_challenge', return_value="test") @@ -187,7 +187,7 @@ class TestAcme(unittest.TestCase): mock_dns_provider.delete_txt_record = Mock() mock_acme_client = Mock() - result = plugin.finalize_authorizations(mock_acme_client, "account_number", mock_dns_provider, mock_authz) + result = plugin.finalize_authorizations(mock_acme_client, "account_number", mock_dns_provider, mock_authz, {}) self.assertEqual(result, mock_authz) @patch('lemur.plugins.lemur_acme.plugin.current_app')