From cf2c46e612cca80ff9748783f61367fad4efbbe1 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 30 Aug 2014 18:27:21 +0200 Subject: [PATCH] use FQDN typed params in api, remove unused parse_name (which was problematic anyway) --- conftest.py | 9 +-- nsupdate/api/_tests/test_api.py | 8 +-- nsupdate/api/views.py | 30 ++++----- nsupdate/main/_tests/test_dnstools.py | 21 +----- nsupdate/main/dnstools.py | 92 +++++++++------------------ nsupdate/main/models.py | 6 +- nsupdate/main/views.py | 2 +- 7 files changed, 61 insertions(+), 107 deletions(-) diff --git a/conftest.py b/conftest.py index a02f7a1..be07351 100644 --- a/conftest.py +++ b/conftest.py @@ -32,7 +32,7 @@ SECURE = False # TLS/SNI support on python 2.x sucks :( from django.utils.translation import activate -from nsupdate.main.dnstools import update_ns +from nsupdate.main.dnstools import update_ns, FQDN @pytest.yield_fixture(scope="function") @@ -43,13 +43,14 @@ def ddns_hostname(): """ hostname = "test%d" % randint(1000000000, 2000000000) yield hostname - update_ns(hostname + '.' + TESTDOMAIN, 'A', action='del') - update_ns(hostname + '.' + TESTDOMAIN, 'AAAA', action='del') + fqdn = FQDN(hostname, TESTDOMAIN) + update_ns(fqdn, 'A', action='del') + update_ns(fqdn, 'AAAA', action='del') @pytest.yield_fixture(scope="function") def ddns_fqdn(ddns_hostname): - yield ddns_hostname + '.' + TESTDOMAIN + yield FQDN(ddns_hostname, TESTDOMAIN) # Note: fixture must be "function" scope (default), see https://github.com/pelme/pytest_django/issues/33 diff --git a/nsupdate/api/_tests/test_api.py b/nsupdate/api/_tests/test_api.py index 982c7d9..6ee07dc 100644 --- a/nsupdate/api/_tests/test_api.py +++ b/nsupdate/api/_tests/test_api.py @@ -4,7 +4,7 @@ Tests for api package. from django.core.urlresolvers import reverse -from nsupdate.main.dnstools import query_ns +from nsupdate.main.dnstools import query_ns, FQDN from nsupdate.main.models import Domain from conftest import TESTDOMAIN, TEST_HOST, TEST_HOST2, TEST_SECRET, TEST_SECRET2 @@ -13,7 +13,7 @@ USERNAME = 'test' PASSWORD = 'pass' BASEDOMAIN = "nsupdate.info" -HOSTNAME = 'nsupdate-ddns-client-unittest.' + BASEDOMAIN +TEST_HOST_OTHER = FQDN('nsupdate-ddns-client-unittest', BASEDOMAIN) def test_myip(client): @@ -124,14 +124,14 @@ def test_nic_update_authorized_update_other_services(client): # must be good (was different IP) assert response.content == b'good 1.2.3.4' # now check if it updated the other service also: - assert query_ns(HOSTNAME, 'A') == '1.2.3.4' + assert query_ns(TEST_HOST_OTHER, 'A') == '1.2.3.4' response = client.get(reverse('nic_update') + '?myip=2.3.4.5', HTTP_AUTHORIZATION=make_basic_auth_header(TEST_HOST, TEST_SECRET)) assert response.status_code == 200 # must be good (was different IP) assert response.content == b'good 2.3.4.5' # now check if it updated the other service also: - assert query_ns(HOSTNAME, 'A') == '2.3.4.5' + assert query_ns(TEST_HOST_OTHER, 'A') == '2.3.4.5' def test_nic_update_authorized_badagent(client, settings): diff --git a/nsupdate/api/views.py b/nsupdate/api/views.py index d30535c..21a8158 100644 --- a/nsupdate/api/views.py +++ b/nsupdate/api/views.py @@ -224,9 +224,9 @@ class NicUpdateView(View): ipaddr = request.META.get('REMOTE_ADDR') secure = request.is_secure() if delete: - return _delete(host, hostname, ipaddr, secure, logger=logger) + return _delete(host, ipaddr, secure, logger=logger) else: - return _update(host, hostname, ipaddr, secure, logger=logger) + return _update(host, ipaddr, secure, logger=logger) class NicDeleteView(NicUpdateView): @@ -282,9 +282,9 @@ class AuthorizedNicUpdateView(View): ipaddr = request.META.get('REMOTE_ADDR') secure = request.is_secure() if delete: - return _delete(host, hostname, ipaddr, secure, logger=logger) + return _delete(host, ipaddr, secure, logger=logger) else: - return _update(host, hostname, ipaddr, secure, logger=logger) + return _update(host, ipaddr, secure, logger=logger) class AuthorizedNicDeleteView(AuthorizedNicUpdateView): @@ -300,12 +300,11 @@ class AuthorizedNicDeleteView(AuthorizedNicUpdateView): return super(AuthorizedNicDeleteView, self).get(request, logger=logger, delete=delete) -def _update(host, hostname, ipaddr, secure=False, logger=None): +def _update(host, ipaddr, secure=False, logger=None): """ common code shared by the 2 update views :param host: host object - :param hostname: hostname (fqdn) :param ipaddr: new ip addr (v4 or v6) :param secure: True if we use TLS/https :param logger: a logger object @@ -330,9 +329,10 @@ def _update(host, hostname, ipaddr, secure=False, logger=None): # some people manage to even give a non-ascii string instead of an ip addr return Response('dnserr') # there should be a better response code for this host.poke(kind, secure) + fqdn = host.get_fqdn() try: - update(hostname, ipaddr) - logger.info('%s - received good update -> ip: %s tls: %r' % (hostname, ipaddr, secure)) + update(fqdn, ipaddr) + logger.info('%s - received good update -> ip: %s tls: %r' % (fqdn, ipaddr, secure)) # now check if there are other services we shall relay updates to: for hc in host.serviceupdaterhostconfigs.all(): if (kind == 'ipv4' and hc.give_ipv4 and hc.service.accept_ipv4 @@ -351,23 +351,22 @@ def _update(host, hostname, ipaddr, secure=False, logger=None): logger.exception("the dyndns2 updater raised an exception [%r]" % kwargs) return Response('good %s' % ipaddr) except SameIpError: - logger.warning('%s - received no-change update, ip: %s tls: %r' % (hostname, ipaddr, secure)) + logger.warning('%s - received no-change update, ip: %s tls: %r' % (fqdn, ipaddr, secure)) host.register_client_fault() return Response('nochg %s' % ipaddr) except (DnsUpdateError, NameServerNotAvailable) as e: msg = str(e) logger.error('%s - received update that resulted in a dns error [%s], ip: %s tls: %r' % ( - hostname, msg, ipaddr, secure)) + fqdn, msg, ipaddr, secure)) host.register_server_fault() return Response('dnserr') -def _delete(host, hostname, ipaddr, secure=False, logger=None): +def _delete(host, ipaddr, secure=False, logger=None): """ common code shared by the 2 delete views :param host: host object - :param hostname: hostname (fqdn) :param ipaddr: ip addr (to determine record type A or AAAA) :param secure: True if we use TLS/https :param logger: a logger object @@ -392,15 +391,16 @@ def _delete(host, hostname, ipaddr, secure=False, logger=None): # some people manage to even give a non-ascii string instead of an ip addr return Response('dnserr') # there should be a better response code for this host.poke(kind, secure) + fqdn = host.get_fqdn() try: rdtype = 'A' if kind == 'ipv4' else 'AAAA' - delete(hostname, rdtype) - logger.info('%s - received delete for record %s, tls: %r' % (hostname, rdtype, secure)) + delete(fqdn, rdtype) + logger.info('%s - received delete for record %s, tls: %r' % (fqdn, rdtype, secure)) # XXX unclear what to do for "other services" we relay updates to return Response('deleted %s' % rdtype) except (DnsUpdateError, NameServerNotAvailable) as e: msg = str(e) logger.error('%s - received delete for record %s that resulted in a dns error [%s], tls: %r' % ( - hostname, rdtype, msg, secure)) + fqdn, rdtype, msg, secure)) host.register_server_fault() return Response('dnserr') diff --git a/nsupdate/main/_tests/test_dnstools.py b/nsupdate/main/_tests/test_dnstools.py index a7b0afc..1997a9d 100644 --- a/nsupdate/main/_tests/test_dnstools.py +++ b/nsupdate/main/_tests/test_dnstools.py @@ -10,12 +10,12 @@ pytestmark = pytest.mark.django_db from dns.resolver import NXDOMAIN, NoAnswer -from ..dnstools import (add, delete, update, query_ns, rev_lookup, parse_name, update_ns, +from ..dnstools import (add, delete, update, query_ns, rev_lookup, update_ns, SameIpError, DnsUpdateError, FQDN) # see also conftest.py BASEDOMAIN = 'nsupdate.info' -INVALID_HOST = 'test999.' + BASEDOMAIN # this can't get updated +INVALID_HOST = FQDN('test999', BASEDOMAIN) # this can't get updated class TestFQDN(object): @@ -128,23 +128,6 @@ class TestReverseLookup(object): class TestUpdate(object): - def test_parse1(self): - host, domain = 'test', BASEDOMAIN - origin, relname = parse_name(host + '.' + domain) - assert str(origin) == domain + '.' - assert str(relname) == host - - def test_parse2(self): - host, domain = 'prefix.test', BASEDOMAIN - origin, relname = parse_name(host + '.' + domain) - assert str(origin) == domain + '.' - assert str(relname) == 'prefix.test' - - def test_parse_with_origin(self): - origin, relname = parse_name('foo.bar.baz.org', 'bar.baz.org') - assert str(origin) == 'bar.baz.org' + '.' - assert str(relname) == 'foo' - def test_add_del_v4(self, ddns_fqdn): host, ip = ddns_fqdn, '1.1.1.1' remove_records(host) diff --git a/nsupdate/main/dnstools.py b/nsupdate/main/dnstools.py index 480fdcc..948438e 100644 --- a/nsupdate/main/dnstools.py +++ b/nsupdate/main/dnstools.py @@ -98,22 +98,22 @@ def check_ip(ipaddr, keys=('ipv4', 'ipv6')): return keys[af == dns.inet.AF_INET6] -def add(fqdn, ipaddr, ttl=60, origin=None): +def add(fqdn, ipaddr, ttl=60): """ intelligent dns adder - first does a lookup on the master server to find the current ip and only sends an 'add' if there is no such entry. otherwise send an 'upd' if the if we have a different ip. - :param fqdn: fully qualified domain name (str) + :param fqdn: fully qualified domain name (FQDN) :param ipaddr: new ip address :param ttl: time to live, default 60s (int) - :param origin: origin zone (optional, str) :raises: SameIpError if new and old IP is the same :raises: ValueError if ipaddr is no valid ip address string """ + assert isinstance(fqdn, FQDN) rdtype = check_ip(ipaddr, keys=('A', 'AAAA')) try: - current_ipaddr = query_ns(fqdn, rdtype, origin=origin) + current_ipaddr = query_ns(fqdn, rdtype) # check if ip really changed ok = ipaddr != current_ipaddr action = 'upd' @@ -125,43 +125,43 @@ def add(fqdn, ipaddr, ttl=60, origin=None): # only send an add/update if the ip really changed as the update # causes write I/O on the nameserver and also traffic to the # dns slaves (they get a notify if we update the zone). - update_ns(fqdn, rdtype, ipaddr, action=action, ttl=ttl, origin=origin) + update_ns(fqdn, rdtype, ipaddr, action=action, ttl=ttl) else: raise SameIpError -def delete(fqdn, rdtype=None, origin=None): +def delete(fqdn, rdtype=None): """ dns deleter - :param fqdn: fully qualified domain name (str) + :param fqdn: fully qualified domain name (FQDN) :param rdtype: 'A', 'AAAA' or None (deletes 'A' and 'AAAA') - :param origin: origin zone (optional, str) """ + assert isinstance(fqdn, FQDN) if rdtype is not None: assert rdtype in ['A', 'AAAA', ] rdtypes = [rdtype, ] else: rdtypes = ['A', 'AAAA'] for rdtype in rdtypes: - update_ns(fqdn, rdtype, action='del', origin=origin) + update_ns(fqdn, rdtype, action='del') -def update(fqdn, ipaddr, ttl=60, origin=None): +def update(fqdn, ipaddr, ttl=60): """ intelligent dns updater - first does a lookup on the master server to find the current ip and only sends a dynamic update if we have a different ip. - :param fqdn: fully qualified domain name (str) + :param fqdn: fully qualified domain name (FQDN) :param ipaddr: new ip address :param ttl: time to live, default 60s (int) - :param origin: origin zone (optional, str) :raises: SameIpError if new and old IP is the same :raises: ValueError if ipaddr is no valid ip address string """ + assert isinstance(fqdn, FQDN) rdtype = check_ip(ipaddr, keys=('A', 'AAAA')) try: - current_ipaddr = query_ns(fqdn, rdtype, origin=origin) + current_ipaddr = query_ns(fqdn, rdtype) # check if ip really changed ok = ipaddr != current_ipaddr except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer): @@ -171,27 +171,23 @@ def update(fqdn, ipaddr, ttl=60, origin=None): # only send an update if the ip really changed as the update # causes write I/O on the nameserver and also traffic to the # dns slaves (they get a notify if we update the zone). - update_ns(fqdn, rdtype, ipaddr, action='upd', ttl=ttl, origin=origin) + update_ns(fqdn, rdtype, ipaddr, action='upd', ttl=ttl) else: raise SameIpError -def query_ns(qname, rdtype, origin=None): +def query_ns(fqdn, rdtype): """ query a dns name from our master server - :param qname: the query name - :type qname: dns.name.Name object or str + :param fqdn: fqdn to query the name server for + :type fqdn: dnstools.FQDN :param rdtype: the query type :type rdtype: int or str - :param origin: origin zone - :type origin: str or None :return: IP (as str) :raises: see dns.resolver.Resolver.query """ - origin, name = parse_name(qname, origin) - fqdn = name + origin - assert fqdn.is_absolute() + assert isinstance(fqdn, FQDN) nameserver, origin = get_ns_info(fqdn)[0:2] resolver = dns.resolver.Resolver(configure=False) # we do not configure it from resolv.conf, but patch in the values we @@ -204,13 +200,13 @@ def query_ns(qname, rdtype, origin=None): # (used if flags = None is given). Thus, we explicitly give flags (all off): resolver.flags = 0 try: - answer = resolver.query(fqdn, rdtype) + answer = resolver.query(str(fqdn), rdtype) ip = str(list(answer)[0]) - logger.debug("query: %s answer: %s" % (fqdn.to_text(), ip)) + logger.debug("query: %s answer: %s" % (fqdn, ip)) return ip except (dns.resolver.Timeout, dns.resolver.NoNameservers): # socket.error also? logger.warning("timeout when querying for name '%s' in zone '%s' with rdtype '%s'." % ( - name, origin, rdtype)) + fqdn.host, origin, rdtype)) set_ns_availability(origin, False) raise @@ -233,53 +229,27 @@ def rev_lookup(ipaddr): return name -def parse_name(fqdn, origin=None): +def get_ns_info(fqdn): """ - Parse a fully qualified domain name into a relative name - and a origin zone. Please note that the origin return value will - have a trailing dot. - - :param fqdn: fully qualified domain name (str) - :param origin: origin zone (optional, str) - :return: origin, relative name (both dns.name.Name) - """ - fqdn = dns.name.from_text(fqdn) - if origin is None: - origin = dns.resolver.zone_for_name(fqdn) - rel_name = fqdn.relativize(origin) - else: - origin = dns.name.from_text(origin) - rel_name = fqdn - origin - return origin, rel_name - - -def get_ns_info(fqdn, origin=None): - """ - Get the master nameserver for the zone, the key secret needed - to update the zone and the key algorithm used. + Get the master nameserver for fqdn, the key secret needed to update the zone and the key algorithm used. :param fqdn: the fully qualified hostname we are dealing with (str) - :param origin: zone we are dealing with, must be with trailing dot (default:autodetect) (str) :return: master nameserver, origin, domain, update keyname, update secret, update algo :raises: NameServerNotAvailable if ns was flagged unavailable in the db """ - fqdn_str = str(fqdn) - origin, name = parse_name(fqdn_str, origin) - origin_str = str(origin) + assert isinstance(fqdn, FQDN) from .models import Domain try: # first we check if we have an entry for the fqdn # single-host update secret use case # XXX we need 2 DB accesses for the usual case just to support this rare case - domain = fqdn_str.rstrip('.') + domain = str(fqdn) d = Domain.objects.get(domain=domain) - keyname = fqdn_str except Domain.DoesNotExist: # now check the base zone, the usual case # zone update secret use case - domain = origin_str.rstrip('.') + domain = fqdn.domain d = Domain.objects.get(domain=domain) - keyname = origin_str if not d.available: if d.last_update + timedelta(seconds=UNAVAILABLE_RETRY) > now(): # if there are troubles with a nameserver, we set available=False @@ -290,24 +260,24 @@ def get_ns_info(fqdn, origin=None): # retry timeout is over, set it available again set_ns_availability(domain, True) algorithm = getattr(dns.tsig, d.nameserver_update_algorithm) - return d.nameserver_ip, origin, domain, name, keyname, d.nameserver_update_secret, algorithm + return d.nameserver_ip, fqdn.domain, domain, fqdn.host, domain, d.nameserver_update_secret, algorithm -def update_ns(fqdn, rdtype='A', ipaddr=None, origin=None, action='upd', ttl=60): +def update_ns(fqdn, rdtype='A', ipaddr=None, action='upd', ttl=60): """ update the master server - :param fqdn: the fully qualified domain name to update (str) + :param fqdn: the fully qualified domain name to update (FQDN) :param rdtype: the record type (default: 'A') (str) :param ipaddr: ip address (v4 or v6), if needed (str) - :param origin: the origin zone to update (default; autodetect) (str) :param action: 'add', 'del' or 'upd' :param ttl: time to live for the added/updated resource, default 60s (int) :return: dns response :raises: DnsUpdateError, Timeout """ + assert isinstance(fqdn, FQDN) assert action in ['add', 'del', 'upd', ] - nameserver, origin, domain, name, keyname, key, algo = get_ns_info(fqdn, origin) + nameserver, origin, domain, name, keyname, key, algo = get_ns_info(fqdn) upd = dns.update.Update(origin, keyring=dns.tsigkeyring.from_text({keyname: key}), keyalgorithm=algo) diff --git a/nsupdate/main/models.py b/nsupdate/main/models.py index a417066..d61e44e 100644 --- a/nsupdate/main/models.py +++ b/nsupdate/main/models.py @@ -181,7 +181,7 @@ class Host(models.Model): index_together = (('subdomain', 'domain'), ) def get_fqdn(self): - return '%s.%s' % (self.subdomain, self.domain.domain) + return dnstools.FQDN(self.subdomain, self.domain.domain) @classmethod def get_by_fqdn(cls, fqdn, **kwargs): @@ -202,7 +202,7 @@ class Host(models.Model): def get_ip(self, kind): record = 'A' if kind == 'ipv4' else 'AAAA' try: - return dnstools.query_ns(self.get_fqdn(), record, origin=self.domain.domain) + return dnstools.query_ns(self.get_fqdn(), record) except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer): return 'none' except (dns.resolver.NoNameservers, dns.resolver.Timeout, dnstools.NameServerNotAvailable): @@ -250,7 +250,7 @@ class Host(models.Model): def pre_delete_host(sender, **kwargs): obj = kwargs['instance'] try: - dnstools.delete(obj.get_fqdn(), origin=obj.domain.domain) + dnstools.delete(obj.get_fqdn()) except (dnstools.Timeout, dnstools.NameServerNotAvailable): # well, we tried to clean up, but we didn't reach the nameserver pass diff --git a/nsupdate/main/views.py b/nsupdate/main/views.py index 187e695..5560e3f 100644 --- a/nsupdate/main/views.py +++ b/nsupdate/main/views.py @@ -194,7 +194,7 @@ class OverviewView(CreateView): def form_valid(self, form): self.object = form.save(commit=False) try: - dnstools.add(self.object.get_fqdn(), self.request.META['REMOTE_ADDR'], origin=self.object.domain.domain) + dnstools.add(self.object.get_fqdn(), self.request.META['REMOTE_ADDR']) except dnstools.Timeout: success, level, msg = False, messages.ERROR, 'Timeout - communicating to name server failed.' except dnstools.NameServerNotAvailable: