Module Name:    src
Committed By:   maxv
Date:           Tue Mar 13 09:04:03 UTC 2018

Modified Files:
        src/sys/net/npf: npf_handler.c npf_inet.c

Log Message:
Fix two consecutive mistakes.

The first mistake was npf_inet.c rev1.37:

        "Don't reassemble ipv6 fragments, instead treat the first fragment
        as a regular packet (subject to filtering rules), and pass
        subsequent fragments in the same group unconditionally."

Doing this was entirely wrong, because then a packet just had to push
the L4 payload in a secondary fragment, and NPF wouldn't apply rules on
it - meaning any IPv6 packet could bypass >=L4 filtering. This mistake
was supposed to be a fix for the second mistake.

The second mistake was that ip6_reass_packet (in npf_reassembly) was
getting called with npc->npc_hlen. But npc_hlen pointed to the last
encountered header in the IPv6 chain, which was not necessarily the
fragment header. So ip6_reass_packet was given garbage, and would fail,
resulting in the packet getting kicked. So basically IPv6 was broken by
NPF.

The first mistake is reverted, and the second one is fixed by doing:

-                       hlen = sizeof(struct ip6_frag);
+                       hlen = 0;

Now the iteration stops on the fragment header, and the call to
ip6_reass_packet is valid.

My npf_inet.c rev1.38 is partially reverted: we don't need to worry
about failing properly to advance; once the packet is reassembled
npf_cache_ip gets called again, and this time the whole chain should be
there.

Tested with a simple UDPv6 server - send a 3000-byte-sized buffer, the
packet gets correctly reassembled by NPF now.


To generate a diff of this commit:
cvs rdiff -u -r1.38 -r1.39 src/sys/net/npf/npf_handler.c
cvs rdiff -u -r1.39 -r1.40 src/sys/net/npf/npf_inet.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/net/npf/npf_handler.c
diff -u src/sys/net/npf/npf_handler.c:1.38 src/sys/net/npf/npf_handler.c:1.39
--- src/sys/net/npf/npf_handler.c:1.38	Thu Mar  8 07:06:13 2018
+++ src/sys/net/npf/npf_handler.c	Tue Mar 13 09:04:02 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_handler.c,v 1.38 2018/03/08 07:06:13 maxv Exp $	*/
+/*	$NetBSD: npf_handler.c,v 1.39 2018/03/13 09:04:02 maxv Exp $	*/
 
 /*-
  * Copyright (c) 2009-2013 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
 
 #ifdef _KERNEL
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_handler.c,v 1.38 2018/03/08 07:06:13 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_handler.c,v 1.39 2018/03/13 09:04:02 maxv Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -166,14 +166,7 @@ npf_packet_handler(npf_t *npf, struct mb
 	/* Determine whether it is an IP fragment. */
 	if (__predict_false(flags & NPC_IPFRAG)) {
 		/*
-		 * We pass IPv6 fragments unconditionally
-		 * The first IPv6 fragment is not marked as such
-		 * and passes through the filter
-		 */
-		if (flags & NPC_IP6)
-			return 0;
-		/*
-		 * Pass to IPv4 reassembly mechanism.
+		 * Pass to IPv4/IPv6 reassembly mechanism.
 		 */
 		error = npf_reassembly(npf, &npc, mp);
 		if (error) {

Index: src/sys/net/npf/npf_inet.c
diff -u src/sys/net/npf/npf_inet.c:1.39 src/sys/net/npf/npf_inet.c:1.40
--- src/sys/net/npf/npf_inet.c:1.39	Thu Mar  8 07:54:14 2018
+++ src/sys/net/npf/npf_inet.c	Tue Mar 13 09:04:02 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_inet.c,v 1.39 2018/03/08 07:54:14 maxv Exp $	*/
+/*	$NetBSD: npf_inet.c,v 1.40 2018/03/13 09:04:02 maxv Exp $	*/
 
 /*-
  * Copyright (c) 2009-2014 The NetBSD Foundation, Inc.
@@ -40,7 +40,7 @@
 
 #ifdef _KERNEL
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_inet.c,v 1.39 2018/03/08 07:54:14 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_inet.c,v 1.40 2018/03/13 09:04:02 maxv Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -358,9 +358,6 @@ npf_cache_ip(npf_cache_t *npc, nbuf_t *n
 		struct ip6_frag *ip6f;
 		size_t off, hlen;
 		int frag_present;
-		bool is_frag;
-		uint8_t onxt;
-		int fragoff;
 
 		ip6 = nbuf_ensure_contig(nbuf, sizeof(struct ip6_hdr));
 		if (ip6 == NULL) {
@@ -373,7 +370,6 @@ npf_cache_ip(npf_cache_t *npc, nbuf_t *n
 		npc->npc_hlen = hlen;
 
 		frag_present = 0;
-		is_frag = false;
 
 		/*
 		 * Advance by the length of the current header.
@@ -396,27 +392,8 @@ npf_cache_ip(npf_cache_t *npc, nbuf_t *n
 				if (ip6f == NULL)
 					return NPC_FMTERR;
 
-				hlen = sizeof(struct ip6_frag);
-
-				/* RFC6946: Skip dummy fragments. */
-				fragoff = ntohs(ip6f->ip6f_offlg & IP6F_OFF_MASK);
-				if (fragoff == 0 &&
-				    !(ip6f->ip6f_offlg & IP6F_MORE_FRAG)) {
-					break;
-				}
-
-				is_frag = true;
-
-				/*
-				 * We treat the first fragment as a regular
-				 * packet and then we pass the rest of the
-				 * fragments unconditionally. This way if
-				 * the first packet passes the rest will
-				 * be able to reassembled, if not they will
-				 * be ignored. We can do better later.
-				 */
-				if (fragoff != 0)
-					flags |= NPC_IPFRAG;
+				hlen = 0;
+				flags |= NPC_IPFRAG;
 
 				break;
 			case IPPROTO_AH:
@@ -430,24 +407,11 @@ npf_cache_ip(npf_cache_t *npc, nbuf_t *n
 			if (!hlen) {
 				break;
 			}
-			onxt = npc->npc_proto;
 			npc->npc_proto = ip6e->ip6e_nxt;
 			npc->npc_hlen += hlen;
 		}
 
 		/*
-		 * We failed to advance. If we are not a fragment, that's
-		 * a format error and we leave. Otherwise, restore npc_hlen
-		 * and npc_proto to their previous (and correct) values.
-		 */
-		if (ip6e == NULL) {
-			if (!is_frag)
-				return NPC_FMTERR;
-			npc->npc_proto = onxt;
-			npc->npc_hlen -= hlen;
-		}
-
-		/*
 		 * Re-fetch the header pointers (nbufs might have been
 		 * reallocated).  Restore the original offset (if any).
 		 */

Reply via email to