git: ee1f417a8609 - main - pf: Check if source nodes use a valid redirection address
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 30 Oct 2025 17:44:40 UTC
The branch main has been updated by ks:
URL: https://cgit.FreeBSD.org/src/commit/?id=ee1f417a8609b8742332950521800502759dd185
commit ee1f417a8609b8742332950521800502759dd185
Author: Kajetan Staszkiewicz <ks@FreeBSD.org>
AuthorDate: 2025-10-20 16:53:50 +0000
Commit: Kajetan Staszkiewicz <ks@FreeBSD.org>
CommitDate: 2025-10-30 17:32:52 +0000
pf: Check if source nodes use a valid redirection address
Source nodes redirect (nat-to, rdr-to, route-to) all further connections
matching the rule which has created the source node. The source node is
valid as long as there are states resulting from the rule or until the
source node lifetime expires. When the rule's redirection pool is
modified (e.g. table contents are changed) the source node is still
valid and it will redirect new connections to invalid target (e.g. a
dead next-hop).
When performing source tracking after finding a source node check if the
redirection address still exists in pool of the rule which has created
this node. If not, delete the source node. This will result in finding a
new redirection address and creation of a new source node.
Reviewed by: kp
Obtained from: OpenBSD
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D53231
---
sys/net/pfvar.h | 4 ++
sys/netpfil/pf/pf.c | 2 +-
sys/netpfil/pf/pf_lb.c | 101 +++++++++++++++++++++++++++++++++++++-
sys/netpfil/pf/pf_table.c | 2 +-
tests/sys/netpfil/pf/src_track.sh | 74 ++++++++++++++++++++++++++++
5 files changed, 179 insertions(+), 4 deletions(-)
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 8aefe514946e..52db00f6ce0b 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2435,6 +2435,7 @@ extern struct pf_ksrc_node *pf_find_src_node(struct pf_addr *,
struct pf_srchash **, pf_sn_types_t, bool);
extern void pf_unlink_src_node(struct pf_ksrc_node *);
extern u_int pf_free_src_nodes(struct pf_ksrc_node_list *);
+extern void pf_free_src_node(struct pf_ksrc_node *);
extern void pf_print_state(struct pf_kstate *);
extern void pf_print_flags(uint16_t);
extern int pf_addr_wrap_neq(struct pf_addr_wrap *,
@@ -2521,6 +2522,9 @@ uint16_t pf_qname2qid(const char *, bool);
void pfr_initialize(void);
void pfr_cleanup(void);
+struct pfr_kentry *
+ pfr_kentry_byaddr(struct pfr_ktable *, struct pf_addr *, sa_family_t,
+ int);
int pfr_match_addr(struct pfr_ktable *, struct pf_addr *, sa_family_t);
void pfr_update_stats(struct pfr_ktable *, struct pf_addr *, sa_family_t,
u_int64_t, int, int, int);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index fd70fb1c8a36..a39f5fe58cd6 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -1007,7 +1007,7 @@ pf_src_node_exists(struct pf_ksrc_node **sn, struct pf_srchash *sh)
return (false);
}
-static void
+void
pf_free_src_node(struct pf_ksrc_node *sn)
{
diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c
index 4b1d74e0e61f..7aeb8266ca8c 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -535,6 +535,63 @@ pf_get_mape_sport(struct pf_pdesc *pd, struct pf_krule *r,
return (1);
}
+static __inline u_short
+pf_check_src_node_valid(struct pf_ksrc_node *sn, struct pf_kpool *rpool)
+{
+ struct pf_addr *raddr, *rmask;
+ struct pf_addr *caddr; /* cached redirection address */
+ struct pf_kpooladdr *pa;
+ sa_family_t raf;
+ sa_family_t caf; /* cached redirection AF */
+ u_short valid = 0;
+
+ KASSERT(sn != NULL, ("sn is NULL"));
+ KASSERT(rpool != NULL, ("rpool is NULL"));
+
+ /* check if the cached entry is still valid */
+
+ if (sn->type == PF_SN_LIMIT) {
+ /* Always valid as it does not store redirection address */
+ return (1);
+ }
+
+ mtx_lock(&rpool->mtx);
+ caddr = &(sn->raddr);
+ caf = sn->raf;
+
+ TAILQ_FOREACH(pa, &rpool->list, entries) {
+ if (PF_AZERO(caddr, caf)) {
+ valid = 1;
+ goto done;
+ } else if (pa->addr.type == PF_ADDR_DYNIFTL) {
+ if (pfr_kentry_byaddr(pa->addr.p.dyn->pfid_kt, caddr, caf, 0)) {
+ valid = 1;
+ goto done;
+ }
+ } else if (pa->addr.type == PF_ADDR_TABLE) {
+ if (pfr_kentry_byaddr(pa->addr.p.tbl, caddr, caf, 0)) {
+ valid = 1;
+ goto done;
+ }
+ } else if (pa->addr.type != PF_ADDR_NOROUTE) {
+ /* PF_ADDR_URPFFAILED, PF_ADDR_RANGE, PF_ADDR_ADDRMASK */
+ raddr = &(pa->addr.v.a.addr);
+ rmask = &(pa->addr.v.a.mask);
+ raf = pa->af;
+ if (raf == caf && pf_match_addr(0, raddr, rmask, caddr, caf)) {
+ valid = 1;
+ goto done;
+ }
+ }
+ /* else PF_ADDR_NOROUTE */
+ }
+
+done:
+ mtx_unlock(&rpool->mtx);
+
+ return (valid);
+}
+
u_short
pf_map_addr(sa_family_t saf, struct pf_krule *r, struct pf_addr *saddr,
struct pf_addr *naddr, struct pfi_kkif **nkif, sa_family_t *naf,
@@ -874,6 +931,45 @@ pf_map_addr_sn(sa_family_t saf, struct pf_krule *r, struct pf_addr *saddr,
if (sn != NULL) {
PF_SRC_NODE_LOCK_ASSERT(sn);
+ /*
+ * Check if source node's redirection address still exists
+ * in pool from which the SN was created. If not, delete it.
+ * Similar to pf_kill_srcnodes(). Unlink the source node
+ * from tree, unlink it from states, then free it. Do not
+ * overlap source node and state locks to avoid LOR.
+ */
+ if (!pf_check_src_node_valid(sn, rpool)) {
+ pf_unlink_src_node(sn);
+ PF_SRC_NODE_UNLOCK(sn);
+ if (V_pf_status.debug >= PF_DEBUG_NOISY) {
+ printf("%s: stale src tracking (%d) ",
+ __func__, sn_type);
+ pf_print_host(saddr, 0, saf);
+ printf(" to ");
+ pf_print_host(&(sn->raddr), 0, sn->raf);
+ if (nkif)
+ printf("@%s", sn->rkif->pfik_name);
+ printf("\n");
+ }
+
+ for (int i = 0; i <= V_pf_hashmask; i++) {
+ struct pf_idhash *ih = &V_pf_idhash[i];
+ struct pf_kstate *st;
+
+ PF_HASHROW_LOCK(ih);
+ LIST_FOREACH(st, &ih->states, entry) {
+ if (st->sns[sn->type] == sn) {
+ st->sns[sn->type] = NULL;
+ }
+ }
+ PF_HASHROW_UNLOCK(ih);
+ }
+ pf_free_src_node(sn);
+ counter_u64_add(V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1);
+ sn = NULL;
+ goto map_addr;
+ }
+
(*naf) = sn->raf;
/* If the supplied address is the same as the current one we've
@@ -902,9 +998,10 @@ pf_map_addr_sn(sa_family_t saf, struct pf_krule *r, struct pf_addr *saddr,
goto done;
}
+map_addr:
/*
- * Source node has not been found. Find a new address and store it
- * in variables given by the caller.
+ * Source node has not been found or is invalid. Find a new address
+ * and store it in variables given by the caller.
*/
if ((reason = pf_map_addr(saf, r, saddr, naddr, nkif, naf, init_addr,
rpool)) != 0) {
diff --git a/sys/netpfil/pf/pf_table.c b/sys/netpfil/pf/pf_table.c
index 73ec18fa7646..cf752ce0de18 100644
--- a/sys/netpfil/pf/pf_table.c
+++ b/sys/netpfil/pf/pf_table.c
@@ -2071,7 +2071,7 @@ pfr_lookup_table(struct pfr_table *tbl)
(struct pfr_ktable *)tbl));
}
-static struct pfr_kentry *
+struct pfr_kentry *
pfr_kentry_byaddr(struct pfr_ktable *kt, struct pf_addr *a, sa_family_t af,
int exact)
{
diff --git a/tests/sys/netpfil/pf/src_track.sh b/tests/sys/netpfil/pf/src_track.sh
index d86b4ce6c466..1b09030f6174 100755
--- a/tests/sys/netpfil/pf/src_track.sh
+++ b/tests/sys/netpfil/pf/src_track.sh
@@ -588,6 +588,79 @@ mixed_af_cleanup()
pft_cleanup
}
+atf_test_case "check_valid" "cleanup"
+check_valid_head()
+{
+ atf_set descr 'Test if source node is invalidated on change in redirection pool'
+ atf_set require.user root
+ atf_set require.progs python3 scapy
+}
+
+check_valid_body()
+{
+ setup_router_server_nat64
+
+ # Clients will connect from another network behind the router.
+ # This allows for using multiple source addresses.
+ jexec router route add -6 ${net_clients_6}::/${net_clients_6_mask} ${net_tester_6_host_tester}
+
+ jexec server1 ifconfig ${epair_server1}b inet6 ${net_server1_6}::42:1/128 alias
+ jexec server1 ifconfig ${epair_server1}b inet6 ${net_server1_6}::42:2/128 alias
+
+ jexec router pfctl -e
+ pft_set_rules router \
+ "set debug loud " \
+ "set state-policy if-bound" \
+ "table <targets> { ${net_server1_6}::42:1 }" \
+ "pass in on ${epair_tester}b \
+ route-to { (${epair_server1}a <targets>) } \
+ sticky-address \
+ proto tcp \
+ keep state"
+
+ atf_check -s exit:0 ${common_dir}/pft_ping.py \
+ --sendif ${epair_tester}a --replyif ${epair_tester}a \
+ --fromaddr ${net_clients_6}::1 --to ${host_server_6} \
+ --ping-type=tcp3way --send-sport=4201
+
+ # A source node is created using the original redirection target
+ nodes=$(mktemp) || exit 1
+ jexec router pfctl -qvvsS | normalize_pfctl_s > $nodes
+ node_regexp='2001:db8:44::1 -> 2001:db8:4201::42:1 .* states 1,.* route sticky-address'
+ grep -qE "${node_regexp}" $nodes || atf_fail "Source node not found for '${node_regexp}'"
+
+ # Change contents of the redirection table
+ echo ${net_server1_6}::42:2 | jexec router pfctl -Tr -t targets -f -
+
+ atf_check -s exit:0 ${common_dir}/pft_ping.py \
+ --sendif ${epair_tester}a --replyif ${epair_tester}a \
+ --fromaddr ${net_clients_6}::1 --to ${host_server_6} \
+ --ping-type=tcp3way --send-sport=4202
+
+ # The original source node was deleted, a new one was created.
+ # It has 1 states.
+ jexec router pfctl -qvvsS | normalize_pfctl_s > $nodes
+ node_regexp='2001:db8:44::1 -> 2001:db8:4201::42:2 .* states 1,.* route sticky-address'
+ grep -qE "${node_regexp}" $nodes || atf_fail "Source node not found for '${node_regexp}'"
+
+ atf_check -s exit:0 ${common_dir}/pft_ping.py \
+ --sendif ${epair_tester}a --replyif ${epair_tester}a \
+ --fromaddr ${net_clients_6}::1 --to ${host_server_6} \
+ --ping-type=tcp3way --send-sport=4203
+
+ # Without redirection table change the source node is reused.
+ # It has 2 states.
+ jexec router pfctl -qvvsS | normalize_pfctl_s > $nodes
+ node_regexp='2001:db8:44::1 -> 2001:db8:4201::42:2 .* states 2,.* route sticky-address'
+ grep -qE "${node_regexp}" $nodes || atf_fail "Source node not found for '${node_regexp}'"
+}
+
+check_valid_cleanup()
+{
+ pft_cleanup
+}
+
+
atf_init_test_cases()
{
atf_add_test_case "source_track"
@@ -598,4 +671,5 @@ atf_init_test_cases()
atf_add_test_case "sn_types_compat"
atf_add_test_case "sn_types_pass"
atf_add_test_case "mixed_af"
+ atf_add_test_case "check_valid"
}