git: 57e047e51c6d - main - pf: allow scrub rules without fragment reassemble
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 28 Nov 2022 19:22:16 UTC
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);