Date: Thu, 26 Oct 2023 14:16:54 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: ede5d4ff5b39 - main - pf: Fix packet reassembly Message-ID: <202310261416.39QEGs6V055079@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=ede5d4ff5b39ccbc193c30fb6c093c7c4de9a464 commit ede5d4ff5b39ccbc193c30fb6c093c7c4de9a464 Author: Kajetan Staszkiewicz <vegeta@tuxpowered.net> AuthorDate: 2023-10-26 12:26:33 +0000 Commit: Kristof Provost <kp@FreeBSD.org> 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 <kp@FreeBSD.org> +# Copyright (c) 2023 Kajetan Staszkiewicz <vegeta@tuxpowered.net> +# +# 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" +}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202310261416.39QEGs6V055079>