Bug 64240
| Summary: | IPFW tee terminates rule processing | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Ian Freislich <ianf> | ||||
| Component: | kern | Assignee: | Andre Oppermann <andre> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | ||||||
| Priority: | Normal | ||||||
| Version: | 5.2-CURRENT | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
Responsible Changed From-To: freebsd-bugs->ipfw Assign to ipfw mailing list 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;
}
Responsible Changed From-To: ipfw->andre Take over. State Changed From-To: open->analyzed Ian, could you please try the following patch (against -current or 5.3-BETA1), (maybe whitespace mangled): Index: ip_fw_pfil.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_fw_pfil.c,v retrieving revision 1.7 diff -u -p -r1.7 ip_fw_pfil.c --- ip_fw_pfil.c 27 Aug 2004 15:16:22 -0000 1.7 +++ ip_fw_pfil.c 27 Aug 2004 18:52:08 -0000 @@ -100,6 +100,7 @@ ipfw_check_in(void *arg, struct mbuf **m m_tag_delete(*m0, dn_tag); } +again: args.m = *m0; ipfw = ipfw_chk(&args); *m0 = args.m; @@ -127,7 +128,7 @@ ipfw_check_in(void *arg, struct mbuf **m *m0 = NULL; return 0; /* packet consumed */ } else - goto pass; /* continue with packet */ + goto again; /* continue with packet */ } #ifdef IPFIREWALL_FORWARD @@ -182,6 +183,7 @@ ipfw_check_out(void *arg, struct mbuf ** m_tag_delete(*m0, dn_tag); } +again: args.m = *m0; args.oif = ifp; ipfw = ipfw_chk(&args); @@ -209,7 +211,7 @@ ipfw_check_out(void *arg, struct mbuf ** *m0 = NULL; return 0; /* packet consumed */ } else - goto pass; /* continue with packet */ + goto again; /* continue with packet */ } #ifdef IPFIREWALL_FORWARD Thanks -- Andre Andre Oppermann wrote:
> Ian Freislich wrote:
> >
> > Andre Oppermann wrote:
> > > Synopsis: IPFW tee terminates rule processing
> > >
> > > State-Changed-From-To: open->analyzed
> > > State-Changed-By: andre
> > > State-Changed-When: Fri Aug 27 18:52:25 GMT 2004
> > > State-Changed-Why:
> > > Ian,
> > >
> > > could you please try the following patch (against -current or 5.3-BETA1),
> > > (maybe whitespace mangled):
> >
> > Applied by hand because of white-space problems. My computer
> > -CURRENT locked up when I added the tee rule with this patch.
>
> It got into an infinite loop. IPFW2 doesn't update the args.rules
> field when 'tee' is hit. Try the attached patch to netinet/ip_fw2.c
> in addition to the one you already have.
Ok, I've tried it. The tee works, but the packet is still allowed:
[brane-dead] ~ # ipfw l |head -4
00001 tee 300 ip from any to any
00050 divert 5000 ip from 196.7.162.24/29 to 192.168.0.0/24
00300 reject tcp from me to 196.33.166.202 dst-port 80
00300 reject tcp from me to 62.26.220.2 dst-port 80
brane-dead] ~ # telnet 196.33.166.202 80
Trying 196.33.166.202...
Connected to a196-33-166-202.deploy.akamaitechnologies.com.
Escape character is '^]'.
At least it doesn't panic. Have a look at what I did closely. It
was a bit of a hack and a cludge, but what is killing you is the
one_pass flag. Your patch works fine under the following condition:
[brane-dead] ~ # sysctl net.inet.ip.fw.one_pass=0
net.inet.ip.fw.one_pass: 1 -> 0
[brane-dead] ~ # telnet 196.33.166.202 80
Trying 196.33.166.202...
telnet: connect to address 196.33.166.202: Permission denied
telnet: Unable to connect to remote host
[brane-dead] ~ # telnet /tmp/ipacct/ipacct.300
Trying /tmp/ipacct/ipacct.300...
No connection.
Escape character is '^]'.
196.7.162.29 3 196.7.162.29 1 icmp 112 2
196.7.162.29 58846 196.7.162.29 53 udp 146 2
196.7.162.29 53 196.7.162.29 58846 udp 512 2
196.7.162.29 65386 196.33.166.202 80 tcp 64 1 <---
196.7.162.29 22 196.7.18.226 4187 tcp 2232 18
196.7.18.226 4187 196.7.162.29 22 tcp 1872 24
---> only on packet - presumeably the SYN.
You need to get the information back into ipfw that even if one_pass
is true, we need to give this packet another pass because it
re-entered because of a tee rule, and it needs to enter after the
tee rule (not sure if you're doing that explicitly or if it just
happens). Line 1834 of /usr/src/sys/netinet/ip_fw2.c is where this
happens.
Ian
--
Ian Freislich
State Changed From-To: analyzed->patched A fix for 'ipfw tee' to behave correctly is in sys/netinet/ip_fw_pfil.c rev. 1.8. State Changed From-To: patched->closed MFC to RELENG_5 is done. Originator reported everything work as expected now. Case closed. |
Noted in the BUGS section of the ipfw manual page: Packets that match a tee rule should not be immediately accepted, but should continue going through the rule list. This may be fixed in a later version. The following set of rules will cause IP packets to be accepted unconditionally: 00001 tee 5000 ip from any to any 00002 deny ip from any to any Fix: Apply the following patch (5-CURRENT > 20040314) This patch removes a stale comment in ip_fw2.c - args->divert_rule does not exist in the structure. It adds tee_continue to the ip_fw_args structure to defeat the one_pass check when continuing with the checks after teeing the packet. How-To-Repeat: Add a tee rule early in the ipfw ruleset and all packets matching that rule will be copied to the divert port and accepted by the firewall.