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. :)