From 3080a9527c8ea539da8f767d0a735e6b53e8e0fe Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Fri, 17 Jan 2020 18:29:37 -0800 Subject: [PATCH 01/15] 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 02/15] 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 03/15] 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 04/15] 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 05/15] 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 06/15] 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 07/15] 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 08/15] 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 09/15] 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 10/15] 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 11/15] 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 53f81fb09f6ce6d8bb2e1522c78b8e9c694abe55 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Mon, 3 Feb 2020 18:58:31 -0800 Subject: [PATCH 12/15] 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 13/15] 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 14/15] 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 bcdb3173bd0b08e0808955d534f745df4176652d Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Tue, 4 Feb 2020 18:23:17 -0800 Subject: [PATCH 15/15] 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