Date: Mon, 28 Nov 2022 19:22:16 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: 57e047e51c6d - main - pf: allow scrub rules without fragment reassemble Message-ID: <202211281922.2ASJMG4Q052037@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=57e047e51c6daf72912332bc95263084f4f0430c commit 57e047e51c6daf72912332bc95263084f4f0430c Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2022-11-22 13:23:27 +0000 Commit: Kristof Provost <kp@FreeBSD.org> 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202211281922.2ASJMG4Q052037>