On Mon, May 18, 2026 at 06:49:59PM +0530, Anshu Kumari wrote: > Add an RFC 2132 type lookup table mapping DHCP option codes to their > expected value formats, and a dhcp_opt_parse() function that converts > CLI string values into their binary wire representation. > > Wire dhcp_opt_parse() into the --dhcp-opt handler so that values are > validated and encoded at configuration time. > > Link: https://bugs.passt.top/show_bug.cgi?id=192 > Signed-off-by: Anshu Kumari > --- > conf.c | 9 +++ > dhcp.c | 227 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > dhcp.h | 15 ++++ > 3 files changed, 251 insertions(+) > > diff --git a/conf.c b/conf.c > index 61a393f..3ec10ac 100644 > --- a/conf.c > +++ b/conf.c > @@ -1485,6 +1485,7 @@ void conf(struct ctx *c, int argc, char **argv) > unsigned long code; > const char *comma; > char *end; > + int len; > > comma = strchr(optarg, ','); > if (!comma) > @@ -1499,7 +1500,15 @@ void conf(struct ctx *c, int argc, char **argv) > die("Too many --dhcp-opt entries (max %d)", > MAX_CUSTOM_DHCP_OPTS); > > + len = dhcp_opt_parse(code, comma + 1, > + c->custom_opts[c->custom_opts_count].val, > + sizeof(c->custom_opts[0].val)); > + if (len < 0) > + die("Invalid value for DHCP option %lu: %s", > + code, comma + 1); > + > c->custom_opts[c->custom_opts_count].code = code; > + c->custom_opts[c->custom_opts_count].len = len; I don't love the fact that .val and .str permanently store redundant information. Initially I was going to suggest that you delay dhcp_opt_parse() until you're actually processing a DHCP message. However, that's also not great at that point it awkward to handle parse errors. Not sure how best to tackle this for now. > if (snprintf_check(c->custom_opts[c->custom_opts_count].str, > sizeof(c->custom_opts[0].str), > "%s", comma + 1)) > diff --git a/dhcp.c b/dhcp.c > index 1ff8cba..9220516 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -33,6 +33,233 @@ > #include "log.h" > #include "dhcp.h" > > +/** > + * struct dhcp_opt_type_entry - Maps option code to RFC 2132 value type > + * @code: DHCP option code > + * @type: Expected value format > + */ > +static const struct dhcp_opt_type_entry { > + uint8_t code; > + enum dhcp_opt_type type; > +} dhcp_opt_types[] = { > + { 1, DHCP_OPT_IPV4 }, /* Subnet Mask */ > + { 2, DHCP_OPT_UINT32 }, /* Time Offset */ > + { 3, DHCP_OPT_IPV4_LIST }, /* Router */ > + { 4, DHCP_OPT_IPV4_LIST }, /* Time Server */ > + { 5, DHCP_OPT_IPV4_LIST }, /* Name Server */ > + > + { 6, DHCP_OPT_IPV4_LIST }, /* Domain Name Server */ > + { 7, DHCP_OPT_IPV4_LIST }, /* Log Server */ > + { 8, DHCP_OPT_IPV4_LIST }, /* Cookie Server */ > + { 9, DHCP_OPT_IPV4_LIST }, /* LPR Server */ > + { 10, DHCP_OPT_IPV4_LIST }, /* Impress Server */ > + > + { 11, DHCP_OPT_IPV4_LIST }, /* Resource Location Server */ > + { 12, DHCP_OPT_STR }, /* Host Name */ > + { 13, DHCP_OPT_UINT16 }, /* Boot File Size */ > + { 15, DHCP_OPT_STR }, /* Domain Name */ > + { 16, DHCP_OPT_IPV4 }, /* Swap Server */ > + > + { 17, DHCP_OPT_STR }, /* Root Path */ > + { 19, DHCP_OPT_UINT8 }, /* IP Forwarding */ > + { 23, DHCP_OPT_UINT8 }, /* Default IP TTL */ > + { 26, DHCP_OPT_UINT16 }, /* Interface MTU */ > + { 28, DHCP_OPT_IPV4 }, /* Broadcast Address */ > + > + { 33, DHCP_OPT_IPV4_LIST }, /* Static Routes (dest+router pairs) */ > + { 37, DHCP_OPT_UINT8 }, /* TCP Default TTL */ > + { 38, DHCP_OPT_UINT32 }, /* TCP Keepalive Interval */ > + { 40, DHCP_OPT_STR }, /* NIS Domain Name */ > + { 41, DHCP_OPT_IPV4_LIST }, /* NIS Servers */ > + > + { 42, DHCP_OPT_IPV4_LIST }, /* NTP Servers */ > + { 44, DHCP_OPT_IPV4_LIST }, /* NetBIOS Name Server */ > + { 50, DHCP_OPT_IPV4 }, /* Requested IP Address */ > + { 51, DHCP_OPT_UINT32 }, /* IP Address Lease Time */ > + { 53, DHCP_OPT_UINT8 }, /* DHCP Message Type */ > + > + { 54, DHCP_OPT_IPV4 }, /* Server Identifier */ > + { 57, DHCP_OPT_UINT16 }, /* Max DHCP Message Size */ > + { 58, DHCP_OPT_UINT32 }, /* Renewal (T1) Time */ > + { 59, DHCP_OPT_UINT32 }, /* Rebinding (T2) Time */ > + { 60, DHCP_OPT_STR }, /* Vendor Class Identifier */ > + > + { 61, DHCP_OPT_STR }, /* Client Identifier */ > + { 66, DHCP_OPT_STR }, /* TFTP Server Name */ > + { 67, DHCP_OPT_STR }, /* Bootfile Name */ > + { 119, DHCP_OPT_STR }, /* Domain Search List (RFC 3397) */ > + { 121, DHCP_OPT_ROUTES }, /* Classless Static Routes */ > + > + { 252, DHCP_OPT_STR }, /* WPAD URL */ At least initially, I'd suggest we don't permit the user to set options which passt already manages internally (1, 3, 51, 53, 54, 55, and 199 from a quick scan). > +}; > + > +/** > + * dhcp_opt_type_lookup() - Look up the value type for a DHCP option code > + * @code: DHCP option code > + * > + * Return: type from table > + */ > +static enum dhcp_opt_type dhcp_opt_type_lookup(uint8_t code) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(dhcp_opt_types); i++) { > + if (dhcp_opt_types[i].code == code) > + return dhcp_opt_types[i].type; > + } > + > + return DHCP_OPT_NONE; > +} > + > +/** > + * dhcp_opt_parse() - Parse a DHCP option value > + * @code: DHCP option code (determines value type via lookup table) > + * @str: Value string from command line > + * @buf: Output buffer for binary value > + * @buf_len: Size of output buffer > + * > + * Return: number of bytes written to @buf, or -1 on error > + */ > +int dhcp_opt_parse(uint8_t code, const char *str, uint8_t *buf, size_t buf_len) > +{ > + enum dhcp_opt_type type = dhcp_opt_type_lookup(code); > + > + switch (type) { > + case DHCP_OPT_NONE: { > + die("Unsupported DHCP option: %u," > + " see passt(1) for supported codes", > + code); > + } > + case DHCP_OPT_IPV4: { > + struct in_addr addr; > + > + if (inet_pton(AF_INET, str, &addr) != 1) > + return -1; > + if (buf_len < sizeof(addr)) > + return -1; > + memcpy(buf, &addr, sizeof(addr)); > + return sizeof(addr); > + } > + case DHCP_OPT_IPV4_LIST: { > + char *tok, *saveptr; > + char tmp[1024]; Bare 1024 constant isn't ideal. > + int len = 0; > + > + if (snprintf_check(tmp, sizeof(tmp), "%s", str)) > + return -1; > + > + for (tok = strtok_r(tmp, " ", &saveptr); tok; > + tok = strtok_r(NULL, " ", &saveptr)) { Apparently strtok_r() is specified by POSIX, so it should be ok. Might be worth double checking musl has it, though. " " seems an odd choice of delimiter - it's pretty awkward to use on the command line. I guess "," is also problematic since it can be found within various of the of the option values. > + struct in_addr addr; > + > + if (inet_pton(AF_INET, tok, &addr) != 1) > + return -1; > + if (len + (int)sizeof(addr) > (int)buf_len) > + return -1; > + memcpy(buf + len, &addr, sizeof(addr)); > + len += sizeof(addr); > + } > + return len; > + } > + case DHCP_OPT_UINT8: { > + unsigned long val; > + char *end; > + > + val = strtoul(str, &end, 0); > + if (*end || val > 255 || buf_len < 1) > + return -1; > + buf[0] = val; > + return 1; > + } > + case DHCP_OPT_UINT16: { > + unsigned long val; > + char *end; > + > + val = strtoul(str, &end, 0); > + if (*end || val > 65535 || buf_len < 2) > + return -1; > + buf[0] = (val >> 8) & 0xff; > + buf[1] = val & 0xff; > + return 2; > + } > + case DHCP_OPT_UINT32: { > + unsigned long val; > + char *end; > + > + val = strtoul(str, &end, 0); > + if (*end || buf_len < 4) > + return -1; > + buf[0] = (val >> 24) & 0xff; > + buf[1] = (val >> 16) & 0xff; > + buf[2] = (val >> 8) & 0xff; > + buf[3] = val & 0xff; > + return 4; > + } It might be possible to simplify the sections above using a single DHCP_OPT_INTEGER type, plus an extra field giving the integer width. > + case DHCP_OPT_ROUTES: { I'd consider excluding this one for now. For one thing it's rather a lot of code. But more importantly with the multi-address and route monitoring changes we're considering, there's a fairly high chance we might want to manage this one ourselves. > + /* RFC 3442: "CIDR/mask,gateway" entries, space-separated > + * Encodes as: mask-width + significant-octets + router > + * e.g. "192.168.1.0/24,10.0.0.1 0.0.0.0/0,10.0.0.1" > + */ > + char *tok, *saveptr; > + char tmp[1024]; > + int len = 0; > + > + if (snprintf_check(tmp, sizeof(tmp), "%s", str)) > + return -1; > + > + for (tok = strtok_r(tmp, " ", &saveptr); tok; > + tok = strtok_r(NULL, " ", &saveptr)) { > + struct in_addr dest, gw; > + char *slash, *comma; > + unsigned long mask; > + int sig_octets; > + > + slash = strchr(tok, '/'); > + if (!slash) > + return -1; > + *slash = '\0'; > + > + if (inet_pton(AF_INET, tok, &dest) != 1) > + return -1; > + > + comma = strchr(slash + 1, ','); > + if (!comma) > + return -1; > + *comma = '\0'; > + > + mask = strtoul(slash + 1, NULL, 10); > + if (mask > 32) > + return -1; > + > + if (inet_pton(AF_INET, comma + 1, &gw) != 1) > + return -1; > + > + sig_octets = (mask + 7) / 8; > + > + if (len + 1 + sig_octets + 4 > (int)buf_len) > + return -1; > + > + buf[len++] = mask; > + memcpy(buf + len, &dest, sig_octets); > + len += sig_octets; > + memcpy(buf + len, &gw, 4); > + len += 4; > + } > + return len; > + } > + case DHCP_OPT_STR: { > + size_t len = strlen(str); > + > + if (!len || len >= buf_len) > + return -1; > + strncpy((char *)buf, str, buf_len); > + return len; > + } > + } > + > + return -1; > +} > + > /** > * struct opt - DHCP option > * @sent: Convenience flag, set while filling replies > diff --git a/dhcp.h b/dhcp.h > index cd50c99..01b2290 100644 > --- a/dhcp.h > +++ b/dhcp.h > @@ -6,7 +6,22 @@ > #ifndef DHCP_H > #define DHCP_H > > +/** > + * enum dhcp_opt_type - DHCP option value types per RFC 2132 > + */ > +enum dhcp_opt_type { > + DHCP_OPT_NONE, > + DHCP_OPT_STR, > + DHCP_OPT_IPV4, > + DHCP_OPT_IPV4_LIST, > + DHCP_OPT_UINT8, > + DHCP_OPT_UINT16, > + DHCP_OPT_UINT32, > + DHCP_OPT_ROUTES, > +}; > + > int dhcp(const struct ctx *c, struct iov_tail *data); > void dhcp_init(void); > +int dhcp_opt_parse(uint8_t code, const char *str, uint8_t *buf, size_t buf_len); > > #endif /* DHCP_H */ > -- > 2.54.0 > -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson