kern/95084: [patch] IPFW2 ignores "recv/xmit/via any" (IPFW1->2 regression)

Dmitry Pryanishnikov dmitry at atlantis.dp.ua
Wed Mar 29 21:20:34 UTC 2006


>Number:         95084
>Category:       kern
>Synopsis:       [patch] IPFW2 ignores "recv/xmit/via any" (IPFW1->2 regression)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Mar 29 21:20:19 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Dmitry Pryanishnikov
>Release:        FreeBSD 7.0-CURRENT i386
>Organization:
Atlantis ISP
>Environment:
System: FreeBSD homelynx.homenet 7.0-CURRENT FreeBSD 7.0-CURRENT #0: Sun Mar 19 00:33:05 EET 2006 root at homelynx.homenet:/usr/CURRENT/obj/usr/CURRENT/src/sys/lynx i386

>Description:

 IPFW2 (unlike IPFW1) ignores documented construction "recv/xmit/via any".

See ipfw(8):

     recv | xmit | via {ifX | if* | ipno | any}
	Matches packets received, transmitted or going through, respec-
	tively, the interface specified by exact name (ifX), by device
	name (if*), by IP address, or through some interface.
...................................^^^^^^^^^^^^^^^^^^^^^^^^^
        A packet may not have a receive or transmit interface: packets
	originating from the local host have no receive interface, while
	packets destined for the local host have no transmit interface.

So it's OK (and convenient) to use "recv any" as a marker of transit
(in contrast to locally-originated) packets. However, ipfw2 completely
(and silently) ignores "recv/xmit/via any", which can break e.g.
traffic accounting: rule "count all from any to any out recv any" becomes
"count all from any to any out" and thus starts to count all outgoing
traffic instead of just transit one. This can be dangerous because
the change is silent, and can only be found by the analysis of resulting
ruleset.

 Note that "xmit any" seems to be useless (though harmless) because
packets destined for the local host don't hit rules marked "out",
and "xmit" interface is undefined for "in" rules. I don't see the point
in special handling for this case. This fact just probably should be documented.

>How-To-Repeat:

 Do it on transit router which also originates some traffic:

root at homelynx# cat a.sh
ipfw -f flush
ipfw add 10 count all from any to any out
ipfw add 20 count all from any to any out recv any
ipfw add 60000 pass all from any to any
root at homelynx# sh a.sh
...
root at homelynx# ipfw show
00010 216 25555 count ip from any to any out
00020 216 25555 count ip from any to any out
60000 425 52158 allow ip from any to any
65535   0     0 deny ip from any to any

>Fix:

 The following patch fixes the problem in CURRENT:

--- sys/netinet/ip_fw2.c.orig	Tue Mar 14 10:05:15 2006
+++ sys/netinet/ip_fw2.c	Tue Mar 28 21:13:03 2006
@@ -453,7 +453,8 @@
 	if (cmd->name[0] != '\0') { /* match by name */
 		/* Check name */
 		if (cmd->p.glob) {
-			if (fnmatch(cmd->name, ifp->if_xname, 0) == 0)
+			if ((cmd->name[0] == '*' && cmd->name[1] == '\0') ||
+			    fnmatch(cmd->name, ifp->if_xname, 0) == 0)
 				return(1);
 		} else {
 			if (strncmp(ifp->if_xname, cmd->name, IFNAMSIZ) == 0)
--- sbin/ipfw/ipfw2.c.orig	Tue Mar 14 10:02:52 2006
+++ sbin/ipfw/ipfw2.c	Wed Mar 29 00:22:43 2006
@@ -1741,6 +1741,8 @@
 				if (cmdif->name[0] == '\0')
 					printf(" %s %s", s,
 					    inet_ntoa(cmdif->p.ip));
+				else if (strcmp(cmdif->name, "*") == 0)
+					printf(" %s any", s);
 				else
 					printf(" %s %s", s, cmdif->name);
 
@@ -3094,11 +3096,11 @@
 	cmd->o.len |= F_INSN_SIZE(ipfw_insn_if);
 
 	/* Parse the interface or address */
-	if (strcmp(arg, "any") == 0)
-		cmd->o.len = 0;		/* effectively ignore this command */
-	else if (!isdigit(*arg)) {
+	if (!isdigit(*arg)) {
 		strlcpy(cmd->name, arg, sizeof(cmd->name));
-		cmd->p.glob = strpbrk(arg, "*?[") != NULL ? 1 : 0;
+		if (strcmp(arg, "any") == 0)
+		    strcpy(cmd->name, "*");
+		cmd->p.glob = strpbrk(cmd->name, "*?[") != NULL ? 1 : 0;
 	} else if (!inet_aton(arg, &cmd->p.ip))
 		errx(EX_DATAERR, "bad ip address ``%s''", arg);
 }


Patch tries to maintain maximum compatibility (new /sbin/ipfw is even
functional with old kernel and vice versa). It just converts "any" to
logically equivalent "*" (code in ipfw) and optimizes this partucular
construction (code in kernel) to improve scalability (in traffic
accounting session "recv any" tends to repeat many times). With this
patch applied my example works correctly:

root at homelynx# ipfw show
00010  821 333189 count ip from any to any out
00020  751 328766 count ip from any to any out recv any
60000 1638 668605 allow ip from any to any
65535    0      0 deny ip from any to any
>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list