tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: NPF: fast kick

Le 08/03/2018 à 13:28, Mindaugas Rasiukevicius a écrit :

Please CC me when proposing NPF patches and please leave some extra time
for response.

Maxime Villard <> wrote:
Currently, NPF does not immediately kick malformed packets, and performs
very few sanity checks. Here is a patch [1] that fixes that.

There are a few important points:

- There is a good reason for NPF to be *able* to behave as a silent
observer.  It can be used for deep packet inspection, packet analysis,
accounting, etc.  Hence the reason why NPF performs minimalistic sanity
checks.  Please do not assume that the only mode of operation for NPF
is a traditional firewall.

- Having said that, we should certainly have an option (and I agree it
should be on by default) to perform extensive sanity checks and block
anything unusual.  Just please make it a run-time *option* (for now, a
config-level variable will do, and later I will make it changeable via

I don't understand your point. The checks I added enforce some basic aspects
of the specs; a packet that does not respect these aspects is just a malformed
packet, regardless of whether NPF is acting as a firewall or something else.

If NPF wasn't performing these checks, the packet would still be dropped by
the kernel afterwards.

We've already had too many problems because NPF basically doesn't sanitize

- It is better to keep a logical separation between the packet handler
which performs minimalistic sanity checks (just to extract the needed
information for NPF, e.g. populate the npf_cache_t structure) and the
function which performs a thorough validation.  So, can you please
introduce something like npf_deep_validate() (with an option to skip
it) where you abstract thorough protocol-level checks.

While I understand why you want a logical separation, I think that in all
fairness, the result is just shitty.

Typically, before my commit npc_hlen could point past the end of the mbuf. I
didn't even understand whether this was a real issue or not, because it gets
passed in many different components, and I wasn't able to make sure the
overflow was harmless (ie, make sure there are proper length checks in the
JIT code).

There is little interest in maintaining a logical separation if we are unable
to understand what happens in the complex machinery. As well, there is little
interest in passing a packet in this complex machinery if we know we'll end
up kicking it anyway because it is malformed to begin with.

So now let's perform basic sanitization early, outside of any ruleset, by
simply checking the return values of function calls that are *already there*.


Home | Main Index | Thread Index | Old Index