The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=4a7c6d6206cf0851e11ecd38ab5c618f67892dab
commit 4a7c6d6206cf0851e11ecd38ab5c618f67892dab Author: Mark Johnston <[email protected]> AuthorDate: 2024-10-29 14:59:15 +0000 Commit: Mark Johnston <[email protected]> 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"
