Index | Thread | Search

From:
Klemens Nanni <kn@openbsd.org>
Subject:
Re: dhcpd(8): use the rdomain it executes with
To:
David Gwynne <david@gwynne.id.au>, tech@openbsd.org
Date:
Mon, 9 Jun 2025 01:02:51 +0000

Download raw body.

Thread
07.06.2025 07:36, David Gwynne пишет:
> dhcpd is weird about how it interacts with rdomains. it uses the rdomain
> from the first interface given to it on the command line.
> 
> i would argue it's more natural to use the rdomain it's executed
> with, which in turn integrates it better with stuff like rcctl. it
> also makes it work as expected with the "-u" flag.
> 
> it shuffles some of the address/subnet matching to avoid reporting
> overlapping address space if you have multiple interfaces with the same
> networks but in separate rdomains.
> 
> i'm not even sure we need to mention rdomains in the manpage with this
> change either.

I'd remove any mention of it;  logs have per-interface reactions, so if
you setup rdomans(4) and still run into problems, they're all you need.

> 
> tests? ok?

Works and reads OK, but the manual has an earlier untouched mention of
rdomain wrt. the interface argmuments.

On that:  [if [... ifN]] should be just [interface ...], so feel free to
add that to your diff in both manual and usage.

> 
> at some point i should introduce dhcpd to sys/queue.h.
> 
> Index: dhcpd.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.8,v
> diff -u -p -r1.36 dhcpd.8
> --- dhcpd.8	27 Jun 2024 16:39:31 -0000	1.36
> +++ dhcpd.8	7 Jun 2025 04:29:40 -0000
> @@ -132,15 +132,8 @@ on systems where
>  is unable to identify non-broadcast interfaces.
>  .Pp
>  .Nm
> -normally runs in routing domain 0.
> -In order to run in another
> -.Xr rdomain 4 ,
> -.Nm
> -needs to be started with a list of interfaces
> -which share the same routing domain,
> -which allows
> -.Nm
> -to set its own routing domain accordingly.
> +will skip interfaces configured in a different routing domain to
> +the one it was executed in.
>  .Pp
>  DHCP traffic always bypasses IPsec.
>  Otherwise there could be situations when a server has an IPsec SA for the
> Index: dhcpd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.c,v
> diff -u -p -r1.61 dhcpd.c
> --- dhcpd.c	4 Jun 2025 21:16:25 -0000	1.61
> +++ dhcpd.c	7 Jun 2025 04:29:40 -0000
> @@ -71,6 +71,7 @@ struct group root_group;
>  u_int16_t server_port;
>  u_int16_t client_port;
>  
> +int rdomain;
>  struct passwd *pw;
>  int log_priority;
>  int pfpipe[2];
> @@ -87,7 +88,7 @@ char *leased_tab = NULL;
>  int
>  main(int argc, char *argv[])
>  {
> -	int ch, cftest = 0, rdomain = -1, udpsockmode = 0;
> +	int ch, cftest = 0, udpsockmode = 0;
>  	int debug = 0, verbose = 0;
>  	char *sync_iface = NULL;
>  	char *sync_baddr = NULL;
> @@ -98,6 +99,8 @@ main(int argc, char *argv[])
>  	log_init(1, LOG_DAEMON);	/* log to stderr until daemonized */
>  	log_setverbose(1);
>  
> +	rdomain = getrtable();
> +
>  	opterr = 0;
>  	while ((ch = getopt(argc, argv, "A:C:L:c:dfl:nu::vY:y:")) != -1)
>  		switch (ch) {
> @@ -197,11 +200,7 @@ main(int argc, char *argv[])
>  
>  	db_startup();
>  	if (!udpsockmode || argc > 0)
> -		discover_interfaces(&rdomain);
> -
> -	if (rdomain != -1)
> -		if (setrtable(rdomain) == -1)
> -			fatal("setrtable");
> +		discover_interfaces();
>  
>  	if (syncsend || syncrecv) {
>  		if (sync_init(sync_iface, sync_baddr, sync_port) == -1)
> Index: dhcpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.h,v
> diff -u -p -r1.72 dhcpd.h
> --- dhcpd.h	31 May 2025 07:47:45 -0000	1.72
> +++ dhcpd.h	7 Jun 2025 04:29:40 -0000
> @@ -443,7 +443,7 @@ ssize_t receive_packet(struct interface_
>  extern struct interface_info *interfaces;
>  extern struct protocol *protocols;
>  extern struct dhcpd_timeout *timeouts;
> -void discover_interfaces(int *);
> +void discover_interfaces(void);
>  void dispatch(void);
>  int locate_network(struct packet *);
>  void got_one(struct protocol *);
> Index: dispatch.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
> diff -u -p -r1.49 dispatch.c
> --- dispatch.c	4 Jun 2025 21:16:25 -0000	1.49
> +++ dispatch.c	7 Jun 2025 04:29:40 -0000
> @@ -67,6 +67,8 @@
>  #include "log.h"
>  #include "sync.h"
>  
> +extern int rdomain;
> +
>  struct interface_info *interfaces;
>  struct protocol *protocols;
>  struct dhcpd_timeout *timeouts;
> @@ -81,14 +83,14 @@ int get_rdomain(char *);
>     subnet it's on, and add it to the list of interfaces. */
>  
>  void
> -discover_interfaces(int *rdomain)
> +discover_interfaces(void)
>  {
>  	struct interface_info *tmp;
>  	struct interface_info *last, *next;
>  	struct subnet *subnet;
>  	struct shared_network *share;
>  	struct sockaddr_in foo;
> -	int ir = 0, ird;
> +	int ir;
>  	struct ifreq *tif;
>  	struct ifaddrs *ifap, *ifa;
>  
> @@ -99,11 +101,7 @@ discover_interfaces(int *rdomain)
>  	 * If we already have a list of interfaces, the interfaces were
>  	 * requested.
>  	 */
> -	if (interfaces != NULL)
> -		ir = 1;
> -	else
> -		/* must specify an interface when rdomains are used */
> -		*rdomain = 0;
> +	ir = (interfaces != NULL);
>  
>  	/* Cycle through the list of interfaces looking for IP addresses. */
>  	for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> @@ -125,15 +123,6 @@ discover_interfaces(int *rdomain)
>  		if (tmp == NULL && ir)
>  			continue;
>  
> -		ird = get_rdomain(ifa->ifa_name);
> -		if (*rdomain == -1)
> -			*rdomain = ird;
> -		else if (*rdomain != ird && ir)
> -			fatalx("Interface %s is not in rdomain %d",
> -			    tmp->name, *rdomain);
> -		else if (*rdomain != ird && !ir)
> -			continue;
> -
>  		/* If there isn't already an interface by this name,
>  		   allocate one. */
>  		if (tmp == NULL) {
> @@ -150,16 +139,19 @@ discover_interfaces(int *rdomain)
>  		/* If we have the capability, extract link information
>  		   and record it in a linked list. */
>  		if (ifa->ifa_addr->sa_family == AF_LINK) {
> -			struct sockaddr_dl *sdl =
> -			    ((struct sockaddr_dl *)(ifa->ifa_addr));
> +			struct if_data *ifi = ifa->ifa_data;
> +			struct sockaddr_dl *sdl;
> +
> +			if (rdomain != ifi->ifi_rdomain)
> +				continue;
> +
> +			sdl = (struct sockaddr_dl *)ifa->ifa_addr;
>  			tmp->index = sdl->sdl_index;
>  			tmp->hw_address.hlen = sdl->sdl_alen;
>  			tmp->hw_address.htype = HTYPE_ETHER; /* XXX */
>  			memcpy(tmp->hw_address.haddr,
>  			    LLADDR(sdl), sdl->sdl_alen);
>  		} else if (ifa->ifa_addr->sa_family == AF_INET) {
> -			struct iaddr addr;
> -
>  			/* Get a pointer to the address... */
>  			memcpy(&foo, ifa->ifa_addr, sizeof(foo));
>  
> @@ -182,55 +174,27 @@ discover_interfaces(int *rdomain)
>  				tmp->ifp = tif;
>  				tmp->primary_address = foo.sin_addr;
>  			}
> -
> -			/* Grab the address... */
> -			addr.len = 4;
> -			memcpy(addr.iabuf, &foo.sin_addr.s_addr, addr.len);
> -
> -			/* If there's a registered subnet for this address,
> -			   connect it together... */
> -			if ((subnet = find_subnet(addr))) {
> -				/* If this interface has multiple aliases
> -				   on the same subnet, ignore all but the
> -				   first we encounter. */
> -				if (!subnet->interface) {
> -					subnet->interface = tmp;
> -					subnet->interface_address = addr;
> -				} else if (subnet->interface != tmp) {
> -					log_warnx("Multiple %s %s: %s %s",
> -					    "interfaces match the",
> -					    "same subnet",
> -					    subnet->interface->name,
> -					    tmp->name);
> -				}
> -				share = subnet->shared_network;
> -				if (tmp->shared_network &&
> -				    tmp->shared_network != share) {
> -					log_warnx("Interface %s matches %s",
> -					    tmp->name,
> -					    "multiple shared networks");
> -				} else {
> -					tmp->shared_network = share;
> -				}
> -
> -				if (!share->interface) {
> -					share->interface = tmp;
> -				} else if (share->interface != tmp) {
> -					log_warnx("Multiple %s %s: %s %s",
> -					    "interfaces match the",
> -					    "same shared network",
> -					    share->interface->name,
> -					    tmp->name);
> -				}
> -			}
>  		}
>  	}
>  
>  	/* Discard interfaces we can't listen on. */
>  	last = NULL;
>  	for (tmp = interfaces; tmp; tmp = next) {
> +		struct iaddr addr;
> +
>  		next = tmp->next;
>  
> +		if (tmp->index == 0) {
> +			log_warnx("Can't listen on %s - wrong rdomain",
> +			    tmp->name);
> +			/* Remove tmp from the list of interfaces. */
> +			if (!last)
> +				interfaces = interfaces->next;
> +			else
> +				last->next = tmp->next;
> +			continue;
> +		}
> +
>  		if (!tmp->ifp) {
>  			log_warnx("Can't listen on %s - it has no IP address.",
>  			    tmp->name);
> @@ -244,6 +208,47 @@ discover_interfaces(int *rdomain)
>  
>  		memcpy(&foo, &tmp->ifp->ifr_addr, sizeof tmp->ifp->ifr_addr);
>  
> +		/* Grab the address... */
> +		addr.len = 4;
> +		memcpy(addr.iabuf, &foo.sin_addr.s_addr, addr.len);
> +
> +		/* If there's a registered subnet for this address,
> +		   connect it together... */
> +		if ((subnet = find_subnet(addr))) {
> +			/* If this interface has multiple aliases
> +			   on the same subnet, ignore all but the
> +			   first we encounter. */
> +			if (!subnet->interface) {
> +				subnet->interface = tmp;
> +				subnet->interface_address = addr;
> +			} else if (subnet->interface != tmp) {
> +				log_warnx("Multiple %s %s: %s %s",
> +				    "interfaces match the",
> +				    "same subnet",
> +				    subnet->interface->name,
> +				    tmp->name);
> +			}
> +			share = subnet->shared_network;
> +			if (tmp->shared_network &&
> +			    tmp->shared_network != share) {
> +				log_warnx("Interface %s matches %s",
> +				    tmp->name,
> +				    "multiple shared networks");
> +			} else {
> +				tmp->shared_network = share;
> +			}
> +
> +			if (!share->interface) {
> +				share->interface = tmp;
> +			} else if (share->interface != tmp) {
> +				log_warnx("Multiple %s %s: %s %s",
> +				    "interfaces match the",
> +				    "same shared network",
> +				    share->interface->name,
> +				    tmp->name);
> +			}
> +		}
> +
>  		if (!tmp->shared_network) {
>  			log_warnx("Can't listen on %s - dhcpd.conf has no "
>  			    "subnet declaration for %s.", tmp->name,
> @@ -616,22 +621,4 @@ remove_protocol(struct protocol *proto)
>  			free(p);
>  		}
>  	}
> -}
> -
> -int
> -get_rdomain(char *name)
> -{
> -	int rv = 0, s;
> -	struct  ifreq ifr;
> -
> -	if ((s = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
> -		fatal("get_rdomain socket");
> -
> -	memset(&ifr, 0, sizeof(ifr));
> -	strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
> -	if (ioctl(s, SIOCGIFRDOMAIN, (caddr_t)&ifr) != -1)
> -		rv = ifr.ifr_rdomainid;
> -
> -	close(s);
> -	return rv;
>  }
>