ipfw "bug" - recv any = not recv any

[Jeff Kletsky]

I hesitate to call this a "bug" as I don't know all the history behind 
the ipfw2 decisions, so let me toss this out there and see I'm just 
missing something.

Overview
========

The negated operator, "not recv any" was taken to mean "any packet never 
received by an interface" believed to be equivalent to "any packet that 
originated on the  current machine's interfaces"

My logic being:
* If "recv any" means "received by any interface"
* then "not recv any" means "not received by any interface" (which 
should be locally-generated packets)

In practice, both "recv any" and "not recv any" appear to be "no-op" 
phrases.



Application was to be able to discriminate between:

* Packets from the current host that are going out
 (and need stateful rules of the form
  my.outside.IP <==> some.rest.of.world.host)

* Packets received from "inside" hosts that had been NAT-ed
 (and need to be let out without another stateful rule, as one was 
created when the packet was received of the form
  my.private.net.IP <==> some.rest.of.world.host)

Agreed, there are ways to solve this in ipfw2 (tagging, for one), but 
the issue is that there is at least one "reasonable" application for the 
phrase and that the behavior is not what one might expect, in a 
potentially dangerous way.

To replicate
============

1) Identify a "blank" rule

[root at wildside /etc/firewall]# ipfw list 20000
ipfw: rule 20000 does not exist

2) create a rule that does not modify traffic, but logs matches, using 
"not recv any"

[root at wildside /etc/firewall]# ipfw add 20000 count all from any to any 
out not recv any
20000 count ip from any to any out

2a) Expect that (2) would have indication that "not recv any" was present

3) delete the rule just created and create another that is identical, 
but without the "not" modifier

[root at wildside /etc/firewall]# ipfw delete 20000
[root at wildside /etc/firewall]# ipfw add 20000 count all from any to any 
out recv any
20000 count ip from any to any out

3a) Note that the generated rule is the same

(enabling logging confirms that both "recv any" and "not recv any" 
appear to be NOOPs)


Discussion
==========

I didn't find much here, but did dig up an older post that looked to be 
discussing this kind of thing

<http://www.mavetju.org/mail/view_message.php?list=freebsd-ipfw&id=971213>

 > From: Patrick O'Reilly <email originally present here>
 > Date: 11 Mar 2001 23:47:44


In my opinion, the following would be "ideal"

1) "recv any" -- matches packets that have been received by the host 
through one of its interfaces
2) "not recv any" -- does not match packets that have been received by 
the host through one of its interfaces

Unfortunately, implementing (1) would likely break a lot of people's 
rule sets

(2), however, I can't immediately see being used without expecting that 
it would fail to match packets that were received by the current host, 
so its implementation would be a bit "safer" for the community



I took a quick look at the code, as Patrick's message suggests matching 
"NULL"

 > Likewise, Rod Grimes suggested:
 > ------------------
 > No, but it should be trivial to patch the code to allow your !any, if
 > you consider that !any is the same as =null:
 > > ipfw count ip from any to any in recv null
 > > Ie, the recv keyword looks at the ifp in the mbuff, the ifp will be 
null
 > for packets originated on the local machine.

/usr/src/sys/netinet/ip_fw2.c -- seemed to be the place, but the start 
of the s/r that caught my eye is not immediately suggesting that there 
is a way to match a null ifp without patching code as its trapped before 
the check is done

static int
iface_match(struct ifnet *ifp, ipfw_insn_if *cmd)
{
       if (ifp == NULL)        /* no iface with this packet, match fails */
               return 0;
       /* Check by name or by IP address */
       if (cmd->name[0] != '\0') { /* match by name */
               /* Check name */
               if (cmd->p.glob) {
                       if (fnmatch(cmd->name, ifp->if_xname, 0) == 0)
                               return(1);
               } else {
                       if (strncmp(ifp->if_xname, cmd->name, IFNAMSIZ) 
== 0)
                               return(1);
               }



Thoughts?

Worth more than just a doc change?

Thanks for the time,

Jeff