Bug 177808

Summary: [pf] [patch] route-to rule forwarding traffic inspite of state limit
Product: Base System Reporter: Kajetan Staszkiewicz <vegeta>
Component: kernAssignee: Gleb Smirnoff <glebius>
Status: Closed FIXED    
Severity: Affects Only Me CC: glebius
Priority: Normal    
Version: 9.1-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
drop-traffic-on-state-creation-fail.patch none

Description Kajetan Staszkiewicz 2013-04-12 14:50:00 UTC
When a route-to rule is configured with a limit of states is hit, according to manual "further packets that would create state will not match this rule until existing states time out." This is only partially true. State is not created, src-node is not created, rule's action is PF_DROP. But if no next rule changes the routing behavior (e.g. if current rule is "quick"), the packet still gets forwarded according to route definition in this rule (so it was "matched").

Fix: 

That's a quick and dirty hack, I have it tested only with a "quick" rule.--6Kqk37usBkSAgfCLAcWcpPYiX75OQIbQkOGanSPi04sc0Umu
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

--- pf.c.10 2013-04-04 16:56:04.000000000 +0200
+++ pf.c.11 2013-04-12 15:41:53.000000000 +0200
@@ -7148,7 +7148,7 @@
                break;
        default:
                /* pf_route can free the mbuf causing *m0 to become NULL */
-               if (r->rt)
+               if (action == PF_PASS && r->rt)
                        pf_route(m0, r, dir, kif->pfik_ifp, s, &pd);
                break;
        }
@@ -7655,7 +7655,7 @@
                break;
        default:
                /* pf_route6 can free the mbuf causing *m0 to become NULL */
-               if (r->rt)
+               if (action == PF_PASS && r->rt)
                        pf_route6(m0, r, dir, kif->pfik_ifp, s, &pd);
                break;
        }
How-To-Repeat: Feed a quick route-to rule with state limit with some traffic, it still is forwarded by pf.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2013-04-13 00:56:00 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-pf

Over to maintainer(s).
Comment 2 Kajetan Staszkiewicz 2013-11-18 16:13:24 UTC
The attached patch for FreeBSD 10 does basically the same thing, although in a 
way that is easier to understand in code as it performs all actions inside 
pf_test, instead of waiting for pf_check_in to free *m.

-- 
| pozdrawiam / greetings | powered by Debian, FreeBSD and CentOS |
|  Kajetan Staszkiewicz  | jabber,email: vegeta()tuxpowered net  |
|        Vegeta          | www: http://vegeta.tuxpowered.net     |
`------------------------^---------------------------------------'
Comment 3 commit-hook freebsd_committer freebsd_triage 2014-09-01 13:00:57 UTC
A commit references this bug:

Author: glebius
Date: Mon Sep  1 13:00:46 UTC 2014
New revision: 270928
URL: http://svnweb.freebsd.org/changeset/base/270928

Log:
  Explicitly free packet on PF_DROP, otherwise a "quick" rule with
  "route-to" may still forward it.

  PR:		177808
  Submitted by:	Kajetan Staszkiewicz <kajetan.staszkiewicz innogames.de>
  Sponsored by:	InnoGames GmbH

Changes:
  head/sys/netpfil/pf/pf.c
Comment 4 commit-hook freebsd_committer freebsd_triage 2014-09-09 10:29:38 UTC
A commit references this bug:

Author: glebius
Date: Tue Sep  9 10:29:27 UTC 2014
New revision: 271306
URL: http://svnweb.freebsd.org/changeset/base/271306

Log:
  Merge r270928: explicitly free packet on PF_DROP, otherwise a "quick"
  rule with "route-to" may still forward it.

  PR:		177808
  Approved by:	re (gjb)

Changes:
_U  stable/10/
  stable/10/sys/netpfil/pf/pf.c