Index | Thread | Search

From:
Ryan Vogt <rvogt.ca@gmail.com>
Subject:
add source link-layer address option to rad(8)
To:
tech@openbsd.org
Date:
Tue, 7 May 2024 16:15:38 -0700

Download raw body.

Thread
Hi tech@,

This is my first time sending a proposed patch, so hello everyone.
This patch adds support to rad(8) for the source link-layer address
option in router advertisement messages (RFC 4861, section 4.2).

Motivation: if an RA message includes a DNS option (RDNSS, DNSSL, or
both), but the RA message doesn't include the source link-layer
address option, Apple clients (macOS, iOS, etc.) in an IPv6-only
environment will misbehave. I'm more than happy to share the details
of how they misbehave if anyone would like, but I'll leave it at
"misbehave" here for brevity.

Quick notes about the patch:

1. I avoided abbreviating "link-layer address" as "lladrr",
   "linkaddr", etc., where possible, to avoid any confusion with link-
   local addresses. So, while "linklayer_addr" is more verbose, I hope
   it minimizes confusion.

2. RFC 4861 section 4.2 states that the source link-layer address
   option "MAY" be omitted, which might be read as meaning that it
   should be included by default? That said, I didn't want to change
   the default and current behaviour of rad(8) -- that is, not
   including the option -- because excluding it does serve a purpose
   (inbound load sharing, see RFC 4861). So, I made the default in
   rad.conf(5) to exclude the source link-layer address option.

3. I used a "struct nd_opt_hdr" to hold the header for the source
   link-layer address option, instead of making a new struct (e.g.,
   "struct nd_opt_src_linklayer_addr"). I did that because there's
   nothing in the source link-layer address option except the type,
   length, and arbitrary-sized extra data (the arbitrarily sized link-
   layer address), just how a "struct nd_opt_hdr" is defined in
   <netinet/icmp6.h>. I could define a new struct instead if it would
   help readability or consistency.

I'd be grateful for any feedback.

diff --git usr.sbin/rad/frontend.c usr.sbin/rad/frontend.c
index 6748e43732d..b8db63e02b6 100644
--- usr.sbin/rad/frontend.c
+++ usr.sbin/rad/frontend.c
@@ -56,6 +56,7 @@
 #include <sys/uio.h>
 
 #include <net/if.h>
+#include <net/if_dl.h>
 #include <net/if_types.h>
 #include <net/route.h>
 
@@ -128,6 +129,7 @@ void			 frontend_startup(void);
 void			 icmp6_receive(int, short, void *);
 void			 join_all_routers_mcast_group(struct ra_iface *);
 void			 leave_all_routers_mcast_group(struct ra_iface *);
+struct sockaddr_dl	*find_linklayer_sockaddr(struct ifaddrs *, int);
 int			 get_link_state(char *);
 int			 get_ifrdomain(char *);
 void			 merge_ra_interface(char *, char *);
@@ -729,6 +731,23 @@ find_ra_iface_conf(struct ra_iface_conf_head *head, char *if_name)
 	return (NULL);
 }
 
+struct sockaddr_dl*
+find_linklayer_sockaddr(struct ifaddrs *ifap, int if_index)
+{
+	struct ifaddrs	*ifa;
+
+	for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+		if (ifa->ifa_addr == NULL ||
+		    ifa->ifa_addr->sa_family != AF_LINK)
+			continue;
+		if (((struct sockaddr_dl *)ifa->ifa_addr)->sdl_index ==
+		    if_index)
+			return (struct sockaddr_dl *)ifa->ifa_addr;
+	}
+	log_warn("find_linklayer_sockaddr");
+	return (NULL);
+}
+
 int
 get_link_state(char *if_name)
 {
@@ -1110,6 +1129,9 @@ build_packet(struct ra_iface *ra_iface)
 	struct ra_rdnss_conf		*ra_rdnss;
 	struct ra_dnssl_conf		*ra_dnssl;
 	struct ra_pref64_conf		*pref64;
+	struct nd_opt_hdr		*ndopt_srclinklayeraddr;
+	struct ifaddrs			*ifap;
+	struct sockaddr_dl		*sa_linklayer;
 	size_t				 len, label_len;
 	uint8_t				*p, buf[RA_MAX_SIZE];
 	char				*label_start, *label_end;
@@ -1136,6 +1158,20 @@ build_packet(struct ra_iface *ra_iface)
 	    entry)
 		len += sizeof(struct nd_opt_pref64);
 
+	ifap = NULL;
+	sa_linklayer = NULL;
+	if (ra_iface_conf->ra_options.src_linklayer_addr) {
+		if (getifaddrs(&ifap) != 0)
+			log_warn("getifaddrs");
+		else
+			sa_linklayer = find_linklayer_sockaddr(ifap,
+			    ra_iface->if_index);
+		if (sa_linklayer != NULL)
+			/* round up to 8 byte boundary */
+			len += (sizeof(struct nd_opt_hdr) +
+			    sa_linklayer->sdl_alen + 7) & ~7;
+	}
+
 	if (len > sizeof(ra_iface->data))
 		fatalx("%s: packet too big", __func__); /* XXX send multiple */
 
@@ -1283,6 +1319,22 @@ build_packet(struct ra_iface *ra_iface)
 		p += sizeof(struct nd_opt_pref64);
 	}
 
+	if (sa_linklayer != NULL) {
+		ndopt_srclinklayeraddr = (struct nd_opt_hdr *)p;
+		ndopt_srclinklayeraddr->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
+		/* length expressed in units of 8 octets */
+		ndopt_srclinklayeraddr->nd_opt_len =
+		    ((sizeof(struct nd_opt_hdr) + sa_linklayer->sdl_alen + 7) &
+		    ~7) / 8;
+		p += sizeof(struct nd_opt_hdr);
+		memcpy(p, LLADDR(sa_linklayer), sa_linklayer->sdl_alen);
+		p += sa_linklayer->sdl_alen;
+		while (((uintptr_t)p) % 8 != 0)
+			*p++ = '\0';
+	}
+	if (ifap != NULL)
+		freeifaddrs(ifap);
+
 	if (len != ra_iface->datalen || memcmp(buf, ra_iface->data, len)
 	    != 0) {
 		memcpy(ra_iface->data, buf, len);
diff --git usr.sbin/rad/parse.y usr.sbin/rad/parse.y
index 66cc0c0ab64..fb9f4628854 100644
--- usr.sbin/rad/parse.y
+++ usr.sbin/rad/parse.y
@@ -122,6 +122,7 @@ typedef struct {
 %token	CONFIGURATION OTHER LIFETIME REACHABLE TIME RETRANS TIMER
 %token	AUTO PREFIX VALID PREFERENCE PREFERRED LIFETIME ONLINK AUTONOMOUS
 %token	ADDRESS_CONFIGURATION DNS NAMESERVER SEARCH MTU NAT64 HIGH MEDIUM LOW
+%token	SOURCE LINK_LAYER
 
 %token	<v.string>	STRING
 %token	<v.number>	NUMBER
@@ -226,6 +227,9 @@ ra_opt_block	: DEFAULT ROUTER yesno {
 		| RETRANS TIMER NUMBER {
 			ra_options->retrans_timer = $3;
 		}
+		| SOURCE LINK_LAYER ADDRESS yesno {
+			ra_options->src_linklayer_addr = $4;
+		}
 		| MTU NUMBER {
 			ra_options->mtu = $2;
 		}
@@ -523,6 +527,7 @@ lookup(char *s)
 		{"interface",		RA_IFACE},
 		{"lifetime",		LIFETIME},
 		{"limit",		LIMIT},
+		{"link-layer",		LINK_LAYER},
 		{"low",			LOW},
 		{"managed",		MANAGED},
 		{"medium",		MEDIUM},
@@ -539,6 +544,7 @@ lookup(char *s)
 		{"retrans",		RETRANS},
 		{"router",		ROUTER},
 		{"search",		SEARCH},
+		{"source",		SOURCE},
 		{"time",		TIME},
 		{"timer",		TIMER},
 		{"valid",		VALID},
diff --git usr.sbin/rad/printconf.c usr.sbin/rad/printconf.c
index 184a5df2dc1..4cf64fabd93 100644
--- usr.sbin/rad/printconf.c
+++ usr.sbin/rad/printconf.c
@@ -78,6 +78,8 @@ print_ra_options(const char *indent, const struct ra_options_conf *ra_options)
 	printf("%srouter lifetime %d\n", indent, ra_options->router_lifetime);
 	printf("%sreachable time %u\n", indent, ra_options->reachable_time);
 	printf("%sretrans timer %u\n", indent, ra_options->retrans_timer);
+	printf("%ssource link-layer address %s\n", indent,
+	    yesno(ra_options->src_linklayer_addr));
 	if (ra_options->mtu > 0)
 		printf("%smtu %u\n", indent, ra_options->mtu);
 
diff --git usr.sbin/rad/rad.conf.5 usr.sbin/rad/rad.conf.5
index f82ccfd216e..11545c1c23f 100644
--- usr.sbin/rad/rad.conf.5
+++ usr.sbin/rad/rad.conf.5
@@ -127,6 +127,10 @@ The default is medium.
 .\" XXX
 .\" .It Ic retrans timer Ar number
 .\" XXX
+.It Ic source link-layer address Pq Ic yes Ns | Ns Ic no
+Add a source link-layer address option to router advertisement messages, to
+communicate the link-layer address of the sending interface.
+The default is no.
 .El
 .Sh INTERFACES
 A list of interfaces or interface groups to send advertisements on:
diff --git usr.sbin/rad/rad.h usr.sbin/rad/rad.h
index 787e78c7c4a..509f91b208a 100644
--- usr.sbin/rad/rad.h
+++ usr.sbin/rad/rad.h
@@ -97,6 +97,7 @@ struct ra_options_conf {
 	int		router_lifetime;	/* default router lifetime */
 	uint32_t	reachable_time;
 	uint32_t	retrans_timer;
+	int		src_linklayer_addr;
 	uint32_t	mtu;
 	uint32_t	rdns_lifetime;
 	SIMPLEQ_HEAD(, ra_rdnss_conf)		 ra_rdnss_list;