Re: [Dnsmasq-discuss] [PATCH] rfc2131: Fix range address assignment not honoring vendor option filters

2016-09-14 Thread Hans Dedecker
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

2016-09-09 Thread Simon Kelley
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

2016-09-05 Thread Hans Dedecker
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
+   {
+