Bug 177808 - [pf] [patch] route-to rule forwarding traffic inspite of state limit
Summary: [pf] [patch] route-to rule forwarding traffic inspite of state limit
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 9.1-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Gleb Smirnoff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-12 14:50 UTC by Kajetan Staszkiewicz
Modified: 2014-09-09 10:31 UTC (History)
1 user (show)

See Also:


Attachments
drop-traffic-on-state-creation-fail.patch (917 bytes, patch)
2013-11-18 16:13 UTC, Kajetan Staszkiewicz
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 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 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