Fix connect() when used fro TCP sockets in combination with IPv4-mapped IPv6 addresses
ClosedPublic
Actions

Authored by tuexen on Jan 12 2017, 8:43 PM.

Details

Reviewers

hrs
gnn
bz

Group Reviewers

transport

Commits

rS319411: MFC r318649:
rS318649: The connect() system call should return -1 and set errno to EAFNOSUPPORT

Summary

The connect() system call should return -1 and set errno to EAFNOSUPPORT if it is called on a TCP socket

with an IPv6 address and the socket is bound to an IPv4-mapped IPv6 address.
with an IPv4-mapped IPv6 address and the socket is bound to an IPv6 address.

Without this patch, SYN-segments are sent out and eventually the connect() call times out.

Thanks to Jonathan T. Leighton for reporting this issue.

Test Plan

Use a test program to test all address combinations.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 9122
Build 9545: CI src build	Jenkins

Event Timeline

tuexen updated this revision to Diff 23944.Jan 12 2017, 8:43 PM

tuexen retitled this revision from to Fix connect() when used fro TCP sockets in combination with IPv4-mapped IPv6 addresses.

tuexen updated this object.

tuexen edited the test plan for this revision. (Show Details)

tuexen added a reviewer: transport.

tuexen set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptJan 12 2017, 8:43 PM

tuexen added a reviewer: bz.Jan 12 2017, 8:45 PM

bz added a reviewer: hrs.Jan 13 2017, 1:32 AM

Can you attach the test program here?

Here it is

connect_test.c3 KBDownload

.
The usage is
./connect_test local_addr local_port remote_addr remote_port
or
./connect_test remote_addr remote_port
where all addresses need to bei either IPv6 addresses or IPv4-mapped IPv6 addresses.
For example:
./connect_test local_ipv6_address 5000 ::ffff:212.201.121.105 5000
./connext_test ::ffff:local_ipv4_address 5000 2a02:c6a0:4015:10::105 5000

gnn accepted this revision.Jan 13 2017, 3:22 PM

gnn added a reviewer: gnn.

This revision is now accepted and ready to land.Jan 13 2017, 3:22 PM

Stupid question; why are you using the COMPAT macro and not the IN6_IS_ADDR_V4MAPPED()?
Seems you are talking one thing but testing another?

This revision now requires changes to proceed.Jan 14 2017, 3:26 PM

I should add that I used the "request changes" because I want clarification first. You will notice that just swapping out the macros will not really make sense in both cases.

https://tools.ietf.org/html/rfc4291#section-2.5.5

Good question... The answer is that in6p_laddr maps to inp_inc.inc6_laddr see
http://fxr.watson.org/fxr/source/netinet/in_pcb.h#L260
which maps to inp_inc.inc_ie.ie6_laddr
see http://fxr.watson.org/fxr/source/netinet/in_pcb.h#L130
which finally maps to
inp_inc.inc_ie.ie_dependladdr.ie6_local
see http://fxr.watson.org/fxr/source/netinet/in_pcb.h#L105
However, ie_dependladdr, is a union as defined in http://fxr.watson.org/fxr/source/netinet/in_pcb.h#L99
So an IPv6 address is stored in ie6_local, an IPv4 address is stored in ie46_local which has the type
struct in_addr_4in6 which is defined in
http://fxr.watson.org/fxr/source/netinet/in_pcb.h#L75
If an IPv4-mapped IPv6 address is provided in the bind call, it is stored in the ie46_local with ia46_pad32[0], ia46_pad32[1], ia46_pad32[2] being zero. That is what I'm testing for.
This was the only way I found to figure out whether the local address is an IPv4 or IPv6 address...

Any further clarification needed or any comments or questions?

Does udp6_connect() have a similar problem to tcp6_usr_connect()? Looks like it.

So, looking at http://fxr.watson.org/fxr/source/netinet/tcp_usrreq.c#L317 (tcp6_usr_bind) I see we clearly set INP_IPV[46] depending on whether (a) it's IPv6 only, (b) it's a v4mapped address on an IPv6 socket, or (c) worst case to check it's an IPv6 socket with an unspecified address (in which case we (unless people use BINDANY (case I did not check) will always bind to an IPv6 address).

So as in in6_mapped_sockaddr() you should be able to check properly on the socket depending on the INP_IPV[46] flags if I am not mistaken. I think that's the proper solution?

Use the inp_vflag as suggested by bz. I was confused by tcp_usrreq.c which resulted in the wrong setting of inp_vflag, This patch ensures also that the inp_vflag is set correctly.
It handles connect() calls for TCP, UDP and UDPLite. SCTP deals with this case in a different way.

Harbormaster failed remote builds in B9122: Diff 28114!May 6 2017, 6:31 PM

Have you tested this for various combinations, including unbound sockets?
I think in case of unbound sockets this will no longer work, as I don't think INP_IPV[46] would be set at that point yet, are they?

I tested the following six cases using the test program posted earlier in this conversation:

./connect_test 2003:cd:6bd7:7400:20d:b9ff:fe42:7338 5000 2a02:c6a0:4015:10::105 5000 (ECONNREFUSED)
./connect_test 2003:cd:6bd7:7400:20d:b9ff:fe42:7338 5000 ::ffff:212.201.121.105 5000 (EAFNOSUPPORT)
./connect_test ::ffff:192.168.1.203 5000 2a02:c6a0:4015:10::105 5000 (EAFNOSUPPORT)
./connect_test ::ffff:192.168.1.203 5000 ::ffff:212.201.121.105 5000 (ECONNREFUSED)
./connect_test ::ffff:212.201.121.105 5000 (ECONNREFUSED)
./connect_test 2a02:c6a0:4015:10::105 5000 (ECONNREFUSED)

The last two cover the unbound socket case. Please note that at the destination host there is no listening socket. This is the reason why ECONNREFUSED is some of the cases. I verified using wireshark that the packets on the wire look as expected.

The inp_vflag is initially set in tcp_attach (see tcp_usrreq.c and tcp_usrreq.c). This covers unbound sockets, doesn't it?