Tables under different anchors may have the same name, but pfctl warns
about such scenarios upon table creation to avoid mixups. Unique and
descriptive names are highly recommended (for sanity).
# pfctl -T replace -t t1
1 table created.
no changes.
# pfctl -T replace -t t1 -a a1
pfctl: warning: namespace collision with <t1> global table.
1 table created.
no changes.
Adding a table to yet another anchor will only ever warn about the one
in the main anchor:
# pfctl -T replace -t t1 -a a2
pfctl: warning: namespace collision with <t1> global table.
1 table created.
no changes.
Adding equally named tables in non-main anchors only will never produce
warnings:
# pfctl -T add -t t2 -a a1
1 table created.
0/0 addresses added.
# pfctl -T add -t t2 -a a2
1 table created.
0/0 addresses added.
Things to improve:
- spot all collisions
- warn on dry runs (`-n') as well
- name offending anchors
For this the anchor name must be passed in, but this is impossible to
do from pfctl's main() upon processing `-f file', where it's done now
since the file may contain table definitions.
This diff moves the call to process_tabledefs() inside the parser where
it's only called when tables actually occur. This way both table and
anchor name is known and warn_namespace_collision()'s `filter == NULL'
semantic can be dropped to further simplify the function.
Besides new warnings on standard error, there's no functional change as
ruleset production/manipulation is not effected; regress passes.
# ./obj/pfctl -T replace -t t3
1 table created.
no changes.
# ./obj/pfctl -T replace -t t3 -a a1
pfctl: warning: table <t3> already defined in anchor "/"
1 table created.
no changes.
# ./obj/pfctl -T replace -t t3 -a a2 -n
pfctl: warning: table <t3> already defined in anchor "/"
pfctl: warning: table <t3> already defined in anchor "a1"
1 table created (dummy).
The main anchor is denoted as "/" as it keeps the logic around warnx(3)
simple while still allowing to copy/paste it as is.
Since it's not just about collisions with the "global" tables (those in
the main anchor), I renamed the function to warn_duplicate_tables() at
no cost/blame churn since signature and callers are touched anyway.
Feedback? Objections? OK?
Index: pfctl.h
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl.h,v
retrieving revision 1.57
diff -u -p -r1.57 pfctl.h
--- pfctl.h 6 Sep 2018 15:07:33 -0000 1.57
+++ pfctl.h 29 Dec 2018 15:59:11 -0000
@@ -79,7 +79,7 @@ void pfctl_clear_tables(const char *, i
void pfctl_show_tables(const char *, int);
int pfctl_command_tables(int, char *[], char *, const char *, char *,
const char *, int);
-void warn_namespace_collision(const char *);
+void warn_duplicate_tables(const char *, const char *);
void pfctl_show_ifaces(const char *, int);
FILE *pfctl_fopen(const char *, const char *);
Index: pfctl.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.361
diff -u -p -r1.361 pfctl.c
--- pfctl.c 27 Dec 2018 16:33:44 -0000 1.361
+++ pfctl.c 29 Dec 2018 15:59:11 -0000
@@ -2690,8 +2690,6 @@ main(int argc, char *argv[])
if (pfctl_rules(dev, rulesopt, opts, optimize,
anchorname, NULL))
error = 1;
- else if (!(opts & PF_OPT_NOACTION))
- warn_namespace_collision(NULL);
}
if (opts & PF_OPT_ENABLE)
Index: parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.688
diff -u -p -r1.688 parse.y
--- parse.y 15 Nov 2018 03:22:01 -0000 1.688
+++ parse.y 29 Dec 2018 15:59:28 -0000
@@ -4075,6 +4075,7 @@ process_tabledef(char *name, struct tabl
if (pf->opts & PF_OPT_VERBOSE)
print_tabledef(name, opts->flags, opts->init_addr,
&opts->init_nodes);
+ warn_duplicate_tables(name, pf->anchor->path);
if (!(pf->opts & PF_OPT_NOACTION) &&
pfctl_define_table(name, opts->flags, opts->init_addr,
pf->anchor->path, &ab, pf->anchor->ruleset.tticket)) {
Index: pfctl_table.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl_table.c,v
retrieving revision 1.78
diff -u -p -r1.78 pfctl_table.c
--- pfctl_table.c 15 Oct 2018 21:15:35 -0000 1.78
+++ pfctl_table.c 29 Dec 2018 15:59:21 -0000
@@ -94,7 +94,8 @@ static const char *istats_text[2][2][2]
goto _error; \
} \
if (nadd) { \
- warn_namespace_collision(table.pfrt_name); \
+ warn_duplicate_tables(table.pfrt_name, \
+ table.pfrt_anchor); \
xprintf(opts, "%d table created", nadd); \
if (opts & PF_OPT_NOACTION) \
return (0); \
@@ -537,12 +538,10 @@ pfctl_define_table(char *name, int flags
}
void
-warn_namespace_collision(const char *filter)
+warn_duplicate_tables(const char *tablename, const char *anchorname)
{
struct pfr_buffer b;
struct pfr_table *t;
- const char *name = NULL, *lastcoll;
- int coll = 0;
bzero(&b, sizeof(b));
b.pfrb_type = PFRB_TABLES;
@@ -558,22 +557,13 @@ warn_namespace_collision(const char *fil
PFRB_FOREACH(t, &b) {
if (!(t->pfrt_flags & PFR_TFLAG_ACTIVE))
continue;
- if (filter != NULL && strcmp(filter, t->pfrt_name))
+ if (!strcmp(anchorname, t->pfrt_anchor))
continue;
- if (!t->pfrt_anchor[0])
- name = t->pfrt_name;
- else if (name != NULL && !strcmp(name, t->pfrt_name)) {
- coll++;
- lastcoll = name;
- name = NULL;
- }
+ if (!strcmp(tablename, t->pfrt_name))
+ warnx("warning: table <%s> already defined"
+ " in anchor \"%s\"", tablename,
+ t->pfrt_anchor[0] ? t->pfrt_anchor : "/");
}
- if (coll == 1)
- warnx("warning: namespace collision with <%s> global table.",
- lastcoll);
- else if (coll > 1)
- warnx("warning: namespace collisions with %d global tables.",
- coll);
pfr_buf_clear(&b);
}
===================================================================
Stats: --- 21 lines 671 chars
Stats: +++ 10 lines 470 chars
Stats: -11 lines
Stats: -201 chars
