This change allows user to define table inside the anchor like that:

	anchor foo {
		table <bar> { 192.168.1.1 }
		pass in from <bar> to <self>
	}
Without this diff one must either create table <bar> in main
ruleset (root) or use 'pfctl -a foo -t bar -T add 192.168.1.1'
This glitch is hard to notice. Not many human admins try to attach
tables to non-global anchors. Deamons which configure pf(4) automatically
at run time such as relayd(8) and spamd(8) create tables attached to
thair anchors (for example 'relayd/*') but the deamons use way similar
to pfctl(8) to add and manage those tables.

The reason why I'd like to seal this gap is that my long term goal
is to turn global `pfr_ktable` in pf(4) into member of pf_anchor.
So each ruleset will get its own tree of tables.

feedback and OK bluhm@

1 files changed, 52 insertions, 10 deletions

diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 27cc175c871..825bd4cf04c 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl.c,v 1.394 2024/02/02 08:23:29 sashan Exp $ */
+/*	$OpenBSD: pfctl.c,v 1.395 2024/07/14 19:51:08 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1424,6 +1424,41 @@ pfctl_check_qassignments(struct pf_ruleset *rs)
 	return (errs);
 }
 
+static int
+pfctl_load_tables(struct pfctl *pf, char *path, struct pf_anchor *a)
+{
+	struct pfr_ktable *kt, *ktw;
+	struct pfr_uktable *ukt;
+	uint32_t ticket;
+	char anchor_path[PF_ANCHOR_MAXPATH];
+	int e;
+
+	RB_FOREACH_SAFE(kt, pfr_ktablehead, &pfr_ktables, ktw) {
+		if (strcmp(kt->pfrkt_anchor, a->path) != 0)
+			continue;
+
+		if (path != NULL && *path) {
+			strlcpy(anchor_path, kt->pfrkt_anchor,
+			    sizeof (anchor_path));
+			snprintf(kt->pfrkt_anchor, PF_ANCHOR_MAXPATH, "%s/%s",
+			    path, anchor_path);
+		}
+		ukt = (struct pfr_uktable *) kt;
+		ticket = pfctl_get_ticket(pf->trans, PF_TRANS_TABLE, path);
+		e = pfr_ina_define(&ukt->pfrukt_t, ukt->pfrukt_addrs.pfrb_caddr,
+		    ukt->pfrukt_addrs.pfrb_size, NULL, NULL, ticket,
+		    ukt->pfrukt_init_addr ? PFR_FLAG_ADDRSTOO : 0);
+		if (e != 0)
+			err(1, "%s pfr_ina_define() %s@%s", __func__,
+			    kt->pfrkt_name, kt->pfrkt_anchor);
+		RB_REMOVE(pfr_ktablehead, &pfr_ktables, kt);
+		pfr_buf_clear(&ukt->pfrukt_addrs);
+		free(ukt);
+	}
+
+	return (0);
+}
+
 int
 pfctl_load_ruleset(struct pfctl *pf, char *path, struct pf_ruleset *rs,
     int depth)
@@ -1469,6 +1504,8 @@ pfctl_load_ruleset(struct pfctl *pf, char *path, struct pf_ruleset *rs,
 			if ((error = pfctl_load_ruleset(pf, path,
 			    &r->anchor->ruleset, depth + 1)))
 				goto error;
+			if ((error = pfctl_load_tables(pf, path, r->anchor)))
+				goto error;
 		} else if (pf->opts & PF_OPT_VERBOSE)
 			printf("\n");
 		free(r);
@@ -1495,8 +1532,11 @@ pfctl_load_rule(struct pfctl *pf, char *path, struct pf_rule *r, int depth)
 
 	bzero(&pr, sizeof(pr));
 	/* set up anchor before adding to path for anchor_call */
-	if ((pf->opts & PF_OPT_NOACTION) == 0)
+	if ((pf->opts & PF_OPT_NOACTION) == 0) {
+		if (pf->trans == NULL)
+			errx(1, "pfctl_load_rule: no transaction");
 		pr.ticket = pfctl_get_ticket(pf->trans, PF_TRANS_RULESET, path);
+	}
 	if (strlcpy(pr.anchor, path, sizeof(pr.anchor)) >= sizeof(pr.anchor))
 		errx(1, "pfctl_load_rule: strlcpy");
 
@@ -1535,8 +1575,8 @@ int
 pfctl_rules(int dev, char *filename, int opts, int optimize,
     char *anchorname, struct pfr_buffer *trans)
 {
-#define ERR(x) do { warn(x); goto _error; } while(0)
-#define ERRX(x) do { warnx(x); goto _error; } while(0)
+#define ERR(...) do { warn(__VA_ARGS__); goto _error; } while(0)
+#define ERRX(...) do { warnx(__VA_ARGS__); goto _error; } while(0)
 
 	struct pfr_buffer	*t, buf;
 	struct pfctl		 pf;
@@ -1549,9 +1589,13 @@ pfctl_rules(int dev, char *filename, int opts, int optimize,
 	RB_INIT(&pf_anchors);
 	memset(&pf_main_anchor, 0, sizeof(pf_main_anchor));
 	pf_init_ruleset(&pf_main_anchor.ruleset);
+	memset(&pf, 0, sizeof(pf));
+	memset(&trs, 0, sizeof(trs));
+
 	if (trans == NULL) {
 		bzero(&buf, sizeof(buf));
 		buf.pfrb_type = PFRB_TRANS;
+		pf.trans = &buf;
 		t = &buf;
 		osize = 0;
 	} else {
@@ -1559,20 +1603,18 @@ pfctl_rules(int dev, char *filename, int opts, int optimize,
 		osize = t->pfrb_size;
 	}
 
-	memset(&pf, 0, sizeof(pf));
-	memset(&trs, 0, sizeof(trs));
 	if ((path = calloc(1, PATH_MAX)) == NULL)
-		ERRX("pfctl_rules: calloc");
+		ERR("%s: calloc", __func__);
 	if (strlcpy(trs.pfrt_anchor, anchorname,
 	    sizeof(trs.pfrt_anchor)) >= sizeof(trs.pfrt_anchor))
-		ERRX("pfctl_rules: strlcpy");
+		ERRX("%s: strlcpy", __func__);
 	pf.dev = dev;
 	pf.opts = opts;
 	pf.optimize = optimize;
 
 	/* non-brace anchor, create without resolving the path */
 	if ((pf.anchor = calloc(1, sizeof(*pf.anchor))) == NULL)
-		ERRX("pfctl_rules: calloc");
+		ERR("%s: calloc", __func__);
 	rs = &pf.anchor->ruleset;
 	pf_init_ruleset(rs);
 	rs->anchor = pf.anchor;
@@ -1637,7 +1679,7 @@ pfctl_rules(int dev, char *filename, int opts, int optimize,
 		/*
 		 * process "load anchor" directives that might have used queues
 		 */
-		if (pfctl_load_anchors(dev, &pf, t) == -1)
+		if (pfctl_load_anchors(dev, &pf) == -1)
 			ERRX("load anchors");
 		pfctl_clear_queues(&qspecs);
 		pfctl_clear_queues(&rootqs);