From nobody Thu Sep 25 12:41:40 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4cXYHp1KBDz68SZ4; Thu, 25 Sep 2025 12:41:42 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4cXYHn16BKz42Yc; Thu, 25 Sep 2025 12:41:41 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1758804101; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=C3qh3FhBzFw8kL15RnTDrQ0h7vzV7tJuR3THky6uKyU=; b=S7KLWXGitllZadWqU9UD3AnAPop2RSkmk55Lmnfow1qakuZJGwDPZvEpVAbxvUJdHuN3q5 JQnuyHO6HdlUV1LnHu/tgoXZMARpvrPa2A2s1n4IQfpefryDRKirSlb69uNelfJeF1Y7Iu AXjdRN0662ZtCCQLw8hGiGfxxnh9tdXlld39RALGVGxqfccP7H/TBsaeFWfAGXu4TM1lYU DCwxv4zDXyY0yNysiJBXB8zmYY8mcKyKHJvLKNaKiWaApSMxb9sXTEX9pbS/il8goR8UP3 akwihXjcuAvN7t7RS/VHwoHYMpgdlJBDJsk7ZdPfzsTuVRZCFSe14Dt3Vvb4EQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1758804101; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=C3qh3FhBzFw8kL15RnTDrQ0h7vzV7tJuR3THky6uKyU=; b=oEOcej+svr1qWiqFEcgbXSBZUFu5jWxuEeYBfgm4n2l5zuTKmDAVvT63Xv0kVZ/3ApI1YM kCE+Zy529OCfqHZ1d28IOnKtNp1YR/AwsAzSoXVL+wS7uRKB2agGtB9nNbloSpxYVEy7LM jVezz8KtJNK0g1XdLhEHs4vEyQEExftt8p3OfB5e5B813ziJPlIZ52Ywc9r0CVkK4GRQvV 7bhvo/BIjDBRunrsBmxNCD5hqHRkmmP1mB5IHSvR3d/K3q98VHgwHHOEr7Uxc3cdtOfaP1 CopqQp6tq7OymNQA/vkrIqppZInesE4QUaQ474Fi0uCCUxtsf3DjfAZZTR9SRQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1758804101; a=rsa-sha256; cv=none; b=odYUX3RC6i9miduA8DypmG79KRRzyiF1QXv/AkOx3VcVMI+iHCLr3PqMyk07nEVnVeaXe9 eeCTz7d381RrlRzSOcP5vgdgUaT44uN+k996qrhr8kocHLzfGwGmY/CXQQf8x1UnAYfxZc mL0mTytsO/uTI5LsUKl55/9dXUCVcFJ+5F/lyLdAEtPjn3kdVNBoJuD/6IQO0ZjvTH1uKQ aDxsb+TQPMN5ggOcIw0pBme6BnZk+JPgIvi5nH0fvL5s18yfr1EPOq+FlZyzqUf/OaDUz3 W+5YV4ufc/ZISNHWcs8MZP5eBci/BuNqzvluKq+7AwLCRj96UN4XvhMTqOhMRA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4cXYHn0dmqz1Gvg; Thu, 25 Sep 2025 12:41:41 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 58PCfeUn004640; Thu, 25 Sep 2025 12:41:40 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 58PCfeEK004637; Thu, 25 Sep 2025 12:41:40 GMT (envelope-from git) Date: Thu, 25 Sep 2025 12:41:40 GMT Message-Id: <202509251241.58PCfeEK004637@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 2be46b52f5db - main - pfctl: fix once rules List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 2be46b52f5db0630550ec60ad8f92a7e7d7d78ab Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=2be46b52f5db0630550ec60ad8f92a7e7d7d78ab commit 2be46b52f5db0630550ec60ad8f92a7e7d7d78ab Author: Kristof Provost AuthorDate: 2025-08-27 19:32:33 +0000 Commit: Kristof Provost 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 , thanks! While here, remove an unneeded cast and make pfctl_add_rule() void as it always returned 0. OK sashan Obtained from: OpenBSD, kn , 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 *);