There's a (subtle) bug in anchor creation/handling I haven't quite
pinned down yet:
Nested brace anchors with names end up being loaded under a different
name if their ruleset is empty:
$ pfctl -aa1 -vnf-
anchor a2 {
}
match
^D
anchor "a1/a2" all
match all
$ pfctl -vnf-
anchor a1 {
anchor a2 {
}
match
}
^D
anchor "a1" all {
anchor "_1/a2" all
match all
}
But it works fine without nesting or anonymous brace anchors:
$ pfctl -vnf-
$ pfctl -vnf-
anchor a1 {
}
anchor a2 {
anchor {
}
}
anchor {
}
^D
anchor "a1" all
anchor "a2" all {
anchor all
}
anchor all
Due to how the parser works, an anchor's ruleset `{ ... }' is fully
parsed before the anchor rule `anchor a' itself, so a temporary anchor
gets created to load the ruleset, continue parsing and eventually move
it place the anchor rule completes (see sbin/pfctl/parse.y:836).
That's where these "_1/a2" come from; anchor names starting with an
underscore are reservered for internal use and therefore rejected on
user-input. Any attempt to fill them will fail, hence one must at least
completely refill the parent anchor to get rid of them.
Still looking into the underlying problem, I'd like to generally prevent
such usage of empty brace-delimited blocks as they're of no use and will
only cause trouble.
* `anchor a {}' is equivalent to `anchor a'
* `anchor {}' cannot be filled with `pfctl -a' as it has no name and
`anchor' is invalid, so that is even more useless
If one ever used brace anchors with empty rulesets, above things would
happen. If not, anchors are either filled right away or braces are left
out in the first place, so I'd argue throwing a syntax error here is a
breaking syntax change without actual breakage - quite the opposite.
Thanks to benno for going into yacc details with me.
pfctl regress passes.
Feedback? Objections? OK?
Index: share/man/man5/pf.conf.5
===================================================================
RCS file: /cvs/src/share/man/man5/pf.conf.5,v
retrieving revision 1.577
diff -u -p -r1.577 pf.conf.5
--- share/man/man5/pf.conf.5 12 Jul 2018 05:54:49 -0000 1.577
+++ share/man/man5/pf.conf.5 30 Dec 2018 19:54:52 -0000
@@ -1822,7 +1822,7 @@ An anchor rule can also contain a filter
in a brace-delimited block.
In that case, no separate loading of rules into the anchor
is required.
-Brace delimited blocks may contain rules or other brace-delimited blocks.
+Brace delimited blocks must contain rules or other brace-delimited blocks.
When an anchor is populated this way the anchor name becomes optional.
Since the parser specification for anchor names is a string,
double quote characters
Index: sbin/pfctl/parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.688
diff -u -p -r1.688 parse.y
--- sbin/pfctl/parse.y 15 Nov 2018 03:22:01 -0000 1.688
+++ sbin/pfctl/parse.y 30 Dec 2018 19:58:16 -0000
@@ -501,6 +501,7 @@ int parseport(char *, struct range *r, i
%type <v.weight> optweight
%type <v.i> dir af optimizer syncookie_val
%type <v.i> sourcetrack flush unaryop statelock
+%type <v.i> pfa_anchorlist
%type <v.b> action
%type <v.b> flags flag blockspec prio
%type <v.range> portplain portstar portrange
@@ -813,11 +814,11 @@ anchorname : STRING { $$ =
$1; }
| /* empty */ { $$ = NULL; }
;
-pfa_anchorlist : /* empty */
- | pfa_anchorlist '\n'
- | pfa_anchorlist pfrule '\n'
- | pfa_anchorlist anchorrule '\n'
- | pfa_anchorlist include '\n'
+pfa_anchorlist : /* empty */ { $$ = 0; }
+ | pfa_anchorlist '\n' { $$ = 1; }
+ | pfa_anchorlist pfrule '\n' { $$ = 1; }
+ | pfa_anchorlist anchorrule '\n' { $$ = 1; }
+ | pfa_anchorlist include '\n' { $$ = 1; }
;
pfa_anchor : '{'
@@ -844,6 +845,10 @@ pfa_anchor : '{'
pf->anchor = rs->anchor;
} '\n' pfa_anchorlist '}'
{
+ if (!$4) {
+ yyerror("brace anchors must not be empty");
+ YYERROR;
+ }
pf->alast = pf->anchor;
pf->asd--;
pf->anchor = pf->astack[pf->asd];
