The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=c114db294d5d0cf82eb010c09061330aa0fdc925
commit c114db294d5d0cf82eb010c09061330aa0fdc925
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2025-06-24 11:27:56 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2025-06-27 14:55:16 +0000
pf: Refactor the six ways to find TCP options into one new function.
As a result:
- MSS and WSCALE option candidates must now meet their min type length.
- 'max-mss' is now more tolerant of malformed option lists.
These changes were immaterial to the live traffic I've examined.
OK sashan@ mpi@
Obtained from: OpenBSD, procter <proc...@openbsd.org>, 672fea2ccb
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sys/net/pfvar.h | 2 +
sys/netpfil/pf/pf.c | 198 ++++++++++++++++++-------------------
sys/netpfil/pf/pf_norm.c | 247 +++++++++++++++++++----------------------------
3 files changed, 193 insertions(+), 254 deletions(-)
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 9fc2a00dca77..71cb1862aabf 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2601,6 +2601,8 @@ int pf_tag_packet(struct pf_pdesc *, int);
int pf_addr_cmp(struct pf_addr *, struct pf_addr *,
sa_family_t);
+uint8_t* pf_find_tcpopt(u_int8_t *, u_int8_t *, size_t,
+ u_int8_t, u_int8_t);
u_int16_t pf_get_mss(struct pf_pdesc *);
u_int8_t pf_get_wscale(struct pf_pdesc *);
struct mbuf *pf_build_tcp(const struct pf_krule *, sa_family_t,
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 4ce2df2f0e31..f4d6f3dcb869 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -3933,55 +3933,42 @@ static int
pf_modulate_sack(struct pf_pdesc *pd, struct tcphdr *th,
struct pf_state_peer *dst)
{
- int hlen = (th->th_off << 2) - sizeof(*th), thoptlen = hlen;
- u_int8_t opts[TCP_MAXOLEN], *opt = opts;
- int copyback = 0, i, olen;
- struct sackblk sack;
-
-#define TCPOLEN_SACKLEN (TCPOLEN_SACK + 2)
- if (hlen < TCPOLEN_SACKLEN || hlen > MAX_TCPOPTLEN ||
- !pf_pull_hdr(pd->m, pd->off + sizeof(*th), opts, hlen, NULL, NULL,
pd->af))
- return 0;
-
- while (hlen >= TCPOLEN_SACKLEN) {
- size_t startoff = opt - opts;
- olen = opt[1];
- switch (*opt) {
- case TCPOPT_EOL: /* FALLTHROUGH */
- case TCPOPT_NOP:
- opt++;
- hlen--;
- break;
- case TCPOPT_SACK:
- if (olen > hlen)
- olen = hlen;
- if (olen >= TCPOLEN_SACKLEN) {
- for (i = 2; i + TCPOLEN_SACK <= olen;
- i += TCPOLEN_SACK) {
- memcpy(&sack, &opt[i], sizeof(sack));
- pf_patch_32(pd,
- &sack.start,
- htonl(ntohl(sack.start) -
dst->seqdiff),
- PF_ALGNMNT(startoff));
- pf_patch_32(pd,
- &sack.end,
- htonl(ntohl(sack.end) -
dst->seqdiff),
- PF_ALGNMNT(startoff));
- memcpy(&opt[i], &sack, sizeof(sack));
- copyback = 1;
- }
- }
- /* FALLTHROUGH */
- default:
- if (olen < 2)
- olen = 2;
- hlen -= olen;
- opt += olen;
+ struct sackblk sack;
+ int copyback = 0, i;
+ int olen, optsoff;
+ uint8_t opts[MAX_TCPOPTLEN], *opt, *eoh;
+
+ olen = (pd->hdr.tcp.th_off << 2) - sizeof(struct tcphdr);
+ optsoff = pd->off + sizeof(struct tcphdr);
+#define TCPOLEN_MINSACK (TCPOLEN_SACK + 2)
+ if (olen < TCPOLEN_MINSACK ||
+ !pf_pull_hdr(pd->m, optsoff, opts, olen, NULL, NULL, pd->af))
+ return (0);
+
+ eoh = opts + olen;
+ opt = opts;
+ while ((opt = pf_find_tcpopt(opt, opts, olen,
+ TCPOPT_SACK, TCPOLEN_MINSACK)) != NULL)
+ {
+ size_t safelen = MIN(opt[1], (eoh - opt));
+ for (i = 2; i + TCPOLEN_SACK <= safelen; i += TCPOLEN_SACK) {
+ size_t startoff = (opt + i) - opts;
+ memcpy(&sack, &opt[i], sizeof(sack));
+ pf_patch_32(pd, &sack.start,
+ htonl(ntohl(sack.start) - dst->seqdiff),
+ PF_ALGNMNT(startoff));
+ pf_patch_32(pd, &sack.end,
+ htonl(ntohl(sack.end) - dst->seqdiff),
+ PF_ALGNMNT(startoff + sizeof(sack.start)));
+ memcpy(&opt[i], &sack, sizeof(sack));
}
+ copyback = 1;
+ opt += opt[1];
}
if (copyback)
- m_copyback(pd->m, pd->off + sizeof(*th), thoptlen,
(caddr_t)opts);
+ m_copyback(pd->m, optsoff, olen, (caddr_t)opts);
+
return (copyback);
}
@@ -4965,83 +4952,86 @@ pf_socket_lookup(struct pf_pdesc *pd)
return (1);
}
-u_int8_t
-pf_get_wscale(struct pf_pdesc *pd)
+/* post: r => (r[0] == type /\ r[1] >= min_typelen >= 2 "validity"
+ * /\ (eoh - r) >= min_typelen >= 2 "safety" )
+ *
+ * warning: r + r[1] may exceed opts bounds for r[1] > min_typelen
+ */
+uint8_t*
+pf_find_tcpopt(u_int8_t *opt, u_int8_t *opts, size_t hlen, u_int8_t type,
+ u_int8_t min_typelen)
{
- struct tcphdr *th = &pd->hdr.tcp;
- int hlen;
- u_int8_t hdr[60];
- u_int8_t *opt, optlen;
- u_int8_t wscale = 0;
+ uint8_t *eoh = opts + hlen;
- hlen = th->th_off << 2; /* hlen <= sizeof(hdr) */
- if (hlen <= sizeof(struct tcphdr))
- return (0);
- if (!pf_pull_hdr(pd->m, pd->off, hdr, hlen, NULL, NULL, pd->af))
- return (0);
- opt = hdr + sizeof(struct tcphdr);
- hlen -= sizeof(struct tcphdr);
- while (hlen >= 3) {
+ if (min_typelen < 2)
+ return (NULL);
+
+ while ((eoh - opt) >= min_typelen) {
switch (*opt) {
case TCPOPT_EOL:
+ /* FALLTHROUGH - Workaround the failure of some
+ systems to NOP-pad their bzero'd option buffers,
+ producing spurious EOLs */
case TCPOPT_NOP:
- ++opt;
- --hlen;
- break;
- case TCPOPT_WINDOW:
- wscale = opt[2];
- if (wscale > TCP_MAX_WINSHIFT)
- wscale = TCP_MAX_WINSHIFT;
- wscale |= PF_WSCALE_FLAG;
- /* FALLTHROUGH */
+ opt++;
+ continue;
default:
- optlen = opt[1];
- if (optlen < 2)
- optlen = 2;
- hlen -= optlen;
- opt += optlen;
- break;
+ if (opt[0] == type &&
+ opt[1] >= min_typelen)
+ return (opt);
}
+
+ opt += MAX(opt[1], 2); /* evade infinite loops */
+ }
+
+ return (NULL);
+}
+
+u_int8_t
+pf_get_wscale(struct pf_pdesc *pd)
+{
+ int olen;
+ uint8_t opts[MAX_TCPOPTLEN], *opt;
+ uint8_t wscale = 0;
+
+ olen = (pd->hdr.tcp.th_off << 2) - sizeof(struct tcphdr);
+ if (olen < TCPOLEN_WINDOW || !pf_pull_hdr(pd->m,
+ pd->off + sizeof(struct tcphdr), opts, olen, NULL, NULL, pd->af))
+ return (0);
+
+ opt = opts;
+ while ((opt = pf_find_tcpopt(opt, opts, olen,
+ TCPOPT_WINDOW, TCPOLEN_WINDOW)) != NULL) {
+ wscale = opt[2];
+ wscale = MIN(wscale, TCP_MAX_WINSHIFT);
+ wscale |= PF_WSCALE_FLAG;
+
+ opt += opt[1];
}
+
return (wscale);
}
u_int16_t
pf_get_mss(struct pf_pdesc *pd)
{
- struct tcphdr *th = &pd->hdr.tcp;
- int hlen;
- u_int8_t hdr[60];
- u_int8_t *opt, optlen;
+ int olen;
+ uint8_t opts[MAX_TCPOPTLEN], *opt;
u_int16_t mss = V_tcp_mssdflt;
- hlen = th->th_off << 2; /* hlen <= sizeof(hdr) */
- if (hlen <= sizeof(struct tcphdr))
- return (0);
- if (!pf_pull_hdr(pd->m, pd->off, hdr, hlen, NULL, NULL, pd->af))
+ olen = (pd->hdr.tcp.th_off << 2) - sizeof(struct tcphdr);
+ if (olen < TCPOLEN_MAXSEG || !pf_pull_hdr(pd->m,
+ pd->off + sizeof(struct tcphdr), opts, olen, NULL, NULL, pd->af))
return (0);
- opt = hdr + sizeof(struct tcphdr);
- hlen -= sizeof(struct tcphdr);
- while (hlen >= TCPOLEN_MAXSEG) {
- switch (*opt) {
- case TCPOPT_EOL:
- case TCPOPT_NOP:
- ++opt;
- --hlen;
- break;
- case TCPOPT_MAXSEG:
- memcpy(&mss, (opt + 2), 2);
- mss = ntohs(mss);
- /* FALLTHROUGH */
- default:
- optlen = opt[1];
- if (optlen < 2)
- optlen = 2;
- hlen -= optlen;
- opt += optlen;
- break;
- }
+
+ opt = opts;
+ while ((opt = pf_find_tcpopt(opt, opts, olen,
+ TCPOPT_MAXSEG, TCPOLEN_MAXSEG)) != NULL) {
+ memcpy(&mss, (opt + 2), 2);
+ mss = ntohs(mss);
+ opt += opt[1];
}
+
return (mss);
}
diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c
index 94a436cdbfd6..369292ca365e 100644
--- a/sys/netpfil/pf/pf_norm.c
+++ b/sys/netpfil/pf/pf_norm.c
@@ -1499,8 +1499,8 @@ pf_normalize_tcp_init(struct pf_pdesc *pd, struct tcphdr
*th,
struct pf_state_peer *src)
{
u_int32_t tsval, tsecr;
- u_int8_t hdr[60];
- u_int8_t *opt;
+ int olen;
+ uint8_t opts[MAX_TCPOPTLEN], *opt;
KASSERT((src->scrub == NULL),
("pf_normalize_tcp_init: src->scrub != NULL"));
@@ -1535,43 +1535,25 @@ pf_normalize_tcp_init(struct pf_pdesc *pd, struct
tcphdr *th,
if ((tcp_get_flags(th) & TH_SYN) == 0)
return (0);
- if (th->th_off > (sizeof(struct tcphdr) >> 2) && src->scrub &&
- pf_pull_hdr(pd->m, pd->off, hdr, th->th_off << 2, NULL, NULL,
pd->af)) {
- /* Diddle with TCP options */
- int hlen;
- opt = hdr + sizeof(struct tcphdr);
- hlen = (th->th_off << 2) - sizeof(struct tcphdr);
- while (hlen >= TCPOLEN_TIMESTAMP) {
- switch (*opt) {
- case TCPOPT_EOL: /* FALLTHROUGH */
- case TCPOPT_NOP:
- opt++;
- hlen--;
- break;
- case TCPOPT_TIMESTAMP:
- if (opt[1] >= TCPOLEN_TIMESTAMP) {
- src->scrub->pfss_flags |=
- PFSS_TIMESTAMP;
- src->scrub->pfss_ts_mod =
- arc4random();
-
- /* note PFSS_PAWS not set yet */
- memcpy(&tsval, &opt[2],
- sizeof(u_int32_t));
- memcpy(&tsecr, &opt[6],
- sizeof(u_int32_t));
- src->scrub->pfss_tsval0 = ntohl(tsval);
- src->scrub->pfss_tsval = ntohl(tsval);
- src->scrub->pfss_tsecr = ntohl(tsecr);
- getmicrouptime(&src->scrub->pfss_last);
- }
- /* FALLTHROUGH */
- default:
- hlen -= MAX(opt[1], 2);
- opt += MAX(opt[1], 2);
- break;
- }
- }
+ olen = (th->th_off << 2) - sizeof(*th);
+ if (olen < TCPOLEN_TIMESTAMP || !pf_pull_hdr(pd->m,
+ pd->off + sizeof(*th), opts, olen, NULL, NULL, pd->af))
+ return (0);
+
+ opt = opts;
+ while ((opt = pf_find_tcpopt(opt, opts, olen,
+ TCPOPT_TIMESTAMP, TCPOLEN_TIMESTAMP)) != NULL) {
+ src->scrub->pfss_flags |= PFSS_TIMESTAMP;
+ src->scrub->pfss_ts_mod = arc4random();
+ /* note PFSS_PAWS not set yet */
+ memcpy(&tsval, &opt[2], sizeof(u_int32_t));
+ memcpy(&tsecr, &opt[6], sizeof(u_int32_t));
+ src->scrub->pfss_tsval0 = ntohl(tsval);
+ src->scrub->pfss_tsval = ntohl(tsval);
+ src->scrub->pfss_tsecr = ntohl(tsecr);
+ getmicrouptime(&src->scrub->pfss_last);
+
+ opt += opt[1];
}
return (0);
@@ -1611,13 +1593,12 @@ pf_normalize_tcp_stateful(struct pf_pdesc *pd,
struct pf_state_peer *src, struct pf_state_peer *dst, int *writeback)
{
struct timeval uptime;
- u_int32_t tsval, tsecr;
u_int tsval_from_last;
- u_int8_t hdr[60];
- u_int8_t *opt;
+ uint32_t tsval, tsecr;
int copyback = 0;
int got_ts = 0;
- size_t startoff;
+ int olen;
+ uint8_t opts[MAX_TCPOPTLEN], *opt;
KASSERT((src->scrub || dst->scrub),
("%s: src->scrub && dst->scrub!", __func__));
@@ -1654,80 +1635,64 @@ pf_normalize_tcp_stateful(struct pf_pdesc *pd,
unhandled_af(pd->af);
}
- if (th->th_off > (sizeof(struct tcphdr) >> 2) &&
+ olen = (th->th_off << 2) - sizeof(*th);
+
+ if (olen >= TCPOLEN_TIMESTAMP &&
((src->scrub && (src->scrub->pfss_flags & PFSS_TIMESTAMP)) ||
(dst->scrub && (dst->scrub->pfss_flags & PFSS_TIMESTAMP))) &&
- pf_pull_hdr(pd->m, pd->off, hdr, th->th_off << 2, NULL, NULL,
pd->af)) {
- /* Diddle with TCP options */
- int hlen;
- opt = hdr + sizeof(struct tcphdr);
- hlen = (th->th_off << 2) - sizeof(struct tcphdr);
- while (hlen >= TCPOLEN_TIMESTAMP) {
- startoff = opt - (hdr + sizeof(struct tcphdr));
- switch (*opt) {
- case TCPOPT_EOL: /* FALLTHROUGH */
- case TCPOPT_NOP:
- opt++;
- hlen--;
- break;
- case TCPOPT_TIMESTAMP:
- /* Modulate the timestamps. Can be used for
- * NAT detection, OS uptime determination or
- * reboot detection.
- */
-
- if (got_ts) {
- /* Huh? Multiple timestamps!? */
- if (V_pf_status.debug >= PF_DEBUG_MISC)
{
- DPFPRINTF(("multiple TS??\n"));
- pf_print_state(state);
- printf("\n");
- }
- REASON_SET(reason, PFRES_TS);
- return (PF_DROP);
- }
- if (opt[1] >= TCPOLEN_TIMESTAMP) {
- memcpy(&tsval, &opt[2],
- sizeof(u_int32_t));
- if (tsval && src->scrub &&
- (src->scrub->pfss_flags &
- PFSS_TIMESTAMP)) {
- tsval = ntohl(tsval);
- copyback += pf_patch_32(pd,
- &opt[2],
- htonl(tsval +
- src->scrub->pfss_ts_mod),
- PF_ALGNMNT(startoff));
- }
-
- /* Modulate TS reply iff valid (!0) */
- memcpy(&tsecr, &opt[6],
- sizeof(u_int32_t));
- if (tsecr && dst->scrub &&
- (dst->scrub->pfss_flags &
- PFSS_TIMESTAMP)) {
- tsecr = ntohl(tsecr)
- - dst->scrub->pfss_ts_mod;
- copyback += pf_patch_32(pd,
- &opt[6],
- htonl(tsecr),
- PF_ALGNMNT(startoff));
- }
- got_ts = 1;
+ pf_pull_hdr(pd->m, pd->off + sizeof(*th), opts, olen, NULL, NULL,
pd->af)) {
+ /* Modulate the timestamps. Can be used for NAT detection, OS
+ * uptime determination or reboot detection.
+ */
+ opt = opts;
+ while ((opt = pf_find_tcpopt(opt, opts, olen,
+ TCPOPT_TIMESTAMP, TCPOLEN_TIMESTAMP)) != NULL) {
+ uint8_t *ts = opt + 2;
+ uint8_t *tsr = opt + 6;
+
+ if (got_ts) {
+ /* Huh? Multiple timestamps!? */
+ if (V_pf_status.debug >= PF_DEBUG_MISC) {
+ printf("pf: %s: multiple TS??",
__func__);
+ pf_print_state(state);
+ printf("\n");
}
- /* FALLTHROUGH */
- default:
- hlen -= MAX(opt[1], 2);
- opt += MAX(opt[1], 2);
- break;
+ REASON_SET(reason, PFRES_TS);
+ return (PF_DROP);
}
+
+ memcpy(&tsval, ts, sizeof(u_int32_t));
+ memcpy(&tsecr, tsr, sizeof(u_int32_t));
+
+ /* modulate TS */
+ if (tsval && src->scrub &&
+ (src->scrub->pfss_flags & PFSS_TIMESTAMP)) {
+ /* tsval used further on */
+ tsval = ntohl(tsval);
+ pf_patch_32(pd,
+ ts, htonl(tsval + src->scrub->pfss_ts_mod),
+ PF_ALGNMNT(ts - opts));
+ copyback = 1;
+ }
+
+ /* modulate TS reply if any (!0) */
+ if (tsecr && dst->scrub &&
+ (dst->scrub->pfss_flags & PFSS_TIMESTAMP)) {
+ /* tsecr used further on */
+ tsecr = ntohl(tsecr) - dst->scrub->pfss_ts_mod;
+ pf_patch_32(pd, tsr, htonl(tsecr),
+ PF_ALGNMNT(tsr - opts));
+ copyback = 1;
+ }
+
+ got_ts = 1;
+ opt += opt[1];
}
+
if (copyback) {
/* Copyback the options, caller copys back header */
*writeback = 1;
- m_copyback(pd->m, pd->off + sizeof(struct tcphdr),
- (th->th_off << 2) - sizeof(struct tcphdr), hdr +
- sizeof(struct tcphdr));
+ m_copyback(pd->m, pd->off + sizeof(*th), olen, opts);
}
}
@@ -1999,50 +1964,32 @@ pf_normalize_tcp_stateful(struct pf_pdesc *pd,
int
pf_normalize_mss(struct pf_pdesc *pd)
{
- struct tcphdr *th = &pd->hdr.tcp;
- u_int16_t *mss;
- int thoff;
- int opt, cnt, optlen = 0;
- u_char opts[TCP_MAXOLEN];
- u_char *optp = opts;
- size_t startoff;
-
- thoff = th->th_off << 2;
- cnt = thoff - sizeof(struct tcphdr);
-
- if (cnt <= 0 || cnt > MAX_TCPOPTLEN || !pf_pull_hdr(pd->m,
- pd->off + sizeof(*th), opts, cnt, NULL, NULL, pd->af))
+ int olen, optsoff;
+ uint8_t opts[MAX_TCPOPTLEN], *opt;
+
+ olen = (pd->hdr.tcp.th_off << 2) - sizeof(struct tcphdr);
+ optsoff = pd->off + sizeof(struct tcphdr);
+ if (olen < TCPOLEN_MAXSEG ||
+ !pf_pull_hdr(pd->m, optsoff, opts, olen, NULL, NULL, pd->af))
return (0);
- for (; cnt > 0; cnt -= optlen, optp += optlen) {
- startoff = optp - opts;
- opt = optp[0];
- if (opt == TCPOPT_EOL)
- break;
- if (opt == TCPOPT_NOP)
- optlen = 1;
- else {
- if (cnt < 2)
- break;
- optlen = optp[1];
- if (optlen < 2 || optlen > cnt)
- break;
- }
- switch (opt) {
- case TCPOPT_MAXSEG:
- mss = (u_int16_t *)(optp + 2);
- if ((ntohs(*mss)) > pd->act.max_mss) {
- pf_patch_16(pd,
- mss, htons(pd->act.max_mss),
- PF_ALGNMNT(startoff));
- m_copyback(pd->m, pd->off + sizeof(*th),
- thoff - sizeof(*th), opts);
- m_copyback(pd->m, pd->off, sizeof(*th),
(caddr_t)th);
- }
- break;
- default:
- break;
+ opt = opts;
+ while ((opt = pf_find_tcpopt(opt, opts, olen,
+ TCPOPT_MAXSEG, TCPOLEN_MAXSEG)) != NULL) {
+ uint16_t mss;
+ uint8_t *mssp = opt + 2;
+ memcpy(&mss, mssp, sizeof(mss));
+ if (ntohs(mss) > pd->act.max_mss) {
+ size_t mssoffopts = mssp - opts;
+ pf_patch_16(pd, &mss,
+ htons(pd->act.max_mss), PF_ALGNMNT(mssoffopts));
+ m_copyback(pd->m, optsoff + mssoffopts,
+ sizeof(mss), (caddr_t)&mss);
+ m_copyback(pd->m, pd->off,
+ sizeof(struct tcphdr), (caddr_t)&pd->hdr.tcp);
}
+
+ opt += opt[1];
}
return (0);
