Replace deprecated gethostbyname()

[Bischoff]

I get the following warnings when building mrouted:

I: binary-or-shlib-calls-gethostbyname /usr/sbin/mtrace
I: binary-or-shlib-calls-gethostbyname /usr/sbin/map-mbone
I: binary-or-shlib-calls-gethostbyname /usr/sbin/mrouted
I: binary-or-shlib-calls-gethostbyname /usr/sbin/mrinfo
The binary calls gethostbyname(). Please port the code to use getaddrinfo().

It seems that gethostbyname() is not IPv6-compatible, and that it is the reason why it should be replaced.

(and I have not the time to do the corresponding pull request, sorry)

[troglobit]

Thanks for the report, and reminder! Great to have it here in the tracker for followup.

For anyone feeling up to it, looking in to how pimd implements this is probably a good idea. Much of the code in pimd is based on mrouted (and these days share the same license). So could be a simple porting job.

[Bischoff]

I don't think pimd is IPv6-compatible. This means that they had less incentive to convert existing gethostaddr() calls.

This does not means that pimd source code can't be reused. But I don't see any call to gethostbyname() nor getaddrinfo() (don't they ever convert a host name to an IP address? I certainly missed some point).

[Bischoff]

As far as I know, DVMRP has no IPv6 version (it is based on IGMP, which in IPv6 world is replaced with ICMPv6 multicast listener discovery functions). This reduces the interest of having this issue solved.

However, I think the issue still would be interesting to solve, because it's bad to use a deprecated function, no matter for what reason it was deprecated.

[troglobit]

pimd is not IPv6-compatible, there is a pim6d project for that. Both mrouted and pimd are for IPv4.

It's probably an easy job to replace gethostbyname() with getaddrinfo().

[Bischoff]

Yes, it's probably not too difficult, but still, it's a more complicated function signature, you have to loop through a linked list of answers, etc. Ideally, you should also try to connect to all found answers to keep only active IP addresses...

I have seen tutorials around that pretend it easier than it really is (for example, they forget to zero the hints structure before using it).

I have also found a post saying that gethostbyname() has security issues too. So we have a good reason to fix this, even forgetting about IPv6.

[troglobit]

Then the easiest way is probably to do like pimd does: to just allow interface names or IP addresses
and rely on, e.g. inet_aton(). The whole point of the code anyway is to figure out which interface to use, so simplify is probably better.

[Bischoff]

Ah. That explains why I did not find any conversion from hostname to IP address in pimd source code :-) .

[troglobit]

;-)

[Bischoff]

Just a small preliminary analysis:

$ grep -R gethostbyname 
cfparse.y:          if ((hp = gethostbyname($1)) == NULL || hp->h_length != sizeof($$)) {
mtrace.c:    if (dots <= 0) e = gethostbyname(name);
mtrace.c:       hp = gethostbyname(myhostname);
mapper.c:    struct hostent *e = gethostbyname(name);
mrinfo.c:               hp = gethostbyname(host);

If I am not wrong, the occurrences in mtrace.c, mapper.c, and mrinfo.c are for getting "real" remote hostnames in order to query them with map_mbone, mrinfo, and mtrace commands. These ones should be converted to getaddrinfo() as initially planned.

Now for the cfparse.y occurrence. It seems to me it is a bison lexxer used to parse /etc/mrouted.conf configuration file. This is the one where a conversion to inet_aton() would probably be enough. This would break any existing configuration file where admins would have put a hostname instead of an IP address, but such configuration files seem bogus to me anyway, so breaking compatibility with the existing here seems more or less okay (you know, "it's for your own good" justifications...)

What do you think? Keep in mind I'm new to this source code and I might as well say nonsense.

[troglobit]

Yep, agreed.

I've been meaning to move these old helper tools out of the mrouted repo, so not really a big rush to fix them. Not even sure if they fill any real purpose anymore.

[Bischoff]

Indeed. I think I have memories the mbone has been phased out, so at least the mbone_map could be removed. But my memories might be deceiving me, it would need at least a check.

[troglobit]

Finally got to resolve this issue! \o/

[Bischoff]

Congrats 😸 ! I did not check, but feel free to close anyway.

[troglobit]

It'll be closed automatically when i merge the dev branch. (Doing static code analysis atm)

Happy Holidays!