Hello,
this diff is part of the 'big patch' [1] to pfctl I've sent while back. The
pfctl fails to handle nested 'load anchor' statements properly, when ruleset is
being loaded to non-root anchor (e.g. pfctl -a regress ...), see [1] to find
more details about the issue.
The first step towards the fix is to fix pfctl_rules() to let it recursively
load rulesets defined as 'load anchor':
1661 /* process "load anchor" directives that might have used queues */
1662 if (!anchorname[0]) {
1663 if (pfctl_load_anchors(dev, &pf, t) == -1)
1664 ERRX("load anchors");
1665 pfctl_clear_queues(&qspecs);
1666 pfctl_clear_queues(&rootqs);
1667 }
the current version recurses only, when pfctl is running _without_ '-a' option.
Also I'm piggy backing yet another fix to 'name vs. path'. the bug occurs on
parse.y, where we handle 'load anchor'. I've just spotted this problem, when
I was hunting down the problem with '-a'.
OK?
thanks and
regards
sasha
[1] https://marc.info/?l=openbsd-tech&m=151148479226177&w=2
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index e1dcfbc382f..1016c909ccc 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -939,7 +939,7 @@ anchorrule : ANCHOR anchorname dir quick interface af
proto fromto
loadrule : LOAD ANCHOR string FROM string {
struct loadanchors *loadanchor;
- if (strlen(pf->anchor->name) + 1 +
+ if (strlen(pf->anchor->path) + 1 +
strlen($3) >= PATH_MAX) {
yyerror("anchorname %s too long, max %u\n",
$3, PATH_MAX - 1);
@@ -954,7 +954,7 @@ loadrule : LOAD ANCHOR string FROM string {
err(1, "loadrule: malloc");
if (pf->anchor->name[0])
snprintf(loadanchor->anchorname, PATH_MAX,
- "%s/%s", pf->anchor->name, $3);
+ "%s/%s", pf->anchor->path, $3);
else
strlcpy(loadanchor->anchorname, $3, PATH_MAX);
if ((loadanchor->filename = strdup($5)) == NULL)
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index d6cfc132c0e..044e8767bae 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1658,20 +1658,21 @@ pfctl_rules(int dev, char *filename, int opts, int
optimize,
free(path);
path = NULL;
- /* process "load anchor" directives that might have used queues */
- if (!anchorname[0]) {
+ if (trans == NULL) {
+ /*
+ * process "load anchor" directives that might have used queues
+ */
if (pfctl_load_anchors(dev, &pf, t) == -1)
ERRX("load anchors");
pfctl_clear_queues(&qspecs);
pfctl_clear_queues(&rootqs);
- }
- if (trans == NULL && (opts & PF_OPT_NOACTION) == 0) {
- if (!anchorname[0])
- if (pfctl_load_options(&pf))
+ if ((opts & PF_OPT_NOACTION) == 0) {
+ if (!anchorname[0] && pfctl_load_options(&pf))
goto _error;
- if (pfctl_trans(dev, t, DIOCXCOMMIT, osize))
- ERR("DIOCXCOMMIT");
+ if (pfctl_trans(dev, t, DIOCXCOMMIT, osize))
+ ERR("DIOCXCOMMIT");
+ }
}
return (0);
