From owner-freebsd-current@freebsd.org Thu May 19 00:19:25 2016 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D8879B417DE for ; Thu, 19 May 2016 00:19:25 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from mailman.ysv.freebsd.org (mailman.ysv.freebsd.org [IPv6:2001:1900:2254:206a::50:5]) by mx1.freebsd.org (Postfix) with ESMTP id C8B2316E0 for ; Thu, 19 May 2016 00:19:25 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: by mailman.ysv.freebsd.org (Postfix) id C80DFB417DD; Thu, 19 May 2016 00:19:25 +0000 (UTC) Delivered-To: current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C7B8CB417DC for ; Thu, 19 May 2016 00:19:25 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from gw.catspoiler.org (unknown [IPv6:2602:304:b010:ef20::f2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "gw.catspoiler.org", Issuer "gw.catspoiler.org" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 9827816DF; Thu, 19 May 2016 00:19:25 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.15.2/8.15.2) with ESMTP id u4J0JFXN080729; Wed, 18 May 2016 17:19:20 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Message-Id: <201605190019.u4J0JFXN080729@gw.catspoiler.org> Date: Wed, 18 May 2016 17:19:15 -0700 (PDT) From: Don Lewis Subject: Re: r299512 breaks dhclient on some networks To: ian.freislich@capeaugusta.com, cem@FreeBSD.org cc: current@freebsd.org In-Reply-To: <15379a54-3e6e-2c2c-b41c-8def3af65bad@capeaugusta.com> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 May 2016 00:19:25 -0000 On 18 May, Ian FREISLICH wrote: > Hi > > I cannot for the life of me figure out why the change in r299512 breaks > DHCP on one network I use but not on another network. > > The only clue I can find is that the request whose response is ignored > has the following client ID: > 1:6:0:22:5f:70:a1:df > > The request whose response is use has this client ID: > 1:0:22:5f:70:a1:df > > Here's a dhcpdump of the request/offer that gets ignored. > > --------------------------------------------------------------------------- > > TIME: 2016-05-18 18:46:39.134 > IP: 0.0.0.0 (00:22:5f:70:a1:df) > 255.255.255.255 (ff:ff:ff:ff:ff:ff) > OP: 1 (BOOTPREQUEST) > HTYPE: 1 (Ethernet) > HLEN: 6 > HOPS: 0 > XID: 92a34fc3 > SECS: 0 > FLAGS: 0 > CIADDR: 0.0.0.0 > YIADDR: 0.0.0.0 > SIADDR: 0.0.0.0 > GIADDR: 0.0.0.0 > CHADDR: 00:22:5f:70:a1:df:00:00:00:00:00:00:00:00:00:00 > SNAME: . > FNAME: . > OPTION: 53 ( 1) DHCP message type 1 (DHCPDISCOVER) > OPTION: 61 ( 8) Client-identifier 01:06:00:22:5f:70:a1:df > OPTION: 12 ( 3) Host name zen > OPTION: 55 ( 9) Parameter Request List 1 (Subnet mask) > 28 (Broadcast address) > 2 (Time offset) > 121 (Classless Static Route) > 3 (Routers) > 15 (Domainname) > 6 (DNS server) > 12 (Host name) > 119 (Domain Search) > > --------------------------------------------------------------------------- > > TIME: 2016-05-18 18:46:39.134 > IP: 10.0.0.1 (4c:5e:0c:62:4f:82) > 10.0.0.80 (00:22:5f:70:a1:df) > OP: 2 (BOOTPREPLY) > HTYPE: 1 (Ethernet) > HLEN: 6 > HOPS: 0 > XID: 92a34fc3 > SECS: 0 > FLAGS: 0 > CIADDR: 0.0.0.0 > YIADDR: 10.0.0.80 > SIADDR: 10.0.0.1 > GIADDR: 0.0.0.0 > CHADDR: 00:22:5f:70:a1:df:00:00:00:00:00:00:00:00:00:00 > SNAME: . > FNAME: . > OPTION: 53 ( 1) DHCP message type 2 (DHCPOFFER) > OPTION: 54 ( 4) Server identifier 10.0.0.1 > OPTION: 51 ( 4) IP address leasetime 259200 (3d) > OPTION: 1 ( 4) Subnet mask 255.255.255.0 > OPTION: 3 ( 4) Routers 10.0.0.1 > OPTION: 6 ( 4) DNS server 10.0.0.1 > --------------------------------------------------------------------------- > > > And here's the request/offer that works (with the r299512 backed out) > > --------------------------------------------------------------------------- > > TIME: 2016-05-18 18:35:33.817 > IP: 10.0.0.220 (00:22:5f:70:a1:df) > 255.255.255.255 (ff:ff:ff:ff:ff:ff) > OP: 1 (BOOTPREQUEST) > HTYPE: 1 (Ethernet) > HLEN: 6 > HOPS: 0 > XID: 866cfd85 > SECS: 4 > FLAGS: 0 > CIADDR: 0.0.0.0 > YIADDR: 0.0.0.0 > SIADDR: 0.0.0.0 > GIADDR: 0.0.0.0 > CHADDR: 00:22:5f:70:a1:df:00:00:00:00:00:00:00:00:00:00 > SNAME: . > FNAME: . > OPTION: 53 ( 1) DHCP message type 3 (DHCPREQUEST) > OPTION: 50 ( 4) Request IP address 10.0.0.220 > OPTION: 61 ( 7) Client-identifier 01:00:22:5f:70:a1:df > OPTION: 12 ( 3) Host name zen > OPTION: 55 ( 9) Parameter Request List 1 (Subnet mask) > 28 (Broadcast address) > 2 (Time offset) > 121 (Classless Static Route) > 3 (Routers) > 15 (Domainname) > 6 (DNS server) > 12 (Host name) > 119 (Domain Search) > > --------------------------------------------------------------------------- > > TIME: 2016-05-18 18:35:33.817 > IP: 10.0.0.1 (4c:5e:0c:62:4f:82) > 10.0.0.220 (00:22:5f:70:a1:df) > OP: 2 (BOOTPREPLY) > HTYPE: 1 (Ethernet) > HLEN: 6 > HOPS: 0 > XID: 866cfd85 > SECS: 0 > FLAGS: 0 > CIADDR: 0.0.0.0 > YIADDR: 10.0.0.220 > SIADDR: 10.0.0.1 > GIADDR: 0.0.0.0 > CHADDR: 00:22:5f:70:a1:df:00:00:00:00:00:00:00:00:00:00 > SNAME: . > FNAME: . > OPTION: 53 ( 1) DHCP message type 5 (DHCPACK) > OPTION: 54 ( 4) Server identifier 10.0.0.1 > OPTION: 51 ( 4) IP address leasetime 259200 (3d) > OPTION: 1 ( 4) Subnet mask 255.255.255.0 > OPTION: 3 ( 4) Routers 10.0.0.1 > OPTION: 6 ( 4) DNS server 10.0.0.1 > --------------------------------------------------------------------------- It looks to me like r299512 is changing the format of the client identifier by inserting the struct hardware hlen field into it. That's not valid if htype is non-zero the way I interpret RFC 2132. On the other hand, I would think that the server would interpret the client ID as an opaque cookie, so I wouldn't think it would make a difference (other than thinking this is a new client) unless your server is configured to look for specific client IDs. I think that r299512 is mostly incorrect and should be reverted. The problem reported by CID 1008682 is an overrun of hw_address.haddr. struct hardware looks like this: struct hardware { u_int8_t htype; u_int8_t hlen; u_int8_t haddr[16]; }; I think the correct fix is just this: size_t hwlen = MIN(ip->hw_address.hlen, sizeof(ip->hw_address.haddr)); The old code was wrong because sizeof(client_ident)-1 is the same as sizeof(struct hardware)-1, when it should be -2 to exclude both htype and hlen from the calculation. The fix for CID 1305550 looks correct. The following patch has been compile tested: Index: dhclient.c =================================================================== --- dhclient.c (revision 300099) +++ dhclient.c (working copy) @@ -1572,18 +1572,16 @@ } /* set unique client identifier */ - struct hardware client_ident; + char client_ident[sizeof(struct hardware)]; if (!options[DHO_DHCP_CLIENT_IDENTIFIER]) { size_t hwlen = MIN(ip->hw_address.hlen, - sizeof(client_ident.haddr)); - client_ident.htype = ip->hw_address.htype; - client_ident.hlen = hwlen; - memcpy(client_ident.haddr, ip->hw_address.haddr, hwlen); + sizeof(ip->hw_address.haddr)); + client_ident[0] = ip->hw_address.htype; + memcpy(&client_ident[1], ip->hw_address.haddr, hwlen); options[DHO_DHCP_CLIENT_IDENTIFIER] = &option_elements[DHO_DHCP_CLIENT_IDENTIFIER]; - options[DHO_DHCP_CLIENT_IDENTIFIER]->value = (void *)&client_ident; - hwlen += offsetof(struct hardware, haddr); - options[DHO_DHCP_CLIENT_IDENTIFIER]->len = hwlen; - options[DHO_DHCP_CLIENT_IDENTIFIER]->buf_size = hwlen; + options[DHO_DHCP_CLIENT_IDENTIFIER]->value = client_ident; + options[DHO_DHCP_CLIENT_IDENTIFIER]->len = hwlen+1; + options[DHO_DHCP_CLIENT_IDENTIFIER]->buf_size = hwlen+1; options[DHO_DHCP_CLIENT_IDENTIFIER]->timeout = 0xFFFFFFFF; }