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.
Details
Details
- Reviewers
tuexen rscheff - Group Reviewers
transport - Commits
- rS361752: We should never allow either the broadcast or IN_ADDR_ANY to be
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
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| tcp_usrreq.c | ||
|---|---|---|
| 547 ↗ | (On Diff #71820) | 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? |
| tcp_usrreq.c | ||
|---|---|---|
| 547 ↗ | (On Diff #71820) | 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. |
Comment Actions
@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...
Comment Actions
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?
Comment Actions
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.
| tcp_usrreq.c | ||
|---|---|---|
| 547 ↗ | (On Diff #71820) | Yeah I should use those. |
Comment Actions
Note that we don't want to commit these for a while since skyzaller uses these addresses but it
is something we should do.