Hello,
the issue has been reported by Gianni Kapetanakis month ago [1]. It took
several emails to figure out relayd(8) exists after hosts got disabled
by 'relayctl host dis ...'
The thing is that relayd(8) relies on pf(4) to create persistent
tables (PFR_TFLAG_PERSIST) as relayd requests that:
47 void
48 init_tables(struct relayd *env)
49 {
...
62 TAILQ_FOREACH(rdr, env->sc_rdrs, entry) {
63 if (strlcpy(tables[i].pfrt_anchor, RELAYD_ANCHOR "/",
64 sizeof(tables[i].pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE)
65 goto toolong;
66 if (strlcat(tables[i].pfrt_anchor, rdr->conf.name,
67 sizeof(tables[i].pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE)
68 goto toolong;
69 if (strlcpy(tables[i].pfrt_name, rdr->conf.name,
70 sizeof(tables[i].pfrt_name)) >=
71 sizeof(tables[i].pfrt_name))
72 goto toolong;
73 tables[i].pfrt_flags |= PFR_TFLAG_PERSIST;
74 i++;
75 }
unfortunately it's not the case as further investigation revealed [2].
the issue can be easily reproduced by pfctl(8) which also creates
persistent tables on behalf of command line:
pfctl -t foo -T add ...
command above always asks pf(4) to create persistent table, however
pf(4) does not honor persistent flag when <foo> table exists already.
One can verify that using commands as follows:
## create 'referenced' table only (table exists but has no active flag)
# echo 'pass from in <foo> to any' |pfctl -f -
# pfctl -sT -vg
----r-- foo
# create instance of table <foo> using command line:
# pfctl -t foo -T add 192.168.1.0/24
1/1 addresses added.
# pfctl -sT -vg
--a-r-- foo
## create instance of table <bar>, note the table will get 'p' flag
# pfctl -t bar -T add 192.168.10.0/24
1 table created.
1/1 addresses added.
# pfctl -sT -vg
-pa---- bar
--a-r-- foo
one-liner change to sys/net/pf_table.c fixes that it also works for Gianni
Kapetanakis. I'm also adding tests to regress/sys/net/pf_table/Makefile
to cover it.
On system which runs current the test fails with error as follows:
pfctl -a regress/ttest -t instance -T add 192.168.1.0/24
1/1 addresses added.
pfctl -a regress/ttest -sT -vg | diff table-persist.out -
1c1
< -pa-r-- instance regress/ttest
---
> --a-r-- instance regress/ttest
*** Error 1 in . (Makefile:96 'flags')
FAILED
the failure is expected on system without patch. On system with
patch applied all tests do pass.
OK to commit?
thanks and
regards
sashan
[1] https://marc.info/?t=168811270400005&r=1&w=2
[2] https://marc.info/?l=openbsd-bugs&m=168868165801905&w=2
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index 6f23a6f795d..c862c804f84 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -1565,8 +1565,10 @@ pfr_add_tables(struct pfr_table *tbl, int size, int
*nadd, int flags)
xadd++;
} else if (!(flags & PFR_FLAG_DUMMY) &&
!(p->pfrkt_flags & PFR_TFLAG_ACTIVE)) {
- p->pfrkt_nflags = (p->pfrkt_flags &
- ~PFR_TFLAG_USRMASK) | PFR_TFLAG_ACTIVE;
+ p->pfrkt_nflags =
+ (p->pfrkt_flags & ~PFR_TFLAG_USRMASK) |
+ (n->pfrkt_flags & PFR_TFLAG_USRMASK) |
+ PFR_TFLAG_ACTIVE;
SLIST_INSERT_HEAD(&changeq, p, pfrkt_workq);
}
}
diff --git a/regress/sys/net/pf_table/Makefile
b/regress/sys/net/pf_table/Makefile
index a71f0190c73..8911e8a1d35 100644
--- a/regress/sys/net/pf_table/Makefile
+++ b/regress/sys/net/pf_table/Makefile
@@ -1,15 +1,26 @@
# $OpenBSD: Makefile,v 1.3 2017/07/07 23:15:27 bluhm Exp $
-REGRESS_TARGETS= hit miss cleanup
-CLEANFILES= stamp-*
+REGRESS_TARGETS= hit miss cleanup flags
+CLEANFILES= stamp-* \
+ pf-reftab.conf \
+ pf-instance.conf \
+ table-ref.conf \
+ table-pgone.out \
+ table-persist.out \
+ table-ref.out \
+ table-refgone.out
+
stamp-setup:
+ ${SUDO} pfctl -a regress/ttest -Fa
${SUDO} pfctl -qt __regress_tbl -T add -f ${.CURDIR}/table.in
date >$@
cleanup:
rm -f stamp-setup
${SUDO} pfctl -qt __regress_tbl -T kill
+ ${SUDO} pfctl -q -a regress/ttest -Fr
+ ${SUDO} pfctl -q -a regress/ttest -qt instance -T kill
hit: stamp-setup
for i in `cat ${.CURDIR}/table.hit`; do \
@@ -27,6 +38,77 @@ miss: stamp-setup
done; \
exit 0
-.PHONY: hit miss
+#
+# tables <instance> and <reference> are both referenced by rule only
+#
+pf-instab.conf:
+ @echo 'table <instance> { 192.168.1.0/24 }' > $@
+ @echo 'pass in from <instance> to <reference>' >> $@
+
+#
+# table <instance> is active and referred by rule, table <reference>
+# is referenced only.
+pf-reftab.conf:
+ @echo 'pass in from <instance> to <reference>' > $@
+
+#
+# check persistent flag (p) is gone from table <instance> after
+# we load pf-instab.conf. Deals with case when persistent table <instance>
+# exists before pf-instab.conf gets loaded.
+#
+table-pgone.out:
+ @echo '--a-r-- instance regress/ttest' > $@
+ @echo '----r-- reference regress/ttest' >> $@
+
+#
+# verify table <instance> got persistent flag after we
+# run 'pfctl -t instance -T add ...'
+#
+table-persist.out:
+ @echo '-pa-r-- instance regress/ttest' > $@
+ @echo '----r-- reference regress/ttest' >> $@
+
+#
+# verify tables <instance> and <reference> are created on behalf of
+# reference by rule after pf-reftab.conf got loaded.
+#
+table-ref.out:
+ @echo '----r-- instance regress/ttest' > $@
+ @echo '----r-- reference regress/ttest' >> $@
+
+#
+# verify reference to <instance> table (persistent) is gone
+# after rules got flushed
+#
+table-refgone.out:
+ @echo '-pa---- instance regress/ttest' > $@
+
+flags: pf-instab.conf pf-reftab.conf table-pgone.out table-persist.out \
+ table-ref.out table-refgone.out
+ @echo 'loading pf-reftab,conf (tables referenced by rules only)'
+ @cat pf-reftab.conf
+ ${SUDO} pfctl -a regress/ttest -f pf-reftab.conf
+ @echo 'tables <reference> and <instance> should both have ----r--'
+ ${SUDO} pfctl -a regress/ttest -sT -vg | diff table-ref.out -
+ @echo 'creating <instance> table on command line, flags should be:'
+ @cat table-persist.out
+ ${SUDO} pfctl -a regress/ttest -t instance -T add 192.168.1.0/24
+ ${SUDO} pfctl -a regress/ttest -sT -vg | diff table-persist.out -
+ @echo 'flushing rules'
+ ${SUDO} pfctl -a regress/ttest -Fr
+ @echo 'table <reference> should be gone, table <instance> should stay'
+ ${SUDO} pfctl -a regress/ttest -sT -vg | diff table-refgone.out -
+ @echo 'loading pf-instab.conf'
+ @cat pf-instab.conf
+ ${SUDO} pfctl -a regress/ttest -f pf-instab.conf
+ @echo 'table <instance> loses -p- flag:'
+ @cat table-pgone.out
+ ${SUDO} pfctl -a regress/ttest -sT -vg | diff table-pgone.out -
+ @echo 'flusing rules, both tables should be gone'
+ ${SUDO} pfctl -a regress/ttest -Fr
+ @echo 'anchor regress/ttest must be gone'
+ ${SUDO} pfctl -a regress/ttest -sr 2>&1 | grep 'pfctl: Anchor does not
exist'
+
+.PHONY: hit miss flags
.include <bsd.regress.mk>
