From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 32040 invoked from network); 20 Oct 2022 17:26:18 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 20 Oct 2022 17:26:18 -0000 Received: (qmail 29787 invoked by uid 550); 20 Oct 2022 17:26:13 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 29754 invoked from network); 20 Oct 2022 17:26:12 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=I/KYYZYT9Y4PtzuAqYjWvwqxX9GbJdcVjhEo0xR7HN8=; b=Day4GewGTf7LorKYbRkTFICkCUSJgik5mSU8EZo/OOL3ukMS3HonUuZC04GtlfjdN9 hiuFJN2HdxsGlV6X0x1QIJ3UxSeMMsWGpA6yqYd9H23flL/pCcVPk529Rpx/0DM/yW6L YcLntO+LiroRmNzfKu7kH30+NKRikqN0PAR3JsnOBip/4gbymecIiFAxvpzYWPGFZQ3f 3Z+sx1iu2vGB8kzE5wp0vMeWQnE5FSd8mjGvLkSrfIAm0R5OnJgnoT2f/YZ75b8OZvAJ WGu+Ig2HAo4PuNp1KyTk9Fl3ObLZc1kctQdSZ0U1AyaUyHYk7IWE9DnuKkqtXTX809YM r5Rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=I/KYYZYT9Y4PtzuAqYjWvwqxX9GbJdcVjhEo0xR7HN8=; b=jnocSsP7sPy+JsFf1rALwB7VFwfq+Wp7wTPP9aNpgcGcQFa95w9JrqbfRBTkePIcHo KfuzBTJtm0heAZmxx8GlEEyrocwqnlcWmVMKxY+0zWSpmNZcLHAxJYiZAq5XEwgvyUSV NZl6JZF+fw7rUPn3HW4J8vffyCjB2ZkkCYngmwAX2gkv8GpDAFF3Q0w5r3pgNECE24M2 Zvo8MwkR2nvhYmohEt+EjTDZPvorKFgo/LuYMQeTLqo/o19iLZk7XgZR32jbLREWGOHp blD9cpFs6xDt5qPRiRCZHVBksqBU+tWzhwtFncstM020XT3YzsEYkzg6xsy70rlNJzzW HoAQ== X-Gm-Message-State: ACrzQf2UfAe5htX2FxSBxr/sycLzVdUQyeYNG3GVld1vzcL6fqe0/vY3 /ySwxXFkNbd6CShZlyH7y0/FJIUnlIRxOPJF6DBxI5XjC4M= X-Google-Smtp-Source: AMsMyM7YLATITA7S2KWkIn/v7PTKWT5KJ7kjhIyFgwQL2mjflVZkXpUI15/8IhVcE1HKEeSDTmso8gt1Rie/zpwL9Fg= X-Received: by 2002:a17:907:7214:b0:791:a4cf:5bb7 with SMTP id dr20-20020a170907721400b00791a4cf5bb7mr8966297ejc.576.1666286760923; Thu, 20 Oct 2022 10:26:00 -0700 (PDT) MIME-Version: 1.0 References: <20221018172727.GK29905@brightrain.aerifal.cx> <20221019142452.GL29905@brightrain.aerifal.cx> <20221020005214.GM29905@brightrain.aerifal.cx> In-Reply-To: <20221020005214.GM29905@brightrain.aerifal.cx> From: Tom Shen Date: Fri, 21 Oct 2022 01:25:50 +0800 Message-ID: To: Rich Felker Cc: musl@lists.openwall.com Content-Type: multipart/mixed; boundary="0000000000007af13505eb7a9ea6" Subject: Re: [musl] gethostbyname2_r returns invalid IPv6 address if DNS server replies IPv4 address --0000000000007af13505eb7a9ea6 Content-Type: multipart/alternative; boundary="0000000000007af13105eb7a9ea4" --0000000000007af13105eb7a9ea4 Content-Type: text/plain; charset="UTF-8" I tested the fix in your earlier email locally with my own test code and getent in Alpine Linux 3.16.2. They both crashed. After debugging, I found in gethostbyname2_r we should return non-zero if no address is returned. Then gethostbyname2 will return NULL. I also check the Linux doc, it says: > Return Value > The gethostbyname() and gethostbyaddr() functions return the hostent structure or *a NULL pointer if an error occurs*. On error, the h_errno variable holds an error number. When non-NULL, the return value may point at static data, see the notes below. Based on your patch (except the "for (i=nq-1; i>=0; i--)"), I made a minor change to address it. Tested with command getent hosts, it works well with my CoreDNS. Although the h_errno is better to be NO_DATA rather than HOST_NOT_FOUND, I think it's not a big issue. The diff file attached. Tom Shen On Thu, Oct 20, 2022 at 8:52 AM Rich Felker wrote: > On Thu, Oct 20, 2022 at 12:00:46AM +0800, Tom Shen wrote: > > > 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. > > > > The getent Linux manual states that when using database hosts, > > gethostbyname2 would be called. > > https://man7.org/linux/man-pages/man1/getent.1.html . Maybe we can > persuade > > the third party service to use getent ahosts. > > Uhg, lovely that it's specified like that... But really, it only needs > to behave "as if" it did that, which can be done better using > getaddrinfo with the right options to get the behavior the legacy APIs > would have given, but in a better way. So I think it's still worth > pursuing this kind of improvement too. Orthogonal to your problem > report, though. > > > > 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. > > > > True. Maybe we should not use this "rewrite type AAAA A" feature in > > CoreDNS. Just on Internet there are some posts suggested it to fix the > name > > resolution issues caused by unexpected IPv6 logic. We already removed it > > from our system since an incident. I am not an expert in DNS though, so > > cannot comment more about the standards. > > > > Anyway, although rewriting the DNS request type may make no sense, we > > should either return correct IPv6 address (e.g. ::ffff:10.96.36.74) or > IPv4 > > address (10.96.36.74) or NODATA, rather than an invalid IPv6 address > > (a60:244a::) :-). I got your point in last email, and I also have the > same > > idea, returning NODATA is a good (or only) solution. In getent hosts > > command, when getting empty result for AF_INET6, it will call > > gethostbyname2 with AF_INET (this is also the behavior of glibc's getent > > https://codebrowser.dev/glibc/glibc/nss/getent.c.html), then we will get > > correct IPv4 address. > > > > > But I have a proposed fix here. > > > > Thank you so much, Rich! Your change looks better than what I would do. I > > will try it out. If tested good, would we merge the fix into later > releases? > > Yes, I'm getting it merged now. Thanks for the report. In some sense > this was a long-known issue (that "malicious" nameservers could give > wrong-RR-type answers -- see CVE-2017-15650) and it's nice to finally > have good reason to put in the attention to fixing it. > > Rich > --0000000000007af13105eb7a9ea4 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I tested the fix in your earlier email locally with my own= test code and getent in Alpine Linux 3.16.2. They both crashed. After debu= gging, I found in gethostbyname2_r we should return non-zero if no address = is returned. Then gethostbyname2 will return NULL. I also check the Linux d= oc, it says:

> Return Value
> The gethostbyna= me() and gethostbyaddr() functions return the hostent structure or *a NULL = pointer if an error occurs*. On error, the h_errno variable holds an error = number. When non-NULL, the return value may point at static data, see the n= otes below.

Based on your patch (except the "for (i= =3Dnq-1; i>=3D0; i--)"), I made a minor change to address it. Teste= d with command getent hosts, it works well with my CoreDNS. Although=C2=A0t= he h_errno is better to be NO_DATA rather than HOST_NOT_FOUND, I think it&#= 39;s not a big issue.

The diff file attached.

Tom Shen

On Thu, Oct 20, 2022 at 8:52 AM Rich Fel= ker <dalias@libc.org> wrote:
On Thu, Oct 20, 2= 022 at 12:00:46AM +0800, Tom Shen wrote:
> > Right. I'm saying the maintainers of this version of getent s= hould be
> > informed that they're using a backwards nonstandard function = here and
> > that it would work better with getaddrinfo.
>
> The getent Linux manual states that when using database hosts,
> gethostbyname2 would be called.
> https://man7.org/linux/man-pages/man1/getent= .1.html . Maybe we can persuade
> the third party service to use getent ahosts.

Uhg, lovely that it's specified like that... But really, it only needs<= br> to behave "as if" it did that, which can be done better using
getaddrinfo with the right options to get the behavior the legacy APIs
would have given, but in a better way. So I think it's still worth
pursuing this kind of improvement too. Orthogonal to your problem
report, though.

> > I inquired about this and got the response that there might not b= e an
> > explicit rule against it, "much like no RFC prohibits you fr= om 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, an= d
> > 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, especia= lly
> > not anything more-useful than just returning NODATA in place of t= he v6
> > result you want to suppress.
>
> True. Maybe we should not use this "rewrite type AAAA A" fea= ture in
> CoreDNS. Just on Internet there are some posts suggested it to fix the= name
> resolution issues caused by unexpected IPv6 logic. We already removed = it
> from our system since an incident. I am not an expert in DNS though, s= o
> cannot comment more about the standards.
>
> Anyway, although rewriting the DNS request type may make no sense, we<= br> > should either return correct IPv6 address (e.g. ::ffff:10.96.36.74) or= IPv4
> address (10.96.36.74) or NODATA, rather than an invalid IPv6 address > (a60:244a::) :-). I got your point in last email, and I also have the = same
> idea, returning NODATA is a good (or only) solution. In getent hosts > command, when getting empty result for AF_INET6, it will call
> gethostbyname2 with AF_INET (this is also the behavior of glibc's = getent
> https://codebrowser.dev/glibc/glibc/nss/g= etent.c.html), then we will get
> correct IPv4 address.
>
> > But I have a proposed fix here.
>
> Thank you so much, Rich! Your change looks better than what I would do= . I
> will try it out. If tested good, would we merge the fix into later rel= eases?

Yes, I'm getting it merged now. Thanks for the report. In some sense this was a long-known issue (that "malicious" nameservers could g= ive
wrong-RR-type answers -- see CVE-2017-15650) and it's nice to finally have good reason to put in the attention to fixing it.

Rich
--0000000000007af13105eb7a9ea4-- --0000000000007af13505eb7a9ea6 Content-Type: application/octet-stream; name="gethostbyname2_r.diff" Content-Disposition: attachment; filename="gethostbyname2_r.diff" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_l9hbwieq0 ZGlmZiAtLWdpdCBhL3NyYy9uZXR3b3JrL2dldGhvc3RieW5hbWUyX3IuYyBiL3NyYy9uZXR3b3Jr L2dldGhvc3RieW5hbWUyX3IuYwppbmRleCBjOWYzYWNjNC4uNDY1M2FhYzggMTAwNjQ0Ci0tLSBh L3NyYy9uZXR3b3JrL2dldGhvc3RieW5hbWUyX3IuYworKysgYi9zcmMvbmV0d29yay9nZXRob3N0 YnluYW1lMl9yLmMKQEAgLTIyLDcgKzIyLDcgQEAgaW50IGdldGhvc3RieW5hbWUyX3IoY29uc3Qg Y2hhciAqbmFtZSwgaW50IGFmLAogCWlmIChjbnQ8MCkgc3dpdGNoIChjbnQpIHsKIAljYXNlIEVB SV9OT05BTUU6CiAJCSplcnIgPSBIT1NUX05PVF9GT1VORDsKLQkJcmV0dXJuIDA7CisJCXJldHVy biBFTk9EQVRBOwogCWNhc2UgRUFJX0FHQUlOOgogCQkqZXJyID0gVFJZX0FHQUlOOwogCQlyZXR1 cm4gRUFHQUlOOwo= --0000000000007af13505eb7a9ea6--