Date: Tue, 27 Sep 2022 10:02:18 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 0bf18fcd54f1 - stable/13 - pfctl: fix recrusive printing of anchors Message-ID: <202209271002.28RA2INE029516@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=0bf18fcd54f1acd8709b45bb0c0ff97827298a22 commit 0bf18fcd54f1acd8709b45bb0c0ff97827298a22 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2022-09-01 08:16:24 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2022-09-27 07:55:46 +0000 pfctl: fix recrusive printing of anchors Fix a couple of problems with printing of anchors, in particular recursive printing, both of inline anchors and when requested explicitly with a '*' in the anchor. - Correct recursive printing of wildcard anchors (recurse into child anchors rather than rules, which don't exist) - Print multi-part anchor paths correctly (pr6065) - Fix comments and prevent users from specifying multi-component names for inline anchors. tested by phessler ok henning Also fix the relevant pfctl test case to reflect the new (and now correct) behaviour). MFC after: 3 weeks Obtained from: OpenBSD (mcbride, f9a568a27c740528301ca3419316c85a9fc7f1de) Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D36416 (cherry picked from commit 585a5ed0bef4a0b874c8fa495ae53901799759c3) --- sbin/pfctl/parse.y | 16 ++++- sbin/pfctl/pfctl.c | 122 +++++++++++++++++++++++++++------------ sbin/pfctl/pfctl_parser.c | 6 +- sbin/pfctl/tests/files/pf0100.ok | 10 ++-- 4 files changed, 107 insertions(+), 47 deletions(-) diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index e08996a54250..043f8faee27b 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -856,7 +856,12 @@ pfa_anchor : '{' pf->asd++; pf->bn++; - /* create a holding ruleset in the root */ + /* + * Anchor contents are parsed before the anchor rule + * production completes, so we don't know the real + * location yet. Create a holding ruleset in the root; + * contents will be moved afterwards. + */ snprintf(ta, PF_ANCHOR_NAME_SIZE, "_%d", pf->bn); rs = pf_find_or_create_ruleset(ta); if (rs == NULL) @@ -893,7 +898,14 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto memset(&r, 0, sizeof(r)); if (pf->astack[pf->asd + 1]) { - /* move inline rules into relative location */ + if ($2 && strchr($2, '/') != NULL) { + free($2); + yyerror("anchor paths containing '/' " + "cannot be used for inline anchors."); + YYERROR; + } + + /* Move inline rules into relative location. */ pfctl_anchor_setup(&r, &pf->astack[pf->asd]->ruleset, $2 ? $2 : pf->alast->name); diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index 165114781a1f..5b170219e2ab 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -97,7 +97,7 @@ int pfctl_load_syncookies(struct pfctl *, u_int8_t); int pfctl_get_pool(int, struct pfctl_pool *, u_int32_t, u_int32_t, int, char *); void pfctl_print_rule_counters(struct pfctl_rule *, int); -int pfctl_show_rules(int, char *, int, enum pfctl_show, char *, int); +int pfctl_show_rules(int, char *, int, enum pfctl_show, char *, int, int); int pfctl_show_nat(int, char *, int, char *, int); int pfctl_show_src_nodes(int, int); int pfctl_show_states(int, const char *, int); @@ -1027,7 +1027,7 @@ pfctl_print_title(char *title) int pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, - char *anchorname, int depth) + char *anchorname, int depth, int wildcard) { struct pfctl_rules_info ri; struct pfctl_rule rule; @@ -1035,15 +1035,65 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, u_int32_t nr, header = 0; int rule_numbers = opts & (PF_OPT_VERBOSE2 | PF_OPT_DEBUG); int numeric = opts & PF_OPT_NUMERIC; - int len = strlen(path); - int brace; - int ret; - char *p; + int len = strlen(path), ret = 0; + char *npath, *p; - if (path[0]) - snprintf(&path[len], MAXPATHLEN - len, "/%s", anchorname); - else - snprintf(&path[len], MAXPATHLEN - len, "%s", anchorname); + /* + * Truncate a trailing / and * on an anchorname before searching for + * the ruleset, this is syntactic sugar that doesn't actually make it + * to the kernel. + */ + if ((p = strrchr(anchorname, '/')) != NULL && + p[1] == '*' && p[2] == '\0') { + p[0] = '\0'; + } + + if (anchorname[0] == '/') { + if ((npath = calloc(1, MAXPATHLEN)) == NULL) + errx(1, "pfctl_rules: calloc"); + snprintf(npath, MAXPATHLEN, "%s", anchorname); + } else { + if (path[0]) + snprintf(&path[len], MAXPATHLEN - len, "/%s", anchorname); + else + snprintf(&path[len], MAXPATHLEN - len, "%s", anchorname); + npath = path; + } + + /* + * If this anchor was called with a wildcard path, go through + * the rulesets in the anchor rather than the rules. + */ + if (wildcard && (opts & PF_OPT_RECURSE)) { + struct pfioc_ruleset prs; + u_int32_t mnr, nr; + + memset(&prs, 0, sizeof(prs)); + memcpy(prs.path, npath, sizeof(prs.path)); + if (ioctl(dev, DIOCGETRULESETS, &prs)) { + if (errno == EINVAL) + fprintf(stderr, "Anchor '%s' " + "not found.\n", anchorname); + else + err(1, "DIOCGETRULESETS"); + } + mnr = prs.nr; + + pfctl_print_rule_counters(&rule, opts); + for (nr = 0; nr < mnr; ++nr) { + prs.nr = nr; + if (ioctl(dev, DIOCGETRULESET, &prs)) + err(1, "DIOCGETRULESET"); + INDENT(depth, !(opts & PF_OPT_VERBOSE)); + printf("anchor \"%s\" all {\n", prs.name); + pfctl_show_rules(dev, npath, opts, + format, prs.name, depth + 1, 0); + INDENT(depth, !(opts & PF_OPT_VERBOSE)); + printf("}\n"); + } + path[len] = '\0'; + return (0); + } if (opts & PF_OPT_SHOWALL) { ret = pfctl_get_rules_info(dev, &ri, PF_PASS, path); @@ -1134,32 +1184,30 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, break; } case PFCTL_SHOW_RULES: - brace = 0; if (rule.label[0] && (opts & PF_OPT_SHOWALL)) labels = 1; INDENT(depth, !(opts & PF_OPT_VERBOSE)); + print_rule(&rule, anchor_call, rule_numbers, numeric); + + /* + * If this is a 'unnamed' brace notation + * anchor, OR the user has explicitly requested + * recursion, print it recursively. + */ if (anchor_call[0] && - ((((p = strrchr(anchor_call, '_')) != NULL) && - ((void *)p == (void *)anchor_call || - *(--p) == '/')) || (opts & PF_OPT_RECURSE))) { - brace++; - int aclen = strlen(anchor_call); - if (anchor_call[aclen - 1] == '*') - anchor_call[aclen - 2] = '\0'; - } - p = &anchor_call[0]; - - print_rule(&rule, p, rule_numbers, numeric); - if (brace) + (((p = strrchr(anchor_call, '/')) ? + p[1] == '_' : anchor_call[0] == '_') || + opts & PF_OPT_RECURSE)) { printf(" {\n"); - else - printf("\n"); - pfctl_print_rule_counters(&rule, opts); - if (brace) { - pfctl_show_rules(dev, path, opts, format, - p, depth + 1); + pfctl_print_rule_counters(&rule, opts); + pfctl_show_rules(dev, npath, opts, format, + anchor_call, depth + 1, + rule.anchor_wildcard); INDENT(depth, !(opts & PF_OPT_VERBOSE)); printf("}\n"); + } else { + printf("\n"); + pfctl_print_rule_counters(&rule, opts); } break; case PFCTL_SHOW_NOTHING: @@ -1167,12 +1215,10 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } pfctl_clear_pool(&rule.rpool); } - path[len] = '\0'; - return (0); error: path[len] = '\0'; - return (-1); + return (ret); } int @@ -1611,7 +1657,7 @@ pfctl_load_rule(struct pfctl *pf, char *path, struct pfctl_rule *r, int depth) if (pf->opts & PF_OPT_VERBOSE) { INDENT(depth, !(pf->opts & PF_OPT_VERBOSE2)); - print_rule(r, r->anchor ? r->anchor->name : "", + print_rule(r, name, pf->opts & PF_OPT_VERBOSE2, pf->opts & PF_OPT_NUMERIC); } @@ -2543,12 +2589,12 @@ main(int argc, char *argv[]) case 'r': pfctl_load_fingerprints(dev, opts); pfctl_show_rules(dev, path, opts, PFCTL_SHOW_RULES, - anchorname, 0); + anchorname, 0, 0); break; case 'l': pfctl_load_fingerprints(dev, opts); pfctl_show_rules(dev, path, opts, PFCTL_SHOW_LABELS, - anchorname, 0); + anchorname, 0, 0); break; case 'n': pfctl_load_fingerprints(dev, opts); @@ -2581,12 +2627,12 @@ main(int argc, char *argv[]) pfctl_load_fingerprints(dev, opts); pfctl_show_nat(dev, path, opts, anchorname, 0); - pfctl_show_rules(dev, path, opts, 0, anchorname, 0); + pfctl_show_rules(dev, path, opts, 0, anchorname, 0, 0); pfctl_show_altq(dev, ifaceopt, opts, 0); pfctl_show_states(dev, ifaceopt, opts); pfctl_show_src_nodes(dev, opts); pfctl_show_status(dev, opts); - pfctl_show_rules(dev, path, opts, 1, anchorname, 0); + pfctl_show_rules(dev, path, opts, 1, anchorname, 0, 0); pfctl_show_timeouts(dev, opts); pfctl_show_limits(dev, opts); pfctl_show_tables(anchorname, opts); @@ -2607,7 +2653,7 @@ main(int argc, char *argv[]) if ((opts & PF_OPT_CLRRULECTRS) && showopt == NULL) pfctl_show_rules(dev, path, opts, PFCTL_SHOW_NOTHING, - anchorname, 0); + anchorname, 0, 0); if (clearopt != NULL) { if (anchorname[0] == '_' || strstr(anchorname, "/_") != NULL) diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c index d22d7fe12375..2ed3f2c96607 100644 --- a/sbin/pfctl/pfctl_parser.c +++ b/sbin/pfctl/pfctl_parser.c @@ -700,6 +700,7 @@ print_rule(struct pfctl_rule *r, const char *anchor_call, int verbose, int numer "anchor", "nat-anchor", "nat-anchor", "binat-anchor", "binat-anchor", "rdr-anchor", "rdr-anchor" }; int i, opts; + char *p; if (verbose) printf("@%d ", r->nr); @@ -708,9 +709,10 @@ print_rule(struct pfctl_rule *r, const char *anchor_call, int verbose, int numer else if (r->action > PF_NORDR) printf("action(%d)", r->action); else if (anchor_call[0]) { - if (anchor_call[0] == '_') { + p = strrchr(anchor_call, '/'); + if (p ? p[1] == '_' : anchor_call[0] == '_') printf("%s", anchortypes[r->action]); - } else + else printf("%s \"%s\"", anchortypes[r->action], anchor_call); } else { diff --git a/sbin/pfctl/tests/files/pf0100.ok b/sbin/pfctl/tests/files/pf0100.ok index 6d1740f308bc..9f4427379bc7 100644 --- a/sbin/pfctl/tests/files/pf0100.ok +++ b/sbin/pfctl/tests/files/pf0100.ok @@ -1,14 +1,14 @@ pass all flags S/SA keep state -anchor "/b" all -anchor "/3" all +anchor "a/b" all +anchor "1/2/3" all anchor "relative" all { pass in on lo0 all flags S/SA keep state label "TEST1" } -anchor "/*" all -anchor "/*" all +anchor "camield/*" all +anchor "relayd/*" all anchor "foo" in on lo0 all { anchor "bar" in all { - anchor "/3" all + anchor "/1/2/3" all anchor "/relative" all pass in on lo0 all flags S/SA keep state label "FOO" }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202209271002.28RA2INE029516>