Jamulus: ParseNetworkAddress might select IPv6 addresses over IPv4 when a hostname has both

Created on 13 Nov 2020  路  9Comments  路  Source: corrados/jamulus

Hi, thanks a lot for Jamulus. I'm trying to take a look at the feasibility of adding IPv6 support, and I believe I found a bug along the way.

If a hostname has both A and AAAA records, NetworkUtil::ParseNetworkAddress() might pick the address from the AAAA record instead of the one from the A record, and fail to connect to a server running on that host.

When given a hostname, NetworkUtil::ParseNetworkAddress() uses QHostInfo::fromName() to perform the lookup, and currently picks the first result from the list. The documentation of this method does not specify anything about the order of the list for cases where there are multiple addresses. Looking a few layers down in the implementation, it calls getaddrinfo(), whose behaviour seems to be system-dependent (see e.g. https://stackoverflow.com/a/11326970).

On my computer, getaddrinfo() returns IPv6 addresses before IPv4 addresses (when both are available). So when I try to connect to a hostname that has both A and AAAA records, Jamulus selects the IPv6 address, and fails to connect to the server running on that host.

(curiously, what happens when an IPv6 CHostAddress makes its way down to CSocket::SendPacket() is that toIPv4Address() fails and returns 0, which is interpreted as 0.0.0.0, so it tries to connect to localhost)

My suggestion is to make NetworkUtil::ParseNetworkAddress() look for the first IPv4 address in the list of returned addresses. The question is what to do if the returned result is non-empty, but only contains IPv6 addresses. Even though the rest of the function has some support for IPv6, I think it would make the most sense to return false when this happens. Right now, there's nothing useful that can be done with an IPv6 address anyway. If that sounds good, I could produce a pull request.

improvement

All 9 comments

Thanks for your investigation. So your change would be only be in function NetworkUtil::ParseNetworkAddress(), right?

I also experienced this problem while wanting to connect to localhost. A long term fix should be full ipv6 support.

@corrados: Yes, I think changing that method should be enough. I tried it out and it seems to solve the issue (including connecting to localhost).

@ann0see: I agree. I think some things have happened since #46 was opened that might make adding IPv6 support a bit easier than it was then. I will try things out a bit more and hopefully come up with a proposal for how support could be added.

Selecting ipv6 before ipv4 is a bug in linux distros that have been zealously promoting ipv6.
The solution is to modify /etc/gai.conf (file probably doesn't exist). Here is my home made gai.conf
precedence ::1/128 50 0
precedence ::/0 40 1
precedence 2002::/16 30 2
precedence ::/96 20 3
precedence ::ffff:0:0/96 100 4

Calling a configuration choice -- it's config, there to enable that choice, explicitly -- a bug is really rather a political statement in itself, I feel.

Devices may not have IPv4 addresses at all, so no matter how you order the preference of selection, you'll still get an IPv6 address. Not currently likely but...

Jamulus should support IPv6, really - it does expand the range of problems users are likely to find of course. (i.e. the local IPv6 address gets selected but there's no routing at all for IPv6 being the most likely...)

I would like ipv6 implemented correctly and I see an ipv6 problem already. ipv4 was made without consideration that it would be superseded, thus ipv4 applications were coded similarly to how Jamulus is (they break when gai.conf is not configured with regard to ipv4). Now, ipv6 applications will repeat this problem if it is not addressed. Therefore gai.conf should not break ipv4, And ipv6 should not break when ipv4 is given precedence in gai.conf. (ipv6 should be ipv4 aware... but ipv4 must remain ignorant of ipv6)

  • Note that my gai.conf above gives precedence to ipv4.

OK, I see what you mean -- _if_ you have both IPv4 and IPv6 addressing _and if_ you have IPv4 applications _and if_ you use gai.conf to set the precedence of addressing, _then_ the IPv4 stack shouldn't be broken. Agreed.

Do we need both #46 and this issue?

I think it makes sense to have both. While they originated from the same issue (hosts with both v4 and v6 addresses), #46 quickly turned into a general IPv6 feature request, while this one regards a very specific bug when trying to use IPv4 on IPv6-enabled hosts.

Though I agree that it would be nicer to have just one IPv6 issue. I'm not 100% on how to follow the contribution guidelines here, but since the suggested fix is so small, I will go ahead and create a pull request for this one.

Was this page helpful?
0 / 5 - 0 ratings