From nobody Thu Oct 26 14:16:54 2023 X-Original-To: dev-commits-src-all@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 4SGSXk50jTz4xsKC; Thu, 26 Oct 2023 14:16:54 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4SGSXk4P7lz4G2C; Thu, 26 Oct 2023 14:16:54 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1698329814; 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=whw/laHq22ql/dD5uP3+E27NuAbw6pOvCIbm0vHuGZU=; b=XE3aTL8eRViGvmcb4p4HMWmxsmFIW6TsB9WqXZ9tFVV7PENyNl3cT2nTvAaXu6bEVsOZKa Wo11FTWKB1nevaKGgp3rq7FPVypOb24EjRolpZXBNjH0txUSd2dSkYYvcROEHpsO4hMNSA P21rJpcgvP7RrPU2Nfw0NbmtbUsEDIsHBXyUMLfC984UJ7s3asWQjTkNqhv1AjoHcxYTG0 +ar2lI4nexJgZJt4t6FYKwhjhsYBI4+U+IkN7t3qGGGMChoJxo7sQoE3VtqISqNoCJxlGF DXoe8z6mBuuZu2SerDeybnLDWebapvjnPKuYsWGOThHtkoxkGxw4jJttqwQxlA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1698329814; 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=whw/laHq22ql/dD5uP3+E27NuAbw6pOvCIbm0vHuGZU=; b=rNpiK5qK82y4cfewrI1FJJwqqmg5GvrlMbjRbhHZEatbEHPXTGOrgjvfmK0k8BhmzLMtrH wZKAwQrm9wvOk/WJI73QfmJcVchsidX7sYXg6lwrBzrimVSf+z4icvYiBvhQs43KCF0Agy 5VdFqeGTdRehy1TrysGWNIo0+y+4TOdWUvJeD9E+RsNrl+IoD//sF/9aBarkijyGbN9PrC a6tee1BgggOaSfrIc4UR0t52ZNAuJP9IHdZj1O21PyP3C0l5Z0pq05oPkpEMd5GyHdLcfI nu0rn9N11DzwyiYfuRGVGpfugKu7tk/tTNqUXlDdPaspE3jmsjEmtNMSLI+l6A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1698329814; a=rsa-sha256; cv=none; b=w/KFXT8HVbMsLOwXBHFiSmKEo3FvR5d5Y9ErFHxT3TRBFjemM8r6W7BRpbI+rRLcpBVGrz LcVap4Ljm13jq5odk+iOpRbGwePtMXjNZG8wDrYcqdGcB7efqK34a1+NpTumRyxHLa992T 9p+fkEZTV95EvPCzGmLpMJEKu4FQARfG+DgF8KiEx7aHZFQwEV66X2Z6oG2q4hUqhErSlx jOTl5r+81VDVQHX93EKEBEH3LqmTBLdagwsSU8+sqBBlo/uUcsRsHLOhw8Q8lXbydu07Ug Dbq+0Hhe587wsGVM3i+YYo75DvqZWNE/mchym/3Rjh3CXWOT/LukJyhf9aZb/Q== 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 4SGSXk3R64zb0F; Thu, 26 Oct 2023 14:16:54 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 39QEGsD6055082; Thu, 26 Oct 2023 14:16:54 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 39QEGs6V055079; Thu, 26 Oct 2023 14:16:54 GMT (envelope-from git) Date: Thu, 26 Oct 2023 14:16:54 GMT Message-Id: <202310261416.39QEGs6V055079@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: ede5d4ff5b39 - main - pf: Fix packet reassembly List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@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: ede5d4ff5b39ccbc193c30fb6c093c7c4de9a464 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=ede5d4ff5b39ccbc193c30fb6c093c7c4de9a464 commit ede5d4ff5b39ccbc193c30fb6c093c7c4de9a464 Author: Kajetan Staszkiewicz AuthorDate: 2023-10-26 12:26:33 +0000 Commit: Kristof Provost CommitDate: 2023-10-26 13:25:44 +0000 pf: Fix packet reassembly Don't drop fragmented packets when reassembly is disabled, they can be matched by rules with "fragment" keyword. Ensure that presence of scrub rules forces old behaviour. Reviewed by: kp Sponsored by: InnoGames GmbH Differential Revision: https://reviews.freebsd.org/D42355 --- sys/netpfil/pf/pf_norm.c | 51 ++++---- tests/sys/netpfil/pf/Makefile | 1 + tests/sys/netpfil/pf/fragmentation_compat.sh | 11 -- .../sys/netpfil/pf/fragmentation_no_reassembly.sh | 130 +++++++++++++++++++++ 4 files changed, 162 insertions(+), 31 deletions(-) diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c index d63cf0ebe54e..9963fe2b741b 100644 --- a/sys/netpfil/pf/pf_norm.c +++ b/sys/netpfil/pf/pf_norm.c @@ -1043,14 +1043,22 @@ pf_normalize_ip(struct mbuf **m0, struct pfi_kkif *kif, u_short *reason, int ip_len; int tag = -1; int verdict; - int srs; + bool scrub_compat; PF_RULES_RASSERT(); r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr); - /* Check if there any scrub rules. Lack of scrub rules means enforced - * packet normalization operation just like in OpenBSD. */ - srs = (r != NULL); + /* + * Check if there are any scrub rules, matching or not. + * Lack of scrub rules means: + * - enforced packet normalization operation just like in OpenBSD + * - fragment reassembly depends on V_pf_status.reass + * With scrub rules: + * - packet normalization is performed if there is a matching scrub rule + * - fragment reassembly is performed if the matching rule has no + * PFRULE_FRAGMENT_NOREASS flag + */ + scrub_compat = (r != NULL); while (r != NULL) { pf_counter_u64_add(&r->evaluations, 1); if (pfi_kkif_match(r->kif, kif) == r->ifnot) @@ -1076,7 +1084,7 @@ pf_normalize_ip(struct mbuf **m0, struct pfi_kkif *kif, u_short *reason, break; } - if (srs) { + if (scrub_compat) { /* With scrub rules present IPv4 normalization happens only * if one of rules has matched and it's not a "no scrub" rule */ if (r == NULL || r->action == PF_NOSCRUB) @@ -1087,12 +1095,6 @@ pf_normalize_ip(struct mbuf **m0, struct pfi_kkif *kif, u_short *reason, pf_counter_u64_add_protected(&r->bytes[pd->dir == PF_OUT], pd->tot_len); pf_counter_u64_critical_exit(); pf_rule_to_actions(r, &pd->act); - } else if ((!V_pf_status.reass && (h->ip_off & htons(IP_MF | IP_OFFMASK)))) { - /* With no scrub rules IPv4 fragment reassembly depends on the - * global switch. Fragments can be dropped early if reassembly - * is disabled. */ - REASON_SET(reason, PFRES_NORM); - goto drop; } /* Check for illegal packets */ @@ -1107,9 +1109,10 @@ pf_normalize_ip(struct mbuf **m0, struct pfi_kkif *kif, u_short *reason, } /* Clear IP_DF if the rule uses the no-df option or we're in no-df mode */ - if ((((r && r->rule_flag & PFRULE_NODF) || - (V_pf_status.reass & PF_REASS_NODF)) && h->ip_off & htons(IP_DF) - )) { + if (((!scrub_compat && V_pf_status.reass & PF_REASS_NODF) || + (r != NULL && r->rule_flag & PFRULE_NODF)) && + (h->ip_off & htons(IP_DF)) + ) { u_int16_t ip_off = h->ip_off; h->ip_off &= htons(~IP_DF); @@ -1143,7 +1146,9 @@ pf_normalize_ip(struct mbuf **m0, struct pfi_kkif *kif, u_short *reason, goto bad; } - if (r==NULL || !(r->rule_flag & PFRULE_FRAGMENT_NOREASS)) { + if ((!scrub_compat && V_pf_status.reass) || + (r != NULL && !(r->rule_flag & PFRULE_FRAGMENT_NOREASS)) + ) { max = fragoff + ip_len; /* Fully buffer all of the fragments @@ -1203,14 +1208,20 @@ pf_normalize_ip6(struct mbuf **m0, struct pfi_kkif *kif, int ooff; u_int8_t proto; int terminal; - int srs; + bool scrub_compat; PF_RULES_RASSERT(); r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr); - /* Check if there any scrub rules. Lack of scrub rules means enforced - * packet normalization operation just like in OpenBSD. */ - srs = (r != NULL); + /* + * Check if there are any scrub rules, matching or not. + * Lack of scrub rules means: + * - enforced packet normalization operation just like in OpenBSD + * With scrub rules: + * - packet normalization is performed if there is a matching scrub rule + * XXX: Fragment reassembly always performed for IPv6! + */ + scrub_compat = (r != NULL); while (r != NULL) { pf_counter_u64_add(&r->evaluations, 1); if (pfi_kkif_match(r->kif, kif) == r->ifnot) @@ -1235,7 +1246,7 @@ pf_normalize_ip6(struct mbuf **m0, struct pfi_kkif *kif, break; } - if (srs) { + if (scrub_compat) { /* With scrub rules present IPv6 normalization happens only * if one of rules has matched and it's not a "no scrub" rule */ if (r == NULL || r->action == PF_NOSCRUB) diff --git a/tests/sys/netpfil/pf/Makefile b/tests/sys/netpfil/pf/Makefile index c9adab5626d4..9337b95baf4e 100644 --- a/tests/sys/netpfil/pf/Makefile +++ b/tests/sys/netpfil/pf/Makefile @@ -13,6 +13,7 @@ ATF_TESTS_SH+= altq \ forward \ fragmentation_compat \ fragmentation_pass \ + fragmentation_no_reassembly \ get_state \ icmp \ killstate \ diff --git a/tests/sys/netpfil/pf/fragmentation_compat.sh b/tests/sys/netpfil/pf/fragmentation_compat.sh index f66ebbad6b33..21ce6b734ea1 100644 --- a/tests/sys/netpfil/pf/fragmentation_compat.sh +++ b/tests/sys/netpfil/pf/fragmentation_compat.sh @@ -300,17 +300,6 @@ reassemble_body() atf_check -s exit:0 -o ignore ping -c 1 192.0.2.2 jexec alcatraz pfctl -e - pft_set_rules alcatraz \ - "pass out" \ - "block in" \ - "pass in inet proto icmp all icmp-type echoreq" - - # Single fragment passes - atf_check -s exit:0 -o ignore ping -c 1 192.0.2.2 - - # But a fragmented ping does not - atf_check -s exit:2 -o ignore ping -c 1 -s 2000 192.0.2.2 - pft_set_rules alcatraz \ "scrub in" \ "pass out" \ diff --git a/tests/sys/netpfil/pf/fragmentation_no_reassembly.sh b/tests/sys/netpfil/pf/fragmentation_no_reassembly.sh new file mode 100644 index 000000000000..fb5c15ac2ff8 --- /dev/null +++ b/tests/sys/netpfil/pf/fragmentation_no_reassembly.sh @@ -0,0 +1,130 @@ +# +# SPDX-License-Identifier: BSD-2-Clause +# +# Copyright (c) 2017 Kristof Provost +# Copyright (c) 2023 Kajetan Staszkiewicz +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +. $(atf_get_srcdir)/utils.subr + +atf_test_case "match_full_v4" "cleanup" +match_full_v4_head() +{ + atf_set descr 'Matching non-fragmented IPv4 packets' + atf_set require.user root + atf_set require.progs scapy +} + +match_full_v4_body() +{ + setup_router_dummy_ipv4 + + # Sanity check. + ping_dummy_check_request exit:0 --ping-type=icmp + + # Only non-fragmented packets are passed + jexec router pfctl -e + pft_set_rules router \ + "pass out" \ + "block in" \ + "pass in inet proto icmp all icmp-type echoreq" + ping_dummy_check_request exit:0 --ping-type=icmp + ping_dummy_check_request exit:1 --ping-type=icmp --send-length=2000 --send-frag-length 1000 +} + +match_full_v4_cleanup() +{ + pft_cleanup +} + + +atf_test_case "match_fragment_v4" "cleanup" +match_fragment_v4_head() +{ + atf_set descr 'Matching fragmented IPv4 packets' + atf_set require.user root + atf_set require.progs scapy +} + +match_fragment_v4_body() +{ + setup_router_dummy_ipv4 + + # Sanity check. + ping_dummy_check_request exit:0 --ping-type=icmp + + # Only fragmented packets are passed + pft_set_rules router \ + "pass out" \ + "block in" \ + "pass in inet proto icmp fragment" + ping_dummy_check_request exit:1 --ping-type=icmp + ping_dummy_check_request exit:0 --ping-type=icmp --send-length=2000 --send-frag-length 1000 +} + +match_fragment_v4_cleanup() +{ + pft_cleanup +} + + +atf_test_case "compat_override_v4" "cleanup" +compat_override_v4_head() +{ + atf_set descr 'Scrub rules override "set reassemble" for IPv4' + atf_set require.user root + atf_set require.progs scapy +} + +compat_override_v4_body() +{ + setup_router_dummy_ipv4 + + # Sanity check. + ping_dummy_check_request exit:0 --ping-type=icmp + + # The same as match_fragment_v4 but with "set reassemble yes" which + # is ignored because of presence of scrub rules. + # Only fragmented packets are passed. + pft_set_rules router \ + "set reassemble yes" \ + "no scrub" \ + "pass out" \ + "block in" \ + "pass in inet proto icmp fragment" + ping_dummy_check_request exit:1 --ping-type=icmp + ping_dummy_check_request exit:0 --ping-type=icmp --send-length=2000 --send-frag-length 1000 +} + +compat_override_v4_cleanup() +{ + pft_cleanup +} + + +atf_init_test_cases() +{ + atf_add_test_case "match_full_v4" + atf_add_test_case "match_fragment_v4" + atf_add_test_case "compat_override_v4" +}