Date: Fri, 27 Jun 2025 15:16:13 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: c114db294d5d - main - pf: Refactor the six ways to find TCP options into one new function. Message-ID: <202506271516.55RFGDHU049120@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=c114db294d5d0cf82eb010c09061330aa0fdc925 commit c114db294d5d0cf82eb010c09061330aa0fdc925 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-06-24 11:27:56 +0000 Commit: Kristof Provost <kp@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 <procter@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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202506271516.55RFGDHU049120>