Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 Jul 2018 20:38:16 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: getaddrinfo(3) / AI_ADDRCONFIG

On Tue, Jul 10, 2018 at 07:30:05PM -0400, Christopher Friedt wrote:
> On Tue, Jul 10, 2018 at 7:21 PM Christopher Friedt
> <chrisfriedt@...il.com> wrote:
> > test.c, output, strace log, and ifconfig -a output here:
> >
> > https://pastebin.com/UmJi02px
> 
> So it's definitely returning an IPv4 socket (which Thrift throws away
> in favour of the IPv6 socket). Since no adapter has the IPv6 address
> returned,  bind(2) would fail on a subsequent call.

Yes, musl seems to be behaving as expected here, and it's thrift
that's choosing to use the first v6 result, or the last result if
there are no v6 results. Even ignoring issues with inability to use
ipv6, this is probably not desirable on v4-only systems since it may
choose the worst, rather than the best, result, if the order was
meaningful to begin with.

> After applying the patch [1] to musl,
> 
> # ./test -v
> struct addrinfo {
>    ai_flags:        0
>    ai_family:       2
>    ai_socktype:     1
>    ai_protocol:     6
>    ai_addrlen:      16
>    ai_addr:
>       family:       2
>       addr:         127.0.0.1
>       port:         0
>    ai_canonname:    localhost
>    ai_next:         0
> }
> 
> [1] https://patch-diff.githubusercontent.com/raw/cfriedt/musl/pull/1.diff

This patch isn't acceptable as-is, and I don't think it would even fix
your problem if you just did "ip addr del ::1 dev lo" but had an ipv6
address available on some other interface. The only thing that would
help is having AI_ADDRCONFIG omit non-routable results, which is
"morally" what it should do, but contrary to what it's specified to
do.

Maybe you're okay with saying "having some ipv6 addresses but not
supporting ::1 is a pathological configuration and doesn't need to be
supported". If so, I'm happy to have AI_ADDRCONFIG check whether ::1
is routable, and disable IPv6 if it's not. This would basically look
like:

	if (family != AF_INET && (flags & AI_ADDRCONFIG)) {
		/* attempt to connect a UDP socket to ::1 */
		/* ... */
		if (failed && errno==ENETUNREACH) {
			if (family == AF_INET6) return EAI_NONAME;
			family = AF_INET;
		}
	}

Regardless of what's done on the musl side, I think it would make
sense for thrift to change its strategy for selecting an address to
use. If IPv6 is supported, getaddrinfo should always return ::1 before
127.0.0.1 (see RFC 3484/6724), so just using the first address
returned should give the best behavior. For addresses other than
localhost, I'm not sure what you want to happen, but I think if the
hostname has both IPv4 and IPv6 addresses, just binding to the v6 one
will _not_ get you the ability to accept v4 connections (since the
address is different) and you need to bind to both. In general the
correct pattern for listening with getaddrinfo is probably to attempt
to bind each of the results.

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.