From owner-freebsd-current@freebsd.org Thu May 19 03:08:35 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 7493CB40DA2 for ; Thu, 19 May 2016 03:08:35 +0000 (UTC) (envelope-from cse.cem@gmail.com) 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 56A6011A7 for ; Thu, 19 May 2016 03:08:35 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mailman.ysv.freebsd.org (Postfix) id 527B3B40DA1; Thu, 19 May 2016 03:08:35 +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 4FDF4B40DA0 for ; Thu, 19 May 2016 03:08:35 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-io0-f178.google.com (mail-io0-f178.google.com [209.85.223.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 17A7511A6; Thu, 19 May 2016 03:08:34 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-io0-f178.google.com with SMTP id f89so90726592ioi.0; Wed, 18 May 2016 20:08:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :date:message-id:subject:from:to:cc; bh=HoDoIcvunVaFQCXg6Xun7IrWbleRKboxhSMoOa0NkFA=; b=M05KRCqzEpFA6wmjjtgIjLMudfh9dH7Xyv5ybM6p2I0NGzxOpV0Hzrd5rOWw//4g3d iM1BZHwtj9r7NF/5+0mLVWvycXUgAKInw2YV0GDdlYeYHNju1L2Fzm6ZI/o/hVISwymu NY6Bngt2vaOvEQUEwmzHgOwgmr5hLzEslnHIh789TcvEZt2JrKcap08lbBa/rAO6fZ4e jeH8MX1hzutI7L681vUd9OiwrYPDt2BRxrCOQIpNkSYQKawIe/p3JWZkH0djJCuVZzqb RMdEGmCt7YpEhypqCY55b3Hl6YeBVe9p9d+BzMeBvQxHY8dH09C6RDZUfwRkmYXD+ixF K9Tg== X-Gm-Message-State: AOPr4FXxd5EYSVBUIa/3fLF72nrmVdDpybiWlTCuH2+5jXZ7M9AUv3/DkyTbMcp+FPmF1Q== X-Received: by 10.107.166.140 with SMTP id p134mr7615067ioe.121.1463617697962; Wed, 18 May 2016 17:28:17 -0700 (PDT) Received: from mail-io0-f170.google.com (mail-io0-f170.google.com. [209.85.223.170]) by smtp.gmail.com with ESMTPSA id ho7sm9583063igc.16.2016.05.18.17.28.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 May 2016 17:28:17 -0700 (PDT) Received: by mail-io0-f170.google.com with SMTP id d62so87233221iof.2; Wed, 18 May 2016 17:28:17 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.107.46.158 with SMTP id u30mr7595174iou.162.1463617697353; Wed, 18 May 2016 17:28:17 -0700 (PDT) Reply-To: cem@FreeBSD.org Received: by 10.36.205.70 with HTTP; Wed, 18 May 2016 17:28:17 -0700 (PDT) In-Reply-To: <201605190019.u4J0JFXN080729@gw.catspoiler.org> References: <15379a54-3e6e-2c2c-b41c-8def3af65bad@capeaugusta.com> <201605190019.u4J0JFXN080729@gw.catspoiler.org> Date: Wed, 18 May 2016 17:28:17 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: r299512 breaks dhclient on some networks From: Conrad Meyer To: Don Lewis Cc: Ian FREISLICH , current Content-Type: text/plain; charset=UTF-8 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 03:08:35 -0000 On Wed, May 18, 2016 at 5:19 PM, Don Lewis wrote: > > It looks to me like r299512 is changing the format of the client > identifier by inserting the struct hardware hlen field into it. Yes. The problem with r299512 is that it assumed the client_id was actually a valid struct hardware, as the array's size suggested, when in fact it has nothing to do with that struct. > 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. That seems like a pretty reasonable use of the client identifier. Or less reasonably but still expected, only allowing client identifiers of exactly 6 bytes. > 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. Yep. Or equivalently, I've defined the length of client_ident in terms of sizeof(haddr) + 1. The result is the same. > The fix for CID 1305550 looks correct. Maybe; though I reverted it too. Really I think hlen > sizeof(haddr) is invalid, but I'm not familiar enough with dhclient.c to insert that check in the right place. I think throwing in MIN() in an ad-hoc fashion maybe isn't the best approach. Best, Conrad