Fix netstat reporting of accepted TCP/IPv6 connections when net.inet6.ip6.v6only=0

[tuexen]

Summary

When net.inet6.ip6.v6only=0 is true (the non-default setting), accepted TCP/IPV6 connections where reported by netstat incorrectly, since they had both the INP_IPV4 and the INP_IPV6 flag set. netstat than tries to print the source and destination address as IPv4 addresses although they are IPv6 addresses. This patch fixes this issue.

This also confuses also squid when using ACLs.

Thanks to kib@ and gjb@ for reporting the issue

Test Plan

Test with simple test programs and sockstat.

[kib]

Patch looks reasonable, for my limited understanding of the TCP code.

I can confirm that with the patch applied, I see proper tcp6 Proto values in the netstat -f inet6 output, as it should be. I will not be able to test the squid behavior easily because that machine is Atom and system rebuild takes a day. Still, I am quite confident that the issues for both squid and netstat are same and the patch looks correct and behaves right.

[bz]

Reading the comment I though "oh so it was actually a bug in netstat" and might squid be doing. Then I saw the patch. This needs a way better description for the commit message! Also a reference to the commit that changed the original behaviour now making this necessary.

[tuexen]

I can provide a better description. Please note the the tcp6_usr_bind() has already the similar code:
https://svnweb.freebsd.org/base/head/sys/netinet/tcp_usrreq.c?revision=326023&view=markup#l367
and this was not changed recently.

I can also provide an analysis why this change is now needed in the commit message.

[kib]

Can we move this forward, please ?

[kib]

Ping^2.

I also confirmed that the patch fixed squid ACLs on ipv6 addresses.

[tuexen]

I'll bring this up on todays transport telco...

[bz]

Ok, so I think this is not the complete fix.
The tcp6_usr_bind() code you reference has the additional bits to make this right-er.

The case I am thinking about is what happens in the V4MAPPED (this might actually be ok?)
and IN6_IS_ADDR_UNSPECIFIED() case?

An application might listen for both incoming v4 and v6 connections on an IPv6 socket?
If IN6P_IPV6_V6ONLY is not set the proposed patch will no longer do the right thing.

Also this behaviour has not changed in about 10+ years, so why are we seeing this now?
I didn't find the reason in kernel space on a very quick look around.

Can we have a full matrix of test cases and also understand why it's become a problem?

sys/netinet/tcp_syncache.c

706 ↗

(On Diff #36665)

wow that #ifdef INET6 is crazy; how do I get rid of INET... but that's another issue... Sadly I touched this code last about 9-10 years ago..

[tuexen]

Here is the story of related changes:

When connect() was called for an IPv6 socket with a remote address, which is an illegal combination with the address the socket is bound to, an error should be returned instead of sending out strange SYN packets. For details see D9163. The fix was committed in r318649.
The fix allows the sending of IPv4 SYN segments if and only if inp_vflag & INP_IPV4 is true and the sending of IPv6 SYN segments if and only if inp_vflag & INP_IPV6 is true. However, it is assumed that the inp_vflag is setup this was before calling tcp6_usr_connect().

The problem reported in Bug 221385 is caused by the fact, that inp_vflag & INP_IPV4 was true for IPv6 sockets, if IPv4 was enabled by using the IPV6_V6ONLY socketoption, but not when IPv4 was enabled because of the sysctl variable net.inet6.ip6.v6only. This inconsistency was fixed in r322648. This inconsistency was not relevant before r318649, since tcp6_usr_connect() was not checking inp_vflag & INP_IPV4, but actually changed (and the code is still there: tcp_userreq.c:606) it. This resulted in inp_vflag & INP_IPV4 and inp_vflag & INP_IPV6 both being true for IPv6 sockets, which are not IPv6 only.

The above works fine, but has introduced a problem with netstat. If inp_vflag & INP_IPV4 is true, it prints the address as IPv4 addresses, not taking inp_vflag & INP_IPV6 into account. See netstat.c:391. For accepted sockets, which are not IPv6 only, both flags were set since r322648 which resulted in wrongly printing IPv4 addresses instead of IPv6 addresses. The patch in this review fixes it.

This explains why this change is necessary and since when netstat was broken and why. Do you agree?

Regarding the testing: I think the client side testing is covered in the description of D9163 and should be used in combination with netstat. The server side requires IPv6 only enabled/disabled (via socket option and sysctl), bound to an IPv4 and IPv6 address (specific and wildcard) and connecting via IPv4 and IPv6. netstat output should be observed. I think this covers all cases. Do you agree?

[tuexen]

This problem was also reported in PR226421.

[bz]

Based on the F2F discussion:
get this one in and we'll sort the problem of https://tools.ietf.org/html/draft-itojun-v6ops-v4mapped-harmful-02 (still being true or not) out at a later time.

[bz]

https://tools.ietf.org/html/rfc6890 table 20 these days disallows v4-mapped addresses on the wire; yiipppiiie!