Re: [Dnsmasq-discuss] [PATCH] Add support for unique TFTP root per MAC

2017-04-09 Thread Floris Bos

On 04/09/2017 11:28 PM, Simon Kelley wrote:

Patch accepted, with one change



  snprintf(daemon->namebuff+oldlen, sizeof(daemon->namebuff)-oldlen, 
"%.2x-%.2x-%.2x-%.2x-%.2x-%.2x/",


daemon->namebuff is a char *, so sizeof(daemon->namebuff) is 4 or 8 and
sizeof(daemon->namebuff)-oldlen is a negative number which is a large
positive number when promoted  to unsigned size_t.


Ah, you are right.

Somehow I had the impression it was a fixed size array, on which 
sizeof() do would work.
Probably confused it with the other namebuff in tftp_request ( char 
namebuff[IF_NAMESIZE]; )


Sorry, and thanks for the commit.



Yours sincerely,

Floris Bos

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] [PATCH v2] Add --dhcp-reply-delay option to delay DHCP replies

2017-04-09 Thread Simon Kelley
On 30/03/17 12:38, Floris Bos wrote:
> Adds option to delay replying to DHCP packets by one or more seconds.
> This provides a workaround for a PXE boot firmware implementation
> that has a bug causing it to fail if it receives a (proxy) DHCP
> reply instantly.
> 
> On Linux it looks up the exact receive time of the UDP packet with
> the SIOCGSTAMP ioctl to prevent multiple delays if multiple packets
> come in around the same time.
> 
> Signed-off-by: Floris Bos 

Committed, with a minor change for logging.

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] Add support for unique TFTP root per MAC

2017-04-09 Thread Simon Kelley
Patch accepted, with one change


> snprintf(daemon->namebuff+oldlen, sizeof(daemon->namebuff)-oldlen, 
> "%.2x-%.2x-%.2x-%.2x-%.2x-%.2x/",


daemon->namebuff is a char *, so sizeof(daemon->namebuff) is 4 or 8 and
sizeof(daemon->namebuff)-oldlen is a negative number which is a large
positive number when promoted  to unsigned size_t. There's thus
effectively no protection here against buffer overflow.

In such ways are security CVEs seeded :)

A changed sizeof(daemon->namebuff) to (MAXDNAME-1) which is the
buffer-size limit used elsewhere in this code.

Cheers,

Simon.



___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] dnsmasq & fingerbank.org

2017-04-09 Thread Simon Kelley
The current tree already has this functionality, though the code which
implements it is somewhat different. It certainly provides a
DNSMASQ_REQUESTED_OPTIONS variable containing a comma-separated list of
decimal numbers.

Best to look at the current code, and submit a patch if it doesn't
behave as you expect.


Cheers,

Simon.



On 09/04/17 19:22, Denton Gentry wrote:
> https://fingerbank.org/about.html is a database of OS signatures based
> on the DHCP options populated and the order in which they appear. So
> for example, an Android device will typically send
> '1,33,3,6,15,26,28,51,58,59' while iOS sends '1,3,6,15,119,252'.
> 
> We modified dnsmasq to export DHCP OS signatures to a
> DNSMASQ_REQUESTED_OPTIONS environment variable when it forks the
> dhcp-script. The environment variable is formatted to match
> fingerbank. The dhcp-script we run writes the signature to a separate
> file, though of course this behavior is outside of dnsmasq itself.
> 
> Is OS fingerprinting support something which could be more widely useful?
> Should I rebase the patch to the current tree and start discussion?
> 
> 
> 
> ___
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss@lists.thekelleys.org.uk
> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
> 




signature.asc
Description: OpenPGP digital signature
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] bug: trunk DHCP offer/replies being ignored by some devices

2017-04-09 Thread Simon Kelley
On 08/04/17 12:01, Floris Bos wrote:
> On 04/08/2017 12:00 AM, Simon Kelley wrote:
>>
>> But RFC 6842 assures us that no clients are broken by this change :)
>>
>> The options here, as I see it are
>>
>> 1) revert the change and don't support 6842
>> 2) provide a way to disable the client-id reply for broken clients.
>> 3) provide a flag to disable the client-id for all clients.
>> 4) make the new behaviour optional, and provide a flag to enable it.
>> 5) declare it No Our Problem and get the broken clients fixed.
> 
> What about only sending client-id back if chaddr is zeroed?
> 
> Isn't that the corner case 6842 is actually trying to fix?
> 
> ==
>In some cases, a client may not have a valid hardware address to
>populate the 'chaddr' field and may set the field to all zeroes. One
>such example is when DHCP is used to assign an IP address to a mobile
>phone or a tablet and where the 'chaddr' field is set to zero in DHCP
>request packets.
> ==
> 

A good suggestion. The bald assertion that DHCP-relays drop packets
without chaddrs in 6842 seems quite dodgy to me, why should they, unless
the chaddr is needed to return the reply to the client?


I'll go offline from here and talk to some DHCP people...

Cheers,

Simon.




___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] dnsmasq treats Islands of Security as bogus

2017-04-09 Thread Simon Kelley
On 08/04/17 17:33, Patryk Szczygłowski wrote:
> 2017-04-04 22:24 GMT+01:00 Simon Kelley :
> 
>> Which version of dnsmasq are you using? I just tested this domain using
>> the development code, and got the correct result.
>>
> 
> 
> dnsmasq - 2.73-3
> 
> This is the version currently distributed by Turris Omnia (openwrt-based).
> 
> 


2.73 is old and gnarly in DNSSEC terms - it's been a long, hard road to
get this right in dnsmasq. I believe that the most recent public release
- 2.76 should behave correctly in this case.

I be grateful if you could test that, as a check that my tests are valid.


Cheers


Simon.







signature.asc
Description: OpenPGP digital signature
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


[Dnsmasq-discuss] dnsmasq & fingerbank.org

2017-04-09 Thread Denton Gentry
https://fingerbank.org/about.html is a database of OS signatures based
on the DHCP options populated and the order in which they appear. So
for example, an Android device will typically send
'1,33,3,6,15,26,28,51,58,59' while iOS sends '1,3,6,15,119,252'.

We modified dnsmasq to export DHCP OS signatures to a
DNSMASQ_REQUESTED_OPTIONS environment variable when it forks the
dhcp-script. The environment variable is formatted to match
fingerbank. The dhcp-script we run writes the signature to a separate
file, though of course this behavior is outside of dnsmasq itself.

Is OS fingerprinting support something which could be more widely useful?
Should I rebase the patch to the current tree and start discussion?
Allows fingerprinting of DHCP clients in the external
dhcp-script.
---
 src/dhcp-protocol.h |  3 ++-
 src/dnsmasq.h   |  1 +
 src/helper.c| 19 +++
 src/lease.c |  1 +
 src/rfc2131.c   | 11 +++
 5 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/src/dhcp-protocol.h b/src/dhcp-protocol.h
index a2750de..7e3c845 100644
--- a/src/dhcp-protocol.h
+++ b/src/dhcp-protocol.h
@@ -85,6 +85,7 @@
 #define BRDBAND_FORUM_IANA   3561 /* Broadband forum IANA enterprise */
 
 #define DHCP_CHADDR_MAX 16
+#define DHCP_OPT_MAX312
 
 struct dhcp_packet {
   u8 op, htype, hlen, hops;
@@ -92,5 +93,5 @@ struct dhcp_packet {
   u16 secs, flags;
   struct in_addr ciaddr, yiaddr, siaddr, giaddr;
   u8 chaddr[DHCP_CHADDR_MAX], sname[64], file[128];
-  u8 options[312];
+  u8 options[DHCP_OPT_MAX];
 };
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 4d368c5..3beaba7 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -563,6 +563,7 @@ struct dhcp_lease {
   } *slaac_address;
   int vendorclass_count;
 #endif
+  unsigned char req_options[DHCP_OPT_MAX];
   struct dhcp_lease *next;
 };
 
diff --git a/src/helper.c b/src/helper.c
index 24c2afd..95aa657 100644
--- a/src/helper.c
+++ b/src/helper.c
@@ -72,6 +72,7 @@ struct script_data
 #endif
   unsigned char hwaddr[DHCP_CHADDR_MAX];
   char interface[IF_NAMESIZE];
+  unsigned char req_options[DHCP_OPT_MAX];
 };
 
 static struct script_data *buf = NULL;
@@ -499,6 +500,23 @@ int create_helper(int event_fd, int err_fd, uid_t uid, 
gid_t gid, long max_fd)
  
  my_setenv("DNSMASQ_DOMAIN", domain, &err);
  
+ {
+   unsigned char *p;
+   char strbuf[4096] = {0};
+   for (p = data.req_options; *p != OPTION_END; p++)
+ {
+   char d[16];
+   if (p != data.req_options) strcat(strbuf, ",");
+   snprintf(d, sizeof(d), "%u", *p);
+   if ((strlen(strbuf) + strlen(d) + 3) > sizeof(strbuf)) {
+ break;
+   }
+   strcat(strbuf, d);
+ }
+
+   my_setenv("DNSMASQ_REQUESTED_OPTIONS", strbuf, &err);
+ }
+
  end = extradata + data.ed_len;
  buf = extradata;
  
@@ -698,6 +716,7 @@ void queue_script(int action, struct dhcp_lease *lease, 
char *hostname, time_t n
   memcpy(buf->hwaddr, lease->hwaddr, DHCP_CHADDR_MAX);
   if (!indextoname(fd, lease->last_interface, buf->interface))
 buf->interface[0] = 0;
+  memcpy(buf->req_options, lease->req_options, sizeof(buf->req_options));
   
 #ifdef HAVE_BROKEN_RTC 
   buf->length = lease->length;
diff --git a/src/lease.c b/src/lease.c
index 4b4d10a..e0329da 100644
--- a/src/lease.c
+++ b/src/lease.c
@@ -703,6 +703,7 @@ static struct dhcp_lease *lease_allocate(void)
   lease->length = 0x; /* illegal value */
 #endif
   lease->hwaddr_len = 256; /* illegal value */
+  lease->req_options[0] = OPTION_END;
   lease->next = leases;
   leases = lease;
   
diff --git a/src/rfc2131.c b/src/rfc2131.c
index a49e076..4a35356 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -1255,6 +1255,16 @@ size_t dhcp_reply(struct dhcp_context *context, char 
*iface_name, int int_index,
  if (mess->giaddr.s_addr)
lease->giaddr = mess->giaddr;
  
+ if ((opt = option_find(mess, sz, OPTION_REQUESTED_OPTIONS, 
0)))
+   {
+ int len = sizeof(lease->req_options);
+ if (option_len(opt) < len) {
+   len = option_len(opt);
+ }
+ memcpy(lease->req_options, option_ptr(opt, 0), len);
+ lease->req_options[len] = OPTION_END;
+   }
+
  free(lease->extradata);
  lease->extradata = NULL;
  lease->extradata_size = lease->extradata_len = 0;
@@ -1302,6 +1312,7 @@ size_t dhcp_reply(struct dhcp_context *context, char 
*iface_name, int int_index,
ucp++, len--;
  lease_add_extradata(lease, ucp, len, 0);
}
+ 
}
 #endif
}
-- 
1.9.0.rc1.175.g0b1dcb5