Date: Tue, 16 Mar 2004 05:30:47 -0800 (PST) From: Ian FREISLICH <if@hetzner.co.za> To: ipfw@FreeBSD.org Subject: Re: kern/64240: IPFW tee terminates rule processing Message-ID: <200403161330.i2GDUlo6001792@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/64240; it has been noted by GNATS.
From: Ian FREISLICH <if@hetzner.co.za>
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;
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200403161330.i2GDUlo6001792>