Invalid addresses in sending or connecting

[rrs]

Summary

Address 0.0.0.0 and 255.255.255.255 really should not be allowed to be sent
to in TCP but somehow this is overlooked. This patch fixes that.

Test Plan

Make sure we can no longer try to connect to or send to 0.0.0.0 or 255.255.255.255 with pkt-drill or any other favorite tooling

[rscheff]

tcp_usrreq.c

547

Do you want to use INADDR_ANY and INADDR_BROADCAST here instead of the hardcoded numericals?

(Why is the reviews context missing when you generate a Diff, rrs@?)

In the inet6 case below, should IN6_IS_ADDR_UNSPECIFIED (::) also be excluded?

[tuexen]

tcp_usrreq.c

547

I guess using the constants if preferred.

Regarding the IPv6 question. The wirdcard address gets automagically transformed to the loopback address. See in6_pcb.c:378.

I guess the missing context is due to the usage of simply svn diff without requesting the context.

[rscheff]

A commit (rS361080) referenced this Diff due to a mistype, binding the wrong commit here., when D24826 should have been targeted. Reopening, so this actual issue doesn't drop to the floor.

[rscheff]

@rrs can you please re-upload the Diff, with the defined constants from in.h instead of hardcoded numbers.

I don't know how to bind the older (actual) Diff back here, when the commit bound this to a different Diff...

[melifaro]

0.0.0.0 and :: are unicast addresses, and it's a bit unclear to me why transport protocol would want to decide that some underlying addresses are invalid. They also work in some other operating systems.
255.255.255.255 is indeed broadcast and not intended to use with TCP, but the same argument can be made around multicast address range. Do we want to include it as well?

Could you please add a reference to any standard explaining why these should be prohibited?

[rscheff]

TCP is strictly unicast connection oriented. There are other protocols to implement reliable transport over multicast (eg RFC3208). TCP simply does not work with a multicast address as either source or destination, since there are no mechanisms to arbitrate between one-to-many and then many-to-one responses.

Multicast is already restricted in the code (see https://reviews.freebsd.org/source/src/browse/head/sys/netinet/tcp_usrreq.c$323) for both IPv4 and IPv6.

Incoming packets should never contain the placeholder value INADDR_ANY (0.0.0.0 or :: ) but always be resolved to an actual (unicast) address.

For some reason, https://reviews.freebsd.org/D24852?id=71820 does not show any context though.

[rrs]

Hmm I somehow got the wrong patch here.

[rrs]

tcp_usrreq.c

547

Yeah I should use those.

[rrs]

Note that we don't want to commit these for a while since skyzaller uses these addresses but it
is something we should do.

[rscheff]

Sorry to have overlooked this inconsistency earlier.