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. 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 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) {
