Hello,

Petr Hoffmann at Oracle discovered a glitch in ONCE rules and anchors.
Petr's test case, which shows a misbehavior looks as follows:

        echo 'anchor "foo/*"' | pfctl -f -
        pfctl -sr
        # anchor "foo/*" all

        echo 'pass' | pfctl -a foo/bar -f -
        echo 'pass once' | pfctl -a foo/once-bar -f -
        pfctl -a "*" -sr
        # anchor "foo/*" all {
        #   anchor "bar" all {
        #     pass all flags S/SA
        #   }
        #   anchor "once-bar" all {
        #     pass all flags S/SA once
        #   }
        # }


        # let some network traffic to hit once rule. any packet will do
        # show rules in main anchor:
        pfctl -a "*" -sr
        # 
        # What the hell is going on here?! why the rules are gone?

We believe it is a bug. Same bug is found in older versions of PF, before
change 'Let purge thread to remove once rules, not packets.' got in. The change
which has been introduced at g2k16 in Cambridge (pf.c @ 1.983).

We think PF should not be attempting to remove anchor rule, which points at
ONCE rule being removed. Though we still need to remove ruleset, if it becomes
empty after 'once rule' has been removed. The fix comes from Dilli Paudel at
Oracle.  After applying patch below, we get expected behavior.

        # Once expired rule gets purged we get expected output
        pfctl -a "*" -sr
        # anchor "foo/*" all {
        #   anchor "bar" all {
        #     pass all flags S/SA
        #   }
        # }

OK?

thanks and
regards
sasha

--------8<---------------8<---------------8<------------------8<--------
diff -r 5b09c9f9c654 src/sys/net/pf.c
--- a/src/sys/net/pf.c  Fri Oct 21 00:17:58 2016 +0200
+++ b/src/sys/net/pf.c  Fri Oct 21 00:28:29 2016 +0200
@@ -3857,12 +3857,6 @@
 #endif /* NPFSYNC > 0 */
 
        if (r->rule_flag & PFRULE_ONCE) {
-               if ((a != NULL) && TAILQ_EMPTY(a->ruleset->rules.active.ptr)) {
-                       a->rule_flag |= PFRULE_EXPIRED;
-                       a->exptime = time_second;
-                       SLIST_INSERT_HEAD(&pf_rule_gcl, a, gcle);
-               }
-
                r->rule_flag |= PFRULE_EXPIRED;
                r->exptime = time_second;
                SLIST_INSERT_HEAD(&pf_rule_gcl, r, gcle);
diff -r 5b09c9f9c654 src/sys/net/pf_ioctl.c
--- a/src/sys/net/pf_ioctl.c    Fri Oct 21 00:17:58 2016 +0200
+++ b/src/sys/net/pf_ioctl.c    Fri Oct 21 00:28:29 2016 +0200
@@ -315,6 +315,7 @@
                rule->nr = nr++;
        ruleset->rules.active.ticket++;
        pf_calc_skip_steps(ruleset->rules.active.ptr);
+       pf_remove_if_empty_ruleset(ruleset);
 }
 
 u_int16_t

Reply via email to