From nobody Thu Oct 03 11:58:08 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4XK9DJ4ykVz5YDVc; Thu, 03 Oct 2024 11:58:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4XK9DJ4Stmz4MP4; Thu, 3 Oct 2024 11:58:08 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1727956688; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=X/Mi41ywo8d8AU7jtAa3YoRxaGd73/BLP+magnCoHY0=; b=gqF4a459sjt2/i9OR2pW3OVNyxHkzjYu3eNHjjCey/+Pw7MO10ZLT0hElX4E2RWQ+JTuaE QMC2bTV2vLdVUkxx9bw7NAhXTHgoi7gD0eGpSRIPUPLgcnb5XGn2NyZTzyL0eUJa60V7o4 6eNSr9Yo/FZAxVPFBFM+EUki4t8GcAwYqcA8DscQyzrvNTpOpLFiWLGAkXXlMcQy3ljYoB DZHsVW/1py8UBoKIq10obdFA7mapDlakyZIOpZlWR7KhstPjebUIW88tuHymx9hDOzUyHj RvnyEqmXJMwgdOwYikxp5wDk8cBIA6khB+w3wW4nHMoLF0GeGQjFUlKR11yL/Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1727956688; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=X/Mi41ywo8d8AU7jtAa3YoRxaGd73/BLP+magnCoHY0=; b=DJI9/MYLCnb7sTxvxrPnl11YBryW68nt7PpJTQodFFnagfh73ccoH5bI3rzUQ75Vsa9SPo dVG9QmIj78i01CE2vgdPwv31KrJiQs3wVd32GllB4WPB0EKZfMi3C7q8pUB/TNFejdyP5f Kq2Upz+10QCF+iRjFsgSvzMjZR23TRCAjOJ73ndFgxJhaZpMpGnh5K/e2SGgmjHPV2pEG+ RQmx+GIOMgw74S3UJE/HhtsFblsRbl+FxPG4HGEiLMvo/scDbyLlsETsRyfj/Rb4bLqayG VcRjfcu8TpgH8Dgf8CBeVDLGYkHcjMyFGZjNoVxk2pBcK4qOzOTevIxjSKdHPw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1727956688; a=rsa-sha256; cv=none; b=Pb9BR3hwVe2a1vGIAydAHX3OoLdtAi0zC63k/kDfqj0TbaJwtO6bKkNFuNWSgnm0wYs3Vm uPwg98LEG4at+Efp1rWZqqPy1yvxtZgu8JULSC/EhrB1/WdXGHJJoaGWZnbMkYS+fSaRbK PkUVqsgq3Ptn2QMREm9xSQm1u1pZ3446hsOqRzwki3BZA+Qb5iovdD1TG/UKx3jfMdmmq5 uBLzuY8ZeMSiXmwB2AD3kz0jq/PYFBUaQPy8LLcvbIDiG+CcS0Wp78YNNP2jZPU9GxZEQ9 m31jjl3RQzOSrnvB/LaU0jA1oSR+LUpiegZfF+uObTpQY/PV7kV/v+fUHkiCkA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4XK9DJ3vNmzpkV; Thu, 3 Oct 2024 11:58:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 493Bw8sj095467; Thu, 3 Oct 2024 11:58:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 493Bw8hi095463; Thu, 3 Oct 2024 11:58:08 GMT (envelope-from git) Date: Thu, 3 Oct 2024 11:58:08 GMT Message-Id: <202410031158.493Bw8hi095463@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 65074f6f3115 - main - pf: fix double ruleset evaluation for fragments sent to dummynet List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 65074f6f3115a066e5beda31787df626aed03696 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=65074f6f3115a066e5beda31787df626aed03696 commit 65074f6f3115a066e5beda31787df626aed03696 Author: Kajetan Staszkiewicz AuthorDate: 2024-10-03 08:28:57 +0000 Commit: Kristof Provost CommitDate: 2024-10-03 11:49:57 +0000 pf: fix double ruleset evaluation for fragments sent to dummynet The function `pf_setup_pdesc()` handles ruleset evaluation for non-reassembled packets. Having it called before `pf_mtag` is checked for flags `PF_MTAG_FLAG_ROUTE_TO` and `PF_MTAG_FLAG_DUMMYNET` will cause loops for fragmented packets if reassembly is disabled. Move `pd` zeroing and `pf_mtag` extraction from `pf_setup_pdesc()` to a separate function `pf_init_pdesc()` and change the order of function calls: first call `pf_init_pdesc()`, then check if the currently processed packet has been reinjected from dummynet, finally call `pf_setup_pdesc()`. Add functionality of sending UDP packets to `pft_ping.py` with fragmentation support and fix broken IPv6 reassembly. Reviewed by: kp Differential Revision: https://reviews.freebsd.org/D46880 --- sys/netpfil/pf/pf.c | 62 +++++++++-------- tests/sys/netpfil/common/pft_ping.py | 107 +++++++++++++++++++++++++++-- tests/sys/netpfil/common/sniffer.py | 13 ++-- tests/sys/netpfil/pf/fragmentation_pass.sh | 43 ++++++++++++ 4 files changed, 188 insertions(+), 37 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index bbed285ab9f8..7edf65ae4a09 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -8460,6 +8460,13 @@ pf_dummynet_route(struct pf_pdesc *pd, struct pf_kstate *s, return (0); } +static void +pf_init_pdesc(struct pf_pdesc *pd, struct mbuf *m) +{ + memset(pd, 0, sizeof(*pd)); + pd->pf_mtag = pf_find_mtag(m); +} + static int pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0, u_short *action, u_short *reason, struct pfi_kkif *kif, struct pf_krule **a, @@ -8469,21 +8476,18 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0, { struct mbuf *m = *m0; - memset(pd, 0, sizeof(*pd)); + pd->af = af; pd->dir = dir; TAILQ_INIT(&pd->sctp_multihome_jobs); if (default_actions != NULL) memcpy(&pd->act, default_actions, sizeof(pd->act)); - pd->pf_mtag = pf_find_mtag(m); if (pd->pf_mtag && pd->pf_mtag->dnpipe) { pd->act.dnpipe = pd->pf_mtag->dnpipe; pd->act.flags = pd->pf_mtag->dnflags; } - pd->af = af; - switch (af) { #ifdef INET case AF_INET: { @@ -8918,30 +8922,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 return (PF_DROP); } - if (pf_setup_pdesc(af, dir, &pd, m0, &action, &reason, kif, &a, &r, - &s, &ruleset, &off, &hdrlen, inp, default_actions) == -1) { - if (action != PF_PASS) - pd.act.log |= PF_LOG_FORCE; - goto done; - } - m = *m0; - - switch (af) { -#ifdef INET - case AF_INET: - h = mtod(m, struct ip *); - ttl = h->ip_ttl; - break; -#endif -#ifdef INET6 - case AF_INET6: - h6 = mtod(m, struct ip6_hdr *); - ttl = h6->ip6_hlim; - break; -#endif - default: - panic("Unknown af %d", af); - } + pf_init_pdesc(&pd, m); if (pd.pf_mtag != NULL && (pd.pf_mtag->flags & PF_MTAG_FLAG_ROUTE_TO)) { pd.pf_mtag->flags &= ~PF_MTAG_FLAG_ROUTE_TO; @@ -8974,6 +8955,31 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 return (PF_PASS); } + if (pf_setup_pdesc(af, dir, &pd, m0, &action, &reason, kif, &a, &r, + &s, &ruleset, &off, &hdrlen, inp, default_actions) == -1) { + if (action != PF_PASS) + pd.act.log |= PF_LOG_FORCE; + goto done; + } + m = *m0; + + switch (af) { +#ifdef INET + case AF_INET: + h = mtod(m, struct ip *); + ttl = h->ip_ttl; + break; +#endif +#ifdef INET6 + case AF_INET6: + h6 = mtod(m, struct ip6_hdr *); + ttl = h6->ip6_hlim; + break; +#endif + default: + panic("Unknown af %d", af); + } + if (__predict_false(ip_divert_ptr != NULL) && ((mtag = m_tag_locate(m, MTAG_PF_DIVERT, 0, NULL)) != NULL)) { struct pf_divert_mtag *dt = (struct pf_divert_mtag *)(mtag+1); diff --git a/tests/sys/netpfil/common/pft_ping.py b/tests/sys/netpfil/common/pft_ping.py index d8aafc884265..1caa26abe5f6 100644 --- a/tests/sys/netpfil/common/pft_ping.py +++ b/tests/sys/netpfil/common/pft_ping.py @@ -103,7 +103,7 @@ def send_icmp_ping(send_params): ip6 = prepare_ipv6(send_params) icmp = sp.ICMPv6EchoRequest(data=sp.raw(build_payload(send_length))) if send_frag_length: - for packet in sp.fragment(ip6 / icmp, fragsize=send_frag_length): + for packet in sp.fragment6(ip6 / icmp, fragSize=send_frag_length): packets.append(ether / packet) else: packets.append(ether / ip6 / icmp) @@ -141,6 +141,39 @@ def send_tcp_syn(send_params): sp.sendp(req, iface=send_params['sendif'], verbose=False) +def send_udp(send_params): + LOGGER.debug(f'Sending UDP ping') + packets = [] + send_length = send_params['length'] + send_frag_length = send_params['frag_length'] + ether = sp.Ether() + if ':' in send_params['dst_address']: + ip6 = prepare_ipv6(send_params) + udp = sp.UDP( + sport=send_params.get('sport'), dport=send_params.get('dport'), + ) + raw = sp.Raw(load=build_payload(send_length)) + if send_frag_length: + for packet in sp.fragment6(ip6 / udp / raw, fragSize=send_frag_length): + packets.append(ether / packet) + else: + packets.append(ether / ip6 / udp / raw) + else: + ip = prepare_ipv4(send_params) + udp = sp.UDP( + sport=send_params.get('sport'), dport=send_params.get('dport'), + ) + raw = sp.Raw(load=build_payload(send_length)) + if send_frag_length: + for packet in sp.fragment(ip / udp / raw, fragsize=send_frag_length): + packets.append(ether / packet) + else: + packets.append(ether / ip / udp / raw) + + for packet in packets: + sp.sendp(packet, iface=send_params['sendif'], verbose=False) + + def send_ping(ping_type, send_params): if ping_type == 'icmp': send_icmp_ping(send_params) @@ -149,8 +182,10 @@ def send_ping(ping_type, send_params): ping_type == 'tcp3way' ): send_tcp_syn(send_params) + elif ping_type == 'udp': + send_udp(send_params) else: - raise Exception('Unspported ping type') + raise Exception('Unsupported ping type') def check_ipv4(expect_params, packet): @@ -345,6 +380,30 @@ def check_tcp(expect_params, packet): return True +def check_udp(expect_params, packet): + expect_length = expect_params['length'] + udp = packet.getlayer(sp.UDP) + if not udp: + LOGGER.debug('Packet is not UDP!') + return False + raw = packet.getlayer(sp.Raw) + if not raw: + LOGGER.debug('Packet contains no payload!') + return False + if raw.load != build_payload(expect_length): + LOGGER.debug(f'Payload magic does not match len {len(raw.load)} vs {expect_length}!') + return False + orig_chksum = udp.chksum + udp.chksum = None + newpacket = sp.Ether(sp.raw(packet[sp.Ether])) + new_chksum = newpacket[sp.UDP].chksum + if new_chksum and orig_chksum != new_chksum: + LOGGER.debug(f'Wrong UDP checksum {orig_chksum}, expected {new_chksum}!') + return False + + return True + + def check_tcp_syn_request_4(expect_params, packet): if not check_ipv4(expect_params, packet): return False @@ -391,6 +450,14 @@ def check_tcp_3way_4(args, packet): return False +def check_udp_request_4(expect_params, packet): + if not check_ipv4(expect_params, packet): + return False + if not check_udp(expect_params, packet): + return False + return True + + def check_tcp_syn_request_6(expect_params, packet): if not check_ipv6(expect_params, packet): return False @@ -437,6 +504,13 @@ def check_tcp_3way_6(args, packet): return False +def check_udp_request_6(expect_params, packet): + if not check_ipv6(expect_params, packet): + return False + if not check_udp(expect_params, packet): + return False + return True + def check_tcp_syn_request(args, packet): expect_params = args['expect_params'] src_address = expect_params.get('src_address') @@ -481,6 +555,21 @@ def check_tcp_3way(args, packet): return check_tcp_3way_4(args, packet) +def check_udp_request(args, packet): + expect_params = args['expect_params'] + src_address = expect_params.get('src_address') + dst_address = expect_params.get('dst_address') + if not (src_address or dst_address): + raise Exception('Source or destination address must be given to match the tcp syn request!') + if ( + (src_address and ':' in src_address) or + (dst_address and ':' in dst_address) + ): + return check_udp_request_6(expect_params, packet) + else: + return check_udp_request_4(expect_params, packet) + + def setup_sniffer( recvif, ping_type, sniff_type, expect_params, defrag, send_params, ): @@ -494,8 +583,10 @@ def setup_sniffer( checkfn = check_tcp_syn_reply elif ping_type == 'tcp3way' and sniff_type == 'reply': checkfn = check_tcp_3way + elif ping_type == 'udp' and sniff_type == 'request': + checkfn = check_udp_request else: - raise Exception('Unspported ping or sniff type') + raise Exception('Unspported ping and sniff type combination') return Sniffer( {'send_params': send_params, 'expect_params': expect_params}, @@ -513,7 +604,7 @@ def parse_args(): parser.add_argument('--to', required=True, help='The destination IP address for the ping request') parser.add_argument('--ping-type', - choices=('icmp', 'tcpsyn', 'tcp3way'), + choices=('icmp', 'tcpsyn', 'tcp3way', 'udp'), help='Type of ping: ICMP (default) or TCP SYN or 3-way TCP handshake', default='icmp') parser.add_argument('--fromaddr', @@ -612,7 +703,13 @@ def main(): sniffers = [] if send_params['frag_length']: - defrag = True + if ( + (send_params['src_address'] and ':' in send_params['src_address']) or + (send_params['dst_address'] and ':' in send_params['dst_address']) + ): + defrag = 'IPv6' + else: + defrag = 'IPv4' else: defrag = False diff --git a/tests/sys/netpfil/common/sniffer.py b/tests/sys/netpfil/common/sniffer.py index 14305a37278c..583b27d34ca6 100644 --- a/tests/sys/netpfil/common/sniffer.py +++ b/tests/sys/netpfil/common/sniffer.py @@ -56,14 +56,19 @@ class Sniffer(threading.Thread): def run(self): self.packets = [] - if self._defrag: - # With fragment reassembly we can't stop the sniffer after catching - # the good packets, as those have not been reassembled. We must - # wait for sniffer to finish and check returned packets instead. + # With fragment reassembly we can't stop the sniffer after catching + # the good packets, as those have not been reassembled. We must + # wait for sniffer to finish and check returned packets instead. + if self._defrag == 'IPv4': self.packets = sp.sniff(session=sp.IPSession, iface=self._recvif, timeout=self._timeout, started_callback=self._startedCb) for p in self.packets: self._checkPacket(p) + elif self._defrag == 'IPv6': + self.packets = sp.sniff(session=sp.DefaultSession, iface=self._recvif, + timeout=self._timeout, started_callback=self._startedCb) + for p in sp.defragment6(self.packets): + self._checkPacket(p) else: self.packets = sp.sniff(iface=self._recvif, stop_filter=self._checkPacket, timeout=self._timeout, diff --git a/tests/sys/netpfil/pf/fragmentation_pass.sh b/tests/sys/netpfil/pf/fragmentation_pass.sh index d505accba5f2..99d2c827b239 100644 --- a/tests/sys/netpfil/pf/fragmentation_pass.sh +++ b/tests/sys/netpfil/pf/fragmentation_pass.sh @@ -553,6 +553,48 @@ dummynet_nat_cleanup() pft_cleanup } +atf_test_case "dummynet_fragmented" "cleanup" +dummynet_fragmented_head() +{ + atf_set descr 'Test dummynet on NATed fragmented traffic' + atf_set require.user root +} + +dummynet_fragmented_body() +{ + pft_init + dummynet_init + + # No test for IPv6. IPv6 fragment reassembly can't be disabled. + setup_router_dummy_ipv4 + + jexec router dnctl pipe 1 config delay 1 + + pft_set_rules router \ + "set reassemble no" \ + "block" \ + "pass inet6 proto icmp6 icmp6-type { neighbrsol, neighbradv }" \ + "pass in on ${epair_tester}b inet proto udp dnpipe (1, 1)" \ + "pass out on ${epair_server}a inet proto udp" \ + + ping_dummy_check_request exit:0 --ping-type=udp --send-length=10000 --send-frag-length=1280 + + rules=$(mktemp) || exit 1 + jexec router pfctl -qvsr > $rules + + # Count that fragmented packets have hit the rule only once and that + # they have not created states. There is no stateful firewall support + # for fragmented packets. + grep -A2 'pass in on epair0b inet proto udp all keep state dnpipe(1, 1)' $rules | + grep -qE 'Packets: 8\s+Bytes: 10168\s+States: 0\s+' || + atf_fail "Fragmented packets not counted correctly" +} + +dummynet_fragmented_cleanup() +{ + pft_cleanup +} + atf_init_test_cases() { atf_add_test_case "too_many_fragments" @@ -566,4 +608,5 @@ atf_init_test_cases() atf_add_test_case "reassemble_slowpath" atf_add_test_case "dummynet" atf_add_test_case "dummynet_nat" + atf_add_test_case "dummynet_fragmented" }