Re: [Dnsmasq-discuss] [PATCH] Add support for unique TFTP root per MAC
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
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
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
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
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
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
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