On Sat, 2021-11-20 at 14:17 +0000, Stuart Henderson wrote:
> 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

committed, thanks.
> 
> > 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).
> 
I fully agree and that was the line of thought in my original diff.
However, I still think the behaviour should be made consistent between
AI_NUMERICHOST and !AI_NUMERICHOST. Iff someone were to make a solid
argument for the behaviour of !AI_NUMERICHOST in AI_NUMERICHOST my code
is not any more dangerous than the current host_v6 cases in the other
daemons. That was my only point I wanted to make.
> 
> 
> > 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([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?
> > > > > 
> > 
> > [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