Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Cannot pass nameservers with domain input when /etc/resolv.conf has a loopback address #436

Closed
phillip-stephens opened this issue Sep 5, 2024 · 2 comments · Fixed by #437
Assignees
Labels

Comments

@phillip-stephens
Copy link
Contributor

Describe the bug
Running echo "google.com,1.1.1.1" | ./zdns A will fail if /etc/resolv.conf contains a loopback address

cat /etc/resolv.conf
# This is /run/systemd/resolve/stub-resolv.conf managed by man:systemd-resolved(8).
# Do not edit.

...

nameserver 127.0.0.53
options edns0 trust-ad
search .

$ echo "google.com,1.1.1.1" | ./zdns A
{"name":"google.com","results":{"A":{"data":null,"duration":0.00002757,"error":"cannot mix loopback and non-loopback addresses","status":"ILLEGAL_INPUT","timestamp":"2024-09-05T14:31:33Z"}}}

To Reproduce

ZDNS Version

Use main branch zdns

CLI Command

echo "google.com,1.1.1.1" | ./zdns A

/etc/resolv.conf contents

# This is /run/systemd/resolve/stub-resolv.conf managed by man:systemd-resolved(8).
# Do not edit.
#
# This file might be symlinked as /etc/resolv.conf. If you're looking at
# /etc/resolv.conf and seeing this text, you have followed the symlink.
#
# This is a dynamic resolv.conf file for connecting local clients to the
# internal DNS stub resolver of systemd-resolved. This file lists all
# configured search domains.
#
# Run "resolvectl status" to see details about the uplink DNS servers
# currently in use.
#
# Third party programs should typically not access this file directly, but only
# through the symlink at /etc/resolv.conf. To manage man:resolv.conf(5) in a
# different way, replace this symlink by a static file or a different symlink.
#
# See man:systemd-resolved.service(8) for details about the supported modes of
# operation for /etc/resolv.conf.

nameserver 127.0.0.53
options edns0 trust-ad
search .

Expected behavior
Passing nameservers in as input should override whatever defaults ZDNS was setup with and should work

@phillip-stephens
Copy link
Contributor Author

I had forgotten that we could pass in nameservers like this and there was no integration test for it. The issue is when the user does not supply --name-servers and is not, in --iterative mode, we use the OS defaults to determine the recursive resolver. In this case, that resolver is a loopback address so the Resolver struct is populated with a loopback localAddr. When we go to actually process user input and we're given an internet-routable IP, we cannot connect to it and error out.

Current main has the Resolver keeping 2 connection objects:

type ConnectionInfo struct {
	udpClient *dns.Client
	tcpClient *dns.Client
	conn      *dns.Conn
	localAddr net.IP
}

type Resolver struct {
...
	connInfoIPv4 *ConnectionInfo
	connInfoIPv6 *ConnectionInfo

We should add

type Resolver struct {
...
	connInfoIPv4Internet *ConnectionInfo
	connInfoIPv6Internet *ConnectionInfo
        connInfoIPv4Loopback *ConnectionInfo
	connInfoIPv6Loopback *ConnectionInfo

and only populate them on-demand. This way if we never need a loopback UDP socket, we won't occupy one unnecessarily. This is the only way I can think of to handle this since we don't know at Resolver instantiation what nameservers the user is going to request. We just need to be able to handle them either way.

@phillip-stephens
Copy link
Contributor Author

Changes needed:

  • Integration test - echo "zdns-testing.com,1.1.1.1" | ./zdns A
  • Integration test - echo "zdns-testing.com,1.1.1.1" | ./zdns A --name-servers=127.0.0.1
  • Integration test - echo "zdns-testing.com,1.1.1.1" | ./zdns A --name-servers=8.8.8.8 check the correct NS was used
  • Add "internet-facing" and "loopback" connection objects to each Resolver
    • Init. on-demand rather than during Resolver creation
    • Remove all the checks around preventing loopback addresses/nameservers mixing with non-loopback since we can no longer assume we'll only process one or the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant