From: Alexandr Nedvedicky Subject: fix pfctl -nvf output for newly added limiters To: tech@openbsd.org Date: Thu, 29 Jan 2026 01:40:07 +0100 Hello, the issue has been reported off-list by Kristof Provost from FreeBSD. pfctl -nvf should produce output which can be fed back to pfctl(8). It is not case for newly added limiters. Consider rules as follows: source limiter "test" id 1 entries 5 \ limit 1 rate 100/10 \ inet mask 32 pass out all block in all pass in quick proto tcp from any to self port 22 source limiter "test" (no-match) block return running pfctl -nvf on ruleset above gives output as follows on current: source limiter test id 1 limit 5 states 1 rate 100/10 pass out all flags S/SA block drop in all pass in quick inet proto tcp from any to 127.0.0.1 port = 22 flags S/SA source limiter id 1 (no-match) pass in quick inet proto tcp from any to 192.168.2.61 port = 22 flags S/SA source limiter id 1 (no-match) pass in quick inet proto tcp from any to 172.16.1.1 port = 22 flags S/SA source limiter id 1 (no-match) pass in quick inet proto tcp from any to 172.27.232.115 port = 22 flags S/SA source limiter id 1 (no-match) pass in quick inet6 proto tcp from any to ::1 port = 22 flags S/SA source limiter id 1 (no-match) pass in quick on lo0 inet6 proto tcp from any to fe80::1 port = 22 flags S/SA source limiter id 1 (no-match) block return all the definition of limiter above does not match grammar expected by parser. The required modifications are: s/limit/entries s/states/limit The issue has been reported by Kristof. The diff below also makes pfctl(8) to print limiter name instead of id if particular rule refers to limiter. Diff below fixes the output to: source limiter "test" id 1 entries 5 limit 1 rate 100/10 pass out all flags S/SA block drop in all pass in quick inet proto tcp from any to 127.0.0.1 port = 22 flags S/SA source limiter test (no-match) pass in quick inet proto tcp from any to 192.168.2.61 port = 22 flags S/SA source limiter test (no-match) pass in quick inet proto tcp from any to 172.16.1.1 port = 22 flags S/SA source limiter test (no-match) pass in quick inet proto tcp from any to 172.27.232.115 port = 22 flags S/SA source limiter test (no-match) pass in quick inet6 proto tcp from any to ::1 port = 22 flags S/SA source limiter test (no-match) pass in quick on lo0 inet6 proto tcp from any to fe80::1 port = 22 flags S/SA source limiter test (no-match) block return all OK to commit? thanks and regards sashan --------8<---------------8<---------------8<------------------8<-------- diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index be1b4bf497c..262cfc155a6 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -93,7 +93,8 @@ int pfctl_set_synflwats(struct pfctl *, u_int32_t, u_int32_t); void pfctl_print_rule_counters(struct pf_rule *, int); int pfctl_show_statelims(int, enum pfctl_show); int pfctl_show_sourcelims(int, enum pfctl_show, int, const char *); -int pfctl_show_rules(int, char *, int, enum pfctl_show, char *, int, int, +void pfctl_init_show_rules(struct pfctl *, int, int); +int pfctl_show_rules(struct pfctl *, char *, enum pfctl_show, char *, int, int, long); int pfctl_show_src_nodes(int, int); int pfctl_show_states(int, const char *, int, long); @@ -896,6 +897,28 @@ pfctl_print_title(char *title) printf("%s\n", title); } +const char * +pfctl_statelim_id2name(struct pfctl *pf, u_int8_t id) +{ + struct pfctl_statelim *stlim; + + stlim = pfctl_get_statelim_id(pf, id); + if (stlim == NULL) { + stlim = malloc(sizeof(struct pfctl_statelim)); + if (stlim == NULL) + err(1, "malloc"); + memset(stlim, 0, sizeof(struct pfctl_statelim)); + stlim->ioc.id = id; + if (ioctl(pf->dev, DIOCGETSTATELIM, &stlim->ioc) == -1) { + snprintf(stlim->ioc.name, sizeof(stlim->ioc.name), + "?(%u)", id); + } + pfctl_add_statelim(pf, stlim); + } + + return stlim->ioc.name; +} + int pfctl_show_statelims(int dev, enum pfctl_show format) { @@ -1050,6 +1073,26 @@ pfctl_show_sources(int dev, const struct pfioc_sourcelim *srlim, return (0); } +const char * +pfctl_sourcelim_id2name(struct pfctl *pf, u_int8_t id) +{ + struct pfctl_sourcelim *srlim; + + srlim = pfctl_get_sourcelim_id(pf, id); + if (srlim == NULL) { + srlim = malloc(sizeof(struct pfctl_sourcelim)); + memset(srlim, 0, sizeof(struct pfctl_sourcelim)); + srlim->ioc.id = id; + if (ioctl(pf->dev, DIOCGETSOURCELIM, &srlim->ioc) == -1) { + snprintf(srlim->ioc.name, sizeof(srlim->ioc.name), + "?(%u)", id); + } + pfctl_add_sourcelim(pf, srlim); + } + + return srlim->ioc.name; +} + int pfctl_show_sourcelims(int dev, enum pfctl_show format, int opts, const char *idopt) @@ -1167,8 +1210,20 @@ pfctl_kill_source(int dev, const char *idopt, const char *source, int opts) } } +void +pfctl_init_show_rules(struct pfctl *pf, int dev, int opts) +{ + memset(pf, 0, sizeof(*pf)); + pf->dev = dev; + pf->opts = opts; + RBT_INIT(pfctl_statelim_ids, &pf->statelim_ids); + RBT_INIT(pfctl_statelim_nms, &pf->statelim_nms); + RBT_INIT(pfctl_sourcelim_ids, &pf->sourcelim_ids); + RBT_INIT(pfctl_sourcelim_nms, &pf->sourcelim_nms); +} + int -pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, +pfctl_show_rules(struct pfctl *pf, char *path, enum pfctl_show format, char *anchorname, int depth, int wildcard, long shownr) { struct pfioc_rule pr; @@ -1182,10 +1237,10 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } if (anchorname[0] == '\0') { - ret = pfctl_show_statelims(dev, format); + ret = pfctl_show_statelims(pf->dev, format); if (ret != 0) goto error; - ret = pfctl_show_sourcelims(dev, format, opts, NULL); + ret = pfctl_show_sourcelims(pf->dev, format, pf->opts, NULL); if (ret != 0) goto error; } @@ -1214,9 +1269,9 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } memcpy(pr.anchor, npath, sizeof(pr.anchor)); - if (opts & PF_OPT_SHOWALL) { + if (pf->opts & PF_OPT_SHOWALL) { pr.rule.action = PF_PASS; - if (ioctl(dev, DIOCGETRULES, &pr) == -1) { + if (ioctl(pf->dev, DIOCGETRULES, &pr) == -1) { warnx("%s", pf_strerror(errno)); ret = -1; goto error; @@ -1227,17 +1282,17 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, else if (format == PFCTL_SHOW_LABELS && labels) pfctl_print_title("LABEL COUNTERS:"); } - if (opts & PF_OPT_CLRRULECTRS) + if (pf->opts & PF_OPT_CLRRULECTRS) pr.action = PF_GET_CLR_CNTR; pr.rule.action = PF_PASS; - if (ioctl(dev, DIOCGETRULES, &pr) == -1) { + if (ioctl(pf->dev, DIOCGETRULES, &pr) == -1) { warnx("%s", pf_strerror(errno)); ret = -1; goto error; } - while (ioctl(dev, DIOCGETRULE, &pr) != -1) { + while (ioctl(pf->dev, DIOCGETRULE, &pr) != -1) { if (shownr != -1 && shownr != pr.nr) continue; @@ -1248,7 +1303,7 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, switch (format) { case PFCTL_SHOW_LABELS: if (pr.rule.label[0]) { - INDENT(depth, !(opts & PF_OPT_VERBOSE)); + INDENT(depth, !(pf->opts & PF_OPT_VERBOSE)); printf("%s %llu %llu %llu %llu" " %llu %llu %llu %llu\n", pr.rule.label, @@ -1265,10 +1320,10 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } break; case PFCTL_SHOW_RULES: - if (pr.rule.label[0] && (opts & PF_OPT_SHOWALL)) + if (pr.rule.label[0] && (pf->opts & PF_OPT_SHOWALL)) labels = 1; - INDENT(depth, !(opts & PF_OPT_VERBOSE)); - print_rule(&pr.rule, pr.anchor_call, opts); + INDENT(depth, !(pf->opts & PF_OPT_VERBOSE)); + print_rule(pf, &pr.rule, pr.anchor_call, pf->opts); /* * If this is an 'unnamed' brace notation anchor OR @@ -1278,20 +1333,21 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, if (pr.anchor_call[0] && (((p = strrchr(pr.anchor_call, '/')) ? p[1] == '_' : pr.anchor_call[0] == '_') || - opts & PF_OPT_RECURSE)) { + pf->opts & PF_OPT_RECURSE)) { printf(" {\n"); - pfctl_print_rule_counters(&pr.rule, opts); - pfctl_show_rules(dev, npath, opts, format, + pfctl_print_rule_counters(&pr.rule, pf->opts); + pfctl_show_rules(pf, npath, format, pr.anchor_call, depth + 1, pr.rule.anchor_wildcard, -1); - INDENT(depth, !(opts & PF_OPT_VERBOSE)); + INDENT(depth, !(pf->opts & PF_OPT_VERBOSE)); printf("}\n"); } else { if ((pr.rule.rule_flag & PFRULE_EXPIRED) && - !(opts & (PF_OPT_VERBOSE2 | PF_OPT_DEBUG))) + !(pf->opts & + (PF_OPT_VERBOSE2|PF_OPT_DEBUG))) break; printf("\n"); - pfctl_print_rule_counters(&pr.rule, opts); + pfctl_print_rule_counters(&pr.rule, pf->opts); } break; case PFCTL_SHOW_NOTHING: @@ -1310,25 +1366,25 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, * 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)) { + if (wildcard && (pf->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) == -1) + if (ioctl(pf->dev, DIOCGETRULESETS, &prs) == -1) errx(1, "%s", pf_strerror(errno)); mnr = prs.nr; for (nr = 0; nr < mnr; ++nr) { prs.nr = nr; - if (ioctl(dev, DIOCGETRULESET, &prs) == -1) + if (ioctl(pf->dev, DIOCGETRULESET, &prs) == -1) errx(1, "%s", pf_strerror(errno)); - INDENT(depth, !(opts & PF_OPT_VERBOSE)); + INDENT(depth, !(pf->opts & PF_OPT_VERBOSE)); printf("anchor \"%s\" all {\n", prs.name); - pfctl_show_rules(dev, npath, opts, - format, prs.name, depth + 1, 0, shownr); - INDENT(depth, !(opts & PF_OPT_VERBOSE)); + pfctl_show_rules(pf, npath, format, + prs.name, depth + 1, 0, shownr); + INDENT(depth, !(pf->opts & PF_OPT_VERBOSE)); printf("}\n"); } path[len] = '\0'; @@ -2003,7 +2059,7 @@ pfctl_load_rule(struct pfctl *pf, char *path, struct pf_rule *r, int depth) if (pf->opts & PF_OPT_VERBOSE) { INDENT(depth, !(pf->opts & PF_OPT_VERBOSE2)); - print_rule(r, name, pf->opts); + print_rule(pf, r, name, pf->opts); } path[len] = '\0'; return (0); @@ -3019,6 +3075,7 @@ main(int argc, char *argv[]) const char *idopt = NULL; const char *errstr; long shownr = -1; + struct pfctl show_rules_pf; if (argc < 2) usage(); @@ -3247,13 +3304,15 @@ main(int argc, char *argv[]) pfctl_show_anchors(dev, opts, anchorname); break; case SHOWOPT_RULES: + pfctl_init_show_rules(&show_rules_pf, dev, opts); pfctl_load_fingerprints(dev, opts); - pfctl_show_rules(dev, path, opts, PFCTL_SHOW_RULES, + pfctl_show_rules(&show_rules_pf, path, PFCTL_SHOW_RULES, anchorname, 0, anchor_wildcard, shownr); break; case SHOWOPT_LABELS: + pfctl_init_show_rules(&show_rules_pf, dev, opts); pfctl_load_fingerprints(dev, opts); - pfctl_show_rules(dev, path, opts, PFCTL_SHOW_LABELS, + pfctl_show_rules(&show_rules_pf, path, PFCTL_SHOW_LABELS, anchorname, 0, anchor_wildcard, shownr); break; case SHOWOPT_QUEUE: @@ -3276,17 +3335,16 @@ main(int argc, char *argv[]) pfctl_show_limits(dev, opts); break; case SHOWOPT_ALL: - opts |= PF_OPT_SHOWALL; + pfctl_init_show_rules(&show_rules_pf, dev, opts); pfctl_load_fingerprints(dev, opts); - - pfctl_show_rules(dev, path, opts, PFCTL_SHOW_RULES, + pfctl_show_rules(&show_rules_pf, path, PFCTL_SHOW_RULES, anchorname, 0, 0, -1); pfctl_show_queues(dev, ifaceopt, opts, opts & PF_OPT_VERBOSE2); pfctl_show_states(dev, ifaceopt, opts, -1); pfctl_show_src_nodes(dev, opts); pfctl_show_status(dev, opts); - pfctl_show_rules(dev, path, opts, PFCTL_SHOW_LABELS, + pfctl_show_rules(&show_rules_pf, path, PFCTL_SHOW_LABELS, anchorname, 0, 0, -1); pfctl_show_timeouts(dev, opts); pfctl_show_limits(dev, opts); @@ -3316,9 +3374,11 @@ main(int argc, char *argv[]) break; } - if ((opts & PF_OPT_CLRRULECTRS) && showopt == 0) - pfctl_show_rules(dev, path, opts, PFCTL_SHOW_NOTHING, + if ((opts & PF_OPT_CLRRULECTRS) && showopt == 0) { + pfctl_init_show_rules(&show_rules_pf, dev, opts); + pfctl_show_rules(&show_rules_pf, path, PFCTL_SHOW_NOTHING, anchorname, 0, 0, -1); + } if (clearopt != NULL) { switch (*clearopt) { @@ -3531,7 +3591,7 @@ pfctl_get_statelim_id(struct pfctl *pf, uint32_t id) key.ioc.id = id; - return (RBT_FIND(pfctl_statelim_nms, &pf->statelim_nms, &key)); + return (RBT_FIND(pfctl_statelim_ids, &pf->statelim_ids, &key)); } struct pfctl_statelim * @@ -3599,7 +3659,7 @@ pfctl_get_sourcelim_id(struct pfctl *pf, uint32_t id) key.ioc.id = id; - return (RBT_FIND(pfctl_sourcelim_nms, &pf->sourcelim_nms, &key)); + return (RBT_FIND(pfctl_sourcelim_ids, &pf->sourcelim_ids, &key)); } struct pfctl_sourcelim * diff --git a/sbin/pfctl/pfctl.h b/sbin/pfctl/pfctl.h index cf4ac9000a0..14481e6c508 100644 --- a/sbin/pfctl/pfctl.h +++ b/sbin/pfctl/pfctl.h @@ -76,6 +76,8 @@ extern struct pfr_ktablehead pfr_ktables; SLIST_HEAD(pfr_anchors, pfr_anchoritem); +struct pfctl; + int pfr_clr_tables(struct pfr_table *, int *, int); int pfr_add_tables(struct pfr_table *, int, int *, int); int pfr_del_tables(struct pfr_table *, int, int *, int); @@ -127,4 +129,9 @@ int pfctl_show_queues(int, const char *, int, int); void pfctl_err(int, int, const char *, ...); void pfctl_errx(int, int, const char *, ...); +const char + *pfctl_statelim_id2name(struct pfctl *, u_int8_t); +const char + *pfctl_sourcelim_id2name(struct pfctl *, u_int8_t); + #endif /* _PFCTL_H_ */ diff --git a/sbin/pfctl/pfctl_optimize.c b/sbin/pfctl/pfctl_optimize.c index 4c972430d78..71924889efc 100644 --- a/sbin/pfctl/pfctl_optimize.c +++ b/sbin/pfctl/pfctl_optimize.c @@ -392,7 +392,7 @@ optimize_superblock(struct pfctl *pf, struct superblock *block) printf("--- Superblock ---\n"); TAILQ_FOREACH(por, &block->sb_rules, por_entry) { printf(" "); - print_rule(&por->por_rule, por->por_rule.anchor ? + print_rule(pf, &por->por_rule, por->por_rule.anchor ? por->por_rule.anchor->name : "", PF_OPT_DEBUG); } #endif /* OPT_DEBUG */ diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c index 27314f43fe6..5f4c795becb 100644 --- a/sbin/pfctl/pfctl_parser.c +++ b/sbin/pfctl/pfctl_parser.c @@ -719,7 +719,7 @@ print_src_node(struct pf_src_node *sn, int opts) void print_statelim(const struct pfioc_statelim *ioc) { - printf("state limiter %s id %u limit %u", + printf("state limiter \"%s\" id %u limit %u", ioc->name, ioc->id, ioc->limit); if (ioc->rate.limit != 0) printf(" rate %u/%u", ioc->rate.limit, ioc->rate.seconds); @@ -730,7 +730,7 @@ print_statelim(const struct pfioc_statelim *ioc) void print_sourcelim(const struct pfioc_sourcelim *ioc) { - printf("source limiter %s id %u limit %u states %u", + printf("source limiter \"%s\" id %u entries %u limit %u", ioc->name, ioc->id, ioc->entries, ioc->limit); if (ioc->rate.limit != 0) printf(" rate %u/%u", ioc->rate.limit, ioc->rate.seconds); @@ -749,7 +749,8 @@ print_sourcelim(const struct pfioc_sourcelim *ioc) } void -print_rule(struct pf_rule *r, const char *anchor_call, int opts) +print_rule(struct pfctl *pf, struct pf_rule *r, const char *anchor_call, + int opts) { static const char *actiontypes[] = { "pass", "block", "scrub", "no scrub", "nat", "no nat", "binat", "no binat", "rdr", "no rdr", @@ -881,7 +882,7 @@ print_rule(struct pf_rule *r, const char *anchor_call, int opts) printf(" proto %u", r->proto); } print_fromto(&r->src, r->os_fingerprint, &r->dst, r->af, r->proto, - opts); + pf->opts | opts); if (r->rcv_ifname[0]) printf(" %sreceived-on %s", r->rcvifnot ? "!" : "", r->rcv_ifname); @@ -1001,30 +1002,16 @@ print_rule(struct pf_rule *r, const char *anchor_call, int opts) printf(" probability %s%%", buf); } if (r->statelim.id != PF_STATELIM_ID_NONE) { -#if 0 /* XXX need pf to find statelims */ - struct pfctl_statelim *stlim = - pfctl_get_statelim_id(pf, r->statelim); - - if (stlim != NULL) - printf(" state limiter %s", stlim->ioc.name); - else -#endif - printf(" state limiter id %u (%s)", r->statelim.id, - (r->statelim.limiter_action == PF_LIMITER_BLOCK) ? - "block" : "no-match"); + printf(" state limiter %s (%s)", + pfctl_statelim_id2name(pf, r->statelim.id), + (r->statelim.limiter_action == PF_LIMITER_BLOCK) ? + "block" : "no-match"); } if (r->sourcelim.id != PF_SOURCELIM_ID_NONE) { -#if 0 /* XXX need pf to find sourcelims */ - struct pfctl_sourcelim *srlim = - pfctl_get_sourcelim_id(pf, r->sourcelim); - - if (srlim != NULL) - printf(" source limiter %s", srlim->ioc.name); - else -#endif - printf(" source limiter id %u (%s)", r->sourcelim.id, - (r->sourcelim.limiter_action == PF_LIMITER_BLOCK) ? - "block" : "no-match"); + printf(" source limiter %s (%s)", + pfctl_sourcelim_id2name(pf, r->sourcelim.id), + (r->sourcelim.limiter_action == PF_LIMITER_BLOCK) ? + "block" : "no-match"); } if (ropts) { printf(" ("); diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h index 0b78eb27572..6c820096cac 100644 --- a/sbin/pfctl/pfctl_parser.h +++ b/sbin/pfctl/pfctl_parser.h @@ -278,7 +278,7 @@ void print_pool(struct pf_pool *, u_int16_t, u_int16_t, sa_family_t, int, int); void print_src_node(struct pf_src_node *, int); void print_statelim(const struct pfioc_statelim *); void print_sourcelim(const struct pfioc_sourcelim *); -void print_rule(struct pf_rule *, const char *, int); +void print_rule(struct pfctl *pf, struct pf_rule *, const char *, int); void print_tabledef(const char *, int, int, struct node_tinithead *); void print_status(struct pf_status *, struct pfctl_watermarks *, int); void print_queuespec(struct pf_queuespec *);