Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 26 Aug 2022 12:16:03 -0400
From: Rich Felker <dalias@...c.org>
To: Markus Wichmann <nullplan@....net>
Cc: musl@...ts.openwall.com
Subject: Re: IPv4 fallback in __res_msend_rc not functional

On Thu, Aug 25, 2022 at 09:26:13AM -0400, Rich Felker wrote:
> On Thu, Aug 25, 2022 at 04:58:59AM +0200, Markus Wichmann wrote:
> > On Wed, Aug 24, 2022 at 07:32:28PM -0400, Rich Felker wrote:
> > > Does this work?
> > >
> > > diff --git a/src/network/res_msend.c b/src/network/res_msend.c
> > > index 3e018009..105bf598 100644
> > > --- a/src/network/res_msend.c
> > > +++ b/src/network/res_msend.c
> > > @@ -68,14 +68,15 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries,
> > >  	}
> > >
> > >  	/* Get local address and open/bind a socket */
> > > -	sa.sin.sin_family = family;
> > >  	fd = socket(family, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0);
> > >
> > >  	/* Handle case where system lacks IPv6 support */
> > >  	if (fd < 0 && family == AF_INET6 && errno == EAFNOSUPPORT) {
> > >  		fd = socket(AF_INET, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0);
> > >  		family = AF_INET;
> > > +		sl = sizeof sa.sin;
> > >  	}
> > > +	sa.sin.sin_family = family;
> > >  	if (fd < 0 || bind(fd, (void *)&sa, sl) < 0) {
> > >  		if (fd >= 0) close(fd);
> > >  		pthread_setcancelstate(cs, 0);
> > >
> > 
> > That would have been my proposal as well, although I would have added a
> > "if (ns[j].sin.sin_family == family)" in front of the sendto call as
> > well.
> 
> I'd thought about adapting the loop that converts v4 addresses to v6
> to operate in both directions and delete v6 addresses from the list,
> but that's a lot more work and more error-prone. I guess we could do
> your check but it doesn't really matter, and in some sense the
> failures might be "nice to see" in strace to show that the system is
> misconfigured (can't send to the requested v6 nameserver). But..
> 
> > Another question to think about is if the function should terminate
> > early if there are no usable servers in the config. Without the IPv4
> > fallback, this could only happen with conf->nns == 0, but with it, it
> > can also happen when all configured servers are IPv6 and IPv6 is
> > unusable (which the callers can't know). In that case, the function
> > would now not send anything (as sendto() fails silently), and wait in
> > vain for a response.
> 
> Yes.. failing here would require running through the list and checking
> that there's at least one matching family, then figuring out what
> error code to return when there's not. I guess it would be EAI_SYSTEM
> with errno=EAFNOSUPPORT or something like that, or perhaps just
> ENOENT. The case where resolv.conf exists but has no nameservers
> (nns=0) is also not really handled right now and I'm not sure if the
> intent was for it to fail or behave as if resolv.conf was missing.
> Probably it should fail with something like ENOENT, or just continuing
> to timeout as it does now I guess..?

How does this look for handling the cases with no (usable)
nameservers?

diff --git a/src/network/res_msend.c b/src/network/res_msend.c
index 105bf598..1e5f3516 100644
--- a/src/network/res_msend.c
+++ b/src/network/res_msend.c
@@ -47,6 +47,11 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries,
 	struct pollfd pfd;
 	unsigned long t0, t1, t2;
 
+	if (!conf->nns) {
+		errno = ENOENT;
+		return -1;
+	}
+
 	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
 
 	timeout = 1000*conf->timeout;
@@ -72,6 +77,11 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries,
 
 	/* Handle case where system lacks IPv6 support */
 	if (fd < 0 && family == AF_INET6 && errno == EAFNOSUPPORT) {
+		for (i=0; i<nns && conf->ns[nns].family == AF_INET6; i++);
+		if (i==nns) {
+			pthread_setcancelstate(cs, 0);
+			return -1;
+		}
 		fd = socket(AF_INET, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0);
 		family = AF_INET;
 		sl = sizeof sa.sin;

__res_msend returning -1 will cause getaddrinfo to fail with
EAI_SYSTEM, in which case the caller can find the reason in errno.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.