The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=13358e47edbcde3b1d3c2bdb071b908f1d2a4688
commit 13358e47edbcde3b1d3c2bdb071b908f1d2a4688
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2025-06-25 08:57:18 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2025-06-30 07:53:25 +0000
pfctl: fix anchor rules with filter opts, introduce filteropts_to_rule()
Some filter options were parsed but not set on anchor rules due to missing
copies of the respective struct members:
$ cat pf.conf
queue rq on trunk0 bandwidth 1G
queue dq parent rq bandwidth 1G default
anchor a set queue dq
$ pfctl -vnf pf.conf | fgrep queue
anchor "a" all
Fix this by moving common code from `anchorrule' and `pfrule' into a new
helper filteropts_to_rule().
Input from henning and benno
OK henning sashan jca
Obtained from: OpenBSD, kn <k...@openbsd.org>, 27a127e0be
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sbin/pfctl/parse.y | 240 +++++++++++++++++++++++------------------------------
1 file changed, 102 insertions(+), 138 deletions(-)
diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 9b89dc7642c5..ec1681e4a27d 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -424,6 +424,7 @@ int invalid_redirect(struct node_host *, sa_family_t);
u_int16_t parseicmpspec(char *, sa_family_t);
int kw_casecmp(const void *, const void *);
int map_tos(char *string, int *);
+int filteropts_to_rule(struct pfctl_rule *, struct filter_opts *);
struct node_mac* node_mac_from_string(const char *);
struct node_mac* node_mac_from_string_masklen(const char *, int);
struct node_mac* node_mac_from_string_mask(const char *, const char *);
@@ -1021,38 +1022,7 @@ anchorrule : ANCHOR anchorname dir quick interface
af proto fromto
r.direction = $3;
r.quick = $4.quick;
r.af = $6;
- r.prob = $9.prob;
- r.rtableid = $9.rtableid;
- r.ridentifier = $9.ridentifier;
- r.pktrate.limit = $9.pktrate.limit;
- r.pktrate.seconds = $9.pktrate.seconds;
- r.max_pkt_size = $9.max_pkt_size;
-
- if ($9.tag)
- if (strlcpy(r.tagname, $9.tag,
- PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
- yyerror("tag too long, max %u chars",
- PF_TAG_NAME_SIZE - 1);
- YYERROR;
- }
- if ($9.match_tag)
- if (strlcpy(r.match_tagname, $9.match_tag,
- PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
- yyerror("tag too long, max %u chars",
- PF_TAG_NAME_SIZE - 1);
- YYERROR;
- }
- r.match_tag_not = $9.match_tag_not;
- if (rule_label(&r, $9.label))
- YYERROR;
- for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
- free($9.label[i]);
- r.flags = $9.flags.b1;
- r.flagset = $9.flags.b2;
- if (($9.flags.b1 & $9.flags.b2) != $9.flags.b1) {
- yyerror("flags always false");
- YYERROR;
- }
+
if ($9.flags.b1 || $9.flags.b2 || $8.src_os) {
for (proto = $7; proto != NULL &&
proto->proto != IPPROTO_TCP;
@@ -1070,7 +1040,8 @@ anchorrule : ANCHOR anchorname dir quick interface
af proto fromto
}
}
- r.tos = $9.tos;
+ if (filteropts_to_rule(&r, &$9))
+ YYERROR;
if ($9.keep.action) {
yyerror("cannot specify state handling "
@@ -1078,26 +1049,6 @@ anchorrule : ANCHOR anchorname dir quick interface
af proto fromto
YYERROR;
}
- if ($9.match_tag)
- if (strlcpy(r.match_tagname, $9.match_tag,
- PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
- yyerror("tag too long, max %u chars",
- PF_TAG_NAME_SIZE - 1);
- YYERROR;
- }
- r.match_tag_not = $9.match_tag_not;
- if ($9.marker & FOM_PRIO) {
- if ($9.prio == 0)
- r.prio = PF_PRIO_ZERO;
- else
- r.prio = $9.prio;
- }
- if ($9.marker & FOM_SETPRIO) {
- r.set_prio[0] = $9.set_prio[0];
- r.set_prio[1] = $9.set_prio[1];
- r.scrub_flags |= PFSTATE_SETPRIO;
- }
-
decide_address_family($8.src.host, &r.af);
decide_address_family($8.dst.host, &r.af);
@@ -2417,66 +2368,11 @@ pfrule : action dir logquick interface route
af proto fromto
r.log = $3.log;
r.logif = $3.logif;
r.quick = $3.quick;
- r.prob = $9.prob;
- r.rtableid = $9.rtableid;
-
- if ($9.nodf)
- r.scrub_flags |= PFSTATE_NODF;
- if ($9.randomid)
- r.scrub_flags |= PFSTATE_RANDOMID;
- if ($9.minttl)
- r.min_ttl = $9.minttl;
- if ($9.max_mss)
- r.max_mss = $9.max_mss;
- if ($9.marker & FOM_SETTOS) {
- r.scrub_flags |= PFSTATE_SETTOS;
- r.set_tos = $9.settos;
- }
- if ($9.marker & FOM_SCRUB_TCP)
- r.scrub_flags |= PFSTATE_SCRUB_TCP;
-
- if ($9.marker & FOM_PRIO) {
- if ($9.prio == 0)
- r.prio = PF_PRIO_ZERO;
- else
- r.prio = $9.prio;
- }
- if ($9.marker & FOM_SETPRIO) {
- r.set_prio[0] = $9.set_prio[0];
- r.set_prio[1] = $9.set_prio[1];
- r.scrub_flags |= PFSTATE_SETPRIO;
- }
-
- if ($9.marker & FOM_AFTO)
- r.rule_flag |= PFRULE_AFTO;
-
r.af = $6;
- if ($9.tag)
- if (strlcpy(r.tagname, $9.tag,
- PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
- yyerror("tag too long, max %u chars",
- PF_TAG_NAME_SIZE - 1);
- YYERROR;
- }
- if ($9.match_tag)
- if (strlcpy(r.match_tagname, $9.match_tag,
- PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
- yyerror("tag too long, max %u chars",
- PF_TAG_NAME_SIZE - 1);
- YYERROR;
- }
- r.match_tag_not = $9.match_tag_not;
- if (rule_label(&r, $9.label))
- YYERROR;
- for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
- free($9.label[i]);
- r.ridentifier = $9.ridentifier;
- r.flags = $9.flags.b1;
- r.flagset = $9.flags.b2;
- if (($9.flags.b1 & $9.flags.b2) != $9.flags.b1) {
- yyerror("flags always false");
+
+ if (filteropts_to_rule(&r, &$9))
YYERROR;
- }
+
if ($9.flags.b1 || $9.flags.b2 || $8.src_os) {
for (proto = $7; proto != NULL &&
proto->proto != IPPROTO_TCP;
@@ -2494,11 +2390,6 @@ pfrule : action dir logquick interface route
af proto fromto
}
}
- r.tos = $9.tos;
- r.keep_state = $9.keep.action;
- r.pktrate.limit = $9.pktrate.limit;
- r.pktrate.seconds = $9.pktrate.seconds;
- r.max_pkt_size = $9.max_pkt_size;
o = $9.keep.options;
/* 'keep state' by default on pass rules. */
@@ -2732,10 +2623,6 @@ pfrule : action dir logquick interface route
af proto fromto
if (r.keep_state && !statelock)
r.rule_flag |= default_statelock;
- if ($9.fragment)
- r.rule_flag |= PFRULE_FRAGMENT;
- r.allow_opts = $9.allowopts;
-
decide_address_family($8.src.host, &r.af);
decide_address_family($8.dst.host, &r.af);
@@ -2755,24 +2642,6 @@ pfrule : action dir logquick interface route
af proto fromto
YYERROR;
}
}
- if ($9.queues.qname != NULL) {
- if (strlcpy(r.qname, $9.queues.qname,
- sizeof(r.qname)) >= sizeof(r.qname)) {
- yyerror("rule qname too long (max "
- "%d chars)", sizeof(r.qname)-1);
- YYERROR;
- }
- free($9.queues.qname);
- }
- if ($9.queues.pqname != NULL) {
- if (strlcpy(r.pqname, $9.queues.pqname,
- sizeof(r.pqname)) >= sizeof(r.pqname)) {
- yyerror("rule pqname too long (max "
- "%d chars)", sizeof(r.pqname)-1);
- YYERROR;
- }
- free($9.queues.pqname);
- }
#ifdef __FreeBSD__
r.divert.port = $9.divert.port;
#else
@@ -7703,3 +7572,98 @@ node_mac_from_string_mask(const char *str, const char
*mask)
return (m);
}
+
+int
+filteropts_to_rule(struct pfctl_rule *r, struct filter_opts *opts)
+{
+ r->keep_state = opts->keep.action;
+ r->pktrate.limit = opts->pktrate.limit;
+ r->pktrate.seconds = opts->pktrate.seconds;
+ r->prob = opts->prob;
+ r->rtableid = opts->rtableid;
+ r->ridentifier = opts->ridentifier;
+ r->max_pkt_size = opts->max_pkt_size;
+ r->tos = opts->tos;
+
+ if (opts->nodf)
+ r->scrub_flags |= PFSTATE_NODF;
+ if (opts->randomid)
+ r->scrub_flags |= PFSTATE_RANDOMID;
+ if (opts->minttl)
+ r->min_ttl = opts->minttl;
+ if (opts->max_mss)
+ r->max_mss = opts->max_mss;
+
+ if (opts->tag)
+ if (strlcpy(r->tagname, opts->tag,
+ PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
+ yyerror("tag too long, max %u chars",
+ PF_TAG_NAME_SIZE - 1);
+ return (1);
+ }
+ if (opts->match_tag)
+ if (strlcpy(r->match_tagname, opts->match_tag,
+ PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
+ yyerror("tag too long, max %u chars",
+ PF_TAG_NAME_SIZE - 1);
+ return (1);
+ }
+ r->match_tag_not = opts->match_tag_not;
+
+ if (rule_label(r, opts->label))
+ return (1);
+ for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
+ free(opts->label[i]);
+
+ if (opts->marker & FOM_AFTO)
+ r->rule_flag |= PFRULE_AFTO;
+ if (opts->marker & FOM_SCRUB_TCP)
+ r->scrub_flags |= PFSTATE_SCRUB_TCP;
+ if (opts->marker & FOM_PRIO) {
+ if (opts->prio == 0)
+ r->prio = PF_PRIO_ZERO;
+ else
+ r->prio = opts->prio;
+ }
+ if (opts->marker & FOM_SETPRIO) {
+ r->set_prio[0] = opts->set_prio[0];
+ r->set_prio[1] = opts->set_prio[1];
+ r->scrub_flags |= PFSTATE_SETPRIO;
+ }
+ if (opts->marker & FOM_SETTOS) {
+ r->scrub_flags |= PFSTATE_SETTOS;
+ r->set_tos = opts->settos;
+ }
+
+ r->flags = opts->flags.b1;
+ r->flagset = opts->flags.b2;
+ if ((opts->flags.b1 & opts->flags.b2) != opts->flags.b1) {
+ yyerror("flags always false");
+ return (1);
+ }
+
+ if (opts->queues.qname != NULL) {
+ if (strlcpy(r->qname, opts->queues.qname,
+ sizeof(r->qname)) >= sizeof(r->qname)) {
+ yyerror("rule qname too long (max "
+ "%d chars)", sizeof(r->qname)-1);
+ return (1);
+ }
+ free(opts->queues.qname);
+ }
+ if (opts->queues.pqname != NULL) {
+ if (strlcpy(r->pqname, opts->queues.pqname,
+ sizeof(r->pqname)) >= sizeof(r->pqname)) {
+ yyerror("rule pqname too long (max "
+ "%d chars)", sizeof(r->pqname)-1);
+ return (1);
+ }
+ free(opts->queues.pqname);
+ }
+
+ if (opts->fragment)
+ r->rule_flag |= PFRULE_FRAGMENT;
+ r->allow_opts = opts->allowopts;
+
+ return (0);
+}
