WIP: inpcb: laddr checking

[markj]

Summary

The assertions added in commit b0ccf53f2455 ("inpcb: Assert against
wildcard addrs in in_pcblookup_hash_locked()") turned up some
interesting bugs.

In IPv4, it is possible to have a route from address 0.0.0.0; I believe
this is mostly used for DHCP wherein UDP packets are sent with src
address 0.0.0.0. For example, dhclient temporarily assigns this address
to an interface when soliciting leases.

There is nothing preventing 0.0.0.0 from being selected as a src address
by in_pcbladdr(), however. I believe that we should prohibit this,
hence, modify in(6)_pcbladdr() to return EHOSTUNREACH instead of an
unspecified address. I believe this already cannot happen for IPv6,
which is more strict about this, but I'm not certain.

Also ensure that protocols reject unspecified addresses. ip6_input()
already prohibits an unspecified destination address, do the same in
ip_input() per RFC 1122. Also make TCP reject unspecified source
addresses. I'm not quite sure what to do about UDP yet since
udp_input() may legitimately receive packets with an unspecified src
address.

Reported by: syzkaller
Sponsored by: Klara, Inc.
Sponsored by: Modirum MDPay

[glebius]

I'll prod @melifaro to take a look, too.

[melifaro]

Thank you for working on this!

LGTM.

In IPv4, ip_output() does not allow INADDR_ANY as the source, (except when one explicitly sets 0.0.0.0 address on the interface which looks like a dirty hack). I agree we should forbid it for the sockets. If one wants to send such head, they need to use BPF.
I've raised D38908 to remove 0.0.0.0 setting from the base dhclient.

For IPv6: despite the fact that RFC4291 clause 2.5.2 mentions One example of its use is in the Source Address field of any IPv6 packets sent by an initializing host before it has learned its own address , the socket API (RFC 3542 clause 6.2) does not allow to set IPv6 source as unspecified. Thus, we shouldn't allow it on the sockets either. It's still possible to construct such packets with BPF.

sys/netinet/ip_input.c

522

I'd extend it to 0.0.0.0/8 and mention https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml as the document source.

[markj]

sys/netinet/ip_input.c

522

Could we perhaps do this in a follow-up commit? At the moment I'd like to simply make it impossible to trigger the assertions from commit b0ccf53f2455.

[melifaro]

sys/netinet/ip_input.c

522

Sure. It was fine for 30+? years, so we can wait :-) I'd only suggest referring the link above instead of the RFC dated back to 1989 :-)

[markj]

sys/netinet/ip_input.c

522

Well, the table entry for 0.0.0.0/8 references RFC 791 from 1981. :)

[melifaro]

sys/netinet/ip_input.c

522

Let me rephrase :-) I see IANA link as a much newer summary on the topic with pretty good credibility.