Yes,

whereas Martijn's proposal changes *all programs immediately*, and would
require a lot of inspection for downside impacts.

> 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