Date: Wed, 25 Jul 2007 20:55:40 +0200 From: Max Laier <max@love2party.net> To: freebsd-pf@freebsd.org Cc: Jordan Gordeev <jgordeev@dir.bg>, freebsd-questions@freebsd.org, jbronson@wixb.com Subject: Re: pf and keep/modulate state on 6.2 Message-ID: <200707252055.50780.max@love2party.net> In-Reply-To: <46A1EA91.5000306@dir.bg> References: <200702252202.l1PM2r46003312@cheyenne.sixcompanies.com> <200702261159.l1QBx46X006755@cheyenne.sixcompanies.com> <46A1EA91.5000306@dir.bg>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart2701767.eXIVtM0v42 Content-Type: multipart/mixed; boundary="Boundary-01=_wy5pGu5kQ8qazKl" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_wy5pGu5kQ8qazKl Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Saturday 21 July 2007, Jordan Gordeev wrote: > J.D. Bronson wrote: > > At 02:52 AM 02/26/2007, you wrote: > >> Wow, this fixed my FTP-over-DSL-to-6.2 problem too. With modulate > >> state, I was getting ~30K/sec. With just keep state, I'm now getting > >> more like what my connection is capable of. This is between two 6.2 > >> hosts on opposite sides of the Atlantic. > >> > >> Ted, I use pf because I like the format of the configuration file, I > >> like the logging and pftop, and like how it's harder to lock > >> yourself out of a remote machine by accident :) > >> > >> /JMS > > > > I use pf since its newer (I think?) and I came from openbsd..pf just > > works and the config file is nice and sweet. > > > > I had thought that modulate state would put a load on my proc, but > > sheesh, its a p4-3.06 - thats more than robust for a router. > > > > I wonder if we should file a bug on this? > > > > I am glad my post helped here. I still use modulate state for any > > INCOMING connections though (www/smtp/etc). > > I'm replying to an old and long-forgotten thread to report my recent > findings. > There's a bug in PF with modulate/synproxy state. Modulate/synproxy > state modulate sequence numbers, but don't modulate sequence numbers in > TCP SACK options. Some firewalls block TCP segments with sequence > numbers in the SACK option pointing outside the window, which causes > connection stalls. The bug was fixed in OpenBSD with revision 1.509 of > src/sys/net/pf.c about an year and a half ago. The bug is present in > FreeBSD-STABLE. A fix for the bug was imported in FreeBSD-CURRENT with > the big import of PF from OpenBSD 4.1. > I'm CC-ing Max to notify him of the bug present in -STABLE and to ask > him to deal with the issue by either porting the fix from OpenBSD, or > by documenting that modulate/synproxy state is broken. Good catch - sorry for the delay. Here is the diff (almost verbatim from=20 OPENBSD_3_8). Please test and report back. I plan to commit this to=20 RELENG_6 in a bit. =2D-=20 /"\ Best regards, | mlaier@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mlaier@EFnet / \ ASCII Ribbon Campaign | Against HTML Mail and News --Boundary-01=_wy5pGu5kQ8qazKl Content-Type: text/x-diff; charset="iso-8859-1"; name="sack-mod.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="sack-mod.diff" Index: pf.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pf.c,v retrieving revision 1.34.2.4 diff -u -r1.34.2.4 pf.c =2D-- pf.c 19 Sep 2006 15:45:20 -0000 1.34.2.4 +++ pf.c 25 Jul 2007 18:51:35 -0000 @@ -1,5 +1,5 @@ /* $FreeBSD: src/sys/contrib/pf/net/pf.c,v 1.34.2.4 2006/09/19 15:45:20 cs= jp Exp $ */ =2D/* $OpenBSD: pf.c,v 1.483 2005/03/15 17:38:43 dhartmei Exp $ */ +/* $OpenBSD: pf.c,v 1.502.2.1 2006/05/02 22:55:52 brad Exp $ */ =20 /* * Copyright (c) 2001 Daniel Hartmeier @@ -170,6 +170,8 @@ void pf_change_ap(struct pf_addr *, u_int16_t *, u_int16_t *, u_int16_t *, struct pf_addr *, u_int16_t, u_int8_t, sa_family_t); +int pf_modulate_sack(struct mbuf *, int, struct pf_pdesc *, + struct tcphdr *, struct pf_state_peer *); #ifdef INET6 void pf_change_a6(struct pf_addr *, u_int16_t *, struct pf_addr *, u_int8_t); @@ -1483,6 +1485,63 @@ } #endif /* INET6 */ =20 + +/* + * Need to modulate the sequence numbers in the TCP SACK option + */ +int +pf_modulate_sack(struct mbuf *m, int off, struct pf_pdesc *pd, + struct tcphdr *th, struct pf_state_peer *dst) +{ + int hlen =3D (th->th_off << 2) - sizeof(*th), thoptlen =3D hlen; + u_int8_t opts[MAX_TCPOPTLEN], *opt =3D opts; + int copyback =3D 0, i, olen; + struct sackblk sack; + +#define TCPOLEN_SACKLEN (TCPOLEN_SACK + 2) + if (hlen < TCPOLEN_SACKLEN || + !pf_pull_hdr(m, off + sizeof(*th), opts, hlen, NULL, NULL, pd->af)) + return 0; + + while (hlen >=3D TCPOLEN_SACKLEN) { + olen =3D opt[1]; + switch (*opt) { + case TCPOPT_EOL: /* FALLTHROUGH */ + case TCPOPT_NOP: + opt++; + hlen--; + break; + case TCPOPT_SACK: + if (olen > hlen) + olen =3D hlen; + if (olen >=3D TCPOLEN_SACKLEN) { + for (i =3D 2; i + TCPOLEN_SACK <=3D olen; + i +=3D TCPOLEN_SACK) { + memcpy(&sack, &opt[i], sizeof(sack)); + pf_change_a(&sack.start, &th->th_sum, + htonl(ntohl(sack.start) - + dst->seqdiff), 0); + pf_change_a(&sack.end, &th->th_sum, + htonl(ntohl(sack.end) - + dst->seqdiff), 0); + memcpy(&opt[i], &sack, sizeof(sack)); + } + copyback =3D 1; + } + /* FALLTHROUGH */ + default: + if (olen < 2) + olen =3D 2; + hlen -=3D olen; + opt +=3D olen; + } + } + + if (copyback) + m_copyback(m, off + sizeof(*th), thoptlen, opts); + return (copyback); +} + void pf_change_icmp(struct pf_addr *ia, u_int16_t *ip, struct pf_addr *oa, struct pf_addr *na, u_int16_t np, u_int16_t *pc, u_int16_t *h2c, @@ -4577,6 +4636,25 @@ =20 ackskew =3D dst->seqlo - ack; =20 + + /* + * Need to demodulate the sequence numbers in any TCP SACK options + * (Selective ACK). We could optionally validate the SACK values + * against the current ACK window, either forwards or backwards, but + * I'm not confident that SACK has been implemented properly + * everywhere. It wouldn't surprise me if several stacks accidently + * SACK too far backwards of previously ACKed data. There really aren't + * any security implications of bad SACKing unless the target stack + * doesn't validate the option length correctly. Someone trying to + * spoof into a TCP connection won't bother blindly sending SACK + * options anyway. + */ + if (dst->seqdiff && (th->th_off << 2) > sizeof(struct tcphdr)) { + if (pf_modulate_sack(m, off, pd, th, dst)) + copyback =3D 1; + } + + #define MAXACKWINDOW (0xffff + 1500) /* 1500 is an arbitrary fudge factor = */ if (SEQ_GEQ(src->seqhi, end) && /* Last octet inside other's window space */ --Boundary-01=_wy5pGu5kQ8qazKl-- --nextPart2701767.eXIVtM0v42 Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQBGp5y2XyyEoT62BG0RAg79AJ9HAJxUJpQZchuFhvY6v2Zf9k01AQCfWO8A J5pVI8w7EIG9XKg6mznt1Jg= =C9in -----END PGP SIGNATURE----- --nextPart2701767.eXIVtM0v42--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200707252055.50780.max>