Hi,

I noticed that pfctl says 'once' can be used only with pass/block rules,
but it is not true - it can't for block but can for anchor rules:

--8<---------------------------------------------------------------
# echo 'block once' | pfctl -f -
stdin:1: 'once' only applies to pass/block rules
pfctl: Syntax error in config file: pf rules not loaded

# echo 'anchor xxx once' | pfctl -f -
# pfctl -sr
anchor "xxx" all once
--------------------------------------------------------------->8--

The patch below resolves the issue. Note that pf_rule.action has no
special constant for anchors. The patch uses pf_rule.anchor to recognize
an anchor rule. To do this, I moved a piece of code from pfctl_add_rule()
to parse.y so that the anchor is created early and pf_rule.anchor becomes
non-NULL. This further allowed tosimplify expand_rule() and
pfctl_add_rule() by shortening their parameterlists.

The patched pfctl responds as expected:

--8<---------------------------------------------------------------
# echo 'block once' | ./pfctl -f -
# pfctl -sr
block drop all once
# echo 'anchor xxx once' | ./pfctl -f -
stdin:1: 'once' only applies to pass/block rules
pfctl: Syntax error in config file: pf rules not loaded
--------------------------------------------------------------->8--


diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index e8dd97f6222..ce866883b04 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -354,7 +354,7 @@ struct pfctl_watermarks  syncookie_opts;
 int         disallow_table(struct node_host *, const char *);
 int         disallow_urpf_failed(struct node_host *, const char *);
 int         disallow_alias(struct node_host *, const char *);
-int         rule_consistent(struct pf_rule *, int);
+int         rule_consistent(struct pf_rule *);
 int         process_tabledef(char *, struct table_opts *, int);
 void         expand_label_str(char *, size_t, const char *, const char *);
 void         expand_label_if(const char *, char *, size_t, const char *);
@@ -377,8 +377,7 @@ void         expand_rule(struct pf_rule *, int, struct node_if *,
             struct node_proto *,
             struct node_os *, struct node_host *, struct node_port *,
             struct node_host *, struct node_port *, struct node_uid *,
-            struct node_gid *, struct node_if *, struct node_icmp *,
-            const char *);
+            struct node_gid *, struct node_if *, struct node_icmp *);
 int         expand_queue(char *, struct node_if *, struct queue_opts *);
 int         expand_skip_interface(struct node_if *);

@@ -876,6 +875,7 @@ anchorrule    : ANCHOR anchorname dir quick interface af proto fromto
         {
             struct pf_rule    r;
             struct node_proto    *proto;
+            char    *p;

             memset(&r, 0, sizeof(r));
             if (pf->astack[pf->asd + 1]) {
@@ -913,7 +913,33 @@ anchorrule    : ANCHOR anchorname dir quick interface af proto fromto
                         "rules must specify a name");
                     YYERROR;
                 }
+
+                /*
+                 * Don't make non-brace anchors part of the main anchor pool.
+                 */
+                if ((r.anchor = calloc(1, sizeof(*r.anchor))) == NULL) {
+                    err(1, "anchorrule: calloc");
+                }
+ pf_init_ruleset(&r.anchor->ruleset);
+                r.anchor->ruleset.anchor = r.anchor;
+                if (strlcpy(r.anchor->path, $2,
+                    sizeof(r.anchor->path)) >= sizeof(r.anchor->path)) {
+                                   errx(1, "anchorrule: strlcpy");
+                }
+                if ((p = strrchr($2, '/')) != NULL) {
+                    if (strlen(p) == 1) {
+                        yyerror("anchorrule: bad anchor name %s",
+                            $2);
+                        YYERROR;
+                    }
+                } else
+                    p = (char *)$2;
+                if (strlcpy(r.anchor->name, p,
+                    sizeof(r.anchor->name)) >= sizeof(r.anchor->name)) {
+                                   errx(1, "anchorrule: strlcpy");
+                }
             }
+
             r.direction = $3;
             r.quick = $4.quick;
             r.af = $6;
@@ -955,8 +981,7 @@ anchorrule    : ANCHOR anchorname dir quick interface af proto fromto

             expand_rule(&r, 0, $5, NULL, NULL, NULL, $7, $8.src_os,
                 $8.src.host, $8.src.port, $8.dst.host, $8.dst.port,
-                $9.uid, $9.gid, $9.rcv, $9.icmpspec,
-                pf->astack[pf->asd + 1] ? pf->alast->name : $2);
+                $9.uid, $9.gid, $9.rcv, $9.icmpspec);
             free($2);
             pf->astack[pf->asd + 1] = NULL;
         }
@@ -1110,7 +1135,7 @@ antispoof    : ANTISPOOF logquick antispoof_ifspc af antispoof_opts {
                 if (h != NULL)
                     expand_rule(&r, 0, j, NULL, NULL, NULL,
                         NULL, NULL, h, NULL, NULL, NULL,
-                        NULL, NULL, NULL, NULL, "");
+                        NULL, NULL, NULL, NULL);

                 if ((i->ifa_flags & IFF_LOOPBACK) == 0) {
                     bzero(&r, sizeof(r));
@@ -1132,7 +1157,7 @@ antispoof    : ANTISPOOF logquick antispoof_ifspc af antispoof_opts {
                         expand_rule(&r, 0, NULL, NULL,
                             NULL, NULL, NULL, NULL, h,
                             NULL, NULL, NULL, NULL,
-                            NULL, NULL, NULL, "");
+                            NULL, NULL, NULL);
                 } else
                     free(hh);
             }
@@ -1849,7 +1874,7 @@ pfrule        : action dir logquick interface af proto fromto
             expand_rule(&r, 0, $4, &$8.nat, &$8.rdr, &$8.rroute, $6,
                 $7.src_os,
                 $7.src.host, $7.src.port, $7.dst.host, $7.dst.port,
-                $8.uid, $8.gid, $8.rcv, $8.icmpspec, "");
+                $8.uid, $8.gid, $8.rcv, $8.icmpspec);
         }
         ;

@@ -3932,7 +3957,7 @@ disallow_alias(struct node_host *h, const char *fmt)
 }

 int
-rule_consistent(struct pf_rule *r, int anchor_call)
+rule_consistent(struct pf_rule *r)
 {
     int    problems = 0;

@@ -4629,7 +4654,7 @@ expand_rule(struct pf_rule *r, int keeprule, struct node_if *interfaces,
     struct node_host *src_hosts, struct node_port *src_ports,
     struct node_host *dst_hosts, struct node_port *dst_ports,
     struct node_uid *uids, struct node_gid *gids, struct node_if *rcv,
-    struct node_icmp *icmp_types, const char *anchor_call)
+    struct node_icmp *icmp_types)
 {
     sa_family_t         af = r->af;
     int             added = 0, error = 0;
@@ -4845,11 +4870,11 @@ expand_rule(struct pf_rule *r, int keeprule, struct node_if *interfaces,
         error += apply_redirspec(&r->rdr, r, rdr, 1, dst_port);
         error += apply_redirspec(&r->route, r, rroute, 2, dst_port);

-        if (rule_consistent(r, anchor_call[0]) < 0 || error)
+        if (rule_consistent(r) < 0 || error)
             yyerror("skipping rule due to errors");
         else {
             r->nr = pf->astack[pf->asd]->match++;
-            pfctl_add_rule(pf, r, anchor_call);
+            pfctl_add_rule(pf, r);
             added++;
         }
         r->direction = dir;
@@ -4888,7 +4913,7 @@ expand_rule(struct pf_rule *r, int keeprule, struct node_if *interfaces,
             expand_rule(&rb, 1, interface, NULL, &binat, NULL,
                 proto,
                 src_os, dst_host, dst_port, dsth, src_port,
-                uid, gid, rcv, icmp_type, anchor_call);
+                uid, gid, rcv, icmp_type);
         }

         if (osrch && src_host->addr.type == PF_ADDR_DYNIFTL) {
@@ -5875,7 +5900,7 @@ int
 filteropts_to_rule(struct pf_rule *r, struct filter_opts *opts)
 {
     if (opts->marker & FOM_ONCE) {
-        if (r->action != PF_PASS && r->action != PF_MATCH) {
+        if ((r->action != PF_PASS && r->action != PF_DROP) || r->anchor) {
             yyerror("'once' only applies to pass/block rules");
             return (1);
         }
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 40cef57995c..ba1d77a1953 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1112,35 +1112,13 @@ pfctl_show_limits(int dev, int opts)

 /* callbacks for rule/nat/rdr/addr */
 int
-pfctl_add_rule(struct pfctl *pf, struct pf_rule *r, const char *anchor_call)
+pfctl_add_rule(struct pfctl *pf, struct pf_rule *r)
 {
     struct pf_rule        *rule;
     struct pf_ruleset    *rs;
     char             *p;

     rs = &pf->anchor->ruleset;
-    if (anchor_call[0] && r->anchor == NULL) {
-        /*
-         * Don't make non-brace anchors part of the main anchor pool.
-         */
-        if ((r->anchor = calloc(1, sizeof(*r->anchor))) == NULL)
-            err(1, "pfctl_add_rule: calloc");
-
-        pf_init_ruleset(&r->anchor->ruleset);
-        r->anchor->ruleset.anchor = r->anchor;
-        if (strlcpy(r->anchor->path, anchor_call,
-            sizeof(rule->anchor->path)) >= sizeof(rule->anchor->path))
-                        errx(1, "pfctl_add_rule: strlcpy");
-        if ((p = strrchr(anchor_call, '/')) != NULL) {
-            if (strlen(p) == 1)
-                errx(1, "pfctl_add_rule: bad anchor name %s",
-                    anchor_call);
-        } else
-            p = (char *)anchor_call;
-        if (strlcpy(r->anchor->name, p,
-            sizeof(rule->anchor->name)) >= sizeof(rule->anchor->name))
-                        errx(1, "pfctl_add_rule: strlcpy");
-    }

     if ((rule = calloc(1, sizeof(*rule))) == NULL)
         err(1, "calloc");
diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h
index 84876f3ad7a..744ada036c3 100644
--- a/sbin/pfctl/pfctl_parser.h
+++ b/sbin/pfctl/pfctl_parser.h
@@ -218,7 +218,7 @@ int     pf_opt_create_table(struct pfctl *, struct pf_opt_tbl *);
 int     add_opt_table(struct pfctl *, struct pf_opt_tbl **, sa_family_t,
             struct pf_rule_addr *, char *);

-int    pfctl_add_rule(struct pfctl *, struct pf_rule *, const char *);
+int    pfctl_add_rule(struct pfctl *, struct pf_rule *);
 int    pfctl_add_pool(struct pfctl *, struct pf_pool *, sa_family_t, int);
 void    pfctl_move_pool(struct pf_pool *, struct pf_pool *);
 void    pfctl_clear_pool(struct pf_pool *);

Reply via email to