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

3 files changed, 113 insertions, 50 deletions

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 8f92e2f265b..8dbdfded120 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.589 2010/03/23 13:31:29 henning Exp $	*/
+/*	$OpenBSD: parse.y,v 1.590 2010/07/03 02:28:57 mcbride Exp $	*/
 
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
@@ -769,7 +769,12 @@ pfa_anchor	: '{'
 			pf->bn++;
 			pf->brace = 1;
 
-			/* 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)
@@ -806,7 +811,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. */
 				pf_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 907a15f7e76..65c5725d81c 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl.c,v 1.299 2010/07/01 15:12:25 stsp Exp $ */
+/*	$OpenBSD: pfctl.c,v 1.300 2010/07/03 02:28:57 mcbride Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -81,7 +81,7 @@ int	 pfctl_load_logif(struct pfctl *, char *);
 int	 pfctl_load_hostid(struct pfctl *, unsigned int);
 int	 pfctl_load_reassembly(struct pfctl *, u_int32_t);
 void	 pfctl_print_rule_counters(struct pf_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_src_nodes(int, int);
 int	 pfctl_show_states(int, const char *, int);
 int	 pfctl_show_status(int, int);
@@ -750,26 +750,78 @@ 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 pfioc_rule pr;
 	u_int32_t nr, mnr, header = 0;
 	int rule_numbers = opts & (PF_OPT_VERBOSE2 | PF_OPT_DEBUG);
-	int len = strlen(path);
-	int brace;
-	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';
+	}
 
 	memset(&pr, 0, sizeof(pr));
-	memcpy(pr.anchor, path, sizeof(pr.anchor));
+	if (anchorname[0] == '/') {
+		if ((npath = calloc(1, MAXPATHLEN)) == NULL)
+			errx(1, "pfctl_rules: calloc");
+		snprintf(npath, MAXPATHLEN, 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 && 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(&pr.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);
+	}
+
+	memcpy(pr.anchor, npath, sizeof(pr.anchor));
 	if (opts & PF_OPT_SHOWALL) {
 		pr.rule.action = PF_PASS;
 		if (ioctl(dev, DIOCGETRULES, &pr)) {
 			warn("DIOCGETRULES");
+			ret = -1;
 			goto error;
 		}
 		header++;
@@ -786,6 +838,7 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format,
 	pr.rule.action = PF_PASS;
 	if (ioctl(dev, DIOCGETRULES, &pr)) {
 		warn("DIOCGETRULES");
+		ret = -1;
 		goto error;
 	}
 	mnr = pr.nr;
@@ -793,6 +846,7 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format,
 		pr.nr = nr;
 		if (ioctl(dev, DIOCGETRULE, &pr)) {
 			warn("DIOCGETRULE");
+			ret = -1;
 			goto error;
 		}
 
@@ -815,46 +869,42 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format,
 			}
 			break;
 		case PFCTL_SHOW_RULES:
-			brace = 0;
 			if (pr.rule.label[0] && (opts & PF_OPT_SHOWALL))
 				labels = 1;
 			INDENT(depth, !(opts & PF_OPT_VERBOSE));
-			if (pr.anchor_call[0] &&
-			   ((((p = strrchr(pr.anchor_call, '_')) != NULL) &&
-			   ((void *)p == (void *)pr.anchor_call ||
-			   *(--p) == '/')) || (opts & PF_OPT_RECURSE))) {
-				brace++;
-				if ((p = strrchr(pr.anchor_call, '/')) !=
-				    NULL)
-					p++;
-				else
-					p = &pr.anchor_call[0];
-			} else
-				p = &pr.anchor_call[0];
-		
-			print_rule(&pr.rule, p, rule_numbers);
-			if (brace)
+			print_rule(&pr.rule, pr.anchor_call, rule_numbers);
+
+			/*
+			 * If this is a 'unnamed' brace notation
+			 * anchor, OR the user has explicitly requested
+			 * recursion, print it recursively.
+			 */
+       		        if (pr.anchor_call[0] &&
+			    (((p = strrchr(pr.anchor_call, '/')) ?
+			    p[1] == '_' : pr.anchor_call[0] == '_') ||
+			    opts & PF_OPT_RECURSE)) {
 				printf(" {\n");
-			else
-				printf("\n");
-			pfctl_print_rule_counters(&pr.rule, opts);
-			if (brace) { 
-				pfctl_show_rules(dev, path, opts, format,
-				    p, depth + 1);
+				pfctl_print_rule_counters(&pr.rule, opts);
+				pfctl_show_rules(dev, npath, opts, format,
+				    pr.anchor_call, depth + 1,
+				    pr.rule.anchor_wildcard);
 				INDENT(depth, !(opts & PF_OPT_VERBOSE));
 				printf("}\n");
+			} else {
+				printf("\n");
+				pfctl_print_rule_counters(&pr.rule, opts);
 			}
 			break;
 		case PFCTL_SHOW_NOTHING:
 			break;
 		}
 	}
-	path[len] = '\0';
-	return (0);
 
  error:
+	if (path != npath)
+		free(npath);
 	path[len] = '\0';
-	return (-1);
+	return (ret);
 }
 
 int
@@ -1153,7 +1203,7 @@ pfctl_load_rule(struct pfctl *pf, char *path, struct pf_rule *r, int depth)
 			else
 				snprintf(&path[len], MAXPATHLEN - len,
 				    "%s", r->anchor->name);
-			name = path;
+			name = r->anchor->name;
 		} else
 			name = r->anchor->path;
 	} else
@@ -1170,8 +1220,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, r->anchor ? r->anchor->name : "",
-		    pf->opts & PF_OPT_VERBOSE2);
+		print_rule(r, name, pf->opts & PF_OPT_VERBOSE2);
 	}
 	path[len] = '\0';
 	return (0);
@@ -2065,12 +2114,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 'q':
 			pfctl_show_altq(dev, ifaceopt, opts,
@@ -2095,12 +2144,12 @@ main(int argc, char *argv[])
 			opts |= PF_OPT_SHOWALL;
 			pfctl_load_fingerprints(dev, opts);
 
-			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);
@@ -2121,7 +2170,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 9adeed15ba1..38db748eafd 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_parser.c,v 1.267 2010/07/03 02:05:17 mcbride Exp $ */
+/*	$OpenBSD: pfctl_parser.c,v 1.268 2010/07/03 02:28:57 mcbride Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -680,15 +680,17 @@ print_rule(struct pf_rule *r, const char *anchor_call, int verbose)
 	    "anchor", "nat-anchor", "nat-anchor", "binat-anchor",
 	    "binat-anchor", "rdr-anchor", "rdr-anchor" };
 	int	i, opts;
+	char	*p;
 
 	if (verbose)
 		printf("@%d ", r->nr);
 	if (r->action > PF_MATCH)
 		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