On 2021/11/20 10:20, Martijn van Duren wrote:
> On Sun, 2021-11-14 at 22:30 +0100, Sebastian Benoit wrote:
> > 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.
> 
> This is the original message[0] (code committed had some tweaks not in this
> mail).
> In there I mention reyk's commit message saying that host_* could be merged.
> This commit tried to implement that (but apparently doesn't hold true any
> more, or maybe it never did). Moving away from struct address in host() also
> has the benefit that having different implementations of struct address to
> be more forgiving and it's less code overall.
> 
> Since it took over a year to find this particular edge case I think it could
> be a good idea to push this code to the other daemons as well, but I'm short
> on time at the moment.
> 
> Diff below should fix this particular issue and is easy enough to revert if
> we decide to go for the behaviour change of getaddrinfo proposed in my
> previous mail.

ok

> I'm not afraid of moving the other way in getaddrinfo (that AI_NUMERICHOST
> is also subject to the family statement in resolv.conf), because that would

I don't like that at all. And I don't think it matches resolv.conf's
definition of "family":

     family      Specify which type of Internet protocol family to prefer, if
                 a host is reachable using different address families. 

the "if a host is reachable using different address families"
means that this doesn't apply for numeric addresses. (OK there could be
a side-case with CLAT on OS that don't force IPV6_V6ONLY but not on
OpenBSD).



> also break all current host_v6 implementations in parse.y, so that would
> be an overhaul in all parse.y files anyway.
> 
> martijn@
> > 
> > Martijn van Duren(openbsd+t...@list.imperialat.at) 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?
> > > > 
> 
> [0] https://marc.info/?l=openbsd-tech&m=159838549814986&w=2
> 
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
> retrieving revision 1.72
> diff -u -p -r1.72 parse.y
> --- parse.y   25 Oct 2021 11:21:32 -0000      1.72
> +++ parse.y   20 Nov 2021 09:19:00 -0000
> @@ -1600,7 +1600,16 @@ host(const char *s, const char *port, in
>       bzero(&hints, sizeof(hints));
>       hints.ai_family = PF_UNSPEC;
>       hints.ai_socktype = type;
> +     /*
> +      * Without AI_NUMERICHOST getaddrinfo might not resolve ip addresses
> +      * for families not specified in the "family" statement in resolv.conf.
> +      */
> +     hints.ai_flags = AI_NUMERICHOST;
>       error = getaddrinfo(s, port, &hints, &res0);
> +     if (error == EAI_NONAME) {
> +             hints.ai_flags = 0;
> +             error = getaddrinfo(s, port, &hints, &res0);
> +     }
>       if (error == EAI_AGAIN || error == EAI_NODATA || error == EAI_NONAME)
>               return 0;
>       if (error) {
> 

Reply via email to