From owner-freebsd-ipfw@FreeBSD.ORG Tue Mar 16 05:30:47 2004 Return-Path: Delivered-To: freebsd-ipfw@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EA6FB16A4D2 for ; Tue, 16 Mar 2004 05:30:47 -0800 (PST) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id E0C2043D2D for ; Tue, 16 Mar 2004 05:30:47 -0800 (PST) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.10/8.12.10) with ESMTP id i2GDUlbv001796 for ; Tue, 16 Mar 2004 05:30:47 -0800 (PST) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.10/8.12.10/Submit) id i2GDUlo6001792; Tue, 16 Mar 2004 05:30:47 -0800 (PST) (envelope-from gnats) Date: Tue, 16 Mar 2004 05:30:47 -0800 (PST) Message-Id: <200403161330.i2GDUlo6001792@freefall.freebsd.org> To: ipfw@FreeBSD.org From: Ian FREISLICH Subject: Re: kern/64240: IPFW tee terminates rule processing X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Ian FREISLICH List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Mar 2004 13:30:48 -0000 The following reply was made to PR kern/64240; it has been noted by GNATS. From: Ian FREISLICH To: freebsd-gnats-submit@FreeBSD.org Cc: Subject: Re: kern/64240: IPFW tee terminates rule processing Date: Tue, 16 Mar 2004 15:29:55 +0200 The patch in this PR is not quite right. The packet is cloned in the wrong place for reinsertion into the firewall which results in packets being silently dropped. Here is a corrected patch. -- Ian Freislich Index: sbin/ipfw/ipfw.8 =================================================================== RCS file: /home/ncvs/src/sbin/ipfw/ipfw.8,v retrieving revision 1.139 diff -u -d -r1.139 ipfw.8 --- sbin/ipfw/ipfw.8 23 Jan 2004 06:37:19 -0000 1.139 +++ sbin/ipfw/ipfw.8 10 Mar 2004 09:03:06 -0000 @@ -658,10 +658,7 @@ .Xr divert 4 socket bound to port .Ar port . -The search terminates and the original packet is accepted -(but see Section -.Sx BUGS -below). +The search continues at the next rule. .It Cm unreach Ar code Discard packets that match this rule, and try to send an ICMP unreachable notice with code @@ -2169,12 +2166,6 @@ are reassembled before delivery to the socket. The action used on those packet is the one from the rule which matches the first fragment of the packet. -.Pp -Packets that match a -.Cm tee -rule should not be immediately accepted, but should continue -going through the rule list. -This may be fixed in a later version. .Pp Packets diverted to userland, and then reinserted by a userland process may lose various packet attributes. Index: sys/netinet/ip_fastfwd.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_fastfwd.c,v retrieving revision 1.8 diff -u -d -r1.8 ip_fastfwd.c --- sys/netinet/ip_fastfwd.c 25 Feb 2004 19:55:28 -0000 1.8 +++ sys/netinet/ip_fastfwd.c 16 Mar 2004 13:23:21 -0000 @@ -337,11 +337,20 @@ bzero(&args, sizeof(args)); args.m = m; +#ifdef IPDIVERT +tee_again: +#endif ipfw = ip_fw_chk_ptr(&args); m = args.m; M_ASSERTVALID(m); M_ASSERTPKTHDR(m); +#ifdef IPDIVERT + if ((ipfw & IP_FW_PORT_TEE_FLAG) != 0) + clone = divert_clone(m); + else + clone = m; +#endif /* * Packet denied, drop it @@ -368,10 +377,6 @@ /* * Tee packet */ - if ((ipfw & IP_FW_PORT_TEE_FLAG) != 0) - clone = divert_clone(m); - else - clone = m; if (clone == NULL) goto passin; @@ -395,11 +400,12 @@ /* * If this was not tee, we are done */ - m = clone; if ((ipfw & IP_FW_PORT_TEE_FLAG) == 0) return 1; /* Continue if it was tee */ - goto passin; + args.m = clone; + args.tee_continue = 1; + goto tee_again; } #endif if (ipfw == 0 && args.next_hop != NULL) { @@ -512,11 +518,20 @@ args.m = m; args.oif = ifp; +#ifdef IPDIVERT +tee_again2: +#endif ipfw = ip_fw_chk_ptr(&args); m = args.m; M_ASSERTVALID(m); M_ASSERTPKTHDR(m); +#ifdef IPDIVERT + if ((ipfw & IP_FW_PORT_TEE_FLAG) != 0) + clone = divert_clone(m); + else + clone = m; +#endif if ((ipfw & IP_FW_PORT_DENY_FLAG) || m == NULL) { RTFREE(ro.ro_rt); @@ -544,10 +559,6 @@ /* * Tee packet */ - if ((ipfw & IP_FW_PORT_TEE_FLAG) != 0) - clone = divert_clone(m); - else - clone = m; if (clone == NULL) goto passout; @@ -571,13 +582,14 @@ /* * If this was not tee, we are done */ - m = clone; if ((ipfw & IP_FW_PORT_TEE_FLAG) == 0) { RTFREE(ro.ro_rt); return 1; } /* Continue if it was tee */ - goto passout; + args.m =clone; + args.tee_continue = 1; + goto tee_again2; } #endif if (ipfw == 0 && args.next_hop != NULL) { Index: sys/netinet/ip_fw.h =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_fw.h,v retrieving revision 1.83 diff -u -d -r1.83 ip_fw.h --- sys/netinet/ip_fw.h 25 Feb 2004 19:55:28 -0000 1.83 +++ sys/netinet/ip_fw.h 15 Mar 2004 11:33:07 -0000 @@ -400,6 +400,7 @@ int flags; /* for dummynet */ struct ipfw_flow_id f_id; /* grabbed from IP header */ + int tee_continue; /* continue after packet tee */ u_int32_t retval; }; Index: sys/netinet/ip_fw2.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_fw2.c,v retrieving revision 1.56 diff -u -d -r1.56 ip_fw2.c --- sys/netinet/ip_fw2.c 25 Feb 2004 19:55:28 -0000 1.56 +++ sys/netinet/ip_fw2.c 16 Mar 2004 12:47:24 -0000 @@ -1361,10 +1361,6 @@ * args->eh (in) Mac header if present, or NULL for layer3 packet. * args->oif Outgoing interface, or NULL if packet is incoming. * The incoming interface is in the mbuf. (in) - * args->divert_rule (in/out) - * Skip up to the first rule past this rule number; - * upon return, non-zero port number for divert or tee. - * * args->rule Pointer to the last matching rule (in/out) * args->next_hop Socket we are forwarding to (out). * args->f_id Addresses grabbed from the packet (out) @@ -1554,13 +1550,18 @@ * to restart processing. * * If fw_one_pass != 0 then just accept it. - * XXX should not happen here, but optimized out in - * the caller. + * XXX should not happen here unless the packet was tee'd, + * but optimized out in the caller. */ - if (fw_one_pass) { + if (fw_one_pass && !args->tee_continue) { IPFW_UNLOCK(chain); /* XXX optimize */ return 0; } + /* + * Reset this so that fw_one_pass is obeyed if we + * land up here again for reasons other than tee_continue + */ + args->tee_continue = 0; f = args->rule->next_rule; if (f == NULL) @@ -2044,6 +2045,7 @@ cmd->arg1 | IP_FW_PORT_TEE_FLAG; m_tag_prepend(m, mtag); retval = dt->info; + args->rule = f; /* report matching rule */ goto done; } Index: sys/netinet/ip_input.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v retrieving revision 1.266 diff -u -d -r1.266 ip_input.c --- sys/netinet/ip_input.c 1 Mar 2004 22:37:01 -0000 1.266 +++ sys/netinet/ip_input.c 16 Mar 2004 10:48:36 -0000 @@ -304,6 +304,7 @@ struct in_addr pkt_dst; #ifdef IPDIVERT u_int32_t divert_info; /* packet divert/tee info */ + struct mbuf *clone = NULL; /* saved mbuf for tee */ #endif struct ip_fw_args args; int dchg = 0; /* dest changed after fw */ @@ -474,8 +475,17 @@ goto ours; args.m = m; +#ifdef IPDIVERT +tee_again: +#endif i = ip_fw_chk_ptr(&args); m = args.m; +#ifdef IPDIVERT + if ((i & IP_FW_PORT_TEE_FLAG) != 0) + clone = divert_clone(m); + else + clone = NULL; +#endif if ( (i & IP_FW_PORT_DENY_FLAG) || m == NULL) { /* drop */ if (m) @@ -837,14 +847,6 @@ */ divert_info = divert_find_info(m); if (divert_info != 0) { - struct mbuf *clone; - - /* Clone packet if we're doing a 'tee' */ - if ((divert_info & IP_FW_PORT_TEE_FLAG) != 0) - clone = divert_clone(m); - else - clone = NULL; - /* Restore packet header fields to original values */ ip->ip_len += hlen; ip->ip_len = htons(ip->ip_len); @@ -854,12 +856,10 @@ divert_packet(m, 1); ipstat.ips_delivered++; - /* If 'tee', continue with original packet */ + /* If 'tee', continue processing firewall rules + * with the original packet */ if (clone == NULL) return; - m = clone; - ip = mtod(m, struct ip *); - ip->ip_len += hlen; /* * Jump backwards to complete processing of the * packet. We do not need to clear args.next_hop @@ -867,7 +867,9 @@ * doesn't contain a divert packet tag so we won't * re-entry this block. */ - goto pass; + args.m = clone; + args.tee_continue = 1; + goto tee_again; } #endif Index: sys/netinet/ip_output.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_output.c,v retrieving revision 1.211 diff -u -d -r1.211 ip_output.c --- sys/netinet/ip_output.c 2 Mar 2004 14:37:23 -0000 1.211 +++ sys/netinet/ip_output.c 16 Mar 2004 13:25:22 -0000 @@ -730,14 +730,27 @@ */ if (fw_enable && IPFW_LOADED && !args.next_hop) { struct sockaddr_in *old = dst; +#ifdef IPDIVERT + struct mbuf *clone = NULL; +#endif args.m = m; args.next_hop = dst; args.oif = ifp; +#ifdef IPDIVERT +tee_again: +#endif off = ip_fw_chk_ptr(&args); m = args.m; dst = args.next_hop; +#ifdef IPDIVERT + /* Clone packet if we're doing a 'tee' */ + if ((off & IP_FW_PORT_TEE_FLAG) != 0) + clone = divert_clone(m); + else + clone = NULL; +#endif /* * On return we must do the following: * m == NULL -> drop the pkt (old interface, deprecated) @@ -782,14 +795,6 @@ } #ifdef IPDIVERT if (off != 0 && (off & IP_FW_PORT_DYNT_FLAG) == 0) { - struct mbuf *clone; - - /* Clone packet if we're doing a 'tee' */ - if ((off & IP_FW_PORT_TEE_FLAG) != 0) - clone = divert_clone(m); - else - clone = NULL; - /* * XXX * delayed checksums are not currently compatible @@ -807,11 +812,14 @@ /* Deliver packet to divert input routine */ divert_packet(m, 0); - /* If 'tee', continue with original packet */ + /* + * If 'tee', continue processing firewall + * rules with the original packet + */ if (clone != NULL) { - m = clone; - ip = mtod(m, struct ip *); - goto pass; + args.m = clone; + args.tee_continue = 1; + goto tee_again; } goto done; }