git: 4a7c6d6206cf - main - pf: Fix handling of v6 loopback connections with pf syncookies enabled
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 29 Oct 2024 15:50:07 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=4a7c6d6206cf0851e11ecd38ab5c618f67892dab
commit 4a7c6d6206cf0851e11ecd38ab5c618f67892dab
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-10-29 14:59:15 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-10-29 15:01:20 +0000
pf: Fix handling of v6 loopback connections with pf syncookies enabled
The SYN|ACK generated by pf needs to inherit M_LOOP from the original
SYN, otherwise it gets dropped by ip6_input().
Fix this by adding an mbuf_flags argument to pf_build_tcp() that can be
used to set both M_SKIP_FIREWALL and M_LOOP as needed. Set M_LOOP on
the output mbuf if it was generated in response to an mbuf with M_LOOP
set.
Add a regression test case. The v4 case had no problems, but the v6
case fails without this change.
Reviewed by: kp
MFC after: 1 month
Sponsored by: Klara, Inc.
Sponsored by: Zenarmor
Differential Revision: https://reviews.freebsd.org/D47257
---
sys/net/pfvar.h | 4 +-
sys/netpfil/pf/pf.c | 44 ++++++++++++--------
sys/netpfil/pf/pf_syncookies.c | 8 ++--
tests/sys/netpfil/pf/syncookie.sh | 86 +++++++++++++++++++++++++++++++++++++++
4 files changed, 119 insertions(+), 23 deletions(-)
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 30be1128d4d3..0dc5d4363867 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2491,12 +2491,12 @@ u_int8_t pf_get_wscale(struct pf_pdesc *);
struct mbuf *pf_build_tcp(const struct pf_krule *, sa_family_t,
const struct pf_addr *, const struct pf_addr *,
u_int16_t, u_int16_t, u_int32_t, u_int32_t,
- u_int8_t, u_int16_t, u_int16_t, u_int8_t, bool,
+ u_int8_t, u_int16_t, u_int16_t, u_int8_t, int,
u_int16_t, u_int16_t, int);
void pf_send_tcp(const struct pf_krule *, sa_family_t,
const struct pf_addr *, const struct pf_addr *,
u_int16_t, u_int16_t, u_int32_t, u_int32_t,
- u_int8_t, u_int16_t, u_int16_t, u_int8_t, bool,
+ u_int8_t, u_int16_t, u_int16_t, u_int8_t, int,
u_int16_t, u_int16_t, int);
void pf_syncookies_init(void);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index bd8b709e396e..a98baeb4bdec 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -2207,6 +2207,9 @@ pf_intr(void *v)
#ifdef INET
case PFSE_IP: {
if (pf_isforlocal(pfse->pfse_m, AF_INET)) {
+ KASSERT(pfse->pfse_m->m_pkthdr.rcvif == V_loif,
+ ("%s: rcvif != loif", __func__));
+
pfse->pfse_m->m_flags |= M_SKIP_FIREWALL;
pfse->pfse_m->m_pkthdr.csum_flags |=
CSUM_IP_VALID | CSUM_IP_CHECKED;
@@ -2225,7 +2228,11 @@ pf_intr(void *v)
#ifdef INET6
case PFSE_IP6:
if (pf_isforlocal(pfse->pfse_m, AF_INET6)) {
- pfse->pfse_m->m_flags |= M_SKIP_FIREWALL;
+ KASSERT(pfse->pfse_m->m_pkthdr.rcvif == V_loif,
+ ("%s: rcvif != loif", __func__));
+
+ pfse->pfse_m->m_flags |= M_SKIP_FIREWALL |
+ M_LOOP;
ip6_input(pfse->pfse_m);
} else {
ip6_output(pfse->pfse_m, NULL, NULL, 0, NULL,
@@ -2574,7 +2581,8 @@ pf_unlink_state(struct pf_kstate *s)
s->key[PF_SK_WIRE]->port[1],
s->key[PF_SK_WIRE]->port[0],
s->src.seqhi, s->src.seqlo + 1,
- TH_RST|TH_ACK, 0, 0, 0, true, s->tag, 0, s->act.rtableid);
+ TH_RST|TH_ACK, 0, 0, 0, M_SKIP_FIREWALL, s->tag, 0,
+ s->act.rtableid);
}
LIST_REMOVE(s, entry);
@@ -3334,7 +3342,7 @@ pf_build_tcp(const struct pf_krule *r, sa_family_t af,
const struct pf_addr *saddr, const struct pf_addr *daddr,
u_int16_t sport, u_int16_t dport, u_int32_t seq, u_int32_t ack,
u_int8_t tcp_flags, u_int16_t win, u_int16_t mss, u_int8_t ttl,
- bool skip_firewall, u_int16_t mtag_tag, u_int16_t mtag_flags, int rtableid)
+ int mbuf_flags, u_int16_t mtag_tag, u_int16_t mtag_flags, int rtableid)
{
struct mbuf *m;
int len, tlen;
@@ -3380,8 +3388,7 @@ pf_build_tcp(const struct pf_krule *r, sa_family_t af,
m_freem(m);
return (NULL);
}
- if (skip_firewall)
- m->m_flags |= M_SKIP_FIREWALL;
+ m->m_flags |= mbuf_flags;
pf_mtag->tag = mtag_tag;
pf_mtag->flags = mtag_flags;
@@ -3598,13 +3605,13 @@ pf_send_tcp(const struct pf_krule *r, sa_family_t af,
const struct pf_addr *saddr, const struct pf_addr *daddr,
u_int16_t sport, u_int16_t dport, u_int32_t seq, u_int32_t ack,
u_int8_t tcp_flags, u_int16_t win, u_int16_t mss, u_int8_t ttl,
- bool skip_firewall, u_int16_t mtag_tag, u_int16_t mtag_flags, int rtableid)
+ int mbuf_flags, u_int16_t mtag_tag, u_int16_t mtag_flags, int rtableid)
{
struct pf_send_entry *pfse;
struct mbuf *m;
m = pf_build_tcp(r, af, saddr, daddr, sport, dport, seq, ack, tcp_flags,
- win, mss, ttl, skip_firewall, mtag_tag, mtag_flags, rtableid);
+ win, mss, ttl, mbuf_flags, mtag_tag, mtag_flags, rtableid);
if (m == NULL)
return;
@@ -3672,7 +3679,7 @@ pf_return(struct pf_krule *r, struct pf_krule *nr, struct pf_pdesc *pd,
pf_send_tcp(r, pd->af, pd->dst,
pd->src, th->th_dport, th->th_sport,
ntohl(th->th_ack), ack, TH_RST|TH_ACK, 0, 0,
- r->return_ttl, true, 0, 0, rtableid);
+ r->return_ttl, M_SKIP_FIREWALL, 0, 0, rtableid);
}
} else if (pd->proto == IPPROTO_SCTP &&
(r->rule_flag & PFRULE_RETURN)) {
@@ -5537,7 +5544,7 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr, struct pf_krule *a,
s->src.mss = mss;
pf_send_tcp(r, pd->af, pd->dst, pd->src, th->th_dport,
th->th_sport, s->src.seqhi, ntohl(th->th_seq) + 1,
- TH_SYN|TH_ACK, 0, s->src.mss, 0, true, 0, 0,
+ TH_SYN|TH_ACK, 0, s->src.mss, 0, M_SKIP_FIREWALL, 0, 0,
pd->act.rtableid);
REASON_SET(&reason, PFRES_SYNPROXY);
return (PF_SYNPROXY_DROP);
@@ -5898,8 +5905,8 @@ pf_tcp_track_full(struct pf_kstate **state, struct pf_pdesc *pd,
pd->dst, pd->src, th->th_dport,
th->th_sport, ntohl(th->th_ack), 0,
TH_RST, 0, 0,
- (*state)->rule->return_ttl, true, 0, 0,
- (*state)->act.rtableid);
+ (*state)->rule->return_ttl, M_SKIP_FIREWALL,
+ 0, 0, (*state)->act.rtableid);
src->seqlo = 0;
src->seqhi = 1;
src->max_win = 1;
@@ -6035,8 +6042,8 @@ pf_synproxy(struct pf_pdesc *pd, struct pf_kstate **state, u_short *reason)
pf_send_tcp((*state)->rule, pd->af, pd->dst,
pd->src, th->th_dport, th->th_sport,
(*state)->src.seqhi, ntohl(th->th_seq) + 1,
- TH_SYN|TH_ACK, 0, (*state)->src.mss, 0, true, 0, 0,
- (*state)->act.rtableid);
+ TH_SYN|TH_ACK, 0, (*state)->src.mss, 0,
+ M_SKIP_FIREWALL, 0, 0, (*state)->act.rtableid);
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_SYNPROXY_DROP);
} else if ((th->th_flags & (TH_ACK|TH_RST|TH_FIN)) != TH_ACK ||
@@ -6067,8 +6074,9 @@ pf_synproxy(struct pf_pdesc *pd, struct pf_kstate **state, u_short *reason)
&sk->addr[pd->sidx], &sk->addr[pd->didx],
sk->port[pd->sidx], sk->port[pd->didx],
(*state)->dst.seqhi, 0, TH_SYN, 0,
- (*state)->src.mss, 0, false, (*state)->tag, 0,
- (*state)->act.rtableid);
+ (*state)->src.mss, 0,
+ (*state)->orig_kif->pfik_ifp == V_loif ? M_LOOP : 0,
+ (*state)->tag, 0, (*state)->act.rtableid);
REASON_SET(reason, PFRES_SYNPROXY);
return (PF_SYNPROXY_DROP);
} else if (((th->th_flags & (TH_SYN|TH_ACK)) !=
@@ -6082,14 +6090,14 @@ pf_synproxy(struct pf_pdesc *pd, struct pf_kstate **state, u_short *reason)
pf_send_tcp((*state)->rule, pd->af, pd->dst,
pd->src, th->th_dport, th->th_sport,
ntohl(th->th_ack), ntohl(th->th_seq) + 1,
- TH_ACK, (*state)->src.max_win, 0, 0, false,
+ TH_ACK, (*state)->src.max_win, 0, 0, 0,
(*state)->tag, 0, (*state)->act.rtableid);
pf_send_tcp((*state)->rule, pd->af,
&sk->addr[pd->sidx], &sk->addr[pd->didx],
sk->port[pd->sidx], sk->port[pd->didx],
(*state)->src.seqhi + 1, (*state)->src.seqlo + 1,
- TH_ACK, (*state)->dst.max_win, 0, 0, true, 0, 0,
- (*state)->act.rtableid);
+ TH_ACK, (*state)->dst.max_win, 0, 0,
+ M_SKIP_FIREWALL, 0, 0, (*state)->act.rtableid);
(*state)->src.seqdiff = (*state)->dst.seqhi -
(*state)->src.seqlo;
(*state)->dst.seqdiff = (*state)->src.seqhi -
diff --git a/sys/netpfil/pf/pf_syncookies.c b/sys/netpfil/pf/pf_syncookies.c
index da25ac47fe3c..3a0e23100f7c 100644
--- a/sys/netpfil/pf/pf_syncookies.c
+++ b/sys/netpfil/pf/pf_syncookies.c
@@ -298,7 +298,8 @@ pf_syncookie_send(struct pf_pdesc *pd)
iss = pf_syncookie_generate(pd, mss);
pf_send_tcp(NULL, pd->af, pd->dst, pd->src, *pd->dport, *pd->sport,
iss, ntohl(pd->hdr.tcp.th_seq) + 1, TH_SYN|TH_ACK, 0, mss,
- 0, true, 0, 0, pd->act.rtableid);
+ 0, M_SKIP_FIREWALL | (pd->m->m_flags & M_LOOP), 0, 0,
+ pd->act.rtableid);
counter_u64_add(V_pf_status.lcounters[KLCNT_SYNCOOKIES_SENT], 1);
/* XXX Maybe only in adaptive mode? */
atomic_add_64(&V_pf_status.syncookies_inflight[V_pf_syncookie_status.oddeven],
@@ -515,6 +516,7 @@ pf_syncookie_recreate_syn(struct pf_pdesc *pd)
wscale = pf_syncookie_wstab[cookie.flags.wscale_idx];
return (pf_build_tcp(NULL, pd->af, pd->src, pd->dst, *pd->sport,
- *pd->dport, seq, 0, TH_SYN, wscale, mss, pd->ttl, false, 0,
- PF_MTAG_FLAG_SYNCOOKIE_RECREATED, pd->act.rtableid));
+ *pd->dport, seq, 0, TH_SYN, wscale, mss, pd->ttl,
+ (pd->m->m_flags & M_LOOP), 0, PF_MTAG_FLAG_SYNCOOKIE_RECREATED,
+ pd->act.rtableid));
}
diff --git a/tests/sys/netpfil/pf/syncookie.sh b/tests/sys/netpfil/pf/syncookie.sh
index ac7483bc258b..6741ad28adf0 100644
--- a/tests/sys/netpfil/pf/syncookie.sh
+++ b/tests/sys/netpfil/pf/syncookie.sh
@@ -233,6 +233,90 @@ forward_v6_cleanup()
pft_cleanup
}
+loopback_test()
+{
+ local addr port
+
+ addr=$1
+ port=$2
+
+ # syncookies don't work without state tracking enabled.
+ atf_check -e ignore pfctl -e
+ atf_check pfctl -f - <<__EOF__
+set syncookies always
+pass all keep state
+__EOF__
+
+ # Try to transmit data over a loopback connection.
+ cat <<__EOF__ >in
+Creativity, no.
+__EOF__
+ nc -l $addr $port >out &
+
+ atf_check nc -N $addr $port < in
+
+ atf_check -o file:in cat out
+
+ atf_check -e ignore pfctl -d
+}
+
+atf_test_case "loopback" "cleanup"
+loopback_head()
+{
+ atf_set descr 'Make sure that loopback v4 TCP connections work with syncookies on'
+ atf_set require.user root
+}
+
+loopback_body()
+{
+ local epair
+
+ pft_init
+
+ atf_check ifconfig lo0 127.0.0.1/8
+ atf_check ifconfig lo0 up
+
+ loopback_test 127.0.0.1 8080
+
+ epair=$(vnet_mkepair)
+ atf_check ifconfig ${epair}a inet 192.0.2.1/24
+
+ loopback_test 192.0.2.1 8081
+}
+
+loopback_cleanup()
+{
+ pft_cleanup
+}
+
+atf_test_case "loopback_v6" "cleanup"
+loopback_v6_head()
+{
+ atf_set descr 'Make sure that loopback v6 TCP connections work with syncookies on'
+ atf_set require.user root
+}
+
+loopback_v6_body()
+{
+ local epair
+
+ pft_init
+
+ atf_check ifconfig lo0 up
+
+ loopback_test ::1 8080
+
+ epair=$(vnet_mkepair)
+ atf_check ifconfig ${epair}a inet6 2001:db8::1/64
+
+ loopback_test 2001:db8::1 8081
+}
+
+loopback_v6_cleanup()
+{
+ pft_cleanup
+}
+
atf_test_case "nostate" "cleanup"
nostate_head()
{
@@ -483,6 +567,8 @@ atf_init_test_cases()
atf_add_test_case "basic_v6"
atf_add_test_case "forward"
atf_add_test_case "forward_v6"
+ atf_add_test_case "loopback"
+ atf_add_test_case "loopback_v6"
atf_add_test_case "nostate"
atf_add_test_case "nostate_v6"
atf_add_test_case "adaptive"