Bug 95084 - [ipfw] [regression] [patch] IPFW2 ignores "recv/xmit/via any" (IPFW1->2 regression)
Summary: [ipfw] [regression] [patch] IPFW2 ignores "recv/xmit/via any" (IPFW1->2 regre...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 7.0-CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2006-03-29 22:20 UTC by Dmitry Pryanishnikov
Modified: 2022-10-17 12:36 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Pryanishnikov 2006-03-29 22:20:19 UTC
 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.

Fix: The following patch fixes the problem in CURRENT:



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@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--HMjAvl69Sp19cJIDYc3kU2w6s20I02mZlXL7RanjDI9XpuXc
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

--- 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);
 }
How-To-Repeat: 
 Do it on transit router which also originates some traffic:

root@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@homelynx# sh a.sh
...
root@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
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2006-03-31 18:52:29 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-ipfw

Over to maintainer(s).
Comment 2 araujobsdport 2008-02-26 11:31:50 UTC
Hi dear,

For this situation you can't use:
island# ipfw add 10 count all from any to any recv out
island# ipfw show 10
00010      0        0 count ip from any to any recv out

Best Regards,

-- 
Marcelo Araujo            (__)
araujo@FreeBSD.org     \\\'',)
http://www.FreeBSD.org   \/  \ ^
Power To Server.         .\. /_)

Comment 3 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:01:09 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 4 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 12:36:55 UTC
Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>