From 409b499217c50f6f558c103be07d90ca30c1cfc2 Mon Sep 17 00:00:00 2001 From: jenkins-x-bot Date: Sun, 12 Jan 2020 01:25:22 +0200 Subject: [PATCH 01/22] added kubernetes auth for vault --- lemur/plugins/lemur_vault_dest/plugin.py | 60 ++++++++++++++++++------ 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/lemur/plugins/lemur_vault_dest/plugin.py b/lemur/plugins/lemur_vault_dest/plugin.py index e1715592..47206708 100755 --- a/lemur/plugins/lemur_vault_dest/plugin.py +++ b/lemur/plugins/lemur_vault_dest/plugin.py @@ -50,11 +50,19 @@ class VaultSourcePlugin(SourcePlugin): "helpMessage": "Version of the Vault KV API to use", }, { - "name": "vaultAuthTokenFile", + "name": "authenticationMethod", + "type": "select", + "value": "token", + "available": ["token", "kubernetes"], + "required": True, + "helpMessage": "Authentication method to use", + }, + { + "name": "tokenFile/VaultRole", "type": "str", "required": True, - "validation": "(/[^/]+)+", - "helpMessage": "Must be a valid file path!", + "validation": "^([a-zA-Z0-9/._-]+/?)+$", + "helpMessage": "Must be vaild file path for token based auth and valid role if k8s based auth", }, { "name": "vaultMount", @@ -85,7 +93,8 @@ class VaultSourcePlugin(SourcePlugin): cert = [] body = "" url = self.get_option("vaultUrl", options) - token_file = self.get_option("vaultAuthTokenFile", options) + auth_method = self.get_option("authenticationMethod", options) + auth_key = self.get_option("tokenFile/vaultRole", options) mount = self.get_option("vaultMount", options) path = self.get_option("vaultPath", options) obj_name = self.get_option("objectName", options) @@ -93,10 +102,17 @@ class VaultSourcePlugin(SourcePlugin): cert_filter = "-----BEGIN CERTIFICATE-----" cert_delimiter = "-----END CERTIFICATE-----" - with open(token_file, "r") as tfile: - token = tfile.readline().rstrip("\n") + client = hvac.Client(url=url) + if auth_method == 'token': + with open(auth_key, "r") as tfile: + token = tfile.readline().rstrip("\n") + client.token = token + + if auth_method == 'kubernetes': + f = open('/var/run/secrets/kubernetes.io/serviceaccount/token') + jwt = f.read() + client.auth_kubernetes(auth_key, jwt) - client = hvac.Client(url=url, token=token) client.secrets.kv.default_kv_version = api_version path = "{0}/{1}".format(path, obj_name) @@ -160,11 +176,19 @@ class VaultDestinationPlugin(DestinationPlugin): "helpMessage": "Version of the Vault KV API to use", }, { - "name": "vaultAuthTokenFile", + "name": "authenticationMethod", + "type": "select", + "value": "token", + "available": ["token", "kubernetes"], + "required": True, + "helpMessage": "Authentication method to use", + }, + { + "name": "tokenFile/VaultRole", "type": "str", "required": True, - "validation": "(/[^/]+)+", - "helpMessage": "Must be a valid file path!", + "validation": "^([a-zA-Z0-9/._-]+/?)+$", + "helpMessage": "Must be vaild file path for token based auth and valid role if k8s based auth", }, { "name": "vaultMount", @@ -219,7 +243,8 @@ class VaultDestinationPlugin(DestinationPlugin): cname = common_name(parse_certificate(body)) url = self.get_option("vaultUrl", options) - token_file = self.get_option("vaultAuthTokenFile", options) + auth_method = self.get_option("authenticationMethod", options) + auth_key = self.get_option("tokenFile/vaultRole", options) mount = self.get_option("vaultMount", options) path = self.get_option("vaultPath", options) bundle = self.get_option("bundleChain", options) @@ -245,10 +270,17 @@ class VaultDestinationPlugin(DestinationPlugin): exc_info=True, ) - with open(token_file, "r") as tfile: - token = tfile.readline().rstrip("\n") + client = hvac.Client(url=url) + if auth_method == 'token': + with open(auth_key, "r") as tfile: + token = tfile.readline().rstrip("\n") + client.token = token + + if auth_method == 'kubernetes': + f = open('/var/run/secrets/kubernetes.io/serviceaccount/token') + jwt = f.read() + client.auth_kubernetes(auth_key, jwt) - client = hvac.Client(url=url, token=token) client.secrets.kv.default_kv_version = api_version if obj_name: From cad56c813ee653a5712e9860f19c354a8767d99d Mon Sep 17 00:00:00 2001 From: jenkins-x-bot Date: Sun, 12 Jan 2020 01:51:48 +0200 Subject: [PATCH 02/22] fixed lint error --- lemur/plugins/lemur_vault_dest/plugin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lemur/plugins/lemur_vault_dest/plugin.py b/lemur/plugins/lemur_vault_dest/plugin.py index 47206708..cb821a36 100755 --- a/lemur/plugins/lemur_vault_dest/plugin.py +++ b/lemur/plugins/lemur_vault_dest/plugin.py @@ -107,7 +107,7 @@ class VaultSourcePlugin(SourcePlugin): with open(auth_key, "r") as tfile: token = tfile.readline().rstrip("\n") client.token = token - + if auth_method == 'kubernetes': f = open('/var/run/secrets/kubernetes.io/serviceaccount/token') jwt = f.read() @@ -275,7 +275,7 @@ class VaultDestinationPlugin(DestinationPlugin): with open(auth_key, "r") as tfile: token = tfile.readline().rstrip("\n") client.token = token - + if auth_method == 'kubernetes': f = open('/var/run/secrets/kubernetes.io/serviceaccount/token') jwt = f.read() From 8d957f22af40b741829290cee66a45083cb6a8f0 Mon Sep 17 00:00:00 2001 From: jenkins-x-bot Date: Mon, 13 Jan 2020 22:46:34 +0200 Subject: [PATCH 03/22] changed file handling --- lemur/plugins/lemur_vault_dest/plugin.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lemur/plugins/lemur_vault_dest/plugin.py b/lemur/plugins/lemur_vault_dest/plugin.py index cb821a36..d401387b 100755 --- a/lemur/plugins/lemur_vault_dest/plugin.py +++ b/lemur/plugins/lemur_vault_dest/plugin.py @@ -109,8 +109,9 @@ class VaultSourcePlugin(SourcePlugin): client.token = token if auth_method == 'kubernetes': - f = open('/var/run/secrets/kubernetes.io/serviceaccount/token') - jwt = f.read() + token_path = '/var/run/secrets/kubernetes.io/serviceaccount/token' + with open(token_path, 'r') as f: + jwt = f.read() client.auth_kubernetes(auth_key, jwt) client.secrets.kv.default_kv_version = api_version @@ -277,8 +278,9 @@ class VaultDestinationPlugin(DestinationPlugin): client.token = token if auth_method == 'kubernetes': - f = open('/var/run/secrets/kubernetes.io/serviceaccount/token') - jwt = f.read() + token_path = '/var/run/secrets/kubernetes.io/serviceaccount/token' + with open(token_path, 'r') as f: + jwt = f.read() client.auth_kubernetes(auth_key, jwt) client.secrets.kv.default_kv_version = api_version From cd7d9aee55839c8806c72ed403a03c41ef4cbdec Mon Sep 17 00:00:00 2001 From: jenkins-x-bot Date: Mon, 13 Jan 2020 23:09:58 +0200 Subject: [PATCH 04/22] fixed lint error --- lemur/plugins/lemur_vault_dest/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_vault_dest/plugin.py b/lemur/plugins/lemur_vault_dest/plugin.py index d401387b..41b9c252 100755 --- a/lemur/plugins/lemur_vault_dest/plugin.py +++ b/lemur/plugins/lemur_vault_dest/plugin.py @@ -109,7 +109,7 @@ class VaultSourcePlugin(SourcePlugin): client.token = token if auth_method == 'kubernetes': - token_path = '/var/run/secrets/kubernetes.io/serviceaccount/token' + token_path = '/var/run/secrets/kubernetes.io/serviceaccount/token' with open(token_path, 'r') as f: jwt = f.read() client.auth_kubernetes(auth_key, jwt) From 3080a9527c8ea539da8f767d0a735e6b53e8e0fe Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Fri, 17 Jan 2020 18:29:37 -0800 Subject: [PATCH 05/22] adding PowerDNS get_zones functionality and unit tests --- lemur/plugins/lemur_acme/plugin.py | 4 +- lemur/plugins/lemur_acme/powerdns.py | 108 +++++++++++++++ lemur/plugins/lemur_acme/tests/test_acme.py | 12 +- .../plugins/lemur_acme/tests/test_powerdns.py | 124 ++++++++++++++++++ 4 files changed, 241 insertions(+), 7 deletions(-) create mode 100644 lemur/plugins/lemur_acme/powerdns.py create mode 100644 lemur/plugins/lemur_acme/tests/test_powerdns.py diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index e38870d8..93628905 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -31,7 +31,7 @@ from lemur.exceptions import InvalidAuthority, InvalidConfiguration, UnknownProv from lemur.extensions import metrics, sentry from lemur.plugins import lemur_acme as acme from lemur.plugins.bases import IssuerPlugin -from lemur.plugins.lemur_acme import cloudflare, dyn, route53, ultradns +from lemur.plugins.lemur_acme import cloudflare, dyn, route53, ultradns, powerdns from retrying import retry @@ -377,6 +377,7 @@ class AcmeHandler(object): "dyn": dyn, "route53": route53, "ultradns": ultradns, + # "powerdns": powerdns, } provider = provider_types.get(type) if not provider: @@ -436,6 +437,7 @@ class ACMEIssuerPlugin(IssuerPlugin): "dyn": dyn, "route53": route53, "ultradns": ultradns, + # "powerdns": powerdns, } provider = provider_types.get(type) if not provider: diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py new file mode 100644 index 00000000..2f80e54f --- /dev/null +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -0,0 +1,108 @@ +import time +import requests +import json +import sys + +import dns +import dns.exception +import dns.name +import dns.query +import dns.resolver + +from flask import current_app +from lemur.extensions import metrics, sentry + +class Zone: + """ This class implements a PowerDNS zone in JSON. """ + + def __init__(self, _data): + self._data = _data + + @property + def id(self): + """ Zone id, has a trailing "." at the end, which we manually remove. """ + return self._data["id"][:-1] + + @property + def name(self): + """ Zone name, has a trailing "." at the end, which we manually remove. """ + return self._data["name"][:-1] + + @property + def kind(self): + """ Indicates whether the zone is setup as a PRIMARY or SECONDARY """ + return self._data["kind"] + + +def _generate_header(): + """Function to generate the header for a request using the PowerDNS API Key""" + + api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME", "") + api_key = current_app.config.get("ACME_POWERDNS_APIKEY", "") + return {api_key_name: api_key} + + +def _get(path, params=None): + """ + Function to execute a GET request on the given URL (base_uri + path) with given params + Returns JSON response object + """ + base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") + resp = requests.get( + f"{base_uri}{path}", + headers=_generate_header(), + params=params, + verify=True, + ) + resp.raise_for_status() + return resp.json() + + +def get_zones(account_number): + """Get zones from the PowerDNS""" + server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") + path = f"/api/v1/servers/{server_id}/zones" + zones = [] + for elem in _get(path): + zone = Zone(elem) + if zone.kind == 'Master': + zones.append(zone.name) + return zones + +def create_txt_record(domain, token, account_number): + """ + Create a TXT record for the given domain. + + The part of the domain that matches with the zone becomes the zone name. + The remainder becomes the owner name (referred to as node name here) + Example: Let's say we have a zone named "exmaple.com" in PowerDNS and we + get a request to create a cert for lemur.example.com + Domain - _acme-challenge.lemur.example.com + Matching zone - example.com + Owner name - _acme-challenge.lemur + """ + pass + +def wait_for_dns_change(change_id, account_number=None): + """ + Waits and checks if the DNS changes have propagated or not. + + First check the domains authoritative server. Once this succeeds, + we ask a public DNS server (Google <8.8.8.8> in our case). + """ + pass + +def delete_txt_record(change_id, account_number, domain, token): + """ + Delete the TXT record that was created in the create_txt_record() function. + + UltraDNS handles records differently compared to Dyn. It creates an RRSet + which is a set of records of the same type and owner. This means + that while deleting the record, we cannot delete any individual record from + the RRSet. Instead, we have to delete the entire RRSet. If multiple certs are + being created for the same domain at the same time, the challenge TXT records + that are created will be added under the same RRSet. If the RRSet had more + than 1 record, then we create a new RRSet on UltraDNS minus the record that + has to be deleted. + """ + pass diff --git a/lemur/plugins/lemur_acme/tests/test_acme.py b/lemur/plugins/lemur_acme/tests/test_acme.py index 2f9dd719..04997ace 100644 --- a/lemur/plugins/lemur_acme/tests/test_acme.py +++ b/lemur/plugins/lemur_acme/tests/test_acme.py @@ -364,7 +364,7 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.ultradns.requests") @patch("lemur.plugins.lemur_acme.ultradns.current_app") - def test_get_ultradns_token(self, mock_current_app, mock_requests): + def test_ultradns_get_token(self, mock_current_app, mock_requests): # ret_val = json.dumps({"access_token": "access"}) the_response = Response() the_response._content = b'{"access_token": "access"}' @@ -374,7 +374,7 @@ class TestAcme(unittest.TestCase): self.assertTrue(len(result) > 0) @patch("lemur.plugins.lemur_acme.ultradns.current_app") - def test_create_txt_record(self, mock_current_app): + def test_ultradns_create_txt_record(self, mock_current_app): domain = "_acme_challenge.test.example.com" zone = "test.example.com" token = "ABCDEFGHIJ" @@ -395,7 +395,7 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.ultradns.current_app") @patch("lemur.extensions.metrics") - def test_delete_txt_record(self, mock_metrics, mock_current_app): + def test_ultradns_delete_txt_record(self, mock_metrics, mock_current_app): domain = "_acme_challenge.test.example.com" zone = "test.example.com" token = "ABCDEFGHIJ" @@ -418,7 +418,7 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.ultradns.current_app") @patch("lemur.extensions.metrics") - def test_wait_for_dns_change(self, mock_metrics, mock_current_app): + def test_ultradns_wait_for_dns_change(self, mock_metrics, mock_current_app): ultradns._has_dns_propagated = Mock(return_value=True) nameserver = "1.1.1.1" ultradns.get_authoritative_nameserver = Mock(return_value=nameserver) @@ -437,7 +437,7 @@ class TestAcme(unittest.TestCase): } mock_current_app.logger.debug.assert_called_with(log_data) - def test_get_zone_name(self): + def test_ultradns_get_zone_name(self): zones = ['example.com', 'test.example.com'] zone = "test.example.com" domain = "_acme-challenge.test.example.com" @@ -446,7 +446,7 @@ class TestAcme(unittest.TestCase): result = ultradns.get_zone_name(domain, account_number) self.assertEqual(result, zone) - def test_get_zones(self): + def test_ultradns_get_zones(self): account_number = "1234567890" path = "a/b/c" zones = ['example.com', 'test.example.com'] diff --git a/lemur/plugins/lemur_acme/tests/test_powerdns.py b/lemur/plugins/lemur_acme/tests/test_powerdns.py new file mode 100644 index 00000000..f39ef3c5 --- /dev/null +++ b/lemur/plugins/lemur_acme/tests/test_powerdns.py @@ -0,0 +1,124 @@ +import unittest +from requests.models import Response + +from mock import MagicMock, Mock, patch + +from lemur.plugins.lemur_acme import plugin, powerdns + +class TestPowerdns(unittest.TestCase): + @patch("lemur.plugins.lemur_acme.plugin.dns_provider_service") + def setUp(self, mock_dns_provider_service): + self.ACMEIssuerPlugin = plugin.ACMEIssuerPlugin() + self.acme = plugin.AcmeHandler() + mock_dns_provider = Mock() + mock_dns_provider.name = "powerdns" + mock_dns_provider.credentials = "{}" + mock_dns_provider.provider_type = "powerdns" + self.acme.dns_providers_for_domain = { + "www.test.com": [mock_dns_provider], + "test.fakedomain.net": [mock_dns_provider], + } + + @patch("lemur.plugins.lemur_acme.powerdns.requests") + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + def test_powerdns_get_token(self, mock_current_app, mock_requests): + # ret_val = json.dumps({"access_token": "access"}) + the_response = Response() + the_response._content = b'{"access_token": "access"}' + mock_requests.post = Mock(return_value=the_response) + mock_current_app.config.get = Mock(return_value="Test") + result = powerdns.get_powerdns_token() + self.assertTrue(len(result) > 0) + + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + def test_powerdns_create_txt_record(self, mock_current_app): + domain = "_acme_challenge.test.example.com" + zone = "test.example.com" + token = "ABCDEFGHIJ" + account_number = "1234567890" + change_id = (domain, token) + powerdns.get_zone_name = Mock(return_value=zone) + mock_current_app.logger.debug = Mock() + powerdns._post = Mock() + log_data = { + "function": "create_txt_record", + "fqdn": domain, + "token": token, + "message": "TXT record created" + } + result = powerdns.create_txt_record(domain, token, account_number) + mock_current_app.logger.debug.assert_called_with(log_data) + self.assertEqual(result, change_id) + + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + @patch("lemur.extensions.metrics") + def test_powerdns_delete_txt_record(self, mock_metrics, mock_current_app): + domain = "_acme_challenge.test.example.com" + zone = "test.example.com" + token = "ABCDEFGHIJ" + account_number = "1234567890" + change_id = (domain, token) + mock_current_app.logger.debug = Mock() + powerdns.get_zone_name = Mock(return_value=zone) + powerdns._post = Mock() + powerdns._get = Mock() + powerdns._get.return_value = {'zoneName': 'test.example.com.com', + 'rrSets': [{'ownerName': '_acme-challenge.test.example.com.', + 'rrtype': 'TXT (16)', 'ttl': 5, 'rdata': ['ABCDEFGHIJ']}], + 'queryInfo': {'sort': 'OWNER', 'reverse': False, 'limit': 100}, + 'resultInfo': {'totalCount': 1, 'offset': 0, 'returnedCount': 1}} + powerdns._delete = Mock() + mock_metrics.send = Mock() + powerdns.delete_txt_record(change_id, account_number, domain, token) + mock_current_app.logger.debug.assert_not_called() + mock_metrics.send.assert_not_called() + + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + @patch("lemur.extensions.metrics") + def test_powerdns_wait_for_dns_change(self, mock_metrics, mock_current_app): + powerdns._has_dns_propagated = Mock(return_value=True) + nameserver = "1.1.1.1" + powerdns.get_authoritative_nameserver = Mock(return_value=nameserver) + mock_metrics.send = Mock() + domain = "_acme-challenge.test.example.com" + token = "ABCDEFGHIJ" + change_id = (domain, token) + mock_current_app.logger.debug = Mock() + powerdns.wait_for_dns_change(change_id) + # mock_metrics.send.assert_not_called() + log_data = { + "function": "wait_for_dns_change", + "fqdn": domain, + "status": True, + "message": "Record status on Public DNS" + } + mock_current_app.logger.debug.assert_called_with(log_data) + + def test_powerdns_get_zone_name(self): + zones = ['example.com', 'test.example.com'] + zone = "test.example.com" + domain = "_acme-challenge.test.example.com" + account_number = "1234567890" + powerdns.get_zones = Mock(return_value=zones) + result = powerdns.get_zone_name(domain, account_number) + self.assertEqual(result, zone) + + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + def test_powerdns_get_zones(self, mock_current_app): + account_number = "1234567890" + path = "a/b/c" + zones = ['example.com', 'test.example.com'] + get_response = [{'account': '', 'dnssec': 'False', 'id': 'example.com.', 'kind': 'Master', 'last_check': 0, 'masters': [], + 'name': 'example.com.', 'notified_serial': '2019111907', 'serial': '2019111907', + 'url': '/api/v1/servers/localhost/zones/example.com.'}, + {'account': '', 'dnssec': 'False', 'id': 'bad.example.com.', 'kind': 'Secondary', 'last_check': 0, 'masters': [], + 'name': 'bad.example.com.', 'notified_serial': '2018053104', 'serial': '2018053104', + 'url': '/api/v1/servers/localhost/zones/bad.example.com.'}, + {'account': '', 'dnssec': 'False', 'id': 'test.example.com.', 'kind': 'Master', 'last_check': 0, + 'masters': [], 'name': 'test.example.com.', 'notified_serial': '2019112501', 'serial': '2019112501', + 'url': '/api/v1/servers/localhost/zones/test.example.com.'}] + powerdns._get = Mock(path) + powerdns._get.side_effect = [get_response] + mock_current_app.config.get = Mock(return_value="localhost") + result = powerdns.get_zones(account_number) + self.assertEqual(result, zones) \ No newline at end of file From 915ec0ba631fdac9ec6f44c71e90acfba27d32a0 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Tue, 21 Jan 2020 17:08:59 -0800 Subject: [PATCH 06/22] added PowerDNS support for create_txt_record and associated tests --- lemur/plugins/lemur_acme/powerdns.py | 81 +++++++++++++++++- .../plugins/lemur_acme/tests/test_powerdns.py | 84 ++++++++++--------- 2 files changed, 123 insertions(+), 42 deletions(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 2f80e54f..f68828d1 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -57,6 +57,18 @@ def _get(path, params=None): resp.raise_for_status() return resp.json() +def _patch(path, payload): + """ + Function to execute a Patch request on the given URL (base_uri + path) with given data + """ + base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") + resp = requests.patch( + f"{base_uri}{path}", + headers=_generate_header(), + data=json.dumps(payload) + ) + resp.raise_for_status() + def get_zones(account_number): """Get zones from the PowerDNS""" @@ -69,6 +81,23 @@ def get_zones(account_number): zones.append(zone.name) return zones +def _get_zone_name(domain, account_number): + """Get the matching zone for the given domain""" + zones = get_zones(account_number) + zone_name = "" + for z in zones: + if domain.endswith(z): + # Find the most specific zone possible for the domain + # Ex: If fqdn is a.b.c.com, there is a zone for c.com, + # and a zone for b.c.com, we want to use b.c.com. + if z.count(".") > zone_name.count("."): + zone_name = z + if not zone_name: + function = sys._getframe().f_code.co_name + metrics.send(f"{function}.fail", "counter", 1) + raise Exception(f"No PowerDNS zone found for domain: {domain}") + return zone_name + def create_txt_record(domain, token, account_number): """ Create a TXT record for the given domain. @@ -81,7 +110,57 @@ def create_txt_record(domain, token, account_number): Matching zone - example.com Owner name - _acme-challenge.lemur """ - pass + + zone_name = _get_zone_name(domain, account_number) + node_name = domain[:-len(".".join(zone_name))] + + server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") + zone_id = zone_name.join(".") + domain_id = domain.join(".") + + path = f"/api/v1/servers/{server_id}/zones/{zone_id}" + payload = { + "rrsets": [ + { + "name": f"{domain_id}", + "type": "TXT", + "ttl": "300", + "changetype": "REPLACE", + "records": [ + { + "content": f"{token}", + "disabled": "false" + } + ], + "comments": [] + } + ] + } + + try: + _patch(path, payload) + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "token": token, + "message": "TXT record successfully created" + } + current_app.logger.debug(log_data) + except Exception as e: + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "domain": domain, + "token": token, + "Exception": e, + "message": "Unable to create TXT record" + } + current_app.logger.debug(log_data) + + change_id = (domain, token) + return change_id + def wait_for_dns_change(change_id, account_number=None): """ diff --git a/lemur/plugins/lemur_acme/tests/test_powerdns.py b/lemur/plugins/lemur_acme/tests/test_powerdns.py index f39ef3c5..e0808d68 100644 --- a/lemur/plugins/lemur_acme/tests/test_powerdns.py +++ b/lemur/plugins/lemur_acme/tests/test_powerdns.py @@ -5,6 +5,7 @@ from mock import MagicMock, Mock, patch from lemur.plugins.lemur_acme import plugin, powerdns + class TestPowerdns(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.dns_provider_service") def setUp(self, mock_dns_provider_service): @@ -19,37 +20,6 @@ class TestPowerdns(unittest.TestCase): "test.fakedomain.net": [mock_dns_provider], } - @patch("lemur.plugins.lemur_acme.powerdns.requests") - @patch("lemur.plugins.lemur_acme.powerdns.current_app") - def test_powerdns_get_token(self, mock_current_app, mock_requests): - # ret_val = json.dumps({"access_token": "access"}) - the_response = Response() - the_response._content = b'{"access_token": "access"}' - mock_requests.post = Mock(return_value=the_response) - mock_current_app.config.get = Mock(return_value="Test") - result = powerdns.get_powerdns_token() - self.assertTrue(len(result) > 0) - - @patch("lemur.plugins.lemur_acme.powerdns.current_app") - def test_powerdns_create_txt_record(self, mock_current_app): - domain = "_acme_challenge.test.example.com" - zone = "test.example.com" - token = "ABCDEFGHIJ" - account_number = "1234567890" - change_id = (domain, token) - powerdns.get_zone_name = Mock(return_value=zone) - mock_current_app.logger.debug = Mock() - powerdns._post = Mock() - log_data = { - "function": "create_txt_record", - "fqdn": domain, - "token": token, - "message": "TXT record created" - } - result = powerdns.create_txt_record(domain, token, account_number) - mock_current_app.logger.debug.assert_called_with(log_data) - self.assertEqual(result, change_id) - @patch("lemur.plugins.lemur_acme.powerdns.current_app") @patch("lemur.extensions.metrics") def test_powerdns_delete_txt_record(self, mock_metrics, mock_current_app): @@ -94,15 +64,6 @@ class TestPowerdns(unittest.TestCase): } mock_current_app.logger.debug.assert_called_with(log_data) - def test_powerdns_get_zone_name(self): - zones = ['example.com', 'test.example.com'] - zone = "test.example.com" - domain = "_acme-challenge.test.example.com" - account_number = "1234567890" - powerdns.get_zones = Mock(return_value=zones) - result = powerdns.get_zone_name(domain, account_number) - self.assertEqual(result, zone) - @patch("lemur.plugins.lemur_acme.powerdns.current_app") def test_powerdns_get_zones(self, mock_current_app): account_number = "1234567890" @@ -121,4 +82,45 @@ class TestPowerdns(unittest.TestCase): powerdns._get.side_effect = [get_response] mock_current_app.config.get = Mock(return_value="localhost") result = powerdns.get_zones(account_number) - self.assertEqual(result, zones) \ No newline at end of file + self.assertEqual(result, zones) + + def test_powerdns_get_zone_name(self): + zones = ['example.com', 'test.example.com'] + zone = "test.example.com" + domain = "_acme-challenge.test.example.com" + account_number = "1234567890" + powerdns.get_zones = Mock(return_value=zones) + result = powerdns._get_zone_name(domain, account_number) + self.assertEqual(result, zone) + + def mock_current_app_config_get(a, b): + """ Mock of current_app.config.get() """ + config = { + 'ACME_POWERDNS_APIKEYNAME': 'X-API-Key', + 'ACME_POWERDNS_APIKEY': 'KEY', + 'ACME_POWERDNS_DOMAIN': 'http://internal-dnshiddenmaster-1486232504.us-east-1.elb.amazonaws.com', + 'ACME_POWERDNS_SERVERID': 'localhost' + } + return config[a] + + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + # @patch("lemur.plugins.lemur_acme.powerdns.current_app.config.get", side_effect=mock_current_app_config_get) + def test_powerdns_create_txt_record(self, mock_current_app): + domain = "_acme_challenge.test.example.com" + zone = "test.example.com" + token = "ABCDEFGHIJ" + account_number = "1234567890" + change_id = (domain, token) + powerdns._get_zone_name = Mock(return_value=zone) + mock_current_app.logger.debug = Mock() + mock_current_app.config.get = Mock(return_value="localhost") + powerdns._patch = Mock() + log_data = { + "function": "create_txt_record", + "fqdn": domain, + "token": token, + "message": "TXT record successfully created" + } + result = powerdns.create_txt_record(domain, token, account_number) + mock_current_app.logger.debug.assert_called_with(log_data) + self.assertEqual(result, change_id) \ No newline at end of file From 52c7686d58d116f22d28147b42532dcb1bc25759 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Tue, 21 Jan 2020 18:47:21 -0800 Subject: [PATCH 07/22] adding wait_for_dns_change() and tests for PowerDNS ACME plugin --- lemur/plugins/lemur_acme/powerdns.py | 141 +++++++++++++++++- .../plugins/lemur_acme/tests/test_powerdns.py | 44 +++--- 2 files changed, 161 insertions(+), 24 deletions(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index f68828d1..0a3135e6 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -33,6 +33,34 @@ class Zone: """ Indicates whether the zone is setup as a PRIMARY or SECONDARY """ return self._data["kind"] +class Record: + """ + This class implements a PowerDNS record. + + Accepts the response from the API call as the argument. + """ + + def __init__(self, _data): + # Since we are dealing with only TXT records for Lemur, we expect only 1 RRSet in the response. + # Thus we default to picking up the first entry (_data["rrsets"][0]) from the response. + self._data = _data + + @property + def name(self): + return self._data["name"] + + @property + def disabled(self): + return self._data["disabled"] + + @property + def content(self): + return self._data["content"] + + @property + def ttl(self): + return self._data["ttl"] + def _generate_header(): """Function to generate the header for a request using the PowerDNS API Key""" @@ -147,7 +175,7 @@ def create_txt_record(domain, token, account_number): "message": "TXT record successfully created" } current_app.logger.debug(log_data) - except Exception as e: + except requests.exceptions.RequestException as e: function = sys._getframe().f_code.co_name log_data = { "function": function, @@ -161,6 +189,78 @@ def create_txt_record(domain, token, account_number): change_id = (domain, token) return change_id +def _get_authoritative_nameserver(domain): + """Get the authoritative nameserver for the given domain""" + n = dns.name.from_text(domain) + + depth = 2 + default = dns.resolver.get_default_resolver() + nameserver = default.nameservers[0] + + last = False + while not last: + s = n.split(depth) + + last = s[0].to_unicode() == u"@" + sub = s[1] + + query = dns.message.make_query(sub, dns.rdatatype.NS) + response = dns.query.udp(query, nameserver) + + rcode = response.rcode() + if rcode != dns.rcode.NOERROR: + function = sys._getframe().f_code.co_name + metrics.send(f"{function}.error", "counter", 1) + if rcode == dns.rcode.NXDOMAIN: + raise Exception("%s does not exist." % sub) + else: + raise Exception("Error %s" % dns.rcode.to_text(rcode)) + + if len(response.authority) > 0: + rrset = response.authority[0] + else: + rrset = response.answer[0] + + rr = rrset[0] + if rr.rdtype != dns.rdatatype.SOA: + authority = rr.target + nameserver = default.query(authority).rrset[0].to_text() + + depth += 1 + + return nameserver + + +def _get_public_authoritative_nameserver(): + return "8.8.8.8" + +def _has_dns_propagated(name, token, domain): + """ + Check whether the DNS change made by Lemur have propagated to the public DNS or not. + + Invoked by wait_for_dns_change() function + """ + txt_records = [] + try: + dns_resolver = dns.resolver.Resolver() + dns_resolver.nameservers = [domain] + dns_response = dns_resolver.query(name, "TXT") + for rdata in dns_response: + for txt_record in rdata.strings: + txt_records.append(txt_record.decode("utf-8")) + except dns.exception.DNSException: + function = sys._getframe().f_code.co_name + metrics.send(f"{function}.fail", "counter", 1) + return False + + for txt_record in txt_records: + if txt_record == token: + function = sys._getframe().f_code.co_name + metrics.send(f"{function}.success", "counter", 1) + return True + + return False + def wait_for_dns_change(change_id, account_number=None): """ @@ -169,7 +269,44 @@ def wait_for_dns_change(change_id, account_number=None): First check the domains authoritative server. Once this succeeds, we ask a public DNS server (Google <8.8.8.8> in our case). """ - pass + domain, token = change_id + number_of_attempts = 20 + + # Check if Record exists via DNS + nameserver = _get_authoritative_nameserver(domain) + for attempts in range(0, number_of_attempts): + status = _has_dns_propagated(domain, token, nameserver) + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "status": status, + "message": "Record status on ultraDNS authoritative server" + } + current_app.logger.debug(log_data) + if status: + time.sleep(10) + break + time.sleep(10) + if status: + nameserver = _get_public_authoritative_nameserver() + for attempts in range(0, number_of_attempts): + status = _has_dns_propagated(domain, token, nameserver) + log_data = { + "function": function, + "fqdn": domain, + "status": status, + "message": "Record status on Public DNS" + } + current_app.logger.debug(log_data) + if status: + metrics.send(f"{function}.success", "counter", 1) + break + time.sleep(10) + if not status: + metrics.send(f"{function}.fail", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token}) + sentry.captureException(extra={"fqdn": str(domain), "txt_record": str(token)}) + return def delete_txt_record(change_id, account_number, domain, token): """ diff --git a/lemur/plugins/lemur_acme/tests/test_powerdns.py b/lemur/plugins/lemur_acme/tests/test_powerdns.py index e0808d68..d69f890c 100644 --- a/lemur/plugins/lemur_acme/tests/test_powerdns.py +++ b/lemur/plugins/lemur_acme/tests/test_powerdns.py @@ -43,27 +43,6 @@ class TestPowerdns(unittest.TestCase): mock_current_app.logger.debug.assert_not_called() mock_metrics.send.assert_not_called() - @patch("lemur.plugins.lemur_acme.powerdns.current_app") - @patch("lemur.extensions.metrics") - def test_powerdns_wait_for_dns_change(self, mock_metrics, mock_current_app): - powerdns._has_dns_propagated = Mock(return_value=True) - nameserver = "1.1.1.1" - powerdns.get_authoritative_nameserver = Mock(return_value=nameserver) - mock_metrics.send = Mock() - domain = "_acme-challenge.test.example.com" - token = "ABCDEFGHIJ" - change_id = (domain, token) - mock_current_app.logger.debug = Mock() - powerdns.wait_for_dns_change(change_id) - # mock_metrics.send.assert_not_called() - log_data = { - "function": "wait_for_dns_change", - "fqdn": domain, - "status": True, - "message": "Record status on Public DNS" - } - mock_current_app.logger.debug.assert_called_with(log_data) - @patch("lemur.plugins.lemur_acme.powerdns.current_app") def test_powerdns_get_zones(self, mock_current_app): account_number = "1234567890" @@ -123,4 +102,25 @@ class TestPowerdns(unittest.TestCase): } result = powerdns.create_txt_record(domain, token, account_number) mock_current_app.logger.debug.assert_called_with(log_data) - self.assertEqual(result, change_id) \ No newline at end of file + self.assertEqual(result, change_id) + + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + @patch("lemur.extensions.metrics") + def test_powerdns_wait_for_dns_change(self, mock_metrics, mock_current_app): + powerdns._has_dns_propagated = Mock(return_value=True) + nameserver = "1.1.1.1" + powerdns._get_authoritative_nameserver = Mock(return_value=nameserver) + mock_metrics.send = Mock() + domain = "_acme-challenge.test.example.com" + token = "ABCDEFGHIJ" + change_id = (domain, token) + mock_current_app.logger.debug = Mock() + powerdns.wait_for_dns_change(change_id) + # mock_metrics.send.assert_not_called() + log_data = { + "function": "wait_for_dns_change", + "fqdn": domain, + "status": True, + "message": "Record status on Public DNS" + } + mock_current_app.logger.debug.assert_called_with(log_data) \ No newline at end of file From bddae6e4287f8c26e48ff49a185847edd3015f60 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Wed, 22 Jan 2020 16:18:52 -0800 Subject: [PATCH 08/22] adding PowerDNS delete_txt_record with associated tests --- lemur/plugins/lemur_acme/powerdns.py | 289 +++++++++--------- .../plugins/lemur_acme/tests/test_powerdns.py | 79 +++-- 2 files changed, 184 insertions(+), 184 deletions(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 0a3135e6..9591cd01 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -12,6 +12,7 @@ import dns.resolver from flask import current_app from lemur.extensions import metrics, sentry + class Zone: """ This class implements a PowerDNS zone in JSON. """ @@ -33,16 +34,11 @@ class Zone: """ Indicates whether the zone is setup as a PRIMARY or SECONDARY """ return self._data["kind"] -class Record: - """ - This class implements a PowerDNS record. - Accepts the response from the API call as the argument. - """ +class Record: + """ This class implements a PowerDNS record. """ def __init__(self, _data): - # Since we are dealing with only TXT records for Lemur, we expect only 1 RRSet in the response. - # Thus we default to picking up the first entry (_data["rrsets"][0]) from the response. self._data = _data @property @@ -62,44 +58,8 @@ class Record: return self._data["ttl"] -def _generate_header(): - """Function to generate the header for a request using the PowerDNS API Key""" - - api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME", "") - api_key = current_app.config.get("ACME_POWERDNS_APIKEY", "") - return {api_key_name: api_key} - - -def _get(path, params=None): - """ - Function to execute a GET request on the given URL (base_uri + path) with given params - Returns JSON response object - """ - base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") - resp = requests.get( - f"{base_uri}{path}", - headers=_generate_header(), - params=params, - verify=True, - ) - resp.raise_for_status() - return resp.json() - -def _patch(path, payload): - """ - Function to execute a Patch request on the given URL (base_uri + path) with given data - """ - base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") - resp = requests.patch( - f"{base_uri}{path}", - headers=_generate_header(), - data=json.dumps(payload) - ) - resp.raise_for_status() - - def get_zones(account_number): - """Get zones from the PowerDNS""" + """Retrieve authoritative zones from the PowerDNS API and return a list""" server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") path = f"/api/v1/servers/{server_id}/zones" zones = [] @@ -109,43 +69,13 @@ def get_zones(account_number): zones.append(zone.name) return zones -def _get_zone_name(domain, account_number): - """Get the matching zone for the given domain""" - zones = get_zones(account_number) - zone_name = "" - for z in zones: - if domain.endswith(z): - # Find the most specific zone possible for the domain - # Ex: If fqdn is a.b.c.com, there is a zone for c.com, - # and a zone for b.c.com, we want to use b.c.com. - if z.count(".") > zone_name.count("."): - zone_name = z - if not zone_name: - function = sys._getframe().f_code.co_name - metrics.send(f"{function}.fail", "counter", 1) - raise Exception(f"No PowerDNS zone found for domain: {domain}") - return zone_name def create_txt_record(domain, token, account_number): - """ - Create a TXT record for the given domain. - - The part of the domain that matches with the zone becomes the zone name. - The remainder becomes the owner name (referred to as node name here) - Example: Let's say we have a zone named "exmaple.com" in PowerDNS and we - get a request to create a cert for lemur.example.com - Domain - _acme-challenge.lemur.example.com - Matching zone - example.com - Owner name - _acme-challenge.lemur - """ - + """ Create a TXT record for the given domain and token and return a change_id tuple """ zone_name = _get_zone_name(domain, account_number) - node_name = domain[:-len(".".join(zone_name))] - server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") zone_id = zone_name.join(".") domain_id = domain.join(".") - path = f"/api/v1/servers/{server_id}/zones/{zone_id}" payload = { "rrsets": [ @@ -189,6 +119,146 @@ def create_txt_record(domain, token, account_number): change_id = (domain, token) return change_id + +def wait_for_dns_change(change_id, account_number=None): + """ + Checks if changes have propagated to DNS + Verifies both the authoritative DNS server and a public DNS server(Google <8.8.8.8> in our case) + Retries and waits until successful. + """ + domain, token = change_id + number_of_attempts = 20 + + nameserver = _get_authoritative_nameserver(domain) + status = False + for attempts in range(0, number_of_attempts): + status = _has_dns_propagated(domain, token, nameserver) + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "status": status, + "message": "Record status on UltraDNS authoritative server" + } + current_app.logger.debug(log_data) + if status: + time.sleep(10) + break + time.sleep(10) + if status: + nameserver = _get_public_authoritative_nameserver() + for attempts in range(0, number_of_attempts): + status = _has_dns_propagated(domain, token, nameserver) + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "status": status, + "message": "Record status on Public DNS" + } + current_app.logger.debug(log_data) + if status: + metrics.send(f"{function}.success", "counter", 1) + break + time.sleep(10) + if not status: + metrics.send(f"{function}.fail", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token}) + sentry.captureException(extra={"fqdn": str(domain), "txt_record": str(token)}) + + +def delete_txt_record(change_id, account_number, domain, token): + """ Delete the TXT record for the given domain and token """ + zone_name = _get_zone_name(domain, account_number) + server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") + zone_id = zone_name.join(".") + domain_id = domain.join(".") + path = f"/api/v1/servers/{server_id}/zones/{zone_id}" + payload = { + "rrsets": [ + { + "name": f"{domain_id}", + "type": "TXT", + "ttl": "300", + "changetype": "DELETE", + "records": [ + { + "content": f"{token}", + "disabled": "false" + } + ], + "comments": [] + } + ] + } + + try: + _patch(path, payload) + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "token": token, + "message": "TXT record successfully deleted" + } + current_app.logger.debug(log_data) + except requests.exceptions.RequestException as e: + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "domain": domain, + "token": token, + "Exception": e, + "message": "Unable to delete TXT record" + } + current_app.logger.debug(log_data) + + +def _generate_header(): + """Generate a PowerDNS API header and return it as a dictionary""" + api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME", "") + api_key = current_app.config.get("ACME_POWERDNS_APIKEY", "") + return {api_key_name: api_key} + + +def _get(path, params=None): + """ Execute a GET request on the given URL (base_uri + path) and return response as JSON object """ + base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") + resp = requests.get( + f"{base_uri}{path}", + headers=_generate_header(), + params=params, + verify=True, + ) + resp.raise_for_status() + return resp.json() + + +def _patch(path, payload): + """ Execute a Patch request on the given URL (base_uri + path) with given payload """ + base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") + resp = requests.patch( + f"{base_uri}{path}", + headers=_generate_header(), + data=json.dumps(payload) + ) + resp.raise_for_status() + + +def _get_zone_name(domain, account_number): + """Get most specific matching zone for the given domain and return as a String""" + zones = get_zones(account_number) + zone_name = "" + for z in zones: + if domain.endswith(z): + if z.count(".") > zone_name.count("."): + zone_name = z + if not zone_name: + function = sys._getframe().f_code.co_name + metrics.send(f"{function}.fail", "counter", 1) + raise Exception(f"No PowerDNS zone found for domain: {domain}") + return zone_name + + def _get_authoritative_nameserver(domain): """Get the authoritative nameserver for the given domain""" n = dns.name.from_text(domain) @@ -234,12 +304,9 @@ def _get_authoritative_nameserver(domain): def _get_public_authoritative_nameserver(): return "8.8.8.8" -def _has_dns_propagated(name, token, domain): - """ - Check whether the DNS change made by Lemur have propagated to the public DNS or not. - Invoked by wait_for_dns_change() function - """ +def _has_dns_propagated(name, token, domain): + """Check whether the DNS change has propagated to the public DNS""" txt_records = [] try: dns_resolver = dns.resolver.Resolver() @@ -260,65 +327,3 @@ def _has_dns_propagated(name, token, domain): return True return False - - -def wait_for_dns_change(change_id, account_number=None): - """ - Waits and checks if the DNS changes have propagated or not. - - First check the domains authoritative server. Once this succeeds, - we ask a public DNS server (Google <8.8.8.8> in our case). - """ - domain, token = change_id - number_of_attempts = 20 - - # Check if Record exists via DNS - nameserver = _get_authoritative_nameserver(domain) - for attempts in range(0, number_of_attempts): - status = _has_dns_propagated(domain, token, nameserver) - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "fqdn": domain, - "status": status, - "message": "Record status on ultraDNS authoritative server" - } - current_app.logger.debug(log_data) - if status: - time.sleep(10) - break - time.sleep(10) - if status: - nameserver = _get_public_authoritative_nameserver() - for attempts in range(0, number_of_attempts): - status = _has_dns_propagated(domain, token, nameserver) - log_data = { - "function": function, - "fqdn": domain, - "status": status, - "message": "Record status on Public DNS" - } - current_app.logger.debug(log_data) - if status: - metrics.send(f"{function}.success", "counter", 1) - break - time.sleep(10) - if not status: - metrics.send(f"{function}.fail", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token}) - sentry.captureException(extra={"fqdn": str(domain), "txt_record": str(token)}) - return - -def delete_txt_record(change_id, account_number, domain, token): - """ - Delete the TXT record that was created in the create_txt_record() function. - - UltraDNS handles records differently compared to Dyn. It creates an RRSet - which is a set of records of the same type and owner. This means - that while deleting the record, we cannot delete any individual record from - the RRSet. Instead, we have to delete the entire RRSet. If multiple certs are - being created for the same domain at the same time, the challenge TXT records - that are created will be added under the same RRSet. If the RRSet had more - than 1 record, then we create a new RRSet on UltraDNS minus the record that - has to be deleted. - """ - pass diff --git a/lemur/plugins/lemur_acme/tests/test_powerdns.py b/lemur/plugins/lemur_acme/tests/test_powerdns.py index d69f890c..be3a590a 100644 --- a/lemur/plugins/lemur_acme/tests/test_powerdns.py +++ b/lemur/plugins/lemur_acme/tests/test_powerdns.py @@ -21,30 +21,7 @@ class TestPowerdns(unittest.TestCase): } @patch("lemur.plugins.lemur_acme.powerdns.current_app") - @patch("lemur.extensions.metrics") - def test_powerdns_delete_txt_record(self, mock_metrics, mock_current_app): - domain = "_acme_challenge.test.example.com" - zone = "test.example.com" - token = "ABCDEFGHIJ" - account_number = "1234567890" - change_id = (domain, token) - mock_current_app.logger.debug = Mock() - powerdns.get_zone_name = Mock(return_value=zone) - powerdns._post = Mock() - powerdns._get = Mock() - powerdns._get.return_value = {'zoneName': 'test.example.com.com', - 'rrSets': [{'ownerName': '_acme-challenge.test.example.com.', - 'rrtype': 'TXT (16)', 'ttl': 5, 'rdata': ['ABCDEFGHIJ']}], - 'queryInfo': {'sort': 'OWNER', 'reverse': False, 'limit': 100}, - 'resultInfo': {'totalCount': 1, 'offset': 0, 'returnedCount': 1}} - powerdns._delete = Mock() - mock_metrics.send = Mock() - powerdns.delete_txt_record(change_id, account_number, domain, token) - mock_current_app.logger.debug.assert_not_called() - mock_metrics.send.assert_not_called() - - @patch("lemur.plugins.lemur_acme.powerdns.current_app") - def test_powerdns_get_zones(self, mock_current_app): + def test_get_zones(self, mock_current_app): account_number = "1234567890" path = "a/b/c" zones = ['example.com', 'test.example.com'] @@ -63,7 +40,7 @@ class TestPowerdns(unittest.TestCase): result = powerdns.get_zones(account_number) self.assertEqual(result, zones) - def test_powerdns_get_zone_name(self): + def test_get_zone_name(self): zones = ['example.com', 'test.example.com'] zone = "test.example.com" domain = "_acme-challenge.test.example.com" @@ -72,19 +49,8 @@ class TestPowerdns(unittest.TestCase): result = powerdns._get_zone_name(domain, account_number) self.assertEqual(result, zone) - def mock_current_app_config_get(a, b): - """ Mock of current_app.config.get() """ - config = { - 'ACME_POWERDNS_APIKEYNAME': 'X-API-Key', - 'ACME_POWERDNS_APIKEY': 'KEY', - 'ACME_POWERDNS_DOMAIN': 'http://internal-dnshiddenmaster-1486232504.us-east-1.elb.amazonaws.com', - 'ACME_POWERDNS_SERVERID': 'localhost' - } - return config[a] - @patch("lemur.plugins.lemur_acme.powerdns.current_app") - # @patch("lemur.plugins.lemur_acme.powerdns.current_app.config.get", side_effect=mock_current_app_config_get) - def test_powerdns_create_txt_record(self, mock_current_app): + def test_create_txt_record(self, mock_current_app): domain = "_acme_challenge.test.example.com" zone = "test.example.com" token = "ABCDEFGHIJ" @@ -106,21 +72,50 @@ class TestPowerdns(unittest.TestCase): @patch("lemur.plugins.lemur_acme.powerdns.current_app") @patch("lemur.extensions.metrics") - def test_powerdns_wait_for_dns_change(self, mock_metrics, mock_current_app): - powerdns._has_dns_propagated = Mock(return_value=True) + @patch("time.sleep") + def test_wait_for_dns_change(self, mock_sleep, mock_metrics, mock_current_app): nameserver = "1.1.1.1" powerdns._get_authoritative_nameserver = Mock(return_value=nameserver) + powerdns._has_dns_propagated = Mock(return_value=True) mock_metrics.send = Mock() + mock_sleep.return_value = False domain = "_acme-challenge.test.example.com" token = "ABCDEFGHIJ" change_id = (domain, token) mock_current_app.logger.debug = Mock() powerdns.wait_for_dns_change(change_id) - # mock_metrics.send.assert_not_called() - log_data = { + + auth_log_data = { + "function": "wait_for_dns_change", + "fqdn": domain, + "status": True, + "message": "Record status on UltraDNS authoritative server" + } + pub_log_data = { "function": "wait_for_dns_change", "fqdn": domain, "status": True, "message": "Record status on Public DNS" } - mock_current_app.logger.debug.assert_called_with(log_data) \ No newline at end of file + mock_current_app.logger.debug.assert_any_call(auth_log_data) + mock_current_app.logger.debug.assert_any_call(pub_log_data) + + @patch("lemur.plugins.lemur_acme.powerdns.current_app") + def test_delete_txt_record(self, mock_current_app): + domain = "_acme_challenge.test.example.com" + zone = "test.example.com" + token = "ABCDEFGHIJ" + account_number = "1234567890" + change_id = (domain, token) + powerdns._get_zone_name = Mock(return_value=zone) + mock_current_app.logger.debug = Mock() + mock_current_app.config.get = Mock(return_value="localhost") + powerdns._patch = Mock() + log_data = { + "function": "delete_txt_record", + "fqdn": domain, + "token": token, + "message": "TXT record successfully deleted" + } + powerdns.delete_txt_record(change_id, account_number, domain, token) + mock_current_app.logger.debug.assert_called_with(log_data) From c465062673e83bf721c5fd69906445ad035aca05 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Thu, 23 Jan 2020 23:53:38 -0800 Subject: [PATCH 09/22] integrated PowerDNS plugin into dns_providers --- lemur/dns_providers/service.py | 1 + lemur/plugins/lemur_acme/plugin.py | 4 ++-- lemur/plugins/lemur_acme/powerdns.py | 8 ++++---- lemur/plugins/lemur_acme/tests/test_powerdns.py | 5 +---- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lemur/dns_providers/service.py b/lemur/dns_providers/service.py index 29f98a5b..7052b55b 100644 --- a/lemur/dns_providers/service.py +++ b/lemur/dns_providers/service.py @@ -99,6 +99,7 @@ def get_types(): }, {"name": "dyn"}, {"name": "ultradns"}, + {"name": "powerdns"}, ] }, ) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index 93628905..8991efdf 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -377,7 +377,7 @@ class AcmeHandler(object): "dyn": dyn, "route53": route53, "ultradns": ultradns, - # "powerdns": powerdns, + "powerdns": powerdns } provider = provider_types.get(type) if not provider: @@ -437,7 +437,7 @@ class ACMEIssuerPlugin(IssuerPlugin): "dyn": dyn, "route53": route53, "ultradns": ultradns, - # "powerdns": powerdns, + "powerdns": powerdns } provider = provider_types.get(type) if not provider: diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 9591cd01..e0a145e6 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -74,8 +74,8 @@ def create_txt_record(domain, token, account_number): """ Create a TXT record for the given domain and token and return a change_id tuple """ zone_name = _get_zone_name(domain, account_number) server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") - zone_id = zone_name.join(".") - domain_id = domain.join(".") + zone_id = zone_name + "." + domain_id = domain + "." path = f"/api/v1/servers/{server_id}/zones/{zone_id}" payload = { "rrsets": [ @@ -170,8 +170,8 @@ def delete_txt_record(change_id, account_number, domain, token): """ Delete the TXT record for the given domain and token """ zone_name = _get_zone_name(domain, account_number) server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") - zone_id = zone_name.join(".") - domain_id = domain.join(".") + zone_id = zone_name + "." + domain_id = domain + "." path = f"/api/v1/servers/{server_id}/zones/{zone_id}" payload = { "rrsets": [ diff --git a/lemur/plugins/lemur_acme/tests/test_powerdns.py b/lemur/plugins/lemur_acme/tests/test_powerdns.py index be3a590a..f1190732 100644 --- a/lemur/plugins/lemur_acme/tests/test_powerdns.py +++ b/lemur/plugins/lemur_acme/tests/test_powerdns.py @@ -1,8 +1,5 @@ import unittest -from requests.models import Response - -from mock import MagicMock, Mock, patch - +from mock import Mock, patch from lemur.plugins.lemur_acme import plugin, powerdns From b91899fe9992c4b8f78f88d39e5c5297f2ada448 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Tue, 28 Jan 2020 19:13:28 -0800 Subject: [PATCH 10/22] created CLI options for testin ACME over dns. Examle: `acme dnstest -d _acme-chall.foo.com -t token1` --- __init__.py | 0 lemur/acme_providers/__init__.py | 0 lemur/acme_providers/cli.py | 92 ++++++++++++++ lemur/dns_providers/util.py | 103 +++++++++++++++ lemur/manage.py | 2 + lemur/plugins/lemur_acme/powerdns.py | 179 +++++++++------------------ 6 files changed, 254 insertions(+), 122 deletions(-) create mode 100644 __init__.py create mode 100644 lemur/acme_providers/__init__.py create mode 100644 lemur/acme_providers/cli.py create mode 100644 lemur/dns_providers/util.py diff --git a/__init__.py b/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/lemur/acme_providers/__init__.py b/lemur/acme_providers/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/lemur/acme_providers/cli.py b/lemur/acme_providers/cli.py new file mode 100644 index 00000000..fcf426fa --- /dev/null +++ b/lemur/acme_providers/cli.py @@ -0,0 +1,92 @@ +import time +import json + +from flask_script import Manager +from flask import current_app + +from lemur.extensions import sentry +from lemur.extensions import metrics +from lemur.constants import SUCCESS_METRIC_STATUS +from lemur.plugins.lemur_acme.plugin import AcmeHandler + +manager = Manager( + usage="This provides ability to test ACME issuance" +) + + +@manager.option( + "-d", + "--domain", + dest="domain", + required=True, + help="Name of the Domain to store to (ex. \"_acme-chall.test.com\".", +) +@manager.option( + "-t", + "--token", + dest="token", + required=True, + help="Value of the Token to store in DNS as content.", +) +def dnstest(domain, token): + """ + Attempts to create, verify, and delete DNS TXT records with an autodetected provider. + """ + print("[+] Starting ACME Tests.") + change_id = (domain, token) + + acme_handler = AcmeHandler() + acme_handler.autodetect_dns_providers(domain) + if not acme_handler.dns_providers_for_domain[domain]: + metrics.send( + "get_authorizations_no_dns_provider_for_domain", "counter", 1 + ) + raise Exception(f"No DNS providers found for domain: {format(domain)}.") + + # Create TXT Records + for dns_provider in acme_handler.dns_providers_for_domain[domain]: + dns_provider_plugin = acme_handler.get_dns_provider(dns_provider.provider_type) + dns_provider_options = json.loads(dns_provider.credentials) + account_number = dns_provider_options.get("account_id") + + print(f"[+] Creating TXT Record in `{dns_provider.name}` provider") + change_id = dns_provider_plugin.create_txt_record(domain, token, account_number) + + print("[+] Verifying TXT Record has propagated to DNS.") + print("[+] Waiting 60 second before continuing...") + time.sleep(10) + + # Verify TXT Records + for dns_provider in acme_handler.dns_providers_for_domain[domain]: + dns_provider_plugin = acme_handler.get_dns_provider(dns_provider.provider_type) + dns_provider_options = json.loads(dns_provider.credentials) + account_number = dns_provider_options.get("account_id") + + try: + dns_provider_plugin.wait_for_dns_change(change_id, account_number) + print(f"[+] Verfied TXT Record in `{dns_provider.name}` provider") + except Exception: + metrics.send("complete_dns_challenge_error", "counter", 1) + sentry.captureException() + current_app.logger.debug( + f"Unable to resolve DNS challenge for change_id: {change_id}, account_id: " + f"{account_number}", + exc_info=True, + ) + print(f"[+] Unable to Verify TXT Record in `{dns_provider.name}` provider") + + time.sleep(10) + + # Delete TXT Records + for dns_provider in acme_handler.dns_providers_for_domain[domain]: + dns_provider_plugin = acme_handler.get_dns_provider(dns_provider.provider_type) + dns_provider_options = json.loads(dns_provider.credentials) + account_number = dns_provider_options.get("account_id") + + # TODO(csine@: Add Exception Handling + dns_provider_plugin.delete_txt_record(change_id, account_number, domain, token) + print(f"[+] Deleted TXT Record in `{dns_provider.name}` provider") + + status = SUCCESS_METRIC_STATUS + metrics.send("dnstest", "counter", 1, metric_tags={"status": status}) + print("[+] Done with ACME Tests.") diff --git a/lemur/dns_providers/util.py b/lemur/dns_providers/util.py new file mode 100644 index 00000000..6534f6eb --- /dev/null +++ b/lemur/dns_providers/util.py @@ -0,0 +1,103 @@ +import sys +import dns +import dns.exception +import dns.name +import dns.query +import dns.resolver +import re + +from flask import current_app +from lemur.extensions import metrics, sentry + + +class DNSError(Exception): + """Base class for DNS Exceptions.""" + pass + + +class BadDomainError(DNSError): + """Error for when a Bad Domain Name is given.""" + + def __init__(self, message): + self.message = message + + +class DNSResolveError(DNSError): + """Error for DNS Resolution Errors.""" + + def __init__(self, message): + self.message = message + + +def is_valid_domain(domain): + """Checks if a domain is syntactically valid and returns a bool""" + if len(domain) > 253: + return False + if domain[-1] == ".": + domain = domain[:-1] + fqdn_re = re.compile("(?=^.{1,254}$)(^(?:(?!\d+\.|-)[a-zA-Z0-9_\-]{1,63}(? 0: + rrset = response.authority[0] + else: + rrset = response.answer[0] + + rr = rrset[0] + if rr.rdtype != dns.rdatatype.SOA: + authority = rr.target + nameserver = default.query(authority).rrset[0].to_text() + + depth += 1 + + return nameserver + + +def get_dns_records(domain, rdtype, nameserver): + """Retrieves the DNS records matching the name and type and returns a list of records""" + # if not nameserver: + # nameserver = get_authoritative_nameserver(domain)[0] + + records = [] + try: + dns_resolver = dns.resolver.Resolver() + dns_resolver.nameservers = [nameserver] + dns_response = dns_resolver.query(domain, rdtype) + for rdata in dns_response: + for record in rdata.strings: + records.append(record.decode("utf-8")) + except dns.exception.DNSException: + function = sys._getframe().f_code.co_name + metrics.send(f"{function}.fail", "counter", 1) + return records diff --git a/lemur/manage.py b/lemur/manage.py index 7dd3b3b4..2fbbe893 100755 --- a/lemur/manage.py +++ b/lemur/manage.py @@ -17,6 +17,7 @@ from flask_migrate import Migrate, MigrateCommand, stamp from flask_script.commands import ShowUrls, Clean, Server from lemur.dns_providers.cli import manager as dns_provider_manager +from lemur.acme_providers.cli import manager as acme_manager from lemur.sources.cli import manager as source_manager from lemur.policies.cli import manager as policy_manager from lemur.reporting.cli import manager as report_manager @@ -584,6 +585,7 @@ def main(): manager.add_command("policy", policy_manager) manager.add_command("pending_certs", pending_certificate_manager) manager.add_command("dns_providers", dns_provider_manager) + manager.add_command("acme", acme_manager) manager.run() diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index e0a145e6..7e45d581 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -3,11 +3,7 @@ import requests import json import sys -import dns -import dns.exception -import dns.name -import dns.query -import dns.resolver +import lemur.dns_providers.util as dnsutil from flask import current_app from lemur.extensions import metrics, sentry @@ -63,8 +59,25 @@ def get_zones(account_number): server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") path = f"/api/v1/servers/{server_id}/zones" zones = [] - for elem in _get(path): - zone = Zone(elem) + try: + records = _get(path) + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "message": "Retrieved Zones Successfully" + } + current_app.logger.debug(log_data) + except Exception as e: + records = _get(path) + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "message": "Failed to Retrieve Zone Data" + } + current_app.logger.debug(log_data) + + for record in records: + zone = Zone(record) if zone.kind == 'Master': zones.append(zone.name) return zones @@ -80,14 +93,14 @@ def create_txt_record(domain, token, account_number): payload = { "rrsets": [ { - "name": f"{domain_id}", + "name": domain_id, "type": "TXT", - "ttl": "300", + "ttl": 300, "changetype": "REPLACE", "records": [ { - "content": f"{token}", - "disabled": "false" + "content": f"\"{token}\"", + "disabled": False } ], "comments": [] @@ -105,7 +118,7 @@ def create_txt_record(domain, token, account_number): "message": "TXT record successfully created" } current_app.logger.debug(log_data) - except requests.exceptions.RequestException as e: + except Exception as e: function = sys._getframe().f_code.co_name log_data = { "function": function, @@ -122,46 +135,36 @@ def create_txt_record(domain, token, account_number): def wait_for_dns_change(change_id, account_number=None): """ - Checks if changes have propagated to DNS - Verifies both the authoritative DNS server and a public DNS server(Google <8.8.8.8> in our case) + Checks the authoritative DNS Server to see if changes have propagated to DNS Retries and waits until successful. """ domain, token = change_id number_of_attempts = 20 - - nameserver = _get_authoritative_nameserver(domain) - status = False + zone_name = _get_zone_name(domain, account_number) + nameserver = dnsutil.get_authoritative_nameserver(zone_name) + record_found = False for attempts in range(0, number_of_attempts): - status = _has_dns_propagated(domain, token, nameserver) - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "fqdn": domain, - "status": status, - "message": "Record status on UltraDNS authoritative server" - } - current_app.logger.debug(log_data) - if status: - time.sleep(10) + txt_records = dnsutil.get_dns_records(domain, "TXT", nameserver) + for txt_record in txt_records: + if txt_record == token: + record_found = True + break + if record_found: break time.sleep(10) - if status: - nameserver = _get_public_authoritative_nameserver() - for attempts in range(0, number_of_attempts): - status = _has_dns_propagated(domain, token, nameserver) - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "fqdn": domain, - "status": status, - "message": "Record status on Public DNS" - } - current_app.logger.debug(log_data) - if status: - metrics.send(f"{function}.success", "counter", 1) - break - time.sleep(10) - if not status: + + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "status": record_found, + "message": "Record status on PowerDNS authoritative server" + } + current_app.logger.debug(log_data) + + if record_found: + metrics.send(f"{function}.success", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token}) + else: metrics.send(f"{function}.fail", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token}) sentry.captureException(extra={"fqdn": str(domain), "txt_record": str(token)}) @@ -176,14 +179,14 @@ def delete_txt_record(change_id, account_number, domain, token): payload = { "rrsets": [ { - "name": f"{domain_id}", + "name": domain_id, "type": "TXT", - "ttl": "300", + "ttl": 300, "changetype": "DELETE", "records": [ { - "content": f"{token}", - "disabled": "false" + "content": f"\"{token}\"", + "disabled": False } ], "comments": [] @@ -201,7 +204,7 @@ def delete_txt_record(change_id, account_number, domain, token): "message": "TXT record successfully deleted" } current_app.logger.debug(log_data) - except requests.exceptions.RequestException as e: + except Exception as e: function = sys._getframe().f_code.co_name log_data = { "function": function, @@ -217,7 +220,8 @@ def _generate_header(): """Generate a PowerDNS API header and return it as a dictionary""" api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME", "") api_key = current_app.config.get("ACME_POWERDNS_APIKEY", "") - return {api_key_name: api_key} + headers = {api_key_name: api_key} + return headers def _get(path, params=None): @@ -238,8 +242,8 @@ def _patch(path, payload): base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") resp = requests.patch( f"{base_uri}{path}", - headers=_generate_header(), - data=json.dumps(payload) + data=json.dumps(payload), + headers=_generate_header() ) resp.raise_for_status() @@ -258,72 +262,3 @@ def _get_zone_name(domain, account_number): raise Exception(f"No PowerDNS zone found for domain: {domain}") return zone_name - -def _get_authoritative_nameserver(domain): - """Get the authoritative nameserver for the given domain""" - n = dns.name.from_text(domain) - - depth = 2 - default = dns.resolver.get_default_resolver() - nameserver = default.nameservers[0] - - last = False - while not last: - s = n.split(depth) - - last = s[0].to_unicode() == u"@" - sub = s[1] - - query = dns.message.make_query(sub, dns.rdatatype.NS) - response = dns.query.udp(query, nameserver) - - rcode = response.rcode() - if rcode != dns.rcode.NOERROR: - function = sys._getframe().f_code.co_name - metrics.send(f"{function}.error", "counter", 1) - if rcode == dns.rcode.NXDOMAIN: - raise Exception("%s does not exist." % sub) - else: - raise Exception("Error %s" % dns.rcode.to_text(rcode)) - - if len(response.authority) > 0: - rrset = response.authority[0] - else: - rrset = response.answer[0] - - rr = rrset[0] - if rr.rdtype != dns.rdatatype.SOA: - authority = rr.target - nameserver = default.query(authority).rrset[0].to_text() - - depth += 1 - - return nameserver - - -def _get_public_authoritative_nameserver(): - return "8.8.8.8" - - -def _has_dns_propagated(name, token, domain): - """Check whether the DNS change has propagated to the public DNS""" - txt_records = [] - try: - dns_resolver = dns.resolver.Resolver() - dns_resolver.nameservers = [domain] - dns_response = dns_resolver.query(name, "TXT") - for rdata in dns_response: - for txt_record in rdata.strings: - txt_records.append(txt_record.decode("utf-8")) - except dns.exception.DNSException: - function = sys._getframe().f_code.co_name - metrics.send(f"{function}.fail", "counter", 1) - return False - - for txt_record in txt_records: - if txt_record == token: - function = sys._getframe().f_code.co_name - metrics.send(f"{function}.success", "counter", 1) - return True - - return False From f3039a1210a8ec0e9325f7ce1503fd8b9cebf2c0 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Wed, 29 Jan 2020 11:05:46 -0800 Subject: [PATCH 11/22] removed accidently added __init__py file --- __init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 __init__.py diff --git a/__init__.py b/__init__.py deleted file mode 100644 index e69de29b..00000000 From ef115ef2b12ee8bf4c7a7f90bb0b1bf176c51632 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Wed, 29 Jan 2020 11:20:39 -0800 Subject: [PATCH 12/22] moving PowerDNS number_of_attempts to global config variable ACME_POWERDNS_RETRIES --- lemur/plugins/lemur_acme/powerdns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 7e45d581..688a84f2 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -139,7 +139,7 @@ def wait_for_dns_change(change_id, account_number=None): Retries and waits until successful. """ domain, token = change_id - number_of_attempts = 20 + number_of_attempts = current_app.config.get("ACME_POWERDNS_RETRIES", "") zone_name = _get_zone_name(domain, account_number) nameserver = dnsutil.get_authoritative_nameserver(zone_name) record_found = False From 969a7107fef2ed256eccce99a623eda5b22d36e1 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Wed, 29 Jan 2020 13:12:09 -0800 Subject: [PATCH 13/22] fixed PowerDNS Tests --- lemur/plugins/lemur_acme/powerdns.py | 31 +++++++++-------- .../plugins/lemur_acme/tests/test_powerdns.py | 33 +++++++++---------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 688a84f2..1efe0752 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -224,6 +224,21 @@ def _generate_header(): return headers +def _get_zone_name(domain, account_number): + """Get most specific matching zone for the given domain and return as a String""" + zones = get_zones(account_number) + zone_name = "" + for z in zones: + if domain.endswith(z): + if z.count(".") > zone_name.count("."): + zone_name = z + if not zone_name: + function = sys._getframe().f_code.co_name + metrics.send(f"{function}.fail", "counter", 1) + raise Exception(f"No PowerDNS zone found for domain: {domain}") + return zone_name + + def _get(path, params=None): """ Execute a GET request on the given URL (base_uri + path) and return response as JSON object """ base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") @@ -246,19 +261,3 @@ def _patch(path, payload): headers=_generate_header() ) resp.raise_for_status() - - -def _get_zone_name(domain, account_number): - """Get most specific matching zone for the given domain and return as a String""" - zones = get_zones(account_number) - zone_name = "" - for z in zones: - if domain.endswith(z): - if z.count(".") > zone_name.count("."): - zone_name = z - if not zone_name: - function = sys._getframe().f_code.co_name - metrics.send(f"{function}.fail", "counter", 1) - raise Exception(f"No PowerDNS zone found for domain: {domain}") - return zone_name - diff --git a/lemur/plugins/lemur_acme/tests/test_powerdns.py b/lemur/plugins/lemur_acme/tests/test_powerdns.py index f1190732..4c483741 100644 --- a/lemur/plugins/lemur_acme/tests/test_powerdns.py +++ b/lemur/plugins/lemur_acme/tests/test_powerdns.py @@ -67,35 +67,34 @@ class TestPowerdns(unittest.TestCase): mock_current_app.logger.debug.assert_called_with(log_data) self.assertEqual(result, change_id) + @patch("lemur.plugins.lemur_acme.powerdns.dnsutil") @patch("lemur.plugins.lemur_acme.powerdns.current_app") @patch("lemur.extensions.metrics") @patch("time.sleep") - def test_wait_for_dns_change(self, mock_sleep, mock_metrics, mock_current_app): - nameserver = "1.1.1.1" - powerdns._get_authoritative_nameserver = Mock(return_value=nameserver) - powerdns._has_dns_propagated = Mock(return_value=True) - mock_metrics.send = Mock() - mock_sleep.return_value = False + def test_wait_for_dns_change(self, mock_sleep, mock_metrics, mock_current_app, mock_dnsutil): domain = "_acme-challenge.test.example.com" - token = "ABCDEFGHIJ" + token = "ABCDEFG" + zone_name = "test.example.com" + nameserver = "1.1.1.1" change_id = (domain, token) + mock_records = (token,) + + mock_current_app.config.get = Mock(return_value=1) + powerdns._get_zone_name = Mock(return_value=zone_name) + mock_dnsutil.get_authoritative_nameserver = Mock(return_value=nameserver) + mock_dnsutil.get_dns_records = Mock(return_value=mock_records) + mock_sleep.return_value = False + mock_metrics.send = Mock() mock_current_app.logger.debug = Mock() powerdns.wait_for_dns_change(change_id) - auth_log_data = { + log_data = { "function": "wait_for_dns_change", "fqdn": domain, "status": True, - "message": "Record status on UltraDNS authoritative server" + "message": "Record status on PowerDNS authoritative server" } - pub_log_data = { - "function": "wait_for_dns_change", - "fqdn": domain, - "status": True, - "message": "Record status on Public DNS" - } - mock_current_app.logger.debug.assert_any_call(auth_log_data) - mock_current_app.logger.debug.assert_any_call(pub_log_data) + mock_current_app.logger.debug.assert_called_with(log_data) @patch("lemur.plugins.lemur_acme.powerdns.current_app") def test_delete_txt_record(self, mock_current_app): From be7736d350e57d0f0630fe957a30bc0c90a8daa0 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Fri, 31 Jan 2020 13:16:37 -0800 Subject: [PATCH 14/22] adding dns tests and assorted exception handling --- lemur/acme_providers/cli.py | 8 ++++---- lemur/dns_providers/util.py | 6 +----- lemur/plugins/lemur_acme/powerdns.py | 14 +++++++------- lemur/tests/test_dns_providers.py | 13 +++++++++++++ 4 files changed, 25 insertions(+), 16 deletions(-) create mode 100644 lemur/tests/test_dns_providers.py diff --git a/lemur/acme_providers/cli.py b/lemur/acme_providers/cli.py index fcf426fa..a7510d36 100644 --- a/lemur/acme_providers/cli.py +++ b/lemur/acme_providers/cli.py @@ -10,7 +10,7 @@ from lemur.constants import SUCCESS_METRIC_STATUS from lemur.plugins.lemur_acme.plugin import AcmeHandler manager = Manager( - usage="This provides ability to test ACME issuance" + usage="Handles all ACME related tasks" ) @@ -30,7 +30,7 @@ manager = Manager( ) def dnstest(domain, token): """ - Attempts to create, verify, and delete DNS TXT records with an autodetected provider. + Create, verify, and delete DNS TXT records using an autodetected provider. """ print("[+] Starting ACME Tests.") change_id = (domain, token) @@ -53,7 +53,7 @@ def dnstest(domain, token): change_id = dns_provider_plugin.create_txt_record(domain, token, account_number) print("[+] Verifying TXT Record has propagated to DNS.") - print("[+] Waiting 60 second before continuing...") + print("[+] This step could take a while...") time.sleep(10) # Verify TXT Records @@ -64,7 +64,7 @@ def dnstest(domain, token): try: dns_provider_plugin.wait_for_dns_change(change_id, account_number) - print(f"[+] Verfied TXT Record in `{dns_provider.name}` provider") + print(f"[+] Verified TXT Record in `{dns_provider.name}` provider") except Exception: metrics.send("complete_dns_challenge_error", "counter", 1) sentry.captureException() diff --git a/lemur/dns_providers/util.py b/lemur/dns_providers/util.py index 6534f6eb..9154cb92 100644 --- a/lemur/dns_providers/util.py +++ b/lemur/dns_providers/util.py @@ -6,8 +6,7 @@ import dns.query import dns.resolver import re -from flask import current_app -from lemur.extensions import metrics, sentry +from lemur.extensions import metrics class DNSError(Exception): @@ -86,9 +85,6 @@ def get_authoritative_nameserver(domain): def get_dns_records(domain, rdtype, nameserver): """Retrieves the DNS records matching the name and type and returns a list of records""" - # if not nameserver: - # nameserver = get_authoritative_nameserver(domain)[0] - records = [] try: dns_resolver = dns.resolver.Resolver() diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 1efe0752..e30d7ca6 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -67,20 +67,20 @@ def get_zones(account_number): "message": "Retrieved Zones Successfully" } current_app.logger.debug(log_data) + for record in records: + zone = Zone(record) + if zone.kind == 'Master': + zones.append(zone.name) + return zones + except Exception as e: - records = _get(path) function = sys._getframe().f_code.co_name log_data = { "function": function, "message": "Failed to Retrieve Zone Data" } current_app.logger.debug(log_data) - - for record in records: - zone = Zone(record) - if zone.kind == 'Master': - zones.append(zone.name) - return zones + raise def create_txt_record(domain, token, account_number): diff --git a/lemur/tests/test_dns_providers.py b/lemur/tests/test_dns_providers.py new file mode 100644 index 00000000..42a86cca --- /dev/null +++ b/lemur/tests/test_dns_providers.py @@ -0,0 +1,13 @@ +import unittest +from mock import Mock, patch +from lemur.dns_providers import util as dnsutil + + +class TestDNSProvider(unittest.TestCase): + def test_is_valid_domain(self): + self.assertTrue(dnsutil.is_valid_domain("example.com")) + self.assertTrue(dnsutil.is_valid_domain("foo.bar.org")) + self.assertTrue(dnsutil.is_valid_domain("_acme-chall.example.com")) + self.assertFalse(dnsutil.is_valid_domain("e/xample.com")) + self.assertFalse(dnsutil.is_valid_domain("exam\ple.com")) + self.assertFalse(dnsutil.is_valid_domain("*.example.com")) From fb6d369130add96c55c6358476790db208d975c0 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Fri, 31 Jan 2020 16:18:22 -0800 Subject: [PATCH 15/22] removed unnecessary imports in test_dns_providers.py --- lemur/tests/test_dns_providers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lemur/tests/test_dns_providers.py b/lemur/tests/test_dns_providers.py index 42a86cca..b8714a2d 100644 --- a/lemur/tests/test_dns_providers.py +++ b/lemur/tests/test_dns_providers.py @@ -1,5 +1,4 @@ import unittest -from mock import Mock, patch from lemur.dns_providers import util as dnsutil From 7dac0e1dd8978c055a6201d80e67a5cb30525362 Mon Sep 17 00:00:00 2001 From: csine-nflx <59457265+csine-nflx@users.noreply.github.com> Date: Fri, 31 Jan 2020 16:54:25 -0800 Subject: [PATCH 16/22] Update administration.rst --- docs/administration.rst | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/docs/administration.rst b/docs/administration.rst index e292ae03..252a7dec 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -973,6 +973,41 @@ Will be the sender of all notifications, so ensure that it is verified with AWS. SES if the default notification gateway and will be used unless SMTP settings are configured in the application configuration settings. +PowerDNS ACME Plugin +~~~~~~~~~~~~~~~~~~~~~~ + +The following configuration properties are required to use the PowerDNS ACME Plugin for domain validation. + + +.. data:: ACME_POWERDNS_DOMAIN + :noindex: + + This is the FQDN for the PowerDNS API (without path) + + +.. data:: ACME_POWERDNS_SERVERID + :noindex: + + This is the ServerID attribute of the PowerDNS API Server (i.e. "localhost" + + +.. data:: ACME_POWERDNS_APIKEYNAME + :noindex: + + This is the Key name to use for authentication (i.e. "X-API-Key") + + +.. data:: ACME_POWERDNS_APIKEY + :noindex: + + This is the API Key to use for authentication (i.e. "Password") + + +.. data:: ACME_POWERDNS_RETRIES + :noindex: + + This is the number of times DNS Verification should be attempted (i.e. 20) + .. _CommandLineInterface: Command Line Interface @@ -1172,11 +1207,12 @@ Acme Kevin Glisson , Curtis Castrapel , Hossein Shafagh , - Mikhail Khodorovskiy + Mikhail Khodorovskiy , + Chad Sine :Type: Issuer :Description: - Adds support for the ACME protocol (including LetsEncrypt) with domain validation being handled Route53. + Adds support for the ACME protocol (including LetsEncrypt) with domain validation using several providers. Atlas From 53f81fb09f6ce6d8bb2e1522c78b8e9c694abe55 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Mon, 3 Feb 2020 18:58:31 -0800 Subject: [PATCH 17/22] updating based on suggestions in 2911 --- lemur/acme_providers/cli.py | 6 - lemur/dns_providers/util.py | 2 + lemur/plugins/lemur_acme/powerdns.py | 112 +++++++++--------- .../plugins/lemur_acme/tests/test_powerdns.py | 5 +- 4 files changed, 62 insertions(+), 63 deletions(-) diff --git a/lemur/acme_providers/cli.py b/lemur/acme_providers/cli.py index a7510d36..310efad1 100644 --- a/lemur/acme_providers/cli.py +++ b/lemur/acme_providers/cli.py @@ -5,7 +5,6 @@ from flask_script import Manager from flask import current_app from lemur.extensions import sentry -from lemur.extensions import metrics from lemur.constants import SUCCESS_METRIC_STATUS from lemur.plugins.lemur_acme.plugin import AcmeHandler @@ -38,9 +37,6 @@ def dnstest(domain, token): acme_handler = AcmeHandler() acme_handler.autodetect_dns_providers(domain) if not acme_handler.dns_providers_for_domain[domain]: - metrics.send( - "get_authorizations_no_dns_provider_for_domain", "counter", 1 - ) raise Exception(f"No DNS providers found for domain: {format(domain)}.") # Create TXT Records @@ -66,7 +62,6 @@ def dnstest(domain, token): dns_provider_plugin.wait_for_dns_change(change_id, account_number) print(f"[+] Verified TXT Record in `{dns_provider.name}` provider") except Exception: - metrics.send("complete_dns_challenge_error", "counter", 1) sentry.captureException() current_app.logger.debug( f"Unable to resolve DNS challenge for change_id: {change_id}, account_id: " @@ -88,5 +83,4 @@ def dnstest(domain, token): print(f"[+] Deleted TXT Record in `{dns_provider.name}` provider") status = SUCCESS_METRIC_STATUS - metrics.send("dnstest", "counter", 1, metric_tags={"status": status}) print("[+] Done with ACME Tests.") diff --git a/lemur/dns_providers/util.py b/lemur/dns_providers/util.py index 9154cb92..cc8d9bb3 100644 --- a/lemur/dns_providers/util.py +++ b/lemur/dns_providers/util.py @@ -6,6 +6,7 @@ import dns.query import dns.resolver import re +from lemur.extensions import sentry from lemur.extensions import metrics @@ -94,6 +95,7 @@ def get_dns_records(domain, rdtype, nameserver): for record in rdata.strings: records.append(record.decode("utf-8")) except dns.exception.DNSException: + sentry.captureException() function = sys._getframe().f_code.co_name metrics.send(f"{function}.fail", "counter", 1) return records diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index e30d7ca6..4aa55cf8 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -3,11 +3,22 @@ import requests import json import sys +import lemur.common.utils as utils import lemur.dns_providers.util as dnsutil from flask import current_app from lemur.extensions import metrics, sentry +REQUIRED_VARIABLES = [ + "ACME_POWERDNS_APIKEYNAME", + "ACME_POWERDNS_APIKEY", + "ACME_POWERDNS_DOMAIN", +] + + +def _check_conf(): + utils.validate_conf(current_app, REQUIRED_VARIABLES) + class Zone: """ This class implements a PowerDNS zone in JSON. """ @@ -56,37 +67,37 @@ class Record: def get_zones(account_number): """Retrieve authoritative zones from the PowerDNS API and return a list""" - server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") + _check_conf() + server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost") path = f"/api/v1/servers/{server_id}/zones" zones = [] + function = sys._getframe().f_code.co_name + log_data = { + "function": function + } try: records = _get(path) - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "message": "Retrieved Zones Successfully" - } + log_data["message"] = "Retrieved Zones Successfully" current_app.logger.debug(log_data) - for record in records: - zone = Zone(record) - if zone.kind == 'Master': - zones.append(zone.name) - return zones except Exception as e: - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "message": "Failed to Retrieve Zone Data" - } + sentry.captureException() + log_data["message"] = "Failed to Retrieve Zone Data" current_app.logger.debug(log_data) raise + for record in records: + zone = Zone(record) + if zone.kind == 'Master': + zones.append(zone.name) + return zones + def create_txt_record(domain, token, account_number): """ Create a TXT record for the given domain and token and return a change_id tuple """ + _check_conf() zone_name = _get_zone_name(domain, account_number) - server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") + server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost") zone_id = zone_name + "." domain_id = domain + "." path = f"/api/v1/servers/{server_id}/zones/{zone_id}" @@ -107,26 +118,20 @@ def create_txt_record(domain, token, account_number): } ] } - + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "token": token, + } try: _patch(path, payload) - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "fqdn": domain, - "token": token, - "message": "TXT record successfully created" - } + log_data["message"] = "TXT record successfully created" current_app.logger.debug(log_data) except Exception as e: - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "domain": domain, - "token": token, - "Exception": e, - "message": "Unable to create TXT record" - } + sentry.captureException() + log_data["Exception"] = e + log_data["message"] = "Unable to create TXT record" current_app.logger.debug(log_data) change_id = (domain, token) @@ -138,8 +143,9 @@ def wait_for_dns_change(change_id, account_number=None): Checks the authoritative DNS Server to see if changes have propagated to DNS Retries and waits until successful. """ + _check_conf() domain, token = change_id - number_of_attempts = current_app.config.get("ACME_POWERDNS_RETRIES", "") + number_of_attempts = current_app.config.get("ACME_POWERDNS_RETRIES", "3") zone_name = _get_zone_name(domain, account_number) nameserver = dnsutil.get_authoritative_nameserver(zone_name) record_found = False @@ -166,13 +172,13 @@ def wait_for_dns_change(change_id, account_number=None): metrics.send(f"{function}.success", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token}) else: metrics.send(f"{function}.fail", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token}) - sentry.captureException(extra={"fqdn": str(domain), "txt_record": str(token)}) def delete_txt_record(change_id, account_number, domain, token): """ Delete the TXT record for the given domain and token """ + _check_conf() zone_name = _get_zone_name(domain, account_number) - server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "") + server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost") zone_id = zone_name + "." domain_id = domain + "." path = f"/api/v1/servers/{server_id}/zones/{zone_id}" @@ -193,33 +199,27 @@ def delete_txt_record(change_id, account_number, domain, token): } ] } - + function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "token": token + } try: _patch(path, payload) - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "fqdn": domain, - "token": token, - "message": "TXT record successfully deleted" - } + log_data["message"] = "TXT record successfully deleted" current_app.logger.debug(log_data) except Exception as e: - function = sys._getframe().f_code.co_name - log_data = { - "function": function, - "domain": domain, - "token": token, - "Exception": e, - "message": "Unable to delete TXT record" - } + sentry.captureException() + log_data["Exception"] = e + log_data["message"] = "Unable to delete TXT record" current_app.logger.debug(log_data) def _generate_header(): """Generate a PowerDNS API header and return it as a dictionary""" - api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME", "") - api_key = current_app.config.get("ACME_POWERDNS_APIKEY", "") + api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME") + api_key = current_app.config.get("ACME_POWERDNS_APIKEY") headers = {api_key_name: api_key} return headers @@ -241,7 +241,7 @@ def _get_zone_name(domain, account_number): def _get(path, params=None): """ Execute a GET request on the given URL (base_uri + path) and return response as JSON object """ - base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") + base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN") resp = requests.get( f"{base_uri}{path}", headers=_generate_header(), @@ -254,7 +254,7 @@ def _get(path, params=None): def _patch(path, payload): """ Execute a Patch request on the given URL (base_uri + path) with given payload """ - base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "") + base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN") resp = requests.patch( f"{base_uri}{path}", data=json.dumps(payload), diff --git a/lemur/plugins/lemur_acme/tests/test_powerdns.py b/lemur/plugins/lemur_acme/tests/test_powerdns.py index 4c483741..c8b0a11e 100644 --- a/lemur/plugins/lemur_acme/tests/test_powerdns.py +++ b/lemur/plugins/lemur_acme/tests/test_powerdns.py @@ -31,6 +31,7 @@ class TestPowerdns(unittest.TestCase): {'account': '', 'dnssec': 'False', 'id': 'test.example.com.', 'kind': 'Master', 'last_check': 0, 'masters': [], 'name': 'test.example.com.', 'notified_serial': '2019112501', 'serial': '2019112501', 'url': '/api/v1/servers/localhost/zones/test.example.com.'}] + powerdns._check_conf = Mock() powerdns._get = Mock(path) powerdns._get.side_effect = [get_response] mock_current_app.config.get = Mock(return_value="localhost") @@ -53,6 +54,7 @@ class TestPowerdns(unittest.TestCase): token = "ABCDEFGHIJ" account_number = "1234567890" change_id = (domain, token) + powerdns._check_conf = Mock() powerdns._get_zone_name = Mock(return_value=zone) mock_current_app.logger.debug = Mock() mock_current_app.config.get = Mock(return_value="localhost") @@ -77,8 +79,8 @@ class TestPowerdns(unittest.TestCase): zone_name = "test.example.com" nameserver = "1.1.1.1" change_id = (domain, token) + powerdns._check_conf = Mock() mock_records = (token,) - mock_current_app.config.get = Mock(return_value=1) powerdns._get_zone_name = Mock(return_value=zone_name) mock_dnsutil.get_authoritative_nameserver = Mock(return_value=nameserver) @@ -103,6 +105,7 @@ class TestPowerdns(unittest.TestCase): token = "ABCDEFGHIJ" account_number = "1234567890" change_id = (domain, token) + powerdns._check_conf = Mock() powerdns._get_zone_name = Mock(return_value=zone) mock_current_app.logger.debug = Mock() mock_current_app.config.get = Mock(return_value="localhost") From 48bccd6f68f539dbfb1a8be8f303486de6cdf28a Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Mon, 3 Feb 2020 19:08:28 -0800 Subject: [PATCH 18/22] moving _check_config() lower in file, near other private methods --- lemur/plugins/lemur_acme/powerdns.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 4aa55cf8..39793eaf 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -16,10 +16,6 @@ REQUIRED_VARIABLES = [ ] -def _check_conf(): - utils.validate_conf(current_app, REQUIRED_VARIABLES) - - class Zone: """ This class implements a PowerDNS zone in JSON. """ @@ -216,6 +212,10 @@ def delete_txt_record(change_id, account_number, domain, token): current_app.logger.debug(log_data) +def _check_conf(): + utils.validate_conf(current_app, REQUIRED_VARIABLES) + + def _generate_header(): """Generate a PowerDNS API header and return it as a dictionary""" api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME") From 8ea54d7db2ead9af179c224bf8bf2587eb05260a Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Tue, 4 Feb 2020 14:50:56 -0800 Subject: [PATCH 19/22] removing exception if domain zone not found. Logging the issue instead --- lemur/plugins/lemur_acme/powerdns.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 39793eaf..617dbf7a 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -234,8 +234,12 @@ def _get_zone_name(domain, account_number): zone_name = z if not zone_name: function = sys._getframe().f_code.co_name + log_data = { + "function": function, + "fqdn": domain, + "message": "No PowerDNS zone name found.", + } metrics.send(f"{function}.fail", "counter", 1) - raise Exception(f"No PowerDNS zone found for domain: {domain}") return zone_name From 5324290234fdb865a89fcdabe6c4811f0889060b Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Tue, 4 Feb 2020 16:23:53 -0800 Subject: [PATCH 20/22] updating documentation based on feedback --- docs/administration.rst | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/administration.rst b/docs/administration.rst index 252a7dec..8f055147 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -988,7 +988,7 @@ The following configuration properties are required to use the PowerDNS ACME Plu .. data:: ACME_POWERDNS_SERVERID :noindex: - This is the ServerID attribute of the PowerDNS API Server (i.e. "localhost" + This is the ServerID attribute of the PowerDNS API Server (i.e. "localhost") .. data:: ACME_POWERDNS_APIKEYNAME @@ -1106,6 +1106,15 @@ All commands default to `~/.lemur/lemur.conf.py` if a configuration is not speci lemur notify +.. data:: acme + + Handles all ACME related tasks, like ACME plugin testing. + + :: + + lemur acme + + Sub-commands ------------ From bcdb3173bd0b08e0808955d534f745df4176652d Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Tue, 4 Feb 2020 18:23:17 -0800 Subject: [PATCH 21/22] ensuring that "3" is set as an integer instead of a string --- lemur/plugins/lemur_acme/powerdns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/plugins/lemur_acme/powerdns.py b/lemur/plugins/lemur_acme/powerdns.py index 617dbf7a..f3ad9965 100644 --- a/lemur/plugins/lemur_acme/powerdns.py +++ b/lemur/plugins/lemur_acme/powerdns.py @@ -141,7 +141,7 @@ def wait_for_dns_change(change_id, account_number=None): """ _check_conf() domain, token = change_id - number_of_attempts = current_app.config.get("ACME_POWERDNS_RETRIES", "3") + number_of_attempts = current_app.config.get("ACME_POWERDNS_RETRIES", 3) zone_name = _get_zone_name(domain, account_number) nameserver = dnsutil.get_authoritative_nameserver(zone_name) record_found = False From ca8e73286f6f0ade5ffb130e276397e9d6b96273 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Wed, 12 Feb 2020 15:10:24 -0800 Subject: [PATCH 22/22] fixed get_domains() to remove duplicate entries, updated usage and tests --- lemur/plugins/lemur_acme/plugin.py | 14 ++++---------- lemur/plugins/lemur_acme/tests/test_acme.py | 21 +++++++++++++++++++-- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index 8991efdf..95689a13 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -254,8 +254,9 @@ class AcmeHandler(object): domains = [options["common_name"]] if options.get("extensions"): - for name in options["extensions"]["sub_alt_names"]["names"]: - domains.append(name) + for dns_name in options["extensions"]["sub_alt_names"]["names"]: + if dns_name.value not in domains: + domains.append(dns_name.value) current_app.logger.debug("Got these domains: {0}".format(domains)) return domains @@ -640,15 +641,8 @@ class ACMEIssuerPlugin(IssuerPlugin): domains = self.acme.get_domains(issuer_options) if not create_immediately: # Create pending authorizations that we'll need to do the creation - authz_domains = [] - for d in domains: - if type(d) == str: - authz_domains.append(d) - else: - authz_domains.append(d.value) - dns_authorization = authorization_service.create( - account_number, authz_domains, provider_type + account_number, domains, provider_type ) # Return id of the DNS Authorization return None, None, dns_authorization.id diff --git a/lemur/plugins/lemur_acme/tests/test_acme.py b/lemur/plugins/lemur_acme/tests/test_acme.py index 04997ace..990a556e 100644 --- a/lemur/plugins/lemur_acme/tests/test_acme.py +++ b/lemur/plugins/lemur_acme/tests/test_acme.py @@ -1,4 +1,6 @@ import unittest + +from cryptography.x509 import DNSName from requests.models import Response from mock import MagicMock, Mock, patch @@ -74,12 +76,14 @@ class TestAcme(unittest.TestCase): @patch("acme.client.Client") @patch("lemur.plugins.lemur_acme.plugin.current_app") @patch("lemur.plugins.lemur_acme.cloudflare.wait_for_dns_change") + @patch("time.sleep") def test_complete_dns_challenge_success( - self, mock_wait_for_dns_change, mock_current_app, mock_acme + self, mock_sleep, mock_wait_for_dns_change, mock_current_app, mock_acme ): mock_dns_provider = Mock() mock_dns_provider.wait_for_dns_change = Mock(return_value=True) mock_authz = Mock() + mock_sleep.return_value = False mock_authz.dns_challenge.response = Mock() mock_authz.dns_challenge.response.simple_verify = Mock(return_value=True) mock_authz.authz = [] @@ -179,7 +183,7 @@ class TestAcme(unittest.TestCase): options = { "common_name": "test.netflix.net", "extensions": { - "sub_alt_names": {"names": ["test2.netflix.net", "test3.netflix.net"]} + "sub_alt_names": {"names": [DNSName("test2.netflix.net"), DNSName("test3.netflix.net")]} }, } result = self.acme.get_domains(options) @@ -187,6 +191,19 @@ class TestAcme(unittest.TestCase): result, [options["common_name"], "test2.netflix.net", "test3.netflix.net"] ) + @patch("lemur.plugins.lemur_acme.plugin.current_app") + def test_get_domains_san(self, mock_current_app): + options = { + "common_name": "test.netflix.net", + "extensions": { + "sub_alt_names": {"names": [DNSName("test.netflix.net"), DNSName("test2.netflix.net")]} + }, + } + result = self.acme.get_domains(options) + self.assertEqual( + result, [options["common_name"], "test2.netflix.net"] + ) + @patch( "lemur.plugins.lemur_acme.plugin.AcmeHandler.start_dns_challenge", return_value="test",