Re: r299512 breaks dhclient on some networks

From: Don Lewis <truckman_at_FreeBSD.org>
Date: Wed, 18 May 2016 17:19:15 -0700 (PDT)
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)
_at__at_ -1572,18 +1572,16 _at__at_
 	}
 
 	/* 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;
 	}
 
Received on Wed May 18 2016 - 22:19:25 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:05 UTC