On Fri, Dec 24, 2010 at 07:48:23AM +1100, Robert B Mills wrote:
> On Thu, 23 Dec 2010, Robert B Mills wrote:
>
> >Date: Thu, 23 Dec 2010 20:35:13
> >From: Robert B Mills <[email protected]>
> >To: [email protected]
> >Subject: Small coding error in /usr/src/sys/net/pf.c
> >
> >Hello
> >
> >I have been having a problem trying to use the stateful tracking
> >option "override <TABLE> flush" in OpenBSD 4.7-stable. I'm running
> >an i386 GENERIC system as a vmware guest under Windows XP.
> >
> >To reproduce:
> >
> > 1. Install the following basic ruleset on a 4.7 system
> >
> > table <BLACKLIST> persist
> > block drop all
> > block drop quick from <BLACKLIST> to any
> > pass out all
> > pass in inet proto tcp from any to any port ssh keep state \
> > (max 20, max-src-conn-rate 2/20, overload <BLACKLIST> flush)
> >
> > 2. I tested the ruleset by starting 3 ssh sessions from a
> > Windows host to the 4.7 system. When the 3rd ssh session is
> > started, the overload rule is triggered, adding my Windows host
> > to the <BLACKLIST> table. What I see at this point is that the
> > 3rd ssh session does not complete the login, but the first two
> > sessions are fully operational. Using "systat states" I see that
> > the PF states for the 1st two ssh sessions are still up.
> >
> > This means the "flush" has not occurred.
> >
> >I have investigated the source code concerned with this behaviour and
> >I believe there is a coding error in /usr/src/sys/net/pf.c. The following
> >diff shows the small change I made to solve the problem:
> >
> Sorry. It was suggested to me that developers prefer
> "diff -up ...", so here's my diff in that format:
>
> obsd47> diff -up pf.c.orig pf.c --- pf.c.orig Sun Feb 21 05:01:14
> 2010
> +++ pf.c Thu Dec 23 19:11:49 2010
> @@ -477,9 +477,9 @@ pf_src_connlimit(struct pf_state **state)
> if (sk->af ==
> (*state)->key[PF_SK_WIRE]->af &&
> (((*state)->direction == PF_OUT &&
> - PF_AEQ(&sn->addr, &sk->addr[0], sk->af))
> ||
> + PF_AEQ(&sn->addr, &sk->addr[1], sk->af))
> ||
> ((*state)->direction == PF_IN &&
> - PF_AEQ(&sn->addr, &sk->addr[1], sk->af)))
> &&
> + PF_AEQ(&sn->addr, &sk->addr[0], sk->af)))
> &&
> ((*state)->rule.ptr->flush &
> PF_FLUSH_GLOBAL ||
> (*state)->rule.ptr == st->rule.ptr)) {
>
This looks right. For PF_OUT the sk->addr[1] represents the source
and for PF_IN sk->addr[0] would be the source as well. So that matches the
comment above this code block.
OK claudio@
--
:wq Claudio
