Tue Mar 13 09:04:03 2018 UTC ()
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.


(maxv)
diff -r1.38 -r1.39 src/sys/net/npf/npf_handler.c
diff -r1.39 -r1.40 src/sys/net/npf/npf_inet.c
cvs diff -r1.38 -r1.39 src/sys/net/npf/npf_handler.c (expand / switch to unified diff)

--- src/sys/net/npf/npf_handler.c 2018/03/08 07:06:13 1.38
+++ src/sys/net/npf/npf_handler.c 2018/03/13 09:04:02 1.39
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: npf_handler.c,v 1.38 2018/03/08 07:06:13 maxv Exp $ */ 1/* $NetBSD: npf_handler.c,v 1.39 2018/03/13 09:04:02 maxv Exp $ */
2 2
3/*- 3/*-
4 * Copyright (c) 2009-2013 The NetBSD Foundation, Inc. 4 * Copyright (c) 2009-2013 The NetBSD Foundation, Inc.
5 * All rights reserved. 5 * All rights reserved.
6 * 6 *
7 * This material is based upon work partially supported by The 7 * This material is based upon work partially supported by The
8 * NetBSD Foundation under a contract with Mindaugas Rasiukevicius. 8 * NetBSD Foundation under a contract with Mindaugas Rasiukevicius.
9 * 9 *
10 * Redistribution and use in source and binary forms, with or without 10 * Redistribution and use in source and binary forms, with or without
11 * modification, are permitted provided that the following conditions 11 * modification, are permitted provided that the following conditions
12 * are met: 12 * are met:
13 * 1. Redistributions of source code must retain the above copyright 13 * 1. Redistributions of source code must retain the above copyright
14 * notice, this list of conditions and the following disclaimer. 14 * notice, this list of conditions and the following disclaimer.
@@ -27,27 +27,27 @@ @@ -27,27 +27,27 @@
27 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) 27 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
28 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE 28 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
29 * POSSIBILITY OF SUCH DAMAGE. 29 * POSSIBILITY OF SUCH DAMAGE.
30 */ 30 */
31 31
32/* 32/*
33 * NPF packet handler. 33 * NPF packet handler.
34 * 34 *
35 * Note: pfil(9) hooks are currently locked by softnet_lock and kernel-lock. 35 * Note: pfil(9) hooks are currently locked by softnet_lock and kernel-lock.
36 */ 36 */
37 37
38#ifdef _KERNEL 38#ifdef _KERNEL
39#include <sys/cdefs.h> 39#include <sys/cdefs.h>
40__KERNEL_RCSID(0, "$NetBSD: npf_handler.c,v 1.38 2018/03/08 07:06:13 maxv Exp $"); 40__KERNEL_RCSID(0, "$NetBSD: npf_handler.c,v 1.39 2018/03/13 09:04:02 maxv Exp $");
41 41
42#include <sys/types.h> 42#include <sys/types.h>
43#include <sys/param.h> 43#include <sys/param.h>
44 44
45#include <sys/mbuf.h> 45#include <sys/mbuf.h>
46#include <sys/mutex.h> 46#include <sys/mutex.h>
47#include <net/if.h> 47#include <net/if.h>
48#include <net/pfil.h> 48#include <net/pfil.h>
49#include <sys/socketvar.h> 49#include <sys/socketvar.h>
50 50
51#include <netinet/in_systm.h> 51#include <netinet/in_systm.h>
52#include <netinet/in.h> 52#include <netinet/in.h>
53#include <netinet/ip_var.h> 53#include <netinet/ip_var.h>
@@ -156,34 +156,27 @@ npf_packet_handler(npf_t *npf, struct mb @@ -156,34 +156,27 @@ npf_packet_handler(npf_t *npf, struct mb
156 156
157 /* Cache everything. */ 157 /* Cache everything. */
158 flags = npf_cache_all(&npc); 158 flags = npf_cache_all(&npc);
159 159
160 /* If error on the format, leave quickly. */ 160 /* If error on the format, leave quickly. */
161 if (flags & NPC_FMTERR) { 161 if (flags & NPC_FMTERR) {
162 error = EINVAL; 162 error = EINVAL;
163 goto fastout; 163 goto fastout;
164 } 164 }
165 165
166 /* Determine whether it is an IP fragment. */ 166 /* Determine whether it is an IP fragment. */
167 if (__predict_false(flags & NPC_IPFRAG)) { 167 if (__predict_false(flags & NPC_IPFRAG)) {
168 /* 168 /*
169 * We pass IPv6 fragments unconditionally 169 * Pass to IPv4/IPv6 reassembly mechanism.
170 * The first IPv6 fragment is not marked as such 
171 * and passes through the filter 
172 */ 
173 if (flags & NPC_IP6) 
174 return 0; 
175 /* 
176 * Pass to IPv4 reassembly mechanism. 
177 */ 170 */
178 error = npf_reassembly(npf, &npc, mp); 171 error = npf_reassembly(npf, &npc, mp);
179 if (error) { 172 if (error) {
180 con = NULL; 173 con = NULL;
181 goto out; 174 goto out;
182 } 175 }
183 if (*mp == NULL) { 176 if (*mp == NULL) {
184 /* More fragments should come; return. */ 177 /* More fragments should come; return. */
185 return 0; 178 return 0;
186 } 179 }
187 } 180 }
188 181
189 /* Just pass-through if specially tagged. */ 182 /* Just pass-through if specially tagged. */
cvs diff -r1.39 -r1.40 src/sys/net/npf/npf_inet.c (expand / switch to unified diff)

--- src/sys/net/npf/npf_inet.c 2018/03/08 07:54:14 1.39
+++ src/sys/net/npf/npf_inet.c 2018/03/13 09:04:02 1.40
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: npf_inet.c,v 1.39 2018/03/08 07:54:14 maxv Exp $ */ 1/* $NetBSD: npf_inet.c,v 1.40 2018/03/13 09:04:02 maxv Exp $ */
2 2
3/*- 3/*-
4 * Copyright (c) 2009-2014 The NetBSD Foundation, Inc. 4 * Copyright (c) 2009-2014 The NetBSD Foundation, Inc.
5 * All rights reserved. 5 * All rights reserved.
6 * 6 *
7 * This material is based upon work partially supported by The 7 * This material is based upon work partially supported by The
8 * NetBSD Foundation under a contract with Mindaugas Rasiukevicius. 8 * NetBSD Foundation under a contract with Mindaugas Rasiukevicius.
9 * 9 *
10 * Redistribution and use in source and binary forms, with or without 10 * Redistribution and use in source and binary forms, with or without
11 * modification, are permitted provided that the following conditions 11 * modification, are permitted provided that the following conditions
12 * are met: 12 * are met:
13 * 1. Redistributions of source code must retain the above copyright 13 * 1. Redistributions of source code must retain the above copyright
14 * notice, this list of conditions and the following disclaimer. 14 * notice, this list of conditions and the following disclaimer.
@@ -30,27 +30,27 @@ @@ -30,27 +30,27 @@
30 */ 30 */
31 31
32/* 32/*
33 * Various protocol related helper routines. 33 * Various protocol related helper routines.
34 * 34 *
35 * This layer manipulates npf_cache_t structure i.e. caches requested headers 35 * This layer manipulates npf_cache_t structure i.e. caches requested headers
36 * and stores which information was cached in the information bit field. 36 * and stores which information was cached in the information bit field.
37 * It is also responsibility of this layer to update or invalidate the cache 37 * It is also responsibility of this layer to update or invalidate the cache
38 * on rewrites (e.g. by translation routines). 38 * on rewrites (e.g. by translation routines).
39 */ 39 */
40 40
41#ifdef _KERNEL 41#ifdef _KERNEL
42#include <sys/cdefs.h> 42#include <sys/cdefs.h>
43__KERNEL_RCSID(0, "$NetBSD: npf_inet.c,v 1.39 2018/03/08 07:54:14 maxv Exp $"); 43__KERNEL_RCSID(0, "$NetBSD: npf_inet.c,v 1.40 2018/03/13 09:04:02 maxv Exp $");
44 44
45#include <sys/param.h> 45#include <sys/param.h>
46#include <sys/types.h> 46#include <sys/types.h>
47 47
48#include <net/pfil.h> 48#include <net/pfil.h>
49#include <net/if.h> 49#include <net/if.h>
50#include <net/ethertypes.h> 50#include <net/ethertypes.h>
51#include <net/if_ether.h> 51#include <net/if_ether.h>
52 52
53#include <netinet/in_systm.h> 53#include <netinet/in_systm.h>
54#include <netinet/in.h> 54#include <netinet/in.h>
55#include <netinet6/in6_var.h> 55#include <netinet6/in6_var.h>
56#include <netinet/ip.h> 56#include <netinet/ip.h>
@@ -348,116 +348,80 @@ npf_cache_ip(npf_cache_t *npc, nbuf_t *n @@ -348,116 +348,80 @@ npf_cache_ip(npf_cache_t *npc, nbuf_t *n
348 npc->npc_proto = ip->ip_p; 348 npc->npc_proto = ip->ip_p;
349 349
350 npc->npc_ip.v4 = ip; 350 npc->npc_ip.v4 = ip;
351 flags |= NPC_IP4; 351 flags |= NPC_IP4;
352 break; 352 break;
353 } 353 }
354 354
355 case (IPV6_VERSION >> 4): { 355 case (IPV6_VERSION >> 4): {
356 struct ip6_hdr *ip6; 356 struct ip6_hdr *ip6;
357 struct ip6_ext *ip6e; 357 struct ip6_ext *ip6e;
358 struct ip6_frag *ip6f; 358 struct ip6_frag *ip6f;
359 size_t off, hlen; 359 size_t off, hlen;
360 int frag_present; 360 int frag_present;
361 bool is_frag; 
362 uint8_t onxt; 
363 int fragoff; 
364 361
365 ip6 = nbuf_ensure_contig(nbuf, sizeof(struct ip6_hdr)); 362 ip6 = nbuf_ensure_contig(nbuf, sizeof(struct ip6_hdr));
366 if (ip6 == NULL) { 363 if (ip6 == NULL) {
367 return NPC_FMTERR; 364 return NPC_FMTERR;
368 } 365 }
369 366
370 /* Set initial next-protocol value. */ 367 /* Set initial next-protocol value. */
371 hlen = sizeof(struct ip6_hdr); 368 hlen = sizeof(struct ip6_hdr);
372 npc->npc_proto = ip6->ip6_nxt; 369 npc->npc_proto = ip6->ip6_nxt;
373 npc->npc_hlen = hlen; 370 npc->npc_hlen = hlen;
374 371
375 frag_present = 0; 372 frag_present = 0;
376 is_frag = false; 
377 373
378 /* 374 /*
379 * Advance by the length of the current header. 375 * Advance by the length of the current header.
380 */ 376 */
381 off = nbuf_offset(nbuf); 377 off = nbuf_offset(nbuf);
382 while ((ip6e = nbuf_advance(nbuf, hlen, sizeof(*ip6e))) != NULL) { 378 while ((ip6e = nbuf_advance(nbuf, hlen, sizeof(*ip6e))) != NULL) {
383 /* 379 /*
384 * Determine whether we are going to continue. 380 * Determine whether we are going to continue.
385 */ 381 */
386 switch (npc->npc_proto) { 382 switch (npc->npc_proto) {
387 case IPPROTO_HOPOPTS: 383 case IPPROTO_HOPOPTS:
388 case IPPROTO_DSTOPTS: 384 case IPPROTO_DSTOPTS:
389 case IPPROTO_ROUTING: 385 case IPPROTO_ROUTING:
390 hlen = (ip6e->ip6e_len + 1) << 3; 386 hlen = (ip6e->ip6e_len + 1) << 3;
391 break; 387 break;
392 case IPPROTO_FRAGMENT: 388 case IPPROTO_FRAGMENT:
393 if (frag_present++) 389 if (frag_present++)
394 return NPC_FMTERR; 390 return NPC_FMTERR;
395 ip6f = nbuf_ensure_contig(nbuf, sizeof(*ip6f)); 391 ip6f = nbuf_ensure_contig(nbuf, sizeof(*ip6f));
396 if (ip6f == NULL) 392 if (ip6f == NULL)
397 return NPC_FMTERR; 393 return NPC_FMTERR;
398 394
399 hlen = sizeof(struct ip6_frag); 395 hlen = 0;
400 396 flags |= NPC_IPFRAG;
401 /* RFC6946: Skip dummy fragments. */ 
402 fragoff = ntohs(ip6f->ip6f_offlg & IP6F_OFF_MASK); 
403 if (fragoff == 0 && 
404 !(ip6f->ip6f_offlg & IP6F_MORE_FRAG)) { 
405 break; 
406 } 
407 
408 is_frag = true; 
409 
410 /* 
411 * We treat the first fragment as a regular 
412 * packet and then we pass the rest of the 
413 * fragments unconditionally. This way if 
414 * the first packet passes the rest will 
415 * be able to reassembled, if not they will 
416 * be ignored. We can do better later. 
417 */ 
418 if (fragoff != 0) 
419 flags |= NPC_IPFRAG; 
420 397
421 break; 398 break;
422 case IPPROTO_AH: 399 case IPPROTO_AH:
423 hlen = (ip6e->ip6e_len + 2) << 2; 400 hlen = (ip6e->ip6e_len + 2) << 2;
424 break; 401 break;
425 default: 402 default:
426 hlen = 0; 403 hlen = 0;
427 break; 404 break;
428 } 405 }
429 406
430 if (!hlen) { 407 if (!hlen) {
431 break; 408 break;
432 } 409 }
433 onxt = npc->npc_proto; 
434 npc->npc_proto = ip6e->ip6e_nxt; 410 npc->npc_proto = ip6e->ip6e_nxt;
435 npc->npc_hlen += hlen; 411 npc->npc_hlen += hlen;
436 } 412 }
437 413
438 /* 414 /*
439 * We failed to advance. If we are not a fragment, that's 
440 * a format error and we leave. Otherwise, restore npc_hlen 
441 * and npc_proto to their previous (and correct) values. 
442 */ 
443 if (ip6e == NULL) { 
444 if (!is_frag) 
445 return NPC_FMTERR; 
446 npc->npc_proto = onxt; 
447 npc->npc_hlen -= hlen; 
448 } 
449 
450 /* 
451 * Re-fetch the header pointers (nbufs might have been 415 * Re-fetch the header pointers (nbufs might have been
452 * reallocated). Restore the original offset (if any). 416 * reallocated). Restore the original offset (if any).
453 */ 417 */
454 nbuf_reset(nbuf); 418 nbuf_reset(nbuf);
455 ip6 = nbuf_dataptr(nbuf); 419 ip6 = nbuf_dataptr(nbuf);
456 if (off) { 420 if (off) {
457 nbuf_advance(nbuf, off, 0); 421 nbuf_advance(nbuf, off, 0);
458 } 422 }
459 423
460 /* Cache: layer 3 - IPv6. */ 424 /* Cache: layer 3 - IPv6. */
461 npc->npc_alen = sizeof(struct in6_addr); 425 npc->npc_alen = sizeof(struct in6_addr);
462 npc->npc_ips[NPF_SRC] = (npf_addr_t *)&ip6->ip6_src; 426 npc->npc_ips[NPF_SRC] = (npf_addr_t *)&ip6->ip6_src;
463 npc->npc_ips[NPF_DST]= (npf_addr_t *)&ip6->ip6_dst; 427 npc->npc_ips[NPF_DST]= (npf_addr_t *)&ip6->ip6_dst;