From nobody Mon Nov 28 19:22:16 2022 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 4NLb2J47Ssz4hhHv; Mon, 28 Nov 2022 19:22:16 +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 4NLb2J3cfKz3CKw; Mon, 28 Nov 2022 19:22:16 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1669663336; 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=2uIfYfVaIWUyXmoyb64RSmOW75kQOlfkN9GS0RGNgSE=; b=MsGezl1DMfYjc9q8fSSsps16PSjM77Lb/kfT8vuckoYCmsqc5jZogMgwkhLs/d9AvM8QMb qXDjwMNOWS5ZYEJlW4e8Malx3hpIAG1JyHkgwQP2yKewxtgSqPZgh0u2vZekcEBDpoagI4 CDwHpp3EByKEebsGHIZ7Pm2ArXe1z6Yl8dev41bFxXn6MAosuz3KTZKiSVzLk1TGkwXewB Vb3pGSflqEUuRTZfZ5/VYlUccddEanGSwqIy2QgpyGifTNADZfT0Pj0d9dGA8KMbD/5PGN PuWjwIoNsE0OYhRq89MtTn5Ran7AcODHYJcBTJ5itEsLYugG2iFrQCAoNq04yA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1669663336; 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=2uIfYfVaIWUyXmoyb64RSmOW75kQOlfkN9GS0RGNgSE=; b=w+S15cVyiLZfroap5F1+l8YgRZws7KkGxG9/rZ6vO5vzcL9igEv1ADF0dU0j9D8y1Kvi6h DcUE0BSdTpNyDlmcqQ5VkgiMECjAkuAMjo5m/DPqFtZncNjH6YvkGA/IDoBCmdbSSqEcpY JgyvC7MeDMpFKz3w7E7jad1ZNrLSwKnA8+u4HUCXx3CoEan5hiQsSuU2z1wbBWdc0pNCrU 4XkRbMItjgr0ds3kOUEFJCy02yXgUIka4SnQmGwC2Zigd2LwZFz+9EQ2DMjioiQT7SY84u FpIjCDbIKskDj6zblBrWdn1tKOegCkO12sDF8cLQSeCU7FvDdkGg7BoUviaG1Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1669663336; a=rsa-sha256; cv=none; b=ftNxVEfN1y8vSQCT/ev1fExG/Jm4R8eYLYftGURlkEZflOHu/HPflcsw/7qNvjSG6eTyZo ZW/yHH6VaVhOf3oa1hynktC4t+21STv6p0i7Ewc+zAzJgqDacTO2ES2bTW1UhmuALz3WNy OTFJ3OlP/rQB5onKIJKlaT3mQKwauGjF3Gp/hjVH/b1J5ylDb5zryzl6fneY31hPuJUX2L NqeboJQCJ4UzNEIgbK4D3fIyACcHEvigtHr4jPTwhB9rG6JKmYXg8dB093GbN61K1aylGD MMcX0cOKb+7f74GfLUxMlPi8aE0WGK4RcaPZa8X41uitioEWNuOmyCWBKeEBtA== 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 4NLb2J2gJYzpFs; Mon, 28 Nov 2022 19:22:16 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 2ASJMGO1052038; Mon, 28 Nov 2022 19:22:16 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 2ASJMG4Q052037; Mon, 28 Nov 2022 19:22:16 GMT (envelope-from git) Date: Mon, 28 Nov 2022 19:22:16 GMT Message-Id: <202211281922.2ASJMG4Q052037@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: 57e047e51c6d - main - pf: allow scrub rules without fragment reassemble 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: 57e047e51c6daf72912332bc95263084f4f0430c Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=57e047e51c6daf72912332bc95263084f4f0430c commit 57e047e51c6daf72912332bc95263084f4f0430c Author: Kristof Provost AuthorDate: 2022-11-22 13:23:27 +0000 Commit: Kristof Provost CommitDate: 2022-11-28 19:19:05 +0000 pf: allow scrub rules without fragment reassemble scrub rules have defaulted to handling fragments for a long time, but since we removed "fragment crop" and "fragment drop-ovl" in 64b3b4d611 this has become less obvious and more expensive ("reassemble" being the more expensive option, even if it's the one the vast majority of users should be using). Extend the 'scrub' syntax to allow fragment reassembly to be disabled, while retaining the other scrub behaviour (e.g. TTL changes, random-id, ..) using 'scrub fragment no reassemble'. Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D37459 --- sbin/pfctl/parse.y | 3 ++- sbin/pfctl/pfctl_parser.c | 3 ++- sbin/pfctl/tests/files/pf1011.in | 1 + sbin/pfctl/tests/files/pf1011.ok | 1 + sbin/pfctl/tests/files/pf1012.in | 1 + sbin/pfctl/tests/files/pf1012.ok | 1 + sbin/pfctl/tests/pfctl_test_list.inc | 2 ++ share/man/man5/pf.conf.5 | 5 ++++- sys/netpfil/pf/pf.h | 1 + sys/netpfil/pf/pf_norm.c | 39 +++++++++++++++++++----------------- 10 files changed, 36 insertions(+), 21 deletions(-) diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index 1d0494e6d0b9..166cbae79087 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -1528,7 +1528,8 @@ scrub_opt : NODF { } ; -fragcache : FRAGMENT REASSEMBLE { $$ = 0; /* default */ } +fragcache : FRAGMENT REASSEMBLE { $$ = 0; /* default */ } + | FRAGMENT NO REASSEMBLE { $$ = PFRULE_FRAGMENT_NOREASS; } | FRAGMENT FRAGCROP { $$ = 0; } | FRAGMENT FRAGDROP { $$ = 0; } ; diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c index 824e473ea2e9..fca1a7aa7b8f 100644 --- a/sbin/pfctl/pfctl_parser.c +++ b/sbin/pfctl/pfctl_parser.c @@ -1131,7 +1131,8 @@ print_rule(struct pfctl_rule *r, const char *anchor_call, int verbose, int numer if (r->rule_flag & PFRULE_REASSEMBLE_TCP) printf(" reassemble tcp"); - printf(" fragment reassemble"); + printf(" fragment %sreassemble", + r->rule_flag & PFRULE_FRAGMENT_NOREASS ? "no " : ""); } i = 0; while (r->label[i][0]) diff --git a/sbin/pfctl/tests/files/pf1011.in b/sbin/pfctl/tests/files/pf1011.in new file mode 100644 index 000000000000..84f0e7204e40 --- /dev/null +++ b/sbin/pfctl/tests/files/pf1011.in @@ -0,0 +1 @@ +scrub fragment no reassemble diff --git a/sbin/pfctl/tests/files/pf1011.ok b/sbin/pfctl/tests/files/pf1011.ok new file mode 100644 index 000000000000..48572b371d8d --- /dev/null +++ b/sbin/pfctl/tests/files/pf1011.ok @@ -0,0 +1 @@ +scrub all fragment no reassemble diff --git a/sbin/pfctl/tests/files/pf1012.in b/sbin/pfctl/tests/files/pf1012.in new file mode 100644 index 000000000000..9083d1bf5396 --- /dev/null +++ b/sbin/pfctl/tests/files/pf1012.in @@ -0,0 +1 @@ +scrub diff --git a/sbin/pfctl/tests/files/pf1012.ok b/sbin/pfctl/tests/files/pf1012.ok new file mode 100644 index 000000000000..b7f1f454fb6a --- /dev/null +++ b/sbin/pfctl/tests/files/pf1012.ok @@ -0,0 +1 @@ +scrub all fragment reassemble diff --git a/sbin/pfctl/tests/pfctl_test_list.inc b/sbin/pfctl/tests/pfctl_test_list.inc index 0b6d5110034d..0b7d89099efe 100644 --- a/sbin/pfctl/tests/pfctl_test_list.inc +++ b/sbin/pfctl/tests/pfctl_test_list.inc @@ -121,3 +121,5 @@ PFCTL_TEST(1007, "Basic ethernet rule") PFCTL_TEST(1008, "Ethernet rule with mask length") PFCTL_TEST(1009, "Ethernet rule with mask") PFCTL_TEST(1010, "POM_STICKYADDRESS test") +PFCTL_TEST(1011, "Test disabling scrub fragment reassemble") +PFCTL_TEST(1012, "Test scrub fragment reassemble is default") diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 index ed57ad5b1c9f..c7446f53bb49 100644 --- a/share/man/man5/pf.conf.5 +++ b/share/man/man5/pf.conf.5 @@ -28,7 +28,7 @@ .\" ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE .\" POSSIBILITY OF SUCH DAMAGE. .\" -.Dd October 28, 2022 +.Dd November 24, 2022 .Dt PF.CONF 5 .Os .Sh NAME @@ -832,6 +832,9 @@ packet, and only the completed packet is passed on to the filter. The advantage is that filter rules have to deal only with complete packets, and can ignore fragments. The drawback of caching fragments is the additional memory cost. +This is the default behaviour unless no fragment reassemble is specified. +.It Ar no fragment reassemble +Do not reassemble fragments. .It Ar reassemble tcp Statefully normalizes TCP connections. .Ar scrub reassemble tcp diff --git a/sys/netpfil/pf/pf.h b/sys/netpfil/pf/pf.h index cc3063b887c9..3e3ba977285a 100644 --- a/sys/netpfil/pf/pf.h +++ b/sys/netpfil/pf/pf.h @@ -598,6 +598,7 @@ struct pf_rule { /* scrub flags */ #define PFRULE_NODF 0x0100 +#define PFRULE_FRAGMENT_NOREASS 0x0200 #define PFRULE_RANDOMID 0x0800 #define PFRULE_REASSEMBLE_TCP 0x1000 #define PFRULE_SET_TOS 0x2000 diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c index 5827fb0b093b..283f6771221c 100644 --- a/sys/netpfil/pf/pf_norm.c +++ b/sys/netpfil/pf/pf_norm.c @@ -1115,31 +1115,34 @@ pf_normalize_ip(struct mbuf **m0, int dir, struct pfi_kkif *kif, u_short *reason DPFPRINTF(("max packet %d\n", fragoff + ip_len)); goto bad; } - max = fragoff + ip_len; - /* Fully buffer all of the fragments - * Might return a completely reassembled mbuf, or NULL */ - PF_FRAG_LOCK(); - DPFPRINTF(("reass frag %d @ %d-%d\n", h->ip_id, fragoff, max)); - verdict = pf_reassemble(m0, h, dir, reason); - PF_FRAG_UNLOCK(); + if (! (r->rule_flag & PFRULE_FRAGMENT_NOREASS)) { + max = fragoff + ip_len; - if (verdict != PF_PASS) - return (PF_DROP); + /* Fully buffer all of the fragments + * Might return a completely reassembled mbuf, or NULL */ + PF_FRAG_LOCK(); + DPFPRINTF(("reass frag %d @ %d-%d\n", h->ip_id, fragoff, max)); + verdict = pf_reassemble(m0, h, dir, reason); + PF_FRAG_UNLOCK(); - m = *m0; - if (m == NULL) - return (PF_DROP); + if (verdict != PF_PASS) + return (PF_DROP); + + m = *m0; + if (m == NULL) + return (PF_DROP); - h = mtod(m, struct ip *); + h = mtod(m, struct ip *); no_fragment: - /* At this point, only IP_DF is allowed in ip_off */ - if (h->ip_off & ~htons(IP_DF)) { - u_int16_t ip_off = h->ip_off; + /* At this point, only IP_DF is allowed in ip_off */ + if (h->ip_off & ~htons(IP_DF)) { + u_int16_t ip_off = h->ip_off; - h->ip_off &= htons(IP_DF); - h->ip_sum = pf_cksum_fixup(h->ip_sum, ip_off, h->ip_off, 0); + h->ip_off &= htons(IP_DF); + h->ip_sum = pf_cksum_fixup(h->ip_sum, ip_off, h->ip_off, 0); + } } pf_scrub_ip(&m, r->rule_flag, r->min_ttl, r->set_tos);