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 *);