Page MenuHomeFreeBSD

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

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

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...

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...

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?

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

Address whitespace issue.

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...

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