From: Ryan Vogt Subject: Re: Allow rad(8) to advertise shorter lifetimes To: tech@openbsd.org Cc: florian@narrans.de Date: Sun, 14 Sep 2025 17:32:27 -0700 On Sun, Sep 14, 2025 at 01:05:33PM +0200, Florian Obser wrote: > Apologies for my silence and very late response, life happened... > > I fully agree that lifetimes should be limited and a excessively long > lifetimes handed out by a DHCPv6 server are a problem. > > I don't think we need a config option for this. The minimum of what we > find on an interface and what we have in the config file (including > defaults) should win. I like this approach far better. I was overly concerned with maintaining the existing behaviour of rad as a configuration-file option. If a user actually wants rad to hand out unbounded lifetimes, they can use the maximum lifetime (4294967295) in the configuration file. > The following diff, inspired by your work, implements that. > Does this work for you? > Florian It absolutely does. I have only one tiny comment about the code, inline below. I ran this patch through 10 different tests, and all came out with the behaviour I'd expect. For posterity, here's what I tested. For each test, I've used a shorthand for the configuration file. So, [auto:4000/2000, static:9999/8888] refers to the configuration file: interface em1 { auto prefix { valid lifetime 4000 preferred lifetime 2000 } prefix wwww:xxxx:yyyy:zzzz { valid lifetime 9999 preferred lifetime 8888 } } I'll always the use notation vvvv/pppp to refer to a valid lifetime of vvvv and a preferred lifetime of pppp. The static prefix in the configuration file (wwww:xxxx:yyyy:zzzz) always matches the decaying configured address on interface em1, to test the behaviour when an auto prefix configuration and static prefix give different lifetime bounds. And I'll use "->" to indicate what rad sends out as advertisements. For the first 5 tests, a configured address of wwww:xxxx:yyyy:zzzz::1 exists on em1, lifetime 6000/3000, when rad starts. Tests 6-9 introduce a configured address while rad is running. Test 10 resets the lifetime on a configured address while rad is running. 1) Ensure that rad starts using the decaying lifetime when it crosses the bound in the configuration file. [auto:5980/2980] ...decaying configured lifetime on em1 6000/3000... ...rad starts... -> 5980/2980 ...time passes... -> 5800/2800-ish 2) We should get the same behaviour as test 1, even if the prefix is defined as a static prefix in the configuration file. [static:5980/2980] ...decaying configured lifetime on em1 6000/3000... ...rad starts... -> 5980/2980 ...time passes... -> 5800/2800-ish 3) Users should be able to use a maximum value to unbound lifetimes. [auto:4294967295/4294967295] ...decaying configured lifetime on em1 6000/3000... ...rad starts... -> 6000/3000 4) A static configuration should override the auto configuration if it provides a lower lifetime bound. [auto:9999/8888, static:4000/2000] ...decaying configured lifetime on em1 6000/3000... ...rad starts... -> 4000/2000 5) A static configuration should override the auto configuration, even when the static configuration provides a higher lifetime bound. [auto:4000/2000, static:9999/8888] ...decaying configured lifetime on em1 6000/3000... ...rad starts... -> 6000/3000 6) Ensure the behaviour of test 1 is unchanged if the configured lifetime appears while rad is running. [auto:5980/2980] ...no configured lifetime on em1... ...rad starts... -> No prefix information ...place decaying configured lifetime on em1 with 6000/3000... ...almost instantly... -> 5980/2980 ...time passes... -> 5800/2800-ish 7) As with test 6, but without the bound coming into play. [auto:9999/8888] ...no configured lifetime on em1... ...rad starts... -> No prefix information ...place decaying configured address on em1 with 6000/3000... ...almost instantly... -> 5999/2999-ish 8) Ensure the behaviour of test 2 is unchanged if the configured lifetime appears while rad is running. [static:5980/2980] ...no configured lifetime on em1... ...start rad... -> 5980/2980 ...place decaying configured address on em1 with 6000/3000... ...time passes... -> 5800/2800-ish 9) As with test 8, but without the bound coming into play. [static:9999/8888] ...no configured lifetime on em1... ...start rad... -> 9999/8888 ...place decaying configured address on em1 with 6000/3000... ...almost instantly... -> 5999/2999-ish 10) A configured lifetime changes while rad is running. [auto:5890/2890] ...decaying configured lifetime on em1 6000/3000... ...rad starts... -> 5980/2980 ...time passes... -> 5800/2800-ish ...reset decaying configured lifetime on em1 to 6000/3000... ...almost instantly... -> 5980/2980 Every one of these tests seems correct to me. Inline comment below. Cheers, Ryan > diff --git frontend.c frontend.c > index 2a519dac0b1..97f4d0ba708 100644 > --- frontend.c > +++ frontend.c > @@ -110,7 +110,7 @@ struct ra_iface { > int removed; > int link_state; > int prefix_count; > - int ltime_decaying; > + int has_autoconf_prefix; > size_t datalen; > uint8_t data[RA_MAX_SIZE]; > }; > @@ -151,11 +151,12 @@ void unref_icmp6ev(struct ra_iface *); > void set_icmp6sock(int, int); > void add_new_prefix_to_ra_iface(struct ra_iface *r, > struct in6_addr *, int, struct ra_prefix_conf *, > - uint32_t, uint32_t); > + int, uint32_t, uint32_t); > void free_ra_iface(struct ra_iface *); > int in6_mask2prefixlen(struct in6_addr *); > void get_interface_prefixes(struct ra_iface *, > struct ra_prefix_conf *, struct ifaddrs *); > +uint32_t calc_autoconf_ltime(time_t, uint32_t, uint32_t); > int build_packet(struct ra_iface *); > void build_leaving_packet(struct ra_iface *); > void ra_output(struct ra_iface *, struct sockaddr_in6 *); > @@ -979,7 +980,7 @@ merge_ra_interfaces(void) > entry) { > add_new_prefix_to_ra_iface(ra_iface, > &ra_prefix_conf->prefix, > - ra_prefix_conf->prefixlen, ra_prefix_conf, > + ra_prefix_conf->prefixlen, ra_prefix_conf, 0, > ND6_INFINITE_LIFETIME, ND6_INFINITE_LIFETIME); > } > > @@ -1046,7 +1047,7 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf > struct in6_ifreq ifr6; > struct ifaddrs *ifa; > struct sockaddr_in6 *sin6; > - uint32_t decaying_vltime, decaying_pltime; > + uint32_t if_vltime, if_pltime; > int prefixlen; > > for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) { > @@ -1066,8 +1067,8 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf > strlcpy(ifr6.ifr_name, ra_iface->name, sizeof(ifr6.ifr_name)); > memcpy(&ifr6.ifr_addr, sin6, sizeof(ifr6.ifr_addr)); > > - decaying_vltime = ND6_INFINITE_LIFETIME; > - decaying_pltime = ND6_INFINITE_LIFETIME; > + if_vltime = ND6_INFINITE_LIFETIME; > + if_pltime = ND6_INFINITE_LIFETIME; > > if (ioctl(ioctlsock, SIOCGIFALIFETIME_IN6, > (caddr_t)&ifr6) != -1) { > @@ -1075,9 +1076,9 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf > > lifetime = &ifr6.ifr_ifru.ifru_lifetime; > if (lifetime->ia6t_preferred) > - decaying_pltime = lifetime->ia6t_preferred; > + if_pltime = lifetime->ia6t_preferred; > if (lifetime->ia6t_expire) > - decaying_vltime = lifetime->ia6t_expire; > + if_vltime = lifetime->ia6t_expire; > } > > memset(&ifr6, 0, sizeof(ifr6)); > @@ -1096,8 +1097,7 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf > mask_prefix(&sin6->sin6_addr, prefixlen); > > add_new_prefix_to_ra_iface(ra_iface, &sin6->sin6_addr, > - prefixlen, autoprefix_conf, decaying_vltime, > - decaying_pltime); > + prefixlen, autoprefix_conf, 1, if_vltime, if_pltime); > } > } > > @@ -1118,41 +1118,30 @@ find_ra_prefix_conf(struct ra_prefix_conf_head* head, struct in6_addr *prefix, > > void > add_new_prefix_to_ra_iface(struct ra_iface *ra_iface, struct in6_addr *addr, > - int prefixlen, struct ra_prefix_conf *ra_prefix_conf, > - uint32_t decaying_vltime, uint32_t decaying_pltime) > + int prefixlen, struct ra_prefix_conf *ra_prefix_conf, int autoconf, > + uint32_t if_vltime, uint32_t if_pltime) > { > struct ra_prefix_conf *new_ra_prefix_conf; > > if ((new_ra_prefix_conf = find_ra_prefix_conf(&ra_iface->prefixes, addr, > prefixlen)) != NULL) { > - if (decaying_vltime != ND6_INFINITE_LIFETIME || > - decaying_pltime != ND6_INFINITE_LIFETIME) { > - ra_iface->ltime_decaying = 1; > - new_ra_prefix_conf->ltime_decaying = 0; > - if (decaying_vltime != ND6_INFINITE_LIFETIME) { > - new_ra_prefix_conf->vltime = decaying_vltime; > - new_ra_prefix_conf->ltime_decaying |= > - VLTIME_DECAYING; > - } > - if (decaying_pltime != ND6_INFINITE_LIFETIME) { > - new_ra_prefix_conf->pltime = decaying_pltime; > - new_ra_prefix_conf->ltime_decaying |= > - PLTIME_DECAYING; > - } > - } else if (new_ra_prefix_conf->ltime_decaying) { > + int changed = new_ra_prefix_conf->autoconf != autoconf; > + > + new_ra_prefix_conf->autoconf = autoconf; > + new_ra_prefix_conf->if_vltime = if_vltime; > + new_ra_prefix_conf->if_pltime = if_pltime; > + > + if (changed) { > struct ra_prefix_conf *pc; > > - new_ra_prefix_conf->ltime_decaying = 0; > - ra_iface->ltime_decaying = 0; > + ra_iface->has_autoconf_prefix = 0; > SIMPLEQ_FOREACH(pc, &ra_iface->prefixes, entry) { > - if (pc->ltime_decaying) { > - ra_iface->ltime_decaying = 1; > + if (pc->autoconf) { > + ra_iface->has_autoconf_prefix = 1; > break; > } > } > - } else > - log_debug("ignoring duplicate %s/%d prefix", > - in6_to_str(addr), prefixlen); > + } > return; > } > > @@ -1164,24 +1153,36 @@ add_new_prefix_to_ra_iface(struct ra_iface *ra_iface, struct in6_addr *addr, > new_ra_prefix_conf->prefixlen = prefixlen; > new_ra_prefix_conf->vltime = ra_prefix_conf->vltime; > new_ra_prefix_conf->pltime = ra_prefix_conf->pltime; > - if (decaying_vltime != ND6_INFINITE_LIFETIME || > - decaying_pltime != ND6_INFINITE_LIFETIME) { > - ra_iface->ltime_decaying = 1; > - if (decaying_vltime != ND6_INFINITE_LIFETIME) { > - new_ra_prefix_conf->vltime = decaying_vltime; > - new_ra_prefix_conf->ltime_decaying |= VLTIME_DECAYING; > - } > - if (decaying_pltime != ND6_INFINITE_LIFETIME) { > - new_ra_prefix_conf->pltime = decaying_pltime; > - new_ra_prefix_conf->ltime_decaying |= PLTIME_DECAYING; > - } > - } > + new_ra_prefix_conf->autoconf = autoconf; > + if (autoconf) > + ra_iface->has_autoconf_prefix = 1; > + new_ra_prefix_conf->if_vltime = if_vltime; > + new_ra_prefix_conf->if_pltime = if_pltime; > new_ra_prefix_conf->aflag = ra_prefix_conf->aflag; > new_ra_prefix_conf->lflag = ra_prefix_conf->lflag; > SIMPLEQ_INSERT_TAIL(&ra_iface->prefixes, new_ra_prefix_conf, entry); > ra_iface->prefix_count++; > } > > +uint32_t > +calc_autoconf_ltime(time_t t, uint32_t conf_ltime, uint32_t if_ltime) A tiny comment about two of the parameters. Here, - "conf_ltime" refers to the lifetime specified in the prefix configuration; and, - "if_ltime" refers to the configured lifetime on the interface. I misinterpreted "conf_ltime" as "configured lifetime" and had to reread things a few times. Please feel free to disregard, but could I suggest calling the lifetime from the prefix configuration "prefix_conf_ltime" instead of "conf_ltime"? > +{ > + uint32_t ltime; > + > + if (if_ltime == ND6_INFINITE_LIFETIME) > + return conf_ltime; > + > + if (if_ltime > t) > + ltime = if_ltime - t; > + else > + ltime = 0; > + > + if (ltime < conf_ltime) > + return ltime; > + else > + return conf_ltime; > +} > + > int > build_packet(struct ra_iface *ra_iface) > { > @@ -1294,16 +1295,15 @@ build_packet(struct ra_iface *ra_iface) > ndopt_pi->nd_opt_pi_flags_reserved |= > ND_OPT_PI_FLAG_AUTO; > > - if (ra_prefix_conf->ltime_decaying & VLTIME_DECAYING) > - vltime = ra_prefix_conf->vltime < t ? 0 : > - ra_prefix_conf->vltime - t; > - else > - vltime = ra_prefix_conf->vltime; > - if (ra_prefix_conf->ltime_decaying & PLTIME_DECAYING) > - pltime = ra_prefix_conf->pltime < t ? 0 : > - ra_prefix_conf->pltime - t; > - else > + if (ra_prefix_conf->autoconf) { > + pltime = calc_autoconf_ltime(t, > + ra_prefix_conf->pltime, ra_prefix_conf->if_pltime); > + vltime = calc_autoconf_ltime(t, > + ra_prefix_conf->vltime, ra_prefix_conf->if_vltime); > + } else { > pltime = ra_prefix_conf->pltime; > + vltime = ra_prefix_conf->vltime; > + } > > ndopt_pi->nd_opt_pi_valid_time = htonl(vltime); > ndopt_pi->nd_opt_pi_preferred_time = htonl(pltime); > @@ -1430,7 +1430,7 @@ ra_output(struct ra_iface *ra_iface, struct sockaddr_in6 *to) > if (!LINK_STATE_IS_UP(ra_iface->link_state)) > return; > > - if (ra_iface->ltime_decaying) > + if (ra_iface->has_autoconf_prefix) > /* update vltime & pltime */ > build_packet(ra_iface); > > diff --git rad.conf.5 rad.conf.5 > index d03bcc139aa..7211c48d0a1 100644 > --- rad.conf.5 > +++ rad.conf.5 > @@ -207,13 +207,13 @@ The preferred lifetime (pltime) in seconds for addresses generated from this > prefix. > The default is 2700. > This option is ignored if the prefix is discovered from a network interface > -and it has a preferred lifetime configured. > +and it has a lower preferred lifetime. > .It Cm valid lifetime Ar seconds > The valid lifetime (vltime) in seconds for addresses generated from this > prefix. > The default is 5400. > This option is ignored if the prefix is discovered from a network interface > -and it has a valid lifetime configured. > +and it has a lower valid lifetime. > .El > .El > .El > diff --git rad.h rad.h > index 7775ac4a0aa..00b4f1bfeb0 100644 > --- rad.h > +++ rad.h > @@ -35,8 +35,6 @@ > #define MIN_DELAY_BETWEEN_RAS 3 /* 3 seconds */ > #define MAX_SEARCH 1025 /* MAXDNAME in arpa/nameser.h */ > #define DEFAULT_RDNS_LIFETIME 3 * MAX_RTR_ADV_INTERVAL > -#define PLTIME_DECAYING 1 > -#define VLTIME_DECAYING 2 > > #define IMSG_DATA_SIZE(imsg) ((imsg).hdr.len - IMSG_HEADER_SIZE) > > @@ -116,7 +114,9 @@ struct ra_prefix_conf { > int prefixlen; /* prefix length */ > uint32_t vltime; /* valid lifetime */ > uint32_t pltime; /* preferred lifetime */ > - int ltime_decaying; > + uint32_t if_vltime; /* valid lifetime */ > + uint32_t if_pltime; /* preferred lifetime */ > + int autoconf; > int lflag; /* on-link flag*/ > int aflag; /* autonom. addr flag */ > }; >