From 77803587d6e15b41a66fc0ee0a87ad55ee196dfe Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Wed, 3 Aug 2022 18:22:47 +0200 Subject: [PATCH] DNSResolver: Fix use of nameservers with ports IPA DNS zone and forwardzone commands allow to use nameservers with ports as "SERVER_IP port PORT_NUMBER". bind is supporting this syntax, but the Resolver in dnspython that is used to verify the list of forwarders (nameservers) is only allowing to have IP addresses in this list. With dnspython version 2.20 there is a new validator in dns.resolver.BaseResolver that ensures this. Refs: - https://bind9.readthedocs.io/en/v9_18_4/reference.html#zone-statement-grammar - https://github.com/rthalley/dnspython/blob/master/dns/resolver.py#L1094 ipapython/dnsutil.DNSResolver derives from dns.resolver.Resolver. The setter for nameservers has been overloaded in the DNSResolver class to split out the port numbers into the nameserver_ports dict { SERVER_IP: PORT_NUMBER }. After the setter for nameservers succeeded, nameserver_ports is set. nameserver_ports is used in the resolve() method of dns.resolver.Resolver. Additional tests have been added to verify that nameservers and also nameserver_ports are properly set and also valid. Fixes: https://pagure.io/freeipa/issue/9158 Signed-off-by: Thomas Woerner Reviewed-By: Alexander Bokovoy --- ipapython/dnsutil.py | 41 +++++++++++++++++++++++++ ipatests/test_ipapython/test_dnsutil.py | 40 ++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/ipapython/dnsutil.py b/ipapython/dnsutil.py index 4baeaf8cc..58de365ab 100644 --- a/ipapython/dnsutil.py +++ b/ipapython/dnsutil.py @@ -144,6 +144,47 @@ class DNSResolver(dns.resolver.Resolver): nameservers.remove(ipv4_loopback) self.nameservers = nameservers + @dns.resolver.Resolver.nameservers.setter + def nameservers(self, nameservers): + """ + *nameservers*, a ``list`` of nameservers with optional ports: + "SERVER_IP port PORT_NUMBER". + + Overloads dns.resolver.Resolver.nameservers setter to split off ports + into nameserver_ports after setting nameservers successfully with the + setter in dns.resolver.Resolver. + """ + # Get nameserver_ports if it is already set + if hasattr(self, "nameserver_ports"): + nameserver_ports = self.nameserver_ports + else: + nameserver_ports = {} + + # Check nameserver items in list and split out converted port number + # into nameserver_ports: { nameserver: port } + if isinstance(nameservers, list): + _nameservers = [] + for nameserver in nameservers: + splits = nameserver.split() + if len(splits) == 3 and splits[1] == "port": + nameserver = splits[0] + try: + port = int(splits[2]) + if port < 0 or port > 65535: + raise ValueError() + except ValueError: + raise ValueError( + "invalid nameserver: %s is not a valid port" % + splits[2]) + nameserver_ports[nameserver] = port + _nameservers.append(nameserver) + nameservers = _nameservers + + # Call dns.resolver.Resolver.nameservers setter + dns.resolver.Resolver.nameservers.__set__(self, nameservers) + # Set nameserver_ports after successfull call to setter + self.nameserver_ports = nameserver_ports + class DNSZoneAlreadyExists(dns.exception.DNSException): supp_kwargs = {'zone', 'ns'} diff --git a/ipatests/test_ipapython/test_dnsutil.py b/ipatests/test_ipapython/test_dnsutil.py index 5e7a46197..09463c69d 100644 --- a/ipatests/test_ipapython/test_dnsutil.py +++ b/ipatests/test_ipapython/test_dnsutil.py @@ -101,3 +101,43 @@ class TestSortURI: assert dnsutil.sort_prio_weight([h3, h2, h1]) == [h1, h2, h3] assert dnsutil.sort_prio_weight([h3, h3, h3]) == [h3] assert dnsutil.sort_prio_weight([h2, h2, h1, h1]) == [h1, h2] + + +class TestDNSResolver: + def test_nameservers(self): + res = dnsutil.DNSResolver() + res.nameservers = ["4.4.4.4", "8.8.8.8"] + assert res.nameservers == ["4.4.4.4", "8.8.8.8"] + + def test_nameservers_with_ports(self): + res = dnsutil.DNSResolver() + res.nameservers = ["4.4.4.4 port 53", "8.8.8.8 port 8053"] + assert res.nameservers == ["4.4.4.4", "8.8.8.8"] + assert res.nameserver_ports == {"4.4.4.4": 53, "8.8.8.8": 8053} + + res.nameservers = ["4.4.4.4 port 53", "8.8.8.8 port 8053"] + assert res.nameservers == ["4.4.4.4", "8.8.8.8"] + assert res.nameserver_ports == {"4.4.4.4": 53, "8.8.8.8": 8053} + + def test_nameservers_with_bad_ports(self): + res = dnsutil.DNSResolver() + try: + res.nameservers = ["4.4.4.4 port a"] + except ValueError: + pass + else: + pytest.fail("No fail on bad port a") + + try: + res.nameservers = ["4.4.4.4 port -1"] + except ValueError: + pass + else: + pytest.fail("No fail on bad port -1") + + try: + res.nameservers = ["4.4.4.4 port 65536"] + except ValueError: + pass + else: + pytest.fail("No fail on bad port 65536") -- 2.37.1