Page MenuHomeFreeBSD

WIP: inpcb: laddr checking
ClosedPublic

Authored by markj on Feb 13 2023, 11:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 5, 5:32 PM
Unknown Object (File)
Tue, Nov 19, 10:38 AM
Unknown Object (File)
Nov 1 2024, 12:03 PM
Unknown Object (File)
Oct 21 2024, 3:16 AM
Unknown Object (File)
Sep 18 2024, 6:55 PM
Unknown Object (File)
Sep 18 2024, 6:21 AM
Unknown Object (File)
Sep 17 2024, 5:38 PM
Unknown Object (File)
Sep 16 2024, 8:21 PM
Subscribers

Details

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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 49752
Build 46642: arc lint + arc unit

Event Timeline

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

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.

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.

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 :-)

sys/netinet/ip_input.c
522

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

This revision was not accepted when it landed; it landed in state Needs Review.Mar 6 2023, 8:20 PM
This revision was automatically updated to reflect the committed changes.
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.