It's probably an overkill for first implementation, but in the future I think we should support subnet definitions in CIDR notation (e.x.: 192.168.0.0/24) and IP ranges for fine control (e.x.: 192.168.0.100-192.168.0.254).
BRs /Andras On Mon, Aug 9, 2021 at 2:35 PM Florian Obser <[email protected]> wrote: > > On 2021-08-08 12:14 -07, patrick keshishian <[email protected]> wrote: > > On Sun, Aug 08, 2021 at 12:37:54PM +0200, Florian Obser wrote: > >> This implements ignoring of nameservers and / or routes in leases as > >> well as completely ignoring servers (you cannot block rogue DHCP servers > >> in pf because bpf sees packets before pf). > >> > >> Various people voiced the need for these features. > >> Tests, OKs? > >> > >> diff --git dhcpleased.c dhcpleased.c > >> index 36a4a21c21a..e005d7de9ae 100644 > >> --- dhcpleased.c > >> +++ dhcpleased.c > >> @@ -569,6 +569,20 @@ main_dispatch_engine(int fd, short event, void *bula) > >> sizeof(imsg_interface.if_index)); > >> break; > >> } > >> + case IMSG_WITHDRAW_ROUTES: { > >> + struct imsg_configure_interface imsg_interface; > >> + if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_interface)) > >> + fatalx("%s: IMSG_CONFIGURE_INTERFACE wrong " > >> + "length: %lu", __func__, > >> + IMSG_DATA_SIZE(imsg)); > >> + memcpy(&imsg_interface, imsg.data, > >> + sizeof(imsg_interface)); > >> + if (imsg_interface.routes_len >= MAX_DHCP_ROUTES) > >> + fatalx("%s: too many routes in imsg", > >> __func__); > >> + if (imsg_interface.routes_len > 0) > >> + configure_routes(RTM_DELETE, &imsg_interface); > >> + break; > >> + } > >> case IMSG_PROPOSE_RDNS: { > >> struct imsg_propose_rdns rdns; > >> if (IMSG_DATA_SIZE(imsg) != sizeof(rdns)) > >> diff --git dhcpleased.conf.5 dhcpleased.conf.5 > >> index 9e6846f899e..b856113bac1 100644 > >> --- dhcpleased.conf.5 > >> +++ dhcpleased.conf.5 > >> @@ -57,6 +57,17 @@ A list of interfaces to overwrite defaults: > >> .Ic interface > >> options are as follows: > >> .Bl -tag -width Ds > >> +.It Ic ignore dns > >> +Ignore nameservers from leases on this interface. > >> +The default is to not ignore nameservers. > >> +.It Ic ignore routes > >> +Ignore routes from leases on this interface. > >> +The default is to not ignore routes. > >> +.It Ic ignore Ar server-ip > >> +Ignore leases from > >> +.Ar server-ip . > >> +This option can be listed multiple times. > >> +The default is to not ignore servers. > >> .It Ic send client id Ar client-id > >> Send the dhcp client identifier option with a value of > >> .Ar client-id . > >> diff --git dhcpleased.h dhcpleased.h > >> index 7f6ec87ad19..1d20b77dbd3 100644 > >> --- dhcpleased.h > >> +++ dhcpleased.h > >> @@ -151,6 +151,12 @@ > >> #define DHCPRELEASE 7 > >> #define DHCPINFORM 8 > >> > >> +/* Ignore parts of DHCP lease */ > >> +#define IGN_ROUTES 1 > >> +#define IGN_DNS 2 > >> + > >> +#define MAX_SERVERS 16 /* max servers that can be ignores > >> per if */ > > > > Typo in comment: ignored (not ignores) > > thanks, fixed in my tree. > > > > > Should there be a mention of a limitation in the man page where > > it states the option can be listed multiple times? > > I really do hope that 16 is enough for everybody. If it turns out not > then we should probably bite the bullet and implement this with a proper > list. So for now I don't want to document the limitation. > > > > > --patrick > > > > > >> + > >> #define IMSG_DATA_SIZE(imsg) ((imsg).hdr.len - IMSG_HEADER_SIZE) > >> #define DHCP_SNAME_LEN 64 > >> #define DHCP_FILE_LEN 128 > >> @@ -216,6 +222,7 @@ enum imsg_type { > >> IMSG_DECONFIGURE_INTERFACE, > >> IMSG_PROPOSE_RDNS, > >> IMSG_WITHDRAW_RDNS, > >> + IMSG_WITHDRAW_ROUTES, > >> IMSG_REPROPOSE_RDNS, > >> IMSG_REQUEST_REBOOT, > >> }; > >> @@ -246,6 +253,9 @@ struct iface_conf { > >> int vc_id_len; > >> uint8_t *c_id; > >> int c_id_len; > >> + int ignore; > >> + struct in_addr ignore_servers[MAX_SERVERS]; > >> + int ignore_servers_len; > >> }; > >> > >> struct dhcpleased_conf { > >> @@ -304,6 +314,8 @@ const char *sin_to_str(struct sockaddr_in *); > >> > >> /* frontend.c */ > >> struct iface_conf *find_iface_conf(struct iface_conf_head *, char *); > >> +int *changed_ifaces(struct dhcpleased_conf *, struct > >> + dhcpleased_conf *); > >> > >> /* printconf.c */ > >> void print_config(struct dhcpleased_conf *); > >> diff --git engine.c engine.c > >> index 076a57e9ba6..67693c358ee 100644 > >> --- engine.c > >> +++ engine.c > >> @@ -139,6 +139,7 @@ void > >> send_configure_interface(struct dhcpleased_iface *); > >> void send_rdns_proposal(struct dhcpleased_iface > >> *); > >> void send_deconfigure_interface(struct > >> dhcpleased_iface *); > >> void send_rdns_withdraw(struct dhcpleased_iface > >> *); > >> +void send_routes_withdraw(struct dhcpleased_iface > >> *); > >> void parse_lease(struct dhcpleased_iface *, > >> struct imsg_ifinfo *); > >> int engine_imsg_compose_main(int, pid_t, void *, > >> uint16_t); > >> @@ -506,13 +507,37 @@ engine_dispatch_main(int fd, short event, void *bula) > >> IMSG_DATA_SIZE(imsg)); > >> iface_conf->c_id_len = IMSG_DATA_SIZE(imsg); > >> break; > >> - case IMSG_RECONF_END: > >> + case IMSG_RECONF_END: { > >> + struct dhcpleased_iface *iface; > >> + int *ifaces; > >> + int i, if_index; > >> + char *if_name; > >> + char ifnamebuf[IF_NAMESIZE]; > >> + > >> if (nconf == NULL) > >> fatalx("%s: IMSG_RECONF_END without " > >> "IMSG_RECONF_CONF", __func__); > >> + ifaces = changed_ifaces(engine_conf, nconf); > >> merge_config(engine_conf, nconf); > >> nconf = NULL; > >> + for (i = 0; ifaces[i] != 0; i++) { > >> + if_index = ifaces[i]; > >> + if_name = if_indextoname(if_index, ifnamebuf); > >> + iface = get_dhcpleased_iface_by_id(if_index); > >> + if (if_name == NULL || iface == NULL) > >> + continue; > >> + iface_conf = find_iface_conf( > >> + &engine_conf->iface_list, if_name); > >> + if (iface_conf == NULL) > >> + continue; > >> + if (iface_conf->ignore & IGN_DNS) > >> + send_rdns_withdraw(iface); > >> + if (iface_conf->ignore & IGN_ROUTES) > >> + send_routes_withdraw(iface); > >> + } > >> + free(ifaces); > >> break; > >> + } > >> #endif /* SMALL */ > >> default: > >> log_debug("%s: unexpected imsg %d", __func__, > >> @@ -760,6 +785,18 @@ parse_dhcp(struct dhcpleased_iface *iface, struct > >> imsg_dhcp *dhcp) > >> if (inet_ntop(AF_INET, &ip->ip_dst, hbuf_dst, sizeof(hbuf_dst)) == > >> NULL) > >> hbuf_dst[0] = '\0'; > >> > >> +#ifndef SMALL > >> + if (iface_conf != NULL) { > >> + for (i = 0; (int)i < iface_conf->ignore_servers_len; i++) { > >> + if (iface_conf->ignore_servers[i].s_addr == > >> + ip->ip_src.s_addr) { > >> + log_debug("ignoring server %s", hbuf_src); > >> + return; > >> + } > >> + } > >> + } > >> +#endif /* SMALL */ > >> + > >> if (rem < sizeof(*udp)) > >> goto too_short; > >> > >> @@ -1203,13 +1240,30 @@ parse_dhcp(struct dhcpleased_iface *iface, struct > >> imsg_dhcp *dhcp) > >> iface->server_identifier.s_addr = server_identifier.s_addr; > >> iface->requested_ip.s_addr = dhcp_hdr->yiaddr.s_addr; > >> iface->mask.s_addr = subnet_mask.s_addr; > >> - iface->routes_len = routes_len; > >> - memcpy(iface->routes, routes, sizeof(iface->routes)); > >> +#ifndef SMALL > >> + if (iface_conf != NULL && iface_conf->ignore & IGN_ROUTES) { > >> + iface->routes_len = 0; > >> + memset(iface->routes, 0, sizeof(iface->routes)); > >> + } else > >> +#endif /* SMALL */ > >> + { > >> + iface->routes_len = routes_len; > >> + memcpy(iface->routes, routes, sizeof(iface->routes)); > >> + } > >> iface->lease_time = lease_time; > >> iface->renewal_time = renewal_time; > >> iface->rebinding_time = rebinding_time; > >> - memcpy(iface->nameservers, nameservers, > >> - sizeof(iface->nameservers)); > >> + > >> +#ifndef SMALL > >> + if (iface_conf != NULL && iface_conf->ignore & IGN_DNS) { > >> + memset(iface->nameservers, 0, > >> + sizeof(iface->nameservers)); > >> + } else > >> +#endif /* SMALL */ > >> + { > >> + memcpy(iface->nameservers, nameservers, > >> + sizeof(iface->nameservers)); > >> + } > >> > >> iface->siaddr.s_addr = dhcp_hdr->siaddr.s_addr; > >> > >> @@ -1520,6 +1574,28 @@ send_deconfigure_interface(struct dhcpleased_iface > >> *iface) > >> memset(iface->routes, 0, sizeof(iface->routes)); > >> } > >> > >> +void > >> +send_routes_withdraw(struct dhcpleased_iface *iface) > >> +{ > >> + struct imsg_configure_interface imsg; > >> + > >> + if (iface->requested_ip.s_addr == INADDR_ANY || iface->routes_len == > >> 0) > >> + return; > >> + > >> + imsg.if_index = iface->if_index; > >> + imsg.rdomain = iface->rdomain; > >> + imsg.addr.s_addr = iface->requested_ip.s_addr; > >> + imsg.mask.s_addr = iface->mask.s_addr; > >> + imsg.siaddr.s_addr = iface->siaddr.s_addr; > >> + strlcpy(imsg.file, iface->file, sizeof(imsg.file)); > >> + strlcpy(imsg.domainname, iface->domainname, sizeof(imsg.domainname)); > >> + strlcpy(imsg.hostname, iface->hostname, sizeof(imsg.hostname)); > >> + imsg.routes_len = iface->routes_len; > >> + memcpy(imsg.routes, iface->routes, sizeof(imsg.routes)); > >> + engine_imsg_compose_main(IMSG_WITHDRAW_ROUTES, 0, &imsg, > >> + sizeof(imsg)); > >> +} > >> + > >> void > >> log_rdns(struct dhcpleased_iface *iface, int withdraw) > >> { > >> diff --git frontend.c frontend.c > >> index 6e17b9c6c62..e52cfae0b25 100644 > >> --- frontend.c > >> +++ frontend.c > >> @@ -96,8 +96,6 @@ void send_discover(struct iface *); > >> void send_request(struct iface *); > >> void bpf_send_packet(struct iface *, uint8_t *, ssize_t); > >> void udp_send_packet(struct iface *, uint8_t *, ssize_t); > >> -int *changed_ifaces(struct dhcpleased_conf *, struct > >> - dhcpleased_conf *); > >> int iface_conf_cmp(struct iface_conf *, struct iface_conf *); > >> > >> LIST_HEAD(, iface) interfaces; > >> @@ -1215,6 +1213,13 @@ iface_conf_cmp(struct iface_conf *a, struct > >> iface_conf *b) > >> return 1; > >> if (memcmp(a->c_id, b->c_id, a->c_id_len) != 0) > >> return 1; > >> + if (a->ignore != b->ignore) > >> + return 1; > >> + if (a->ignore_servers_len != b->ignore_servers_len) > >> + return 1; > >> + if (memcmp(a->ignore_servers, b->ignore_servers, > >> + a->ignore_servers_len * sizeof (struct in_addr)) != 0) > >> + return 1; > >> return 0; > >> } > >> #endif /* SMALL */ > >> diff --git parse.y parse.y > >> index 947ed038f95..dc6f16d4400 100644 > >> --- parse.y > >> +++ parse.y > >> @@ -108,7 +108,7 @@ typedef struct { > >> > >> %} > >> > >> -%token DHCP_IFACE ERROR SEND VENDOR CLASS ID CLIENT > >> +%token DHCP_IFACE ERROR SEND VENDOR CLASS ID CLIENT IGNORE DNS ROUTES > >> > >> %token <v.string> STRING > >> %token <v.number> NUMBER > >> @@ -275,6 +275,31 @@ ifaceoptsl : SEND VENDOR CLASS ID STRING { > >> iface_conf->c_id[0] = DHO_DHCP_CLIENT_IDENTIFIER; > >> iface_conf->c_id[1] = iface_conf->c_id_len - 2; > >> } > >> + | IGNORE ROUTES { > >> + iface_conf->ignore |= IGN_ROUTES; > >> + } > >> + | IGNORE DNS { > >> + iface_conf->ignore |= IGN_DNS; > >> + } > >> + | IGNORE STRING { > >> + int res; > >> + > >> + if (iface_conf->ignore_servers_len >= MAX_SERVERS) { > >> + yyerror("too many servers to ignore"); > >> + free($2); > >> + YYERROR; > >> + } > >> + res = inet_pton(AF_INET, $2, > >> + &iface_conf->ignore_servers[ > >> + iface_conf->ignore_servers_len++]); > >> + > >> + if (res != 1) { > >> + yyerror("Invalid server IP %s", $2); > >> + free($2); > >> + YYERROR; > >> + } > >> + free($2); > >> + } > >> ; > >> %% > >> > >> @@ -312,8 +337,11 @@ lookup(char *s) > >> static const struct keywords keywords[] = { > >> {"class", CLASS}, > >> {"client", CLIENT}, > >> + {"dns", DNS}, > >> {"id", ID}, > >> + {"ignore", IGNORE}, > >> {"interface", DHCP_IFACE}, > >> + {"routes", ROUTES}, > >> {"send", SEND}, > >> {"vendor", VENDOR}, > >> }; > >> diff --git printconf.c printconf.c > >> index 16b33bbfe40..86eef19baf3 100644 > >> --- printconf.c > >> +++ printconf.c > >> @@ -106,11 +106,24 @@ void > >> print_config(struct dhcpleased_conf *conf) > >> { > >> struct iface_conf *iface; > >> + int i; > >> + char hbuf[INET_ADDRSTRLEN]; > >> > >> SIMPLEQ_FOREACH(iface, &conf->iface_list, entry) { > >> printf("interface %s {\n", iface->name); > >> print_dhcp_options("\t", iface->c_id, iface->c_id_len); > >> print_dhcp_options("\t", iface->vc_id, iface->vc_id_len); > >> + if (iface->ignore & IGN_DNS) > >> + printf("\tignore dns\n"); > >> + if (iface->ignore & IGN_ROUTES) > >> + printf("\tignore routes\n"); > >> + for (i = 0; i < iface->ignore_servers_len; i++) { > >> + if (inet_ntop(AF_INET, &iface->ignore_servers[i], > >> + hbuf, sizeof(hbuf)) == NULL) > >> + continue; > >> + printf("\tignore %s\n", hbuf); > >> + > >> + } > >> printf("}\n"); > >> } > >> } > >> > >> > >> > >> -- > >> I'm not entirely sure you are real. > >> > > > > -- > I'm not entirely sure you are real. >
