Date: Mon, 30 Jun 2025 09:54:31 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 13358e47edbc - main - pfctl: fix anchor rules with filter opts, introduce filteropts_to_rule() Message-ID: <202506300954.55U9sVvw064209@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=13358e47edbcde3b1d3c2bdb071b908f1d2a4688 commit 13358e47edbcde3b1d3c2bdb071b908f1d2a4688 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-06-25 08:57:18 +0000 Commit: Kristof Provost <kp@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 <kn@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); +}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202506300954.55U9sVvw064209>