]> CyberLeo.Net >> Repos - FreeBSD/FreeBSD.git/commit
Fix dst/netmask handling in routing socket code.
authorAlexander V. Chernikov <melifaro@FreeBSD.org>
Tue, 16 Feb 2021 20:30:04 +0000 (20:30 +0000)
committerAlexander V. Chernikov <melifaro@FreeBSD.org>
Thu, 11 Mar 2021 08:25:01 +0000 (08:25 +0000)
commit2d227a6ec371b970c174c0e916af5abd83deded7
treea2595c5c73fd68097bd9f2e97bffa309c61b810c
parent19fdc202976854a973067edd5a3d0c1d13a03846
Fix dst/netmask handling in routing socket code.

Traditionally routing socket code did almost zero checks on
 the input message except for the most basic size checks.

This resulted in the unclear KPI boundary for the routing system code
 (`rtrequest*` and now `rib_action()`) w.r.t message validness.

Multiple potential problems and nuances exists:
* Host bits in RTAX_DST sockaddr. Existing applications do send prefixes
 with hostbits uncleared. Even `route(8)` does this, as they hope the kernel
 would do the job of fixing it. Code inside `rib_action()` needs to handle
 it on its own (see `rt_maskedcopy()` ugly hack).
* There are multiple way of adding the host route: it can be DST without
 netmask or DST with /32(/128) netmask. Also, RTF_HOST has to be set correspondingly.
 Currently, these 2 options create 2 DIFFERENT routes in the kernel.
* no sockaddr length/content checking for the "secondary" fields exists: nothing
 stops rtsock application to send sockaddr_in with length of 25 (instead of 16).
 Kernel will accept it, install to RIB as is and propagate to all rtsock consumers,
 potentially triggering bugs in their code. Same goes for sin_port, sin_zero, etc.

The goal of this change is to make rtsock verify all sockaddr and prefix consistency.
Said differently, `rib_action()` or internals should NOT require to change any of the
 sockaddrs supplied by `rt_addrinfo` structure due to incorrectness.

To be more specific, this change implements the following:
* sockaddr cleanup/validation check is added immediately after getting sockaddrs from rtm.
* Per-family dst/netmask checks clears host bits in dst and zeros all dst/netmask "secondary" fields.
* The same netmask checking code converts /32(/128) netmasks to "host" route case
 (NULL netmask, RTF_HOST), removing the dualism.
* Instead of allowing ANY "known" sockaddr families (0<..<AF_MAX), allow only actually
 supported ones (inet, inet6, link).
* Automatically convert `sockaddr_sdl` (AF_LINK) gateways to
  `sockaddr_sdl_short`.

Reported by: Guy Yur <guyyur at gmail.com>
Reviewed By: donner
Approved by: re(gjb)
Differential Revision: https://reviews.freebsd.org/D28668

(cherry picked from commit e1bdecd9f60a80604a351e38cab7cfc56e308c66)
sys/net/if_llatbl.c
sys/net/rtsock.c
tests/sys/net/routing/rtsock_common.h
usr.sbin/arp/arp.c
usr.sbin/ndp/ndp.c