From: Alexander Bluhm Subject: Re: let 'pfctl -a "*" -sT' show all tables To: Alexandr Nedvedicky Cc: tech@openbsd.org Date: Wed, 10 Jan 2024 15:27:44 +0100 On Tue, Jan 09, 2024 at 06:24:03PM +0100, Alexandr Nedvedicky wrote: > Hello, > > any interest in this? OK bluhm@ > thanks and > regards > sashan > > On Tue, Nov 28, 2023 at 12:54:00AM +0100, Alexandr Nedvedicky wrote: > > Hello, > > > > 3 weeks ago kn@ noticed a glitch in pfctl(8) [1] while he was playing with > > relayd(8). Diff below makes 'pfctl -a "*" -sT' to walk through all > > anchors/rulesets and show tables bound to rulesets. > > > > the relayd(8) creates its tables under 'relayd/*' anchor. Those tables > > are never reported when we run 'pfctl -a "*" -sT'. In order to test my > > change one must create tables under non-root anchor/ruleset: > > > > pfctl -a test -t foo -T add 192.168.1.0/24 > > > > command above creates anchor/ruleset 'test' with table 'foo'. The diff > > makes command 'pfctl -a "*" -sT' to produce output as follows: > > > > lumpy# ./pfctl -a "*" -sT > > foo@test > > > > If we create a more complex structure: > > > > pfctl -t bar -T add 192.168.2.0/24 > > pfctl -a this/is/test -t foobar -T add 172.16.0.0/16 > > > > then one can run 'pfctl -a "*" -sT' to see all tables: > > > > lumpy# ./pfctl -a "*" -sT > > foobar@this/is/test > > foo@test > > bar > > > > we can also ask pfctl(8) to dump tables in ruleset subtree: > > > > lumpy# ./pfctl -a "/this/*" -sT > > foobar@this/is/test > > > > The diff builds on existing function pfctl_recurse() which walks > > all anchors. The current pfctl_recurse() assumes clear action only. > > Therefore I'm introducing a new PF_OPT_CALLSHOW flag as a hint > > for pfctl_recurse() to adjust its output. I must admit I'm not > > happy about it. On the other hand the only alternative I could > > come up with is copy and modify pfctl_recurse(). > > > > OK to commit? > > > > thanks and > > regards > > sashan > > > > [1] https://marc.info/?l=openbsd-bugs&m=169972184623318&w=2 > > > > --------8<---------------8<---------------8<------------------8<-------- > > diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c > > index f4ff345a49a..ae93c8c841f 100644 > > --- a/sbin/pfctl/pfctl.c > > +++ b/sbin/pfctl/pfctl.c > > @@ -118,6 +118,7 @@ int pfctl_recurse(int, int, const char *, > > int pfctl_call_clearrules(int, int, struct pfr_anchoritem *); > > int pfctl_call_cleartables(int, int, struct pfr_anchoritem *); > > int pfctl_call_clearanchors(int, int, struct pfr_anchoritem *); > > +int pfctl_call_showtables(int, int, struct pfr_anchoritem *); > > > > const char *clearopt; > > char *rulesopt; > > @@ -2300,6 +2301,13 @@ pfctl_call_clearrules(int dev, int opts, struct pfr_anchoritem *pfra) > > return (pfctl_clear_rules(dev, opts, pfra->pfra_anchorname)); > > } > > > > +int > > +pfctl_call_showtables(int dev, int opts, struct pfr_anchoritem *pfra) > > +{ > > + pfctl_show_tables(pfra->pfra_anchorname, opts); > > + return (0); > > +} > > + > > int > > pfctl_call_clearanchors(int dev, int opts, struct pfr_anchoritem *pfra) > > { > > @@ -2325,10 +2333,12 @@ pfctl_recurse(int dev, int opts, const char *anchorname, > > * so that failures on one anchor do not prevent clearing others. > > */ > > opts |= PF_OPT_IGNFAIL; > > - printf("Removing:\n"); > > + if ((opts & PF_OPT_CALLSHOW) == 0) > > + printf("Removing:\n"); > > SLIST_FOREACH_SAFE(pfra, anchors, pfra_sle, pfra_save) { > > - printf(" %s\n", (*pfra->pfra_anchorname == '\0') ? > > - "/" : pfra->pfra_anchorname); > > + if ((opts & PF_OPT_CALLSHOW) == 0) > > + printf(" %s\n", (*pfra->pfra_anchorname == '\0') ? > > + "/" : pfra->pfra_anchorname); > > rv |= walkf(dev, opts, pfra); > > SLIST_REMOVE(anchors, pfra, pfr_anchoritem, pfra_sle); > > free(pfra->pfra_anchorname); > > @@ -2747,7 +2757,12 @@ main(int argc, char *argv[]) > > pfctl_show_fingerprints(opts); > > break; > > case 'T': > > - pfctl_show_tables(anchorname, opts); > > + if (opts & PF_OPT_RECURSE) { > > + opts |= PF_OPT_CALLSHOW; > > + pfctl_recurse(dev, opts, anchorname, > > + pfctl_call_showtables); > > + } else > > + pfctl_show_tables(anchorname, opts); > > break; > > case 'o': > > pfctl_load_fingerprints(dev, opts); > > diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h > > index 01a61a49d01..625b9488073 100644 > > --- a/sbin/pfctl/pfctl_parser.h > > +++ b/sbin/pfctl/pfctl_parser.h > > @@ -52,6 +52,7 @@ > > #define PF_OPT_RECURSE 0x04000 > > #define PF_OPT_PORTNAMES 0x08000 > > #define PF_OPT_IGNFAIL 0x10000 > > +#define PF_OPT_CALLSHOW 0x20000 > > > > #define PF_TH_ALL 0xFF > > > > diff --git a/sbin/pfctl/pfctl_table.c b/sbin/pfctl/pfctl_table.c > > index d041b15d568..381a6f190ae 100644 > > --- a/sbin/pfctl/pfctl_table.c > > +++ b/sbin/pfctl/pfctl_table.c > > @@ -369,21 +369,21 @@ print_table(struct pfr_table *ta, int verbose, int debug) > > { > > if (!debug && !(ta->pfrt_flags & PFR_TFLAG_ACTIVE)) > > return; > > - if (verbose) { > > - printf("%c%c%c%c%c%c%c\t%s", > > + if (verbose) > > + printf("%c%c%c%c%c%c%c\t", > > (ta->pfrt_flags & PFR_TFLAG_CONST) ? 'c' : '-', > > (ta->pfrt_flags & PFR_TFLAG_PERSIST) ? 'p' : '-', > > (ta->pfrt_flags & PFR_TFLAG_ACTIVE) ? 'a' : '-', > > (ta->pfrt_flags & PFR_TFLAG_INACTIVE) ? 'i' : '-', > > (ta->pfrt_flags & PFR_TFLAG_REFERENCED) ? 'r' : '-', > > (ta->pfrt_flags & PFR_TFLAG_REFDANCHOR) ? 'h' : '-', > > - (ta->pfrt_flags & PFR_TFLAG_COUNTERS) ? 'C' : '-', > > - ta->pfrt_name); > > - if (ta->pfrt_anchor[0]) > > - printf("\t%s", ta->pfrt_anchor); > > - puts(""); > > - } else > > - puts(ta->pfrt_name); > > + (ta->pfrt_flags & PFR_TFLAG_COUNTERS) ? 'C' : '-'); > > + > > + printf("%s", ta->pfrt_name); > > + if (ta->pfrt_anchor[0] != '\0') > > + printf("@%s", ta->pfrt_anchor); > > + > > + printf("\n"); > > } > > > > void > >