Index | Thread | Search

From:
Alexander Bluhm <alexander.bluhm@gmx.net>
Subject:
Re: let 'pfctl -a "*" -sT' show all tables
To:
Alexandr Nedvedicky <sashan@fastmail.net>
Cc:
tech@openbsd.org
Date:
Wed, 10 Jan 2024 15:27:44 +0100

Download raw body.

Thread
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
> >