Date: Wed, 12 Feb 2025 19:39:06 GMT
From: Kristof Provost <kp@FreeBSD.org>
To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject: git: 5e9272401d80 - main - pf: fold pf_test_state_{tcp,udp,other} into one pf_test_state
Message-ID: <202502121939.51CJd6xR061639@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=5e9272401d80381d95be84ab3b7755a207245268 commit 5e9272401d80381d95be84ab3b7755a207245268 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-02-06 14:36:11 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-02-12 19:38:38 +0000 pf: fold pf_test_state_{tcp,udp,other} into one pf_test_state The _icmp variant stays because it is completely different. pf_setup_pdesc sets us up with access to ports, cksum etc in a protocol independent matter, so we don't need many protocol switches here. tcp and udp were almost identical, the _other case changes significantly - not too unlikely this fixes a subtle bug or two in that case. ok ryan benno bluhm mikeb Obtained from: OpenBSD, henning <henning@openbsd.org>, f6cfeb8e09 Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/netpfil/pf/pf.c | 325 ++++++++++++++-------------------------------------- 1 file changed, 87 insertions(+), 238 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 3957d39edc13..c92faf43d967 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -353,10 +353,8 @@ static int pf_tcp_track_full(struct pf_kstate **, struct pf_pdesc *, u_short *, int *); static int pf_tcp_track_sloppy(struct pf_kstate **, struct pf_pdesc *, u_short *); -static int pf_test_state_tcp(struct pf_kstate **, - struct pf_pdesc *, u_short *); -static int pf_test_state_udp(struct pf_kstate **, - struct pf_pdesc *); +static int pf_test_state(struct pf_kstate **, struct pf_pdesc *, + u_short *); int pf_icmp_state_lookup(struct pf_state_key_cmp *, struct pf_pdesc *, struct pf_kstate **, int, u_int16_t, u_int16_t, @@ -368,8 +366,6 @@ static void pf_sctp_multihome_delayed(struct pf_pdesc *, struct pfi_kkif *, struct pf_kstate *, int); static int pf_test_state_sctp(struct pf_kstate **, struct pf_pdesc *, u_short *); -static int pf_test_state_other(struct pf_kstate **, - struct pf_pdesc *); static u_int16_t pf_calc_mss(struct pf_addr *, sa_family_t, int, u_int16_t); static int pf_check_proto_cksum(struct mbuf *, int, int, @@ -6901,153 +6897,102 @@ pf_synproxy(struct pf_pdesc *pd, struct pf_kstate **state, u_short *reason) } static int -pf_test_state_tcp(struct pf_kstate **state, struct pf_pdesc *pd, - u_short *reason) +pf_test_state(struct pf_kstate **state, struct pf_pdesc *pd, u_short *reason) { struct pf_state_key_cmp key; - struct tcphdr *th = &pd->hdr.tcp; int copyback = 0; - int action = PF_PASS; struct pf_state_peer *src, *dst; + uint8_t psrc, pdst; + int action = PF_PASS; bzero(&key, sizeof(key)); key.af = pd->af; - key.proto = IPPROTO_TCP; + key.proto = pd->virtual_proto; PF_ACPY(&key.addr[pd->sidx], pd->src, key.af); PF_ACPY(&key.addr[pd->didx], pd->dst, key.af); - key.port[pd->sidx] = th->th_sport; - key.port[pd->didx] = th->th_dport; + key.port[pd->sidx] = pd->osport; + key.port[pd->didx] = pd->odport; STATE_LOOKUP(&key, *state, pd); if (pd->dir == (*state)->direction) { src = &(*state)->src; dst = &(*state)->dst; + psrc = PF_PEER_SRC; + pdst = PF_PEER_DST; } else { src = &(*state)->dst; dst = &(*state)->src; + psrc = PF_PEER_DST; + pdst = PF_PEER_SRC; } - if ((action = pf_synproxy(pd, state, reason)) != PF_PASS) - return (action); - - if (dst->state >= TCPS_FIN_WAIT_2 && - src->state >= TCPS_FIN_WAIT_2 && - (((tcp_get_flags(th) & (TH_SYN|TH_ACK)) == TH_SYN) || - ((tcp_get_flags(th) & (TH_SYN|TH_ACK|TH_RST)) == TH_ACK && - pf_syncookie_check(pd) && pd->dir == PF_IN))) { - if (V_pf_status.debug >= PF_DEBUG_MISC) { - printf("pf: state reuse "); - pf_print_state(*state); - pf_print_flags(tcp_get_flags(th)); - printf("\n"); - } - /* XXX make sure it's the same direction ?? */ - pf_set_protostate(*state, PF_PEER_BOTH, TCPS_CLOSED); - pf_unlink_state(*state); - *state = NULL; - return (PF_DROP); - } - - if ((*state)->state_flags & PFSTATE_SLOPPY) { - if (pf_tcp_track_sloppy(state, pd, reason) == PF_DROP) - return (PF_DROP); - } else { - int ret; + switch (pd->virtual_proto) { + case IPPROTO_TCP: { + struct tcphdr *th = &pd->hdr.tcp; - ret = pf_tcp_track_full(state, pd, reason, - ©back); - if (ret == PF_DROP) + if ((action = pf_synproxy(pd, state, reason)) != PF_PASS) + return (action); + if ((*state)->src.state >= TCPS_FIN_WAIT_2 && + (*state)->dst.state >= TCPS_FIN_WAIT_2 && + (((tcp_get_flags(th) & (TH_SYN|TH_ACK)) == TH_SYN) || + ((tcp_get_flags(th) & (TH_SYN|TH_ACK|TH_RST)) == TH_ACK && + pf_syncookie_check(pd) && pd->dir == PF_IN))) { + if (V_pf_status.debug >= PF_DEBUG_MISC) { + printf("pf: state reuse "); + pf_print_state(*state); + pf_print_flags(tcp_get_flags(th)); + printf("\n"); + } + /* XXX make sure it's the same direction ?? */ + pf_set_protostate(*state, PF_PEER_BOTH, TCPS_CLOSED); + pf_unlink_state(*state); + *state = NULL; return (PF_DROP); - } - - /* translate source/destination address, if necessary */ - if ((*state)->key[PF_SK_WIRE] != (*state)->key[PF_SK_STACK]) { - struct pf_state_key *nk; - int afto, sidx, didx; - - if (PF_REVERSED_KEY((*state)->key, pd->af)) - nk = (*state)->key[pd->sidx]; - else - nk = (*state)->key[pd->didx]; - - afto = pd->af != nk->af; - sidx = afto ? pd->didx : pd->sidx; - didx = afto ? pd->sidx : pd->didx; - - if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) || - nk->port[sidx] != th->th_sport) - pf_change_ap(pd->m, pd->src, &th->th_sport, - pd->ip_sum, &th->th_sum, &nk->addr[sidx], - nk->port[sidx], 0, pd->af, nk->af); - - if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) || - nk->port[didx] != th->th_dport) - pf_change_ap(pd->m, pd->dst, &th->th_dport, - pd->ip_sum, &th->th_sum, &nk->addr[didx], - nk->port[didx], 0, pd->af, nk->af); - - if (afto) { - PF_ACPY(&pd->nsaddr, &nk->addr[sidx], nk->af); - PF_ACPY(&pd->ndaddr, &nk->addr[didx], nk->af); - pd->naf = nk->af; - action = PF_AFRT; } + if ((*state)->state_flags & PFSTATE_SLOPPY) { + if (pf_tcp_track_sloppy(state, pd, reason) == PF_DROP) + return (PF_DROP); + } else { + int ret; - copyback = 1; + ret = pf_tcp_track_full(state, pd, reason, + ©back); + if (ret == PF_DROP) + return (PF_DROP); + } + break; } + case IPPROTO_UDP: + /* update states */ + if (src->state < PFUDPS_SINGLE) + pf_set_protostate(*state, psrc, PFUDPS_SINGLE); + if (dst->state == PFUDPS_SINGLE) + pf_set_protostate(*state, pdst, PFUDPS_MULTIPLE); - /* Copyback sequence modulation or stateful scrub changes if needed */ - if (copyback) - m_copyback(pd->m, pd->off, sizeof(*th), (caddr_t)th); - - return (action); -} - -static int -pf_test_state_udp(struct pf_kstate **state, struct pf_pdesc *pd) -{ - struct pf_state_peer *src, *dst; - struct pf_state_key_cmp key; - struct udphdr *uh = &pd->hdr.udp; - uint8_t psrc, pdst; - int action = PF_PASS; - - bzero(&key, sizeof(key)); - key.af = pd->af; - key.proto = IPPROTO_UDP; - PF_ACPY(&key.addr[pd->sidx], pd->src, key.af); - PF_ACPY(&key.addr[pd->didx], pd->dst, key.af); - key.port[pd->sidx] = uh->uh_sport; - key.port[pd->didx] = uh->uh_dport; - - STATE_LOOKUP(&key, *state, pd); + /* update expire time */ + (*state)->expire = pf_get_uptime(); + if (src->state == PFUDPS_MULTIPLE && dst->state == PFUDPS_MULTIPLE) + (*state)->timeout = PFTM_UDP_MULTIPLE; + else + (*state)->timeout = PFTM_UDP_SINGLE; + break; + default: + /* update states */ + if (src->state < PFOTHERS_SINGLE) + pf_set_protostate(*state, psrc, PFOTHERS_SINGLE); + if (dst->state == PFOTHERS_SINGLE) + pf_set_protostate(*state, pdst, PFOTHERS_MULTIPLE); - if (pd->dir == (*state)->direction) { - src = &(*state)->src; - dst = &(*state)->dst; - psrc = PF_PEER_SRC; - pdst = PF_PEER_DST; - } else { - src = &(*state)->dst; - dst = &(*state)->src; - psrc = PF_PEER_DST; - pdst = PF_PEER_SRC; + /* update expire time */ + (*state)->expire = pf_get_uptime(); + if (src->state == PFOTHERS_MULTIPLE && dst->state == PFOTHERS_MULTIPLE) + (*state)->timeout = PFTM_OTHER_MULTIPLE; + else + (*state)->timeout = PFTM_OTHER_SINGLE; + break; } - /* update states */ - if (src->state < PFUDPS_SINGLE) - pf_set_protostate(*state, psrc, PFUDPS_SINGLE); - if (dst->state == PFUDPS_SINGLE) - pf_set_protostate(*state, pdst, PFUDPS_MULTIPLE); - - /* update expire time */ - (*state)->expire = pf_get_uptime(); - if (src->state == PFUDPS_MULTIPLE && dst->state == PFUDPS_MULTIPLE) - (*state)->timeout = PFTM_UDP_MULTIPLE; - else - (*state)->timeout = PFTM_UDP_SINGLE; - /* translate source/destination address, if necessary */ if ((*state)->key[PF_SK_WIRE] != (*state)->key[PF_SK_STACK]) { struct pf_state_key *nk; @@ -7063,16 +7008,18 @@ pf_test_state_udp(struct pf_kstate **state, struct pf_pdesc *pd) didx = afto ? pd->sidx : pd->didx; if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) || - nk->port[sidx] != uh->uh_sport) - pf_change_ap(pd->m, pd->src, &uh->uh_sport, pd->ip_sum, - &uh->uh_sum, &nk->addr[pd->sidx], - nk->port[sidx], 1, pd->af, nk->af); + nk->port[sidx] != pd->osport) + pf_change_ap(pd->m, pd->src, pd->sport, pd->ip_sum, + pd->pcksum, &nk->addr[pd->sidx], + nk->port[sidx], pd->virtual_proto == IPPROTO_UDP, + pd->af, nk->af); if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) || - nk->port[didx] != uh->uh_dport) - pf_change_ap(pd->m, pd->dst, &uh->uh_dport, pd->ip_sum, - &uh->uh_sum, &nk->addr[pd->didx], - nk->port[didx], 1, pd->af, nk->af); + nk->port[didx] != pd->odport) + pf_change_ap(pd->m, pd->dst, pd->dport, pd->ip_sum, + pd->pcksum, &nk->addr[pd->didx], + nk->port[didx], pd->virtual_proto == IPPROTO_UDP, + pd->af, nk->af); if (afto) { PF_ACPY(&pd->nsaddr, &nk->addr[sidx], nk->af); @@ -7081,9 +7028,12 @@ pf_test_state_udp(struct pf_kstate **state, struct pf_pdesc *pd) action = PF_AFRT; } - m_copyback(pd->m, pd->off, sizeof(*uh), (caddr_t)uh); + copyback = 1; } + if (copyback && pd->hdrlen > 0) + m_copyback(pd->m, pd->off, pd->hdrlen, pd->hdr.any); + return (action); } @@ -8672,107 +8622,6 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd, } } -static int -pf_test_state_other(struct pf_kstate **state, struct pf_pdesc *pd) -{ - struct pf_state_peer *src, *dst; - struct pf_state_key_cmp key; - uint8_t psrc, pdst; - int action = PF_PASS; - - bzero(&key, sizeof(key)); - key.af = pd->af; - key.proto = pd->proto; - PF_ACPY(&key.addr[pd->sidx], pd->src, key.af); - PF_ACPY(&key.addr[pd->didx], pd->dst, key.af); - key.port[0] = key.port[1] = 0; - - STATE_LOOKUP(&key, *state, pd); - - if (pd->dir == (*state)->direction) { - src = &(*state)->src; - dst = &(*state)->dst; - psrc = PF_PEER_SRC; - pdst = PF_PEER_DST; - } else { - src = &(*state)->dst; - dst = &(*state)->src; - psrc = PF_PEER_DST; - pdst = PF_PEER_SRC; - } - - /* update states */ - if (src->state < PFOTHERS_SINGLE) - pf_set_protostate(*state, psrc, PFOTHERS_SINGLE); - if (dst->state == PFOTHERS_SINGLE) - pf_set_protostate(*state, pdst, PFOTHERS_MULTIPLE); - - /* update expire time */ - (*state)->expire = pf_get_uptime(); - if (src->state == PFOTHERS_MULTIPLE && dst->state == PFOTHERS_MULTIPLE) - (*state)->timeout = PFTM_OTHER_MULTIPLE; - else - (*state)->timeout = PFTM_OTHER_SINGLE; - - /* translate source/destination address, if necessary */ - if ((*state)->key[PF_SK_WIRE] != (*state)->key[PF_SK_STACK]) { - struct pf_state_key *nk; - int afto; - - if (PF_REVERSED_KEY((*state)->key, pd->af)) - nk = (*state)->key[pd->sidx]; - else - nk = (*state)->key[pd->didx]; - - KASSERT(nk, ("%s: nk is null", __func__)); - KASSERT(pd, ("%s: pd is null", __func__)); - KASSERT(pd->src, ("%s: pd->src is null", __func__)); - KASSERT(pd->dst, ("%s: pd->dst is null", __func__)); - - afto = pd->af != nk->af; - - switch (pd->af) { -#ifdef INET - case AF_INET: - if (!afto && - PF_ANEQ(pd->src, &nk->addr[pd->sidx], AF_INET)) - pf_change_a(&pd->src->v4.s_addr, - pd->ip_sum, - nk->addr[pd->sidx].v4.s_addr, - 0); - - if (!afto && - PF_ANEQ(pd->dst, &nk->addr[pd->didx], AF_INET)) - pf_change_a(&pd->dst->v4.s_addr, - pd->ip_sum, - nk->addr[pd->didx].v4.s_addr, - 0); - - break; -#endif /* INET */ -#ifdef INET6 - case AF_INET6: - if (!afto && - PF_ANEQ(pd->src, &nk->addr[pd->sidx], AF_INET6)) - PF_ACPY(pd->src, &nk->addr[pd->sidx], pd->af); - - if (!afto && - PF_ANEQ(pd->dst, &nk->addr[pd->didx], AF_INET6)) - PF_ACPY(pd->dst, &nk->addr[pd->didx], pd->af); -#endif /* INET6 */ - } - if (afto) { - PF_ACPY(&pd->nsaddr, - &nk->addr[afto ? pd->didx : pd->sidx], nk->af); - PF_ACPY(&pd->ndaddr, - &nk->addr[afto ? pd->sidx : pd->didx], nk->af); - pd->naf = nk->af; - action = PF_AFRT; - } - } - return (action); -} - /* * ipoff and off are measured from the start of the mbuf chain. * h must be at "ipoff" on the mbuf chain. @@ -10439,7 +10288,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 action = pf_normalize_tcp(&pd); if (action == PF_DROP) goto done; - action = pf_test_state_tcp(&s, &pd, &reason); + action = pf_test_state(&s, &pd, &reason); if (action == PF_PASS || action == PF_AFRT) { if (V_pfsync_update_state_ptr != NULL) V_pfsync_update_state_ptr(s); @@ -10465,7 +10314,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 if (action != PF_PASS) break; - action = pf_test_state_tcp(&s, &pd, &reason); + action = pf_test_state(&s, &pd, &reason); if (action != PF_PASS || s == NULL) { action = PF_DROP; break; @@ -10485,7 +10334,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 } case IPPROTO_UDP: { - action = pf_test_state_udp(&s, &pd); + action = pf_test_state(&s, &pd, &reason); if (action == PF_PASS || action == PF_AFRT) { if (V_pfsync_update_state_ptr != NULL) V_pfsync_update_state_ptr(s); @@ -10543,7 +10392,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 } default: - action = pf_test_state_other(&s, &pd); + action = pf_test_state(&s, &pd, &reason); if (action == PF_PASS || action == PF_AFRT) { if (V_pfsync_update_state_ptr != NULL) V_pfsync_update_state_ptr(s);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502121939.51CJd6xR061639>