From owner-freebsd-net@FreeBSD.ORG Sun Mar 15 13:30:23 2015 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id EDE00363; Sun, 15 Mar 2015 13:30:23 +0000 (UTC) Received: from forward7l.mail.yandex.net (forward7l.mail.yandex.net [IPv6:2a02:6b8:0:1819::7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "forwards.mail.yandex.net", Issuer "Certum Level IV CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 980E7E1; Sun, 15 Mar 2015 13:30:23 +0000 (UTC) Received: from smtp3m.mail.yandex.net (smtp3m.mail.yandex.net [77.88.61.130]) by forward7l.mail.yandex.net (Yandex) with ESMTP id AC66CBC1054; Sun, 15 Mar 2015 16:30:19 +0300 (MSK) Received: from smtp3m.mail.yandex.net (localhost [127.0.0.1]) by smtp3m.mail.yandex.net (Yandex) with ESMTP id 27B4B27A038F; Sun, 15 Mar 2015 16:30:19 +0300 (MSK) Received: from unknown (unknown [2a02:6b8:0:6::bb]) by smtp3m.mail.yandex.net (nwsmtp/Yandex) with ESMTPSA id vxyKT49d6m-UIamZA6p; Sun, 15 Mar 2015 16:30:18 +0300 (using TLSv1.2 with cipher AES128-SHA (128/128 bits)) (Client certificate not present) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1426426218; bh=/hjYmjHLKMB4zKcSKVY5l709gSH0uWqpPFJLVbclB2E=; h=Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type; b=VDxAFyjQpUNpBp5hKq1GmS/xBW0L86OK7awyuqmH6jC9tqQuWMOGpz5J0I0uuD2jE ecV1coeUp8CCIi/BxnYMl5A2fLMZbtI+Mo2zQSajsJWsiLyKfVi0yGgBFk1yYZcSes +9f5CmKPYIXWVon2FMn8Zt8F+f8pdaG+RENmwu2g= Authentication-Results: smtp3m.mail.yandex.net; dkim=pass header.i=@yandex.ru Message-ID: <5505891E.4060109@yandex.ru> Date: Sun, 15 Mar 2015 16:29:02 +0300 From: "Andrey V. Elsukov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Kristof Provost , freebsd-net@freebsd.org Subject: Re: Padded packets in ip6_input() References: <20150315063651.GA2036@vega.codepro.be> In-Reply-To: <20150315063651.GA2036@vega.codepro.be> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fehWO7T0modUimUf4LW491lNlD2t7dSp1" Cc: Philip Paeps X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Mar 2015 13:30:24 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --fehWO7T0modUimUf4LW491lNlD2t7dSp1 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 15.03.2015 09:36, Kristof Provost wrote: > Hi, >=20 > While having a quick look at PR 169630 I wound up looking at what > happens with short IP and IPv6 packets in their input paths. >=20 > On Ethernet frames have to have a minimum size and for both legacy and > modern IP it's possible to have shorter packets. In that case the sende= r > just adds some padding after the packet so it can be transported over > Ethernet. >=20 > The way we deal with that is different for IPv4 and IPv6. In v4 we chec= k > packet size (is the packet big enough for what IP claims the size is) > and remove the trailing bytes very early in the processing. In > ip6_input() that isn't done until after the pfil() hook and nexthop > processing. >=20 > I think it's risky to wait with the check and trim (as in PR 169360: > it's possible firewalls would reassemble and include the padding. That'= d > be a bug in the firewall of course, but this would ensure it couldn't > happen at all). > On the flip size, I see no downside of doing the size check earlier. We= > have to check the packet size anyway. >=20 > Below is a patch which does just that. >=20 > commit 04efb7e62ab6ae2d3bdac362b1da8a1de9f0a531 > Author: Kristof Provost > Date: Sun Mar 15 05:25:00 2015 +0100 >=20 > Check ip6 packet size and trim before the firewall > =20 > In the ip6 input path we don't check the packet size ("Is the entir= e IP > frame there?") or trim it down (e.g. on Ethernet where short packet= s get > padding at the tail) until after the pfil() hook. > That means that the firewall can get packets with unwanted trailing= > bytes. This could cause issues with careless reassembly code. > There's no reason to wait with this check so align with the ip4 inp= ut > path and do the check before the pfil() hook. > =20 > Note that we re-read the plen after the pfil() hook, just in case t= he > firewall code does something to the packet length. This may or may = not > be required. >=20 > diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c > index 78e8ef3..d7b20fa 100644 > --- a/sys/netinet6/ip6_input.c > +++ b/sys/netinet6/ip6_input.c > @@ -563,6 +563,26 @@ ip6_input(struct mbuf *m) > in6_ifstat_inc(m->m_pkthdr.rcvif, ifs6_in_addrerr); > goto bad; > } > + > + /* > + * Check that the amount of data in the buffers > + * is as at least much as the IPv6 header would have us expect. > + * Trim mbufs if longer than we expect. > + * Drop packet if shorter than we expect. > + */ > + plen =3D (u_int32_t)ntohs(ip6->ip6_plen); > + if (m->m_pkthdr.len - sizeof(struct ip6_hdr) < plen) { > + IP6STAT_INC(ip6s_tooshort); > + in6_ifstat_inc(m->m_pkthdr.rcvif, ifs6_in_truncated); > + goto bad; > + } This is very rare case, I think, but plen can be zero in case, when jumbo payload option is present. Probably this is the reason why this check is done after hop-by-hop options parsing. --=20 WBR, Andrey V. Elsukov --fehWO7T0modUimUf4LW491lNlD2t7dSp1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVBYkeAAoJEAHF6gQQyKF6dTQIAITQOJ19+d7BElpTRb/klmCf Dqic08AxBckG72MBU2WuzYo7YPgJRQinAHMJG6IJuTmf6r1ngIOQ0F73pVnpW8fp zW9zN5CcedPkqRZZ+Z0bxiudU3hhfD3YlVfowutRFxV+GY72Z1PIQLU5OtYqKTD6 kQfJo/HaaeGNcMUXitWsfrm2Dk+4dgaCvQNSlcmkzNDDIZwMMY77qMnlJ9nqtnQ1 PWZm10diCfRw+TlfpwFG0LvH1tJ0Rm8YXqBPqH3RaoIEQWwtqF8Fxodg+ARBZd7N qSP1h9oDeDj+jNPb3g0/GiRgDrhN5uKOKXlG7DCJWCNYWTygGSjoRrQVzv/Owns= =9F/B -----END PGP SIGNATURE----- --fehWO7T0modUimUf4LW491lNlD2t7dSp1--