Switch from gethostbyname() to getaddrinfo()

[robert-scheck]

Use of gethostbyname (in src/addr.c) may prevent the program from working properly on IPv6 systems. getaddrinfo should be used in preference.

See also: https://bugzilla.redhat.com/show_bug.cgi?id=990560

[ofalk]

Switching from gethostbyname to getaddrinfo isn't so straight forward - it's not a 1:1 replacement. If anyone has some spare-cycles to code this piece, I'm open for some PR :-)

[dwmcrobb]

Given how gethostbyname() is used here, the correct use of getaddrinfo() actually would be a 1:1 replacement for gethostbyname(). Yes it's more lines of code, but it'd be functionally equivalent.

However, I'd argue that just removing it is the right thing to do here. It's never been appropriate to invoke the resolver from addr_pton(); it overloads the function inappropriately. It should always return an error when the first argument is not a valid address string. And it should have done this before IPv6 was common. Today, where it's common to have AAAA and A records for a given host name, it's unquestionably the wrong thing to do. Code that expect to pass a hostname (canonical or not) as the first argument to addr_pton() and get a valid result is just incorrect in today's world.

I'd say make it obvious by removing that 'else if' entirely and updating the manpage. That's what my local patches for the FreeBSD port do. I haven't seen any negative consequences (I've had these patches for many years, maybe a decade), but I don't think I have any code that depends on libdnet that's not my own code, which has never called addr_pton() with a host name. The only things I see in the FreeBSD ports tree that depends on libdnet are the perl5 and python interfaces to libdnet, plus p5-Class-Gomor. I haven't checked the linux distributions but I assume the situation is similar.

Also note that the original code sets errno to EINVAL if host lookup fails, which is misleading at best if a host name is passed as the first argument. "xyqzfooygnfb" isn't an invalid host name, but there are no A or AAAA records for it in my search domains.

Basically... name resolution is orthogonal to address format conversion. Keep in mind that when someone wants name resolution, they should want RFC 8305 behavior in many/most cases (they're doing name lookup simply to get an address for connect(), for example). That's much more involved than just calling getaddrinfo() or the obsolete gethostbyname().

[dwmcrobb]

Pull request created, if you want it.

[dwmcrobb]

Packages on Ubuntu 20.04 that depend on libdumbnet1:

% apt-cache rdepends libdumbnet1
libdumbnet1
Reverse Depends:
  libdumbnet-dev
  snort
  scanssh
  libnet-libdnet-perl
  arpon
  labrea
  farpd
  daemonlogger

snort, scanssh and farpd don't pass host names to addr_pton().
arpon, labrea and daemonlogger don't call addr_pton() at all.

So I don't think my pull request will break debian-based distros in any consequential way; direct dnet-dependent packages are not calling addr_pton() with host names. At the moment I don't have time to chase down the whole dependency tree, but I think it's good to remove the host lookup stuff in addr_pton() and update the documentation.

If desired, there could be a new function that uses gettaddrinfo() and fills an array of struct addr, and possibly another function that just fills one struct addr with the first entry found matching a given family? Family could be 0 for "Don't care".

hname_tonarr(const char *hname, struct addr *addrs, size_t numAddrs, int family);
hname_ton(const char *hname, struct addr *addr, int family);

[ofalk]

Thx a lot @dwmcrobb for your work on that. I'll have a look at the PR!

[dwmcrobb]

I reverted the README.md changes on my addr_pton-no-gethostbyname branch and master branch. So sorry about that goof up on my part!

Let me know if I need to do anything else.

[ofalk]

No worries @dwmcrobb ! I guessed it happened by accident!

[ofalk]

Squash-Merged and close this issue.
Thanks again for your efforts @dwmcrobb !