git: 2be46b52f5db - main - pfctl: fix once rules
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 25 Sep 2025 12:41:40 UTC
The branch main has been updated by kp:
URL: https://cgit.FreeBSD.org/src/commit/?id=2be46b52f5db0630550ec60ad8f92a7e7d7d78ab
commit 2be46b52f5db0630550ec60ad8f92a7e7d7d78ab
Author: Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2025-08-27 19:32:33 +0000
Commit: Kristof Provost <kp@FreeBSD.org>
CommitDate: 2025-09-25 12:41:09 +0000
pfctl: fix once rules
parse.y revision 1.682 from 16.07.2018 errornously allowed `match once' and
`anchor "a" once'.
Fix both by checking for PF_DROP not PF_MATCH and creating anchors in the
parser already such that they can be used to distinguish anchor rules in
the same check as well.
Found and fixed by Petr Hoffmann <petr.hoffmann at oracle dot com>, thanks!
While here, remove an unneeded cast and make pfctl_add_rule() void as it
always returned 0.
OK sashan
Obtained from: OpenBSD, kn <kn@openbsd.org>, 6da84b37b3
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sbin/pfctl/parse.y | 69 +++++++++++++++++++++++++++++++----------------
sbin/pfctl/pfctl.c | 30 ++-------------------
sbin/pfctl/pfctl_parser.h | 2 +-
3 files changed, 49 insertions(+), 52 deletions(-)
diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index c75632c740b3..babe6b99013e 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -372,8 +372,8 @@ int validate_range(uint8_t, uint16_t, uint16_t);
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 pfctl_rule *, int);
-int filter_consistent(struct pfctl_rule *, int);
+int rule_consistent(struct pfctl_rule *);
+int filter_consistent(struct pfctl_rule *);
int nat_consistent(struct pfctl_rule *);
int rdr_consistent(struct pfctl_rule *);
int process_tabledef(char *, struct table_opts *, int);
@@ -403,7 +403,7 @@ void expand_rule(struct pfctl_rule *, bool, 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_icmp *);
int expand_altq(struct pf_altq *, struct node_if *,
struct node_queue *, struct node_queue_bw bwspec,
struct node_queue_opt *);
@@ -994,6 +994,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto
{
struct pfctl_rule r;
struct node_proto *proto;
+ char *p;
if (check_rulestate(PFCTL_STATE_FILTER)) {
if ($2)
@@ -1039,6 +1040,30 @@ 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 = $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;
@@ -1075,8 +1100,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto
expand_rule(&r, false, $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);
+ $8.dst.port, $9.uid, $9.gid, $9.rcv, $9.icmpspec);
free($2);
pf->astack[pf->asd + 1] = NULL;
}
@@ -1099,7 +1123,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto
expand_rule(&r, false, $3, NULL, NULL, NULL,
$5, $6.src_os, $6.src.host, $6.src.port, $6.dst.host,
- $6.dst.port, 0, 0, 0, 0, $2);
+ $6.dst.port, 0, 0, 0, 0);
free($2);
}
| RDRANCHOR string interface af proto fromto rtable {
@@ -1142,7 +1166,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto
expand_rule(&r, false, $3, NULL, NULL, NULL,
$5, $6.src_os, $6.src.host, $6.src.port, $6.dst.host,
- $6.dst.port, 0, 0, 0, 0, $2);
+ $6.dst.port, 0, 0, 0, 0);
free($2);
}
| BINATANCHOR string interface af proto fromto rtable {
@@ -1178,7 +1202,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto
decide_address_family($6.src.host, &r.af);
decide_address_family($6.dst.host, &r.af);
- pfctl_append_rule(pf, &r, $2);
+ pfctl_append_rule(pf, &r);
free($2);
}
;
@@ -1465,7 +1489,7 @@ scrubrule : scrubaction dir logquick interface af proto fromto scrub_opts
expand_rule(&r, false, $4, NULL, NULL, NULL,
$6, $7.src_os, $7.src.host, $7.src.port, $7.dst.host,
- $7.dst.port, NULL, NULL, NULL, NULL, "");
+ $7.dst.port, NULL, NULL, NULL, NULL);
}
;
@@ -1630,7 +1654,7 @@ antispoof : ANTISPOOF logquick antispoof_ifspc af antispoof_opts {
if (h != NULL)
expand_rule(&r, false, j, NULL, NULL,
NULL, NULL, NULL, h, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL, "");
+ NULL, NULL, NULL, NULL, NULL);
if ((i->ifa_flags & IFF_LOOPBACK) == 0) {
bzero(&r, sizeof(r));
@@ -1653,7 +1677,7 @@ antispoof : ANTISPOOF logquick antispoof_ifspc af antispoof_opts {
expand_rule(&r, false, NULL,
NULL, NULL, NULL, NULL,
NULL, h, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, "");
+ NULL, NULL, NULL, NULL);
} else
free(hh);
}
@@ -2736,7 +2760,7 @@ pfrule : action dir logquick interface route af proto fromto
expand_rule(&r, false, $4, $9.nat, $9.rdr, $5.redirspec,
$7, $8.src_os, $8.src.host, $8.src.port, $8.dst.host,
- $8.dst.port, $9.uid, $9.gid, $9.rcv, $9.icmpspec, "");
+ $8.dst.port, $9.uid, $9.gid, $9.rcv, $9.icmpspec);
}
;
@@ -4871,7 +4895,7 @@ natrule : nataction interface af proto fromto tag tagged rtable
expand_rule(&r, false, $2, NULL, $9, NULL, $4,
$5.src_os, $5.src.host, $5.src.port, $5.dst.host,
- $5.dst.port, 0, 0, 0, 0, "");
+ $5.dst.port, 0, 0, 0, 0);
}
;
@@ -5050,7 +5074,7 @@ binatrule : no BINAT natpasslog interface af proto FROM ipspec toipspec tag
free($13);
}
- pfctl_append_rule(pf, &binat, "");
+ pfctl_append_rule(pf, &binat);
}
;
@@ -5277,7 +5301,7 @@ disallow_alias(struct node_host *h, const char *fmt)
}
int
-rule_consistent(struct pfctl_rule *r, int anchor_call)
+rule_consistent(struct pfctl_rule *r)
{
int problems = 0;
@@ -5287,7 +5311,7 @@ rule_consistent(struct pfctl_rule *r, int anchor_call)
case PF_DROP:
case PF_SCRUB:
case PF_NOSCRUB:
- problems = filter_consistent(r, anchor_call);
+ problems = filter_consistent(r);
break;
case PF_NAT:
case PF_NONAT:
@@ -5306,7 +5330,7 @@ rule_consistent(struct pfctl_rule *r, int anchor_call)
}
int
-filter_consistent(struct pfctl_rule *r, int anchor_call)
+filter_consistent(struct pfctl_rule *r)
{
int problems = 0;
@@ -6346,7 +6370,7 @@ expand_rule(struct pfctl_rule *r, bool keeprule,
struct node_os *src_oses, 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_if *rcv, struct node_icmp *icmp_types)
{
sa_family_t af = r->af;
int added = 0, error = 0;
@@ -6513,11 +6537,11 @@ expand_rule(struct pfctl_rule *r, bool keeprule,
error += check_binat_redirspec(src_host, r, af);
}
- 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_append_rule(pf, r, anchor_call);
+ pfctl_append_rule(pf, r);
added++;
}
@@ -6533,8 +6557,7 @@ expand_rule(struct pfctl_rule *r, bool keeprule,
expand_rule(&rdr_rule, true, interface, NULL, rdr_redirspec,
NULL, proto, src_os, dst_host, dst_port,
- rdr_dst_host, src_port, uid, gid, rcv, icmp_type,
- "");
+ rdr_dst_host, src_port, uid, gid, rcv, icmp_type);
}
if (osrch && src_host->addr.type == PF_ADDR_DYNIFTL) {
@@ -7743,7 +7766,7 @@ int
filteropts_to_rule(struct pfctl_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 7e1195fdf2f0..b8f4305a3e38 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1861,14 +1861,12 @@ pfctl_init_rule(struct pfctl_rule *r)
TAILQ_INIT(&(r->route.list));
}
-int
-pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r,
- const char *anchor_call)
+void
+pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r)
{
u_int8_t rs_num;
struct pfctl_rule *rule;
struct pfctl_ruleset *rs;
- char *p;
rs_num = pf_get_ruleset_number(r->action);
if (rs_num == PF_RULESET_MAX)
@@ -1876,29 +1874,6 @@ pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r,
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_append_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_append_rule: strlcpy");
- if ((p = strrchr(anchor_call, '/')) != NULL) {
- if (!strlen(p))
- err(1, "pfctl_append_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_append_rule: strlcpy");
- }
-
if ((rule = calloc(1, sizeof(*rule))) == NULL)
err(1, "calloc");
bcopy(r, rule, sizeof(*rule));
@@ -1910,7 +1885,6 @@ pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r,
pfctl_move_pool(&r->route, &rule->route);
TAILQ_INSERT_TAIL(rs->rules[rs_num].active.ptr, rule, entries);
- return (0);
}
int
diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h
index e96ff0195e03..44ddfb45fbe1 100644
--- a/sbin/pfctl/pfctl_parser.h
+++ b/sbin/pfctl/pfctl_parser.h
@@ -288,7 +288,7 @@ int pfctl_rules(int, char *, int, int, char *, struct pfr_buffer *);
int pfctl_optimize_ruleset(struct pfctl *, struct pfctl_ruleset *);
void pfctl_init_rule(struct pfctl_rule *r);
-int pfctl_append_rule(struct pfctl *, struct pfctl_rule *, const char *);
+void pfctl_append_rule(struct pfctl *, struct pfctl_rule *);
int pfctl_append_eth_rule(struct pfctl *, struct pfctl_eth_rule *,
const char *);
int pfctl_add_altq(struct pfctl *, struct pf_altq *);