Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

reassembled packets and pfil

3 views
Skip to first unread message

Matthew Luckie

unread,
Apr 12, 2010, 9:57:01 PM4/12/10
to
Hi

Reassembled packets are not passed to the packet filter interface for
both IPv4 and IPv6, so a firewall has no effect if the packets arrive
in fragments. Here is a patch to fix this for IPv6. The patch for
IPv4 is similarly trivial, but I have not written / tested it yet.

Is there any particular reason why reassembled packets were not
checked? If the answer is no, I'll send in a PR.

I've tested the patch below.

Matthew

--- sys/netinet6/frag6.c.orig 2008-11-25 15:59:29.000000000 +1300
+++ sys/netinet6/frag6.c 2010-04-13 13:21:02.000000000 +1200
@@ -46,6 +46,7 @@ __FBSDID("$FreeBSD: src/sys/netinet6/fra

#include <net/if.h>
#include <net/route.h>
+#include <net/pfil.h>

#include <netinet/in.h>
#include <netinet/in_var.h>
@@ -568,6 +569,13 @@ insert:
*offp = offset;

IP6Q_UNLOCK();
+
+ if (PFIL_HOOKED(&inet6_pfil_hook) &&
+ (pfil_run_hooks(&inet6_pfil_hook, &m, m->m_pkthdr.rcvif, PFIL_IN, NULL) ||
+ m == NULL)) {
+ return IPPROTO_DONE;
+ }
+
return nxt;

dropfrag:
_______________________________________________
freeb...@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net...@freebsd.org"

Luigi Rizzo

unread,
Apr 13, 2010, 2:57:32 AM4/13/10
to
On Tue, Apr 13, 2010 at 01:57:01PM +1200, Matthew Luckie wrote:
> Hi
>
> Reassembled packets are not passed to the packet filter interface for
> both IPv4 and IPv6, so a firewall has no effect if the packets arrive
> in fragments. Here is a patch to fix this for IPv6. The patch for
> IPv4 is similarly trivial, but I have not written / tested it yet.
>
> Is there any particular reason why reassembled packets were not
> checked? If the answer is no, I'll send in a PR.

I think it was just a random decision -- either pass packets to
the firewall before reassembly as we do, or after reassembly, as
linux does. Both have pros and cons.
Going through the firewall twice, however, is problematic because
far too many things (counters, dummynet, etc.) expect to see each
packet only once.

I think that a patch like the one you propose is very useful (for
ipv4 as well) but it requires a sysctl or other mechanism to make
sure that when it is enabled we don't pass fragments through the
firewall.

cheers
luigi

Matthew Luckie

unread,
Apr 13, 2010, 3:36:55 AM4/13/10
to
>> Is there any particular reason why reassembled packets were not
>> checked? If the answer is no, I'll send in a PR.
>
> I think it was just a random decision -- either pass packets to
> the firewall before reassembly as we do, or after reassembly, as
> linux does. Both have pros and cons.
> Going through the firewall twice, however, is problematic because
> far too many things (counters, dummynet, etc.) expect to see each
> packet only once.

ok, thanks for letting me know.

> I think that a patch like the one you propose is very useful (for
> ipv4 as well) but it requires a sysctl or other mechanism to make
> sure that when it is enabled we don't pass fragments through the
> firewall.

i've looked further into this and I now wonder if is a byproduct of my
use of ipfw. the problem seems to be that offset will always be
non-zero with any packet with a v6 fragment header, so a rule requiring
offset to be zero is never run. i'll spend a bit more time on this
tomorrow, and come back with a patch for ipfw.

Note this code:

offset = ((struct ip6_frag *)ulp)->ip6f_offlg & IP6F_OFF_MASK;
/* Add IP6F_MORE_FRAG for offset of first
* fragment to be != 0. */
offset |= ((struct ip6_frag *)ulp)->ip6f_offlg & IP6F_MORE_FRAG;
if (offset == 0) {
printf("IPFW2: IPV6 - Invalid Fragment Header\n");
if (fw_deny_unknown_exthdrs)
return (IP_FW_DENY);
break;
}

This code seems to be incorrect, per rfc 2460:

In response to an IPv6 packet that is sent to an IPv4 destination
(i.e., a packet that undergoes translation from IPv6 to IPv4), the
originating IPv6 node may receive an ICMP Packet Too Big message
reporting a Next-Hop MTU less than 1280. In that case, the IPv6 node
is not required to reduce the size of subsequent packets to less than
1280, but must include a Fragment header in those packets so that the
IPv6-to-IPv4 translating router can obtain a suitable Identification
value to use in resulting IPv4 fragments. Note that this means the
payload may have to be reduced to 1232 octets (1280 minus 40 for the
IPv6 header and 8 for the Fragment header), and smaller still if
additional extension headers are used.

A stack can send an IPv6 packet with a fragment header attached that
does not have the MF bit set. I'm 90% sure that FreeBSD itself will do
this when it receives a PTB with an MTU value of (say) 1000.

Matthew

Matthew Luckie

unread,
Apr 13, 2010, 6:31:12 PM4/13/10
to
> >I think that a patch like the one you propose is very useful (for
> >ipv4 as well) but it requires a sysctl or other mechanism to make
> >sure that when it is enabled we don't pass fragments through the
> >firewall.
>
> i've looked further into this and I now wonder if is a byproduct of my
> use of ipfw. the problem seems to be that offset will always be
> non-zero with any packet with a v6 fragment header, so a rule requiring
> offset to be zero is never run. i'll spend a bit more time on this
> tomorrow, and come back with a patch for ipfw.

Here's a patch to ipfw. We keep a copy of the MF bit for IPv6
fragments so it can be passed to ipfw_log. Otherwise, the offset
field no longer has the MF bit embedded in it as before.

Note that apart from the various transport-layer checks that require
offset to be zero, the O_FRAG opcode now has a different behaviour.
Only subsequent fragments will match this rule. If you want the exact
same behaviour as before, then

case O_FRAG:
match = (offset != 0);
break;

should become

case O_FRAG:
match = (offset != 0 || ext_hd & EXT_FRAGMENT);
break;

If you are generally happy with this patch, let me know and I'll file
a PR so it doesn't get lost.


--- ip_fw2.c.orig 2008-11-25 15:59:29.000000000 +1300
+++ ip_fw2.c 2010-04-14 10:05:46.000000000 +1200
@@ -758,6 +758,7 @@ ipfw_log(struct ip_fw *f, u_int hlen, st
char *action;
int limit_reached = 0;
char action2[40], proto[128], fragment[32];
+ u_short mf = 0;

fragment[0] = '\0';
proto[0] = '\0';
@@ -903,6 +904,8 @@ ipfw_log(struct ip_fw *f, u_int hlen, st
snprintf(dst, sizeof(dst), "[%s]",
ip6_sprintf(ip6buf, &args->f_id.dst_ip6));

+ mf = offset & IP6F_MORE_FRAG;
+ offset &= IP6F_OFF_MASK;
ip6 = (struct ip6_hdr *)ip;
tcp = (struct tcphdr *)(((char *)ip) + hlen);
udp = (struct udphdr *)(((char *)ip) + hlen);
@@ -972,13 +975,13 @@ ipfw_log(struct ip_fw *f, u_int hlen, st

#ifdef INET6
if (IS_IP6_FLOW_ID(&(args->f_id))) {
- if (offset & (IP6F_OFF_MASK | IP6F_MORE_FRAG))
+ if (offset || mf)
snprintf(SNPARGS(fragment, 0),
" (frag %08x:%d@%d%s)",
args->f_id.frag_id6,
ntohs(ip6->ip6_plen) - hlen,
- ntohs(offset & IP6F_OFF_MASK) << 3,
- (offset & IP6F_MORE_FRAG) ? "+" : "");
+ ntohs(offset) << 3,
+ mf ? "+" : "");
} else
#endif
{
@@ -2151,16 +2154,13 @@ ipfw_chk(struct ip_fw_args *args)

/*
* offset The offset of a fragment. offset != 0 means that
- * we have a fragment at this offset of an IPv4 packet.
- * offset == 0 means that (if this is an IPv4 packet)
- * this is the first or only fragment.
- * For IPv6 offset == 0 means there is no Fragment Header.
- * If offset != 0 for IPv6 always use correct mask to
- * get the correct offset because we add IP6F_MORE_FRAG
- * to be able to dectect the first fragment which would
- * otherwise have offset = 0.
+ * we have a fragment at this offset.
+ * offset == 0 means that this is the first or only fragment.
+ *
+ * mf The MF bit masked out of IPv6 packets.
*/
u_short offset = 0;
+ u_short mf = 0;

/*
* Local copies of addresses. They are only valid if we have
@@ -2311,17 +2311,8 @@ do { \
proto = ((struct ip6_frag *)ulp)->ip6f_nxt;


offset = ((struct ip6_frag *)ulp)->ip6f_offlg &
IP6F_OFF_MASK;

- /* Add IP6F_MORE_FRAG for offset of first
- * fragment to be != 0. */
- offset |= ((struct ip6_frag *)ulp)->ip6f_offlg &
+ mf = ((struct ip6_frag *)ulp)->ip6f_offlg &
IP6F_MORE_FRAG;
- if (offset == 0) {
- printf("IPFW2: IPV6 - Invalid Fragment "
- "Header\n");
- if (fw_deny_unknown_exthdrs)
- return (IP_FW_DENY);
- break;
- }
args->f_id.frag_id6 =
ntohl(((struct ip6_frag *)ulp)->ip6f_ident);
ulp = NULL;
@@ -2904,7 +2895,7 @@ check_body:
case O_LOG:
if (fw_verbose)
ipfw_log(f, hlen, args, m,
- oif, offset, tablearg, ip);
+ oif, offset|mf, tablearg, ip);
match = 1;
break;

0 new messages