Subject: Re: AF_UNSPEC doesn't fall back to IPv4 if only CNAME is returned

Re: AF_UNSPEC doesn't fall back to IPv4 if only CNAME is returned

From: Jakub Hrozek <jhrozek_at_redhat.com>
Date: Thu, 24 Jul 2014 18:07:07 +0200

On Thu, Jul 24, 2014 at 04:53:45PM +0100, David Drysdale wrote:
> On Thu, Jul 24, 2014 at 3:12 PM, Jakub Hrozek <jhrozek_at_redhat.com> wrote:
>
> > On Thu, Jul 24, 2014 at 11:24:23AM +0100, David Drysdale wrote:
> > > On Thu, Jul 24, 2014 at 10:53 AM, Jakub Hrozek <jhrozek_at_redhat.com>
> > wrote:
> > >
> > > > On Thu, Jul 24, 2014 at 09:47:10AM +0100, David Drysdale wrote:
> > > > > On Wed, Jul 23, 2014 at 4:59 PM, Jakub Hrozek <jhrozek_at_redhat.com>
> > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I'm seeing the behaviour described in $SUBJECT with git head of
> > c-ares
> > > > > > and I'd like to discuss the best way of fixing it.
> > > > > >
> > > > > > If you search with ares_gethostbyname() with lookup family set to
> > > > > > AF_UNSPEC,
> > > > > > the first family to try would be AF_INET6. The code would jump to
> > > > > > host_callback() on completing the query and try to parse the
> > returned
> > > > abuf:
> > > > > >
> > > > > > 188 else if (hquery->sent_family == AF_INET6)
> > > > > > 189 {
> > > > > > 190 status = ares_parse_aaaa_reply(abuf, alen, &host,
> > NULL,
> > > > > > NULL);
> > > > > > 191 if ((status == ARES_ENODATA || status ==
> > ARES_EBADRESP)
> > > > &&
> > > > > > 192 hquery->want_family == AF_UNSPEC) {
> > > > > > 193 /* The query returned something but either there
> > were
> > > > no
> > > > > > AAAA
> > > > > > 194 records (e.g. just CNAME) or the response was
> > > > > > malformed. Try
> > > > > > 195 looking up A instead. */
> > > > > >
> > > > > > But since commit b3afe9cbdee797652d8cfba49aa2e9d42bf781fe parsing a
> > > > query
> > > > > > that only contains CNAMES returns ARES_SUCCESS and a hostent that
> > > > contains
> > > > > > the aliases, so the condition on list 191 never matches and
> > AF_INET is
> > > > > > never retried. I don't see a way for ares_parse_aaaa_reply()
> > caller to
> > > > > > see if the data returned contain a real AAAA response or just
> > CNAMEs.
> > > > > >
> > > > > > I think ares_parse_aaaa_reply() behaviour is correct now and
> > > > > > I think host_callback() here is buggy, or rather relies on
> > > > > > bug in ares_parse_aaaa_reply(). Since we apparently can't use
> > > > > > ares_parse_aaaa_reply(), I'd like to suggest a new internal
> > function,
> > > > > > something like:
> > > > > >
> > > > > > int ares_peek_aaaa_reply(const unsigned char *abuf, int alen,
> > > > > > int *naaaa, int *ncname);
> > > > > >
> > > > > > And change the fallback condition to:
> > > > > > status = ares_peek_aaaa_reply(abuf, alen, &naaaa, &ncname);
> > > > > > if ((status == ARES_ENODATA || status == ARES_EBADRESP ||
> > > > > > (status == ARES_SUCCESS && naaaa == 0)) &&
> > > > > > hquery->want_family == AF_UNSPEC) {
> > > > > > }
> > > > > >
> > > > > > Would that be an acceptable solution? Is there any other/better
> > way?
> > > > > >
> > > > > > Thanks for any suggestions.
> > > > > >
> > > > >
> > > > > Could you just extend the check on 191-192 to explicitly look in
> > host for
> > > > > returned addresses?
> > > > > Something like:
> > > > > if ((status == ARES_ENODATA || status == ARES_EBADRESP ||
> > > > > (status == ARES_SUCCESS && host &&
> > host->h_addr_list[0] ==
> > > > > NULL)) &&
> > > > > hquery->want_family == AF_UNSPEC) {
> > > >
> > > > Sure, I was just wondering if this is not considered relying on yet
> > > > another 'side effect'. The 'peek' would really return exactly the data
> > > > host_callback is interested in.
> > > >
> > > > But on the other hand you're right that we should strive for minimal
> > > > changes.
> > > >
> > > > Thanks for the suggestion, David, I've attached a patch.
> > > >
> > >
> > > That seems to work for me -- I've attached a patch to ahost that lets us
> > > test it out (e.g.
> > > "./ahost -t u www.comcast.com.", which is a domain with a CNAME but no
> > AAAA
> > > records).
> >
> > Thanks for the review!
> >
> > > From fcb2d96724c7bcc0efe311c54732d2225f0ac179 Mon Sep 17 00:00:00 2001
> > > From: David Drysdale <drysdale_at_google.com>
> > > Date: Thu, 24 Jul 2014 11:18:46 +0100
> > > Subject: [PATCH] Add -t u option to ahost
> >
> > Can you also fix the usage output? I'm still seeing:
> >
> > ./ahost --help
> > usage: ahost [-t {a|aaaa}] {host|addr} ...
> >
>
> Ah, I missed that. Updated patch attached.

> From 2bc07b2e7432c42a94d65577293602efefd6eddd Mon Sep 17 00:00:00 2001
> From: David Drysdale <drysdale_at_google.com>
> Date: Thu, 24 Jul 2014 11:18:46 +0100
> Subject: [PATCH] Add -t u option to ahost

Looks good to me!

Can you push the patch?
Received on 2014-07-24