From owner-freebsd-net@FreeBSD.ORG Mon Jan 10 23:23:19 2011 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EF39D106564A for ; Mon, 10 Jan 2011 23:23:18 +0000 (UTC) (envelope-from gnn@neville-neil.com) Received: from vps.hungerhost.com (vps.hungerhost.com [216.38.53.176]) by mx1.freebsd.org (Postfix) with ESMTP id ABA098FC0C for ; Mon, 10 Jan 2011 23:23:18 +0000 (UTC) Received: from smtp.hudson-trading.com ([209.249.190.9] helo=gnnmac.hudson-trading.com) by vps.hungerhost.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69) (envelope-from ) id 1PcQGX-0002wV-7e; Mon, 10 Jan 2011 17:31:05 -0500 Mime-Version: 1.0 (Apple Message framework v1082) Content-Type: text/plain; charset=us-ascii From: George Neville-Neil In-Reply-To: <4cfe88b6.1cedd80a.33f5.119d@mx.google.com> Date: Mon, 10 Jan 2011 17:31:04 -0500 Content-Transfer-Encoding: quoted-printable Message-Id: <5B47152C-9076-4E54-BCD8-A66FF3E00A42@neville-neil.com> References: <4cfe88b6.1cedd80a.33f5.119d@mx.google.com> To: Rozhuk.IM@gmail.com X-Mailer: Apple Mail (2.1082) X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vps.hungerhost.com X-AntiAbuse: Original Domain - freebsd.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - neville-neil.com Cc: freebsd-net@freebsd.org Subject: Re: [arp] possible DoS, fixes and improvements X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Jan 2011 23:23:19 -0000 On Dec 7, 2010, at 14:19 , rozhuk.im@gmail.com wrote: > Hi! >=20 >=20 Hi, Sorry it has taken me a while to test this. In the first two cases I = cannot reproduce your results on HEAD. I have attempted to test this with a modified arpwhohas.py = script using PCS (ports/net/pcs). I can send very large requests and I see them arrive with tcpdump but = the kernel just throws them away. I do not disagree that the code might be wrong, in particular in case 1, = but I cannot reproduce your results. If you wish to share your test code with me that would be fine, or you = can try your test against HEAD and let me know your result. > 1. ah->ar_hln - is depend from ar_hrd? > Yes, and for ARPHRD_ETHER is 6 (ETHER_ADDR_LEN) > For ARPHRD_IEEE1394 - sizeof(struct fw_hwaddr) > ah->ar_hln ignored in ether_output: bcopy(ar_tha(ah), edst, = ETHER_ADDR_LEN); >=20 > check in in_arpinput: > if (ifp->if_addrlen !=3D ah->ar_hln) { > LLE_WUNLOCK(la); > log(LOG_WARNING, > "arp from %*D: addr len: new %d, i/f %d > (ignored)", > ifp->if_addrlen, (u_char *) ar_sha(ah), ":", > ah->ar_hln, ifp->if_addrlen); > goto reply; > } > NO DROP!!!! > In reply we get: > (void)memcpy(ar_tha(ah), ar_sha(ah), ah->ar_hln); > (void)memcpy(ar_sha(ah), enaddr, ah->ar_hln); > Or=20 > (void)memcpy(ar_tha(ah), ar_sha(ah), = ah->ar_hln); > (void)memcpy(ar_sha(ah), &lle->ll_addr, = ah->ar_hln); >=20 > How to use it see below. >=20 >=20 > 2. ah->ar_pln - does not checked! > We can make big arp request (512 nulls after struct arphdr + 2*6 + = 2*4) , > valid for host, set ar_plt =3D 255 > And in reply will receive part of stack or core panic: > in_arpinput: > (void)memcpy(ar_spa(ah), &itaddr, ah->ar_pln); > ... > m->m_len =3D sizeof(*ah) + (2 * ah->ar_pln) + (2 * ah->ar_hln); > ( eq arphdr_len(ah) ) >=20 >=20 >=20 > 3. ar_sha(ah) - does not checked for multicast! > Answers to request my be send to multicast addrs > Only broadcast and host addr are checked. > No check is ar_sha(ah) equal to Ethernet.ether_shost > As result: > arp -an > ? (172.16.0.2) at 01:80:c2:00:00:01 on em0 expires in 118 seconds = [ethernet] >=20 >=20 >=20 > 4. holded packet my be sended without any locks >=20 > Current: > if (la->la_hold !=3D NULL) { > struct mbuf *m_hold, *m_hold_next; >=20 > bcopy(L3_ADDR(la), &sa, sizeof(sa)); > LLE_WUNLOCK(la); > for (m_hold =3D la->la_hold, la->la_hold =3D = NULL; > m_hold !=3D NULL; m_hold =3D m_hold_next) { > m_hold_next =3D m_hold->m_nextpkt; > m_hold->m_nextpkt =3D NULL; > (*ifp->if_output)(ifp, m_hold, &sa, = NULL); > } > } else > LLE_WUNLOCK(la); > la->la_hold =3D NULL; > la->la_numheld =3D 0; >=20 > Here we unlock la and then modify them - this is bad idea! > Patched - see in attached patch. >=20 This is now fixed in HEAD. Thanks! Best, George