Skip to content

Commit

Permalink
DNSResolver: Fix use of nameservers with ports
Browse files Browse the repository at this point in the history
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 <twoerner@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
  • Loading branch information
t-woerner authored and flo-renaud committed Aug 16, 2022
1 parent 21091c2 commit 7780358
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 0 deletions.
41 changes: 41 additions & 0 deletions ipapython/dnsutil.py
Expand Up @@ -144,6 +144,47 @@ def read_resolv_conf(self, *args, **kwargs):
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'}
Expand Down
40 changes: 40 additions & 0 deletions ipatests/test_ipapython/test_dnsutil.py
Expand Up @@ -101,3 +101,43 @@ def test_prio(self):
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")

0 comments on commit 7780358

Please sign in to comment.