Paths

Table of Contentst

(Re)allow 0.0.0.0 to be used as a destination address in connect() for TCP
ClosedPublic
Actions

Authored by tuexen on Jun 22 2020, 1:54 PM.

Details

Reviewers

rrs
rscheff
rgrimes
• ian

Group Reviewers

transport

Commits

rS363256: (Re)-allow 0.0.0.0 to be used as an address in connect() for TCP

Summary

In r361752 the connect() call for TCP/IPv4 was changed in a way that using INADDR_ANY or INADDR_BROADCAST results in a failure indicating EAFNOSUPPORT. Using these addresses does not make sense, since 0.0.0.0 is not allowed as a destination address for IPv4 packets on the wire and TCP does not support multicast.

However, this change breaks backwards compatibility, since FreeBSD supports using INADDR_ANY by mapping it in in_pcbconnect_setup() (see in_pcb.c:1335) to the primary local address.

Two people reported this regression reporting that they use 0.0.0.0 as destination address. Therefore, this change is to re-allow INADDR_ANY as the destination address used in connnect() calls for TCP sockets. This also makes IPv4 and IPv6 behaviour more consistent, since IPv6 maps ::0 to ::1 when connecting.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 31879

Event Timeline

tuexen created this revision.Jun 22 2020, 1:54 PM

Herald added a reviewer: transport. · View Herald TranscriptJun 22 2020, 1:54 PM

Herald added subscribers: melifaro, imp. · View Herald Transcript

tuexen requested review of this revision.Jun 22 2020, 1:54 PM

Harbormaster completed remote builds in B31874: Diff 73468.Jun 22 2020, 1:54 PM

tuexen edited the summary of this revision. (Show Details)Jun 22 2020, 2:00 PM

Should this

If the destination address is INADDR_ANY,
use the primary local address.

be documented in a man page in addition to the comment in the code?

This revision is now accepted and ready to land.Jun 22 2020, 2:05 PM

In D25401#560404, @rscheff wrote:

Should this

If the destination address is INADDR_ANY,

use the primary local address.

be documented in a man page in addition to the comment in the code?

Don't know. The man page for connect is pretty generic. Not talking about TCP sockets in particular. So it could go into the man page of TCP. However, this patch just restores a functionality which was there...

In D25401#560408, @tuexen wrote:

In D25401#560404, @rscheff wrote:

Should this

If the destination address is INADDR_ANY,

use the primary local address.

It took me a few "brain loops" to realize you mean the "local loopback interface address" or is there actually some concept of a "primary local IP address."?

be documented in a man page in addition to the comment in the code?

I think we should document this.

Don't know. The man page for connect is pretty generic. Not talking about TCP sockets in particular. So it could go into the man page of TCP. However, this patch just restores a functionality which was there...

The in_pcb_setup code is that called independent of L4? Ie for all sockets? Or have i lost my way in the twisty maze of IP layers?

BUT, given we are in a regressed state I have no problem with delaying any documentation changes for a future or NULL commit.

The in_pcb_setup code is that called independent of L4? Ie for all sockets? Or have i lost my way in the twisty maze of IP layers?

TCP calls it: tcp_usrreq.c:1566. The logic is independent from L4. They are also rewriting INADDR_BROADCAST, which doesn't make sense for TCP, but we are catching this now...

In D25401#560437, @tuexen wrote:

The in_pcb_setup code is that called independent of L4? Ie for all sockets? Or have i lost my way in the twisty maze of IP layers?

TCP calls it: tcp_usrreq.c:1566. The logic is independent from L4. They are also rewriting INADDR_BROADCAST, which doesn't make sense for TCP, but we are catching this now...

Is it not called for ALL L4 protocols is what I was getting at, hence it is not just TCP you should consider being documented. Though the "connect" man page is tearse it does have mention at least in one place of INADDR_BROADCAST and INADDR_NONE in the EACCES case. Also did the earlier change document the additional details of the error return?

In D25401#560438, @rgrimes wrote:

In D25401#560437, @tuexen wrote:

The in_pcb_setup code is that called independent of L4? Ie for all sockets? Or have i lost my way in the twisty maze of IP layers?

TCP calls it: tcp_usrreq.c:1566. The logic is independent from L4. They are also rewriting INADDR_BROADCAST, which doesn't make sense for TCP, but we are catching this now...

Is it not called for ALL L4 protocols is what I was getting at, hence it is not just TCP you should consider being documented. Though the "connect" man page is tearse it does have mention at least in one place of INADDR_BROADCAST and INADDR_NONE in the EACCES case. Also did the earlier change document the additional details of the error return?

I missed that. So we should fail with EACCES instead of EAFNOSUPPORT. I'll update this patch. This means at least the error path is documented.

Indicate EACCES instead of EAFNOSUPPORT in case the user tries to connect to INADDR_BROADCAST as indicated by the man page of connect.

This revision now requires review to proceed.Jun 22 2020, 7:58 PM

Harbormaster completed remote builds in B31878: Diff 73477.Jun 22 2020, 7:58 PM

Address whitespace issue.

Harbormaster completed remote builds in B31879: Diff 73478.Jun 22 2020, 8:00 PM

trasz added a subscriber: trasz.Jun 22 2020, 8:18 PM

Please note that the TCP code indicates EAFNOSUPPORT when you try to connect to a multicast address for IPv4 or IPv6. Note that this is not explicitly mentioned in the man page, but I would expect the same indication for using multicast or broadcast, when it is not allowed by the protocol.

Linux and Solaris return ENETUNREACH when connecting to the broadcast address or a multicast address...

mateusz_serveraptor.com added a subscriber: mateusz_serveraptor.com.Jun 24 2020, 7:51 AM

rrs accepted this revision.Jul 16 2020, 4:33 PM

This revision is now accepted and ready to land.Jul 16 2020, 4:33 PM

Closed by commit rS363256: (Re)-allow 0.0.0.0 to be used as an address in connect() for TCP (authored by tuexen). · Explain WhyJul 16 2020, 4:46 PM

This revision was automatically updated to reflect the committed changes.

tuexen added a commit: rS363256: (Re)-allow 0.0.0.0 to be used as an address in connect() for TCP.