If there is no obvious reason (i.e. be different because you need it for a
specific feature) why not to use the same host*() function as other parse.y?
it would be better to stay in sync with otehrr daemons. That way if there is
an issue in one daemon, we can fix it in all of them.

Or, to turn the argument around: if you have found a way to improve those
functions in parse.y, you could push for your changes to be applied to all
parse.y that use the same function.

This applies to other parse.y functions too. The more they deviate, the
harder maintanance becomes.

Martijn van Duren([email protected]) on 2021.11.14 00:23:59 +0100:
> On Sat, 2021-11-13 at 13:23 +0000, Stuart Henderson wrote:
> > On 2021/08/09 20:55, Martijn van Duren wrote:
> > > On Mon, 2021-08-09 at 11:57 +0200, Martijn van Duren wrote:
> > > > 
> > > > This diff fixes all of the above:
> > > > - Allow any to be used resolving to 0.0.0.0 and ::
> > > > - Set SO_REUSEADDR on sockets, so we can listen on both any and
> > > > ?? localhost
> > > > - Document that we listen on any by default
> > 
> > I've discovered a problem with this, if you have "family inet4" or
> > "family inet6" in resolv.conf then startup fails, either with the
> > implicit listen:
> > 
> > snmpd: Unexpected resolving of ::
> > 
> > or with explicit e.g. "listen on any snmpv3":
> > 
> > /etc/snmpd.conf:3: invalid address: any
> > 
> > Config-based workaround is e.g. "listen on 0.0.0.0 snmpv3"
> > 
> > Should host() use a specific ai_family instead of PF_UNSPEC where we
> > already know that it's a v4 or v6 address? Or just do like httpd/parse.y
> > where host() tries v4, then v6 if that fails, then dns?
> > 
> Actually, this diff isn't the cause of the regression, it's r1.60 of
> parse.y. Prior to that snmpd also used the v4->v6->dns method, similar
> to httpd. I removed these steps to simplify the code and because
> getaddrinfo(3) is supposed to do the same thing as host_v4 and host_v6
> do/did. To be more precise: host_v6 uses getaddrinfo(3), but with the
> AI_NUMERICHOST flag.
> 
> So this brings to light a discrepancy's between calling getaddrinfo(3)
> with and without AI_NUMERICHOST:
> When called without AI_NUMERICHOST the family keyword in resolv.conf(5)
> is "honoured", but when called with AI_NUMERICHOST the family keyword is
> ignored.
> 
> When looking at POSIX[0] it states:
> If the nodename argument is not null, it can be a descriptive name or
> can be an address string. If the specified address family is AF_INET,
> [IP6] [Option Start]  AF_INET6, [Option End] or AF_UNSPEC, valid
> descriptive names include host names. If the specified address family is
> AF_INET or AF_UNSPEC, address strings using Internet standard dot
> notation as specified in inet_addr are valid.
> 
> [IP6] [Option Start] If the specified address family is AF_INET6 or
> AF_UNSPEC, standard IPv6 text forms described in inet_ntop are valid.
> [Option End]
> 
> The way I read this text is that we should always test for numeric
> hostnames (within the requested family of ai_family) regardless of what
> is in resolv.conf and would make us consistent with the AI_NUMERICHOST
> case. This is also consistent with the phrasing in resolv.conf(5), which
> states "inet4     IPv4 queries.", since I don't consider calling
> inet_pton(3) (which is what is called internally) a query.
> Diff below does this and fixes the bug in snmpd(8).
> 
> As for the risks: I think these are nil: It only affects the few
> installs that have set family to a single value. For the people that
> do have it, it will change an IPv{4,6} literal to always be "resolved",
> but if you enter e.g. a v6 numeric address on a v4 only machines it will
> almost always error out on the next step (most probably EHOSTUNREACH or
> EADDRNOTAVAIL).
> 
> regress/lib/libc/asr is currently broken, but with some minimal tweaking
> all test-cases pass.
> 
> martijn@
> 
> [0] 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/getaddrinfo.html
> 
> Index: asr/getaddrinfo_async.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/asr/getaddrinfo_async.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 getaddrinfo_async.c
> --- asr/getaddrinfo_async.c   26 Jan 2021 12:27:28 -0000      1.57
> +++ asr/getaddrinfo_async.c   13 Nov 2021 23:17:43 -0000
> @@ -492,8 +492,8 @@ get_port(const char *servname, const cha
>  }
>  
>  /*
> - * Iterate over the address families that are to be queried. Use the
> - * list on the async context, unless a specific family was given in hints.
> + * Iterate over PF_INET and PF_INET6 unless a specific family was given in
> + * hints. Only to be used when resolving numeric address.
>   */
>  static int
>  iter_family(struct asr_query *as, int first)
> @@ -502,7 +502,7 @@ iter_family(struct asr_query *as, int fi
>               as->as_family_idx = 0;
>               if (as->as.ai.hints.ai_family != PF_UNSPEC)
>                       return as->as.ai.hints.ai_family;
> -             return AS_FAMILY(as);
> +             return PF_INET;
>       }
>  
>       if (as->as.ai.hints.ai_family != PF_UNSPEC)
> @@ -510,7 +510,7 @@ iter_family(struct asr_query *as, int fi
>  
>       as->as_family_idx++;
>  
> -     return AS_FAMILY(as);
> +     return as->as_family_idx == 1 ? PF_INET6 : -1;
>  }
>  
>  /*
> 

Reply via email to