The NPF firewall leaks! (was Re: in_cksum: out of data)

[Tom Ivar Helbekkmo <tih%hamartun.priv.no@localhost>]

Tom Ivar Helbekkmo <tih%hamartun.priv.no@localhost> writes:

> So far, I have just one improvement suggestion for npf: the ability to
> use sets instead of singletons in rules is great, but needs to be
> extended to letting sets of addresses and networks cross address
> families.

I now have one more.  I accidentally created a leak in my npf
configuration, partially caused by looking at the example in the man
page npf.conf(5).

I've got several VLANs, one of them connected to the outside world, and
the others to internal networks with various levels of trust.  To limit
access among them, I've configured npf to handle each VLAN by allowing
all outbound traffic, statefully, while limiting inbound traffic to the
particular connections I want to allow.

The groups typically follow this pattern:

group "vlan10" on $vlan10 {
    pass stateful out final all
    pass in final proto tcp to $somehost port $someservices
    pass in final proto udp to $somehost port $otherservices
    block return in final all
}

Can you spot the vulnerability?

Some of the attack software that probes well-known ports to look for
holes, will respond to a TCP RST by sending a new TCP SYN from the very
same source port.  Guess what npf does then?  :)

Yup, the TCP RST sent by the last line of the above example gets
permitted out by the rule in the first line, updating the connection
state -- and the next connection attempt is permitted.

I had to change the above to this:

group "vlan10" on $vlan10 {
    pass stateful out final proto tcp flags S/SAFR all
    pass out final proto tcp all
    pass stateful out final all
    pass in final proto tcp to $somehost port $someservices
    pass in final proto udp to $somehost port $otherservices
    block return in final all
}

It's fine and all, but I tend to think that the simplistic first version
might automatically expand to the code in the second one.  In fact, the
documentation seems to agree with me:

     By default, a stateful rule implies SYN-only flag check ("flags
     S/SAFR") for the TCP packets.  It is not advisable to change this
     behavior; however, it can be overridden with the flags keyword.

The code or the documentation needs to change.  I vote for the code.  :)

-tih
-- 
I like long walks, especially when they are taken by people who annoy me.