From owner-freebsd-pf@FreeBSD.ORG Wed Jul 25 18:56:10 2007 Return-Path: Delivered-To: freebsd-pf@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7F0A016A418 for ; Wed, 25 Jul 2007 18:56:10 +0000 (UTC) (envelope-from max@love2party.net) Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.174]) by mx1.freebsd.org (Postfix) with ESMTP id E9CAE13C461 for ; Wed, 25 Jul 2007 18:56:09 +0000 (UTC) (envelope-from max@love2party.net) Received: from [88.66.27.41] (helo=amd64.laiers.local) by mrelayeu.kundenserver.de (node=mrelayeu1) with ESMTP (Nemesis), id 0MKwpI-1IDm1Y2MAp-0005VO; Wed, 25 Jul 2007 20:55:54 +0200 From: Max Laier Organization: FreeBSD To: freebsd-pf@freebsd.org Date: Wed, 25 Jul 2007 20:55:40 +0200 User-Agent: KMail/1.9.7 References: <200702252202.l1PM2r46003312@cheyenne.sixcompanies.com> <200702261159.l1QBx46X006755@cheyenne.sixcompanies.com> <46A1EA91.5000306@dir.bg> In-Reply-To: <46A1EA91.5000306@dir.bg> X-Face: ,,8R(x[kmU]tKN@>gtH1yQE4aslGdu+2]; R]*pL,U>^H?)gW@49@wdJ`H<=?utf-8?q?=25=7D*=5FBD=0A=09U=5For=3D=5CmOZf764=26nYj=3DJYbR1PW0ud?=>|!~,,CPC.1-D$FG@0h3#'5"k{V]a~.<=?utf-8?q?mZ=7D44=23Se=7Em=0A=09Fe=7E=5C=5DX5B=5D=5Fxj?=(ykz9QKMw_l0C2AQ]}Ym8)fU MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart2701767.eXIVtM0v42"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <200707252055.50780.max@love2party.net> X-Provags-ID: V01U2FsdGVkX1/WGyraAYjjwEd/xFOnFoj2IAC5nZF87oJPjuF cSf4+hfO0P1e2xKVNebVPinl2+haet/3VsluXIgDGnn5QmVxk6 8dAunxIYi3FF5NDCZX3K5qeGpjN5o3Ra29AcwsGfg8= Cc: Jordan Gordeev , freebsd-questions@freebsd.org, jbronson@wixb.com Subject: Re: pf and keep/modulate state on 6.2 X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: freebsd-pf@freebsd.org List-Id: "Technical discussion and general questions about packet filter \(pf\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jul 2007 18:56:10 -0000 --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--