Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221019142452.GL29905@brightrain.aerifal.cx>
Date: Wed, 19 Oct 2022 10:24:54 -0400
From: Rich Felker <dalias@...c.org>
To: Tom Shen <sjiagc.dev@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: gethostbyname2_r returns invalid IPv6 address if DNS
 server replies IPv4 address

On Wed, Oct 19, 2022 at 11:44:45AM +0800, Tom Shen wrote:
> Thanks for replying!
> 
> > gethostbyname2 is legacy and has lots of inherent problems, so this
> > really should be changed to use getaddrinfo. If nothing else, such a
> > change would make it finish twice as fast.
> 
> Ya, it is obsolete as stated in Linux doc. But a third-party service is
> using "getent hosts" which calls this API.

Right. I'm saying the maintainers of this version of getent should be
informed that they're using a backwards nonstandard function here and
that it would work better with getaddrinfo.

> > Are you saying that the configured nameserver (coredns) is responding
> >   with an answer packet for a different question (A instead of AAAA)
> >   than the query packet asked for? I'm confused why it would do that.
> 
> >   It sounds like this is also wrong behavior by coredns. If it's
> >   rewriting AAAA queries to A for the sake of determining if the name
> >   exists vs NxDomain, that's fine for the outgoing query it performs,
> >   but instead of returning the answer to the rewritten query back to the
> >   asker as an answer that mismatches the question, it should just be
> >   removing the answer section to make it into a NODATA answer (if it's
> >   not already NxDomain).
> 
> Yes. In CoreDNS, we can configure it to rewrite the request type AAAA to A,
> so that the response will be IPv4 family and address. This is a CoreDNS
> feature. I cannot find anything in the DNS standards against it so far.

I inquired about this and got the response that there might not be an
explicit rule against it, "much like no RFC prohibits you from eating
glass". :-) I don't see how the behavior makes any sense though. There
is a risk that strict stub resolvers could consider these packets
non-answers and timeout/fail rather than returning a response, and
like you found, a chance that buggy stub resolvers not doing
sufficient validation could misinterpret the answers. I'm not clear
where these wrong-type answers could give useful results, especially
not anything more-useful than just returning NODATA in place of the v6
result you want to suppress.

> >   The expected behavior of a process receiving such an answer would be
> >   to ignore it as bogus, but maybe we're not doing that, and then
> >   wrongly returning answers that don't match the type we asked for?
> 
> Unfortunately, we return invalid IPv6 address as stated in the previous
> email. But, maybe it's not good to mention it here, glibc can handle this
> situation.
> 
> > I'd rather fix this at the layer the actual bug (missing validation)
> > is at.
> 
> So, let me try to suggest a fix in __lookup_name or musl team would help to
> take a look? So far I cannot find a way to create a pull request to musl
> project.

You can send patches to the list in plain diff or git format-patch
form. Attachments are preferred over inline but inline is okay too if
you're sure your mailer won't mess them up. But I have a proposed fix
here. The last hunk needs to be applied manually since it has context
affected by other DNS work I'm about to push (reversal of the for loop
direction).

Rich

View attachment "badrrtypefix.diff" of type "text/plain" (1947 bytes)

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.