Date: Sun, 15 Mar 2015 16:29:02 +0300 From: "Andrey V. Elsukov" <bu7cher@yandex.ru> To: Kristof Provost <kristof@sigsegv.be>, freebsd-net@freebsd.org Cc: Philip Paeps <philip@freebsd.org> Subject: Re: Padded packets in ip6_input() Message-ID: <5505891E.4060109@yandex.ru> In-Reply-To: <20150315063651.GA2036@vega.codepro.be> References: <20150315063651.GA2036@vega.codepro.be>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <kristof@sigsegv.be>
> 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5505891E.4060109>