Re: [Dnsmasq-discuss] [PATCH] rfc2131: Fix range address assignment not honoring vendor option filters
On Fri, Sep 9, 2016 at 10:55 PM, Simon Kelley wrote: > On 05/09/16 15:35, Hans Dedecker wrote: >> Problem is visible when using multiple dhcp-ranges; one dhcp-range is a >> "catch-all" >> range without tags while the second dhcp-range has tags based on >> vendor-class/user-class/... >> If a client sends a DORA with no specific IP and no vendor/user class it >> will get an IP addres >> from the "catch-all" range; if the client renews (or restarts) with a >> vendor/user class >> and a request IP option holding the "catch-all" IP it will get DHCP options >> from the >> dhcp-range based on vendor-class/user-class but will get the requested IP >> address acked. >> This patch solves this problem as it detects a tag is assigned based on the >> vendor-class/user-class >> and actually checks if the requested IP belongs to the correct dhcp-range; >> if not the client will >> be nacked to it can restart its DORA sequence. >> > > > I'm happy to work on understanding this patch if it's really necessary, > but an alternative way of working around this client behaviour, without > code changes, may be to abandon the "catch all" range, and declare the > two ranges explicitly, > > dhcp-vendorclass=set:special, ... > > dhcp-range = tag:special, .. > > # range used for hosts that don't have a special class > dhcp-range = tag:!special, .. > > > If there are multiple special classes, the catch-all becomes > > dhcp-range = tag:!special1, tag:!special2, tag:!special3 > > Doing it like this, when a previously ordinary client acquires "special" > status, the catch-all range ceases to apply, and the re-addressing > should happen automatically. Maybe I should have added a slightly patch description which describes the problem more clearly. If a client sends a DHCP discover containing the requested IP option dnsmasq acks the IP address (depending on certain conditions like ip address is available in a defined range, no lease, etc ... ) irrespective from the fact a policy class is defined for the client (eg based on the vendor class) instructing it to take an IP address from a different pool. You have no control over neither the presence or the contents of the requested IP option in a discover message sent by the client but I would expect the server to respect the policy class put into place independent from the requested IP option. I don't think this problem can be fixed by config unless I'm missing something; the delivered patch addresses this problem. Bye, Hans > > > Cheers, > > Simon. > > > > > ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] rfc2131: Fix range address assignment not honoring vendor option filters
On 05/09/16 15:35, Hans Dedecker wrote: > Problem is visible when using multiple dhcp-ranges; one dhcp-range is a > "catch-all" > range without tags while the second dhcp-range has tags based on > vendor-class/user-class/... > If a client sends a DORA with no specific IP and no vendor/user class it will > get an IP addres > from the "catch-all" range; if the client renews (or restarts) with a > vendor/user class > and a request IP option holding the "catch-all" IP it will get DHCP options > from the > dhcp-range based on vendor-class/user-class but will get the requested IP > address acked. > This patch solves this problem as it detects a tag is assigned based on the > vendor-class/user-class > and actually checks if the requested IP belongs to the correct dhcp-range; if > not the client will > be nacked to it can restart its DORA sequence. > I'm happy to work on understanding this patch if it's really necessary, but an alternative way of working around this client behaviour, without code changes, may be to abandon the "catch all" range, and declare the two ranges explicitly, dhcp-vendorclass=set:special, ... dhcp-range = tag:special, .. # range used for hosts that don't have a special class dhcp-range = tag:!special, .. If there are multiple special classes, the catch-all becomes dhcp-range = tag:!special1, tag:!special2, tag:!special3 Doing it like this, when a previously ordinary client acquires "special" status, the catch-all range ceases to apply, and the re-addressing should happen automatically. Cheers, Simon. ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] [PATCH] rfc2131: Fix range address assignment not honoring vendor option filters
Problem is visible when using multiple dhcp-ranges; one dhcp-range is a "catch-all" range without tags while the second dhcp-range has tags based on vendor-class/user-class/... If a client sends a DORA with no specific IP and no vendor/user class it will get an IP addres from the "catch-all" range; if the client renews (or restarts) with a vendor/user class and a request IP option holding the "catch-all" IP it will get DHCP options from the dhcp-range based on vendor-class/user-class but will get the requested IP address acked. This patch solves this problem as it detects a tag is assigned based on the vendor-class/user-class and actually checks if the requested IP belongs to the correct dhcp-range; if not the client will be nacked to it can restart its DORA sequence. Signed-off-by: Daniele Lacamera Signed-off-by: Hans Dedecker --- src/rfc2131.c | 81 ++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/src/rfc2131.c b/src/rfc2131.c index 8b99d4b..53e6be1 100644 --- a/src/rfc2131.c +++ b/src/rfc2131.c @@ -64,6 +64,23 @@ static int prune_vendor_opts(struct dhcp_netid *netid); static struct dhcp_opt *pxe_opts(int pxe_arch, struct dhcp_netid *netid, struct in_addr local, time_t now); struct dhcp_boot *find_boot(struct dhcp_netid *netid); static int pxe_uefi_workaround(int pxe_arch, struct dhcp_netid *netid, struct dhcp_packet *mess, struct in_addr local, time_t now, int pxe); + +static struct dhcp_netid *vendor_forced_netid_filter(struct dhcp_context *context, struct dhcp_vendor *vendor, struct dhcp_netid *netid) +{ + struct dhcp_context *context_tmp; + for (context_tmp = context; context_tmp; context_tmp = context_tmp->current) +{ + struct dhcp_netid *f; + for (f = context_tmp->filter; f; f = f->next) +{ + if (strcmp(f->net, netid->net) == 0) +{ + return netid; +} +} +} + return NULL; +} size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, size_t sz, time_t now, int unicast_dest, int *is_inform, int pxe, struct in_addr fallback) @@ -98,6 +115,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, #ifdef HAVE_SCRIPT unsigned char *class = NULL; #endif + struct dhcp_netid *ctx_forced_netid_tag = NULL; + struct dhcp_context *nwd_context = NULL; subnet_addr.s_addr = override.s_addr = 0; @@ -452,6 +471,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, { vendor->netid.next = netid; netid = &vendor->netid; + ctx_forced_netid_tag = vendor_forced_netid_filter(context, vendor, netid); break; } } @@ -1039,13 +1059,59 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, mess->yiaddr = lease->addr; else if (opt && address_available(context, addr, tagif_netid) && !lease_find_by_addr(addr) && !config_find_by_address(daemon->dhcp_conf, addr)) - mess->yiaddr = addr; + { + /* Only accept option 50 either if no tag was forced, or if the tag matches one of the filters + * of the IP range narrowed via option 60. + */ + if ((!ctx_forced_netid_tag) || + (((nwd_context = narrow_context(context, addr, ctx_forced_netid_tag)) != NULL) && + (nwd_context->filter != NULL) && match_netid(nwd_context->filter, ctx_forced_netid_tag, 0))) + { + mess->yiaddr = addr; + } + else + { + struct dhcp_context *context_tmp; + my_syslog(MS_DHCP | LOG_WARNING, _("ignoring option 50 address because netid tag does not match the pool.")); + opt = NULL; + +for (context_tmp = daemon->dhcp; context_tmp; context_tmp = context_tmp->next) + { +if (address_allocate(context_tmp, &mess->yiaddr, emac, emac_len, ctx_forced_netid_tag, now)) + break; + } + + if (!context_tmp) +message = _("wrong option 50 provided, trying to allocate a new address, no address available"); + + ctx_forced_netid_tag = NULL; + } + } else if (emac_len == 0) message = _("no unique-id"); else if (!address_allocate(context, &mess->yiaddr, emac, emac_len, tagif_netid, now)) message = _("no address available"); } + if ((ctx_forced_netid_tag) && + (((nwd_context = narrow_context(context, mess->yiaddr, ctx_forced_netid_tag)) == NULL) || + (nwd_context->filter == NULL) || (!match_netid(nwd_context->filter, ctx_forced_netid_tag, 0 + { +