Date: Sun, 15 Mar 2015 07:36:51 +0100 From: Kristof Provost <kristof@sigsegv.be> To: freebsd-net@freebsd.org Cc: Philip Paeps <philip@freebsd.org> Subject: Padded packets in ip6_input() Message-ID: <20150315063651.GA2036@vega.codepro.be>
next in thread | raw e-mail | index | archive | help
Hi,
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.
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 sender
just adds some padding after the packet so it can be transported over
Ethernet.
The way we deal with that is different for IPv4 and IPv6. In v4 we check
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.
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.
Below is a patch which does just that.
commit 04efb7e62ab6ae2d3bdac362b1da8a1de9f0a531
Author: Kristof Provost <kristof@sigsegv.be>
Date: Sun Mar 15 05:25:00 2015 +0100
Check ip6 packet size and trim before the firewall
In the ip6 input path we don't check the packet size ("Is the entire IP
frame there?") or trim it down (e.g. on Ethernet where short packets 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 input
path and do the check before the pfil() hook.
Note that we re-read the plen after the pfil() hook, just in case the
firewall code does something to the packet length. This may or may not
be required.
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 = (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;
+ }
+ if (m->m_pkthdr.len > sizeof(struct ip6_hdr) + plen) {
+ if (m->m_len == m->m_pkthdr.len) {
+ m->m_len = sizeof(struct ip6_hdr) + plen;
+ m->m_pkthdr.len = sizeof(struct ip6_hdr) + plen;
+ } else
+ m_adj(m, sizeof(struct ip6_hdr) + plen - m->m_pkthdr.len);
+ }
#if 0
/*
* Reject packets with IPv4 compatible addresses (auto tunnel).
@@ -702,25 +722,6 @@ passin:
nxt = ip6->ip6_nxt;
/*
- * 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.
- */
- 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;
- }
- if (m->m_pkthdr.len > sizeof(struct ip6_hdr) + plen) {
- if (m->m_len == m->m_pkthdr.len) {
- m->m_len = sizeof(struct ip6_hdr) + plen;
- m->m_pkthdr.len = sizeof(struct ip6_hdr) + plen;
- } else
- m_adj(m, sizeof(struct ip6_hdr) + plen - m->m_pkthdr.len);
- }
-
- /*
* Forward if desirable.
*/
if (V_ip6_mrouter &&
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150315063651.GA2036>