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
Differential Revision: https://reviews.freebsd.org/D28668
MFC after:	3 days
This commit is contained in:
Alexander V. Chernikov 2021-02-16 20:30:04 +00:00
parent 600eade2fb
commit 2fe5a79425
2 changed files with 195 additions and 10 deletions

View File

@ -70,6 +70,7 @@
#include <netinet/if_ether.h> #include <netinet/if_ether.h>
#include <netinet/ip_carp.h> #include <netinet/ip_carp.h>
#ifdef INET6 #ifdef INET6
#include <netinet6/in6_var.h>
#include <netinet6/ip6_var.h> #include <netinet6/ip6_var.h>
#include <netinet6/scope6_var.h> #include <netinet6/scope6_var.h>
#endif #endif
@ -173,6 +174,7 @@ static int rtsock_msg_buffer(int type, struct rt_addrinfo *rtinfo,
struct walkarg *w, int *plen); struct walkarg *w, int *plen);
static int rt_xaddrs(caddr_t cp, caddr_t cplim, static int rt_xaddrs(caddr_t cp, caddr_t cplim,
struct rt_addrinfo *rtinfo); struct rt_addrinfo *rtinfo);
static int cleanup_xaddrs(struct rt_addrinfo *info);
static int sysctl_dumpentry(struct rtentry *rt, void *vw); static int sysctl_dumpentry(struct rtentry *rt, void *vw);
static int sysctl_dumpnhop(struct rtentry *rt, struct nhop_object *nh, static int sysctl_dumpnhop(struct rtentry *rt, struct nhop_object *nh,
uint32_t weight, struct walkarg *w); uint32_t weight, struct walkarg *w);
@ -636,11 +638,9 @@ fill_addrinfo(struct rt_msghdr *rtm, int len, u_int fibnum, struct rt_addrinfo *
return (EINVAL); return (EINVAL);
info->rti_flags = rtm->rtm_flags; info->rti_flags = rtm->rtm_flags;
if (info->rti_info[RTAX_DST] == NULL || error = cleanup_xaddrs(info);
info->rti_info[RTAX_DST]->sa_family >= AF_MAX || if (error != 0)
(info->rti_info[RTAX_GATEWAY] != NULL && return (error);
info->rti_info[RTAX_GATEWAY]->sa_family >= AF_MAX))
return (EINVAL);
saf = info->rti_info[RTAX_DST]->sa_family; saf = info->rti_info[RTAX_DST]->sa_family;
/* /*
* Verify that the caller has the appropriate privilege; RTM_GET * Verify that the caller has the appropriate privilege; RTM_GET
@ -739,7 +739,14 @@ handle_rtm_get(struct rt_addrinfo *info, u_int fibnum,
RIB_RLOCK(rnh); RIB_RLOCK(rnh);
if (info->rti_info[RTAX_NETMASK] == NULL) { /*
* By (implicit) convention host route (one without netmask)
* means longest-prefix-match request and the route with netmask
* means exact-match lookup.
* As cleanup_xaddrs() cleans up info flags&addrs for the /32,/128
* prefixes, use original data to check for the netmask presence.
*/
if ((rtm->rtm_addrs & RTA_NETMASK) == 0) {
/* /*
* Provide longest prefix match for * Provide longest prefix match for
* address lookup (no mask). * address lookup (no mask).
@ -1286,6 +1293,188 @@ rt_xaddrs(caddr_t cp, caddr_t cplim, struct rt_addrinfo *rtinfo)
return (0); return (0);
} }
static inline void
fill_sockaddr_inet(struct sockaddr_in *sin, struct in_addr addr)
{
const struct sockaddr_in nsin = {
.sin_family = AF_INET,
.sin_len = sizeof(struct sockaddr_in),
.sin_addr = addr,
};
*sin = nsin;
}
static inline void
fill_sockaddr_inet6(struct sockaddr_in6 *sin6, const struct in6_addr *addr6,
uint32_t scopeid)
{
const struct sockaddr_in6 nsin6 = {
.sin6_family = AF_INET6,
.sin6_len = sizeof(struct sockaddr_in6),
.sin6_addr = *addr6,
.sin6_scope_id = scopeid,
};
*sin6 = nsin6;
}
static int
cleanup_xaddrs_gateway(struct rt_addrinfo *info)
{
struct sockaddr *gw = info->rti_info[RTAX_GATEWAY];
switch (gw->sa_family) {
#ifdef INET
case AF_INET:
{
struct sockaddr_in *gw_sin = (struct sockaddr_in *)gw;
if (gw_sin->sin_len < sizeof(struct sockaddr_in)) {
printf("gw sin_len too small\n");
return (EINVAL);
}
fill_sockaddr_inet(gw_sin, gw_sin->sin_addr);
}
break;
#endif
#ifdef INET6
case AF_INET6:
{
struct sockaddr_in6 *gw_sin6 = (struct sockaddr_in6 *)gw;
if (gw_sin6->sin6_len < sizeof(struct sockaddr_in6)) {
printf("gw sin6_len too small\n");
return (EINVAL);
}
fill_sockaddr_inet6(gw_sin6, &gw_sin6->sin6_addr, 0);
break;
}
#endif
case AF_LINK:
{
struct sockaddr_dl_short *gw_sdl;
gw_sdl = (struct sockaddr_dl_short *)gw;
if (gw_sdl->sdl_len < sizeof(struct sockaddr_dl_short)) {
printf("gw sdl_len too small\n");
return (EINVAL);
}
const struct sockaddr_dl_short sdl = {
.sdl_family = AF_LINK,
.sdl_len = sizeof(struct sockaddr_dl_short),
.sdl_index = gw_sdl->sdl_index,
};
*gw_sdl = sdl;
break;
}
}
return (0);
}
static int
cleanup_xaddrs_inet(struct rt_addrinfo *info)
{
struct sockaddr_in *dst_sa, *mask_sa;
/* Check & fixup dst/netmask combination first */
dst_sa = (struct sockaddr_in *)info->rti_info[RTAX_DST];
mask_sa = (struct sockaddr_in *)info->rti_info[RTAX_NETMASK];
struct in_addr mask = {
.s_addr = mask_sa ? mask_sa->sin_addr.s_addr : INADDR_BROADCAST,
};
struct in_addr dst = {
.s_addr = htonl(ntohl(dst_sa->sin_addr.s_addr) & ntohl(mask.s_addr))
};
if (dst_sa->sin_len < sizeof(struct sockaddr_in)) {
printf("dst sin_len too small\n");
return (EINVAL);
}
if (mask_sa && mask_sa->sin_len < sizeof(struct sockaddr_in)) {
printf("mask sin_len too small\n");
return (EINVAL);
}
fill_sockaddr_inet(dst_sa, dst);
if (mask.s_addr != INADDR_BROADCAST)
fill_sockaddr_inet(mask_sa, mask);
else {
info->rti_info[RTAX_NETMASK] = NULL;
info->rti_flags |= RTF_HOST;
info->rti_addrs &= ~RTA_NETMASK;
}
/* Check gateway */
if (info->rti_info[RTAX_GATEWAY] != NULL)
return (cleanup_xaddrs_gateway(info));
return (0);
}
static int
cleanup_xaddrs_inet6(struct rt_addrinfo *info)
{
struct sockaddr_in6 *dst_sa, *mask_sa;
struct in6_addr mask;
/* Check & fixup dst/netmask combination first */
dst_sa = (struct sockaddr_in6 *)info->rti_info[RTAX_DST];
mask_sa = (struct sockaddr_in6 *)info->rti_info[RTAX_NETMASK];
mask = mask_sa ? mask_sa->sin6_addr : in6mask128;
IN6_MASK_ADDR(&dst_sa->sin6_addr, &mask);
if (dst_sa->sin6_len < sizeof(struct sockaddr_in6)) {
printf("dst sin6_len too small\n");
return (EINVAL);
}
if (mask_sa && mask_sa->sin6_len < sizeof(struct sockaddr_in6)) {
printf("mask sin6_len too small\n");
return (EINVAL);
}
fill_sockaddr_inet6(dst_sa, &dst_sa->sin6_addr, 0);
if (!IN6_ARE_ADDR_EQUAL(&mask, &in6mask128))
fill_sockaddr_inet6(mask_sa, &mask, 0);
else {
info->rti_info[RTAX_NETMASK] = NULL;
info->rti_flags |= RTF_HOST;
info->rti_addrs &= ~RTA_NETMASK;
}
/* Check gateway */
if (info->rti_info[RTAX_GATEWAY] != NULL)
return (cleanup_xaddrs_gateway(info));
return (0);
}
static int
cleanup_xaddrs(struct rt_addrinfo *info)
{
int error = EAFNOSUPPORT;
if (info->rti_info[RTAX_DST] == NULL)
return (EINVAL);
switch (info->rti_info[RTAX_DST]->sa_family) {
#ifdef INET
case AF_INET:
error = cleanup_xaddrs_inet(info);
break;
#endif
#ifdef INET6
case AF_INET6:
error = cleanup_xaddrs_inet6(info);
break;
#endif
}
return (error);
}
/* /*
* Fill in @dmask with valid netmask leaving original @smask * Fill in @dmask with valid netmask leaving original @smask
* intact. Mostly used with radix netmasks. * intact. Mostly used with radix netmasks.

View File

@ -826,10 +826,6 @@ _validate_message_sockaddrs(char *buffer, int rtm_len, size_t offset, int rtm_ad
} }
sa = (struct sockaddr *)((char *)sa + SA_SIZE(sa)); sa = (struct sockaddr *)((char *)sa + SA_SIZE(sa));
} }
RTSOCK_ATF_REQUIRE_MSG((struct rt_msghdr *)buffer, parsed_len == rtm_len,
"message len != parsed len: expected %d parsed %d",
rtm_len, (int)parsed_len);
} }
/* /*