[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy "DROP"

On 10/17/2011 05:22 PM, David Stevens wrote:
Stefan Berger<stefanb linux vnet ibm com>  wrote on 10/17/2011 10:29:08

Yes, '_at_the_end_', that's what I thought. I am not sure whether this
particular requirement is the best way to proceed since obviously you
cannot have any other rules with lesser priority after the ones doing
the 'return' -- whatever those rules may be doing.
         Not in the same chain, but  if a chain is checking multiple
allowed values for the same field, that's all that chain should be doing.
Sure, and what if I wanted to do something with packets that were not RETURN'ed, where would I treat them besides *after* the RETURN?
Checking some other condition should be done after passing, e.g., the IP
address checks and would be done because of the "RETURN". Current code
an ACCEPT or REJECT at that point, so *requires* modification of the
existing chain.
         Any current filters are even worse because without RETURN and
CONTINUE, they can only accept or drop a packet without further
Not arguing against that.
With this patchset, you can apply multiple tests to the same packet with
only the restriction that testing other fields in that packet require a
subchain-- something not possible at all today.
         If, for example, you want to allow 100 ports and a particular IP
address, reject everything else, won't that require a "DROP" rule for
65436 ports so that you don't accept either based on just the port or
just the IP address? Or , you'd have to combine the IP address check in
every port-check rule and do it before you do the blanket  "ACCEPT" in
the standard rule.

An alternative would be to say that the existing filters work with the
IP address learning thread and we would need to introduce new filters
for support of multiple IP addresses. Yes, the current filters aren't
prepare for supporting multiple IP addresses per interface.
         Yes, I thought about this, but it really is just duplicating the
entire set with a different name. I think especially without support
for RETURN/CONTINUE, as a practical matter the only way to modify the
current filters to do interesting things is to replace them. Anyone doing
that will not be affected by a change to the standard rules, except for
the possibly trivial addition of a "-j ACCEPT" at the end if they require
a default "ACCEPT" for the modified rules.

I don't think replacing the existing filters would be a problem per-se,
but I don't like that the priority of the rules doing the 'RETURN' is
assumed to be the lowest in the chain and we can just append anything to
the end of the chain -- the filters we are writing are just examples and
someone may come up with a few rules that do something with packets that
were not RETURN'ed and thus needs to have rules executing behind those.
Again, maybe (the less efficient but more generic way of) instantiating
the filters by calling the virNWFilterInstantiateFilterLate() could
solve part of the problem.
         The only use I've added for addRules is the
multiple address check and that should not have other random fields in
the same chain. Better would be to have a different chain linked at the
What if in the future we were to support ebtables logging and wanted to log any packet that has not been handled with a RETURN, thus for example logging MAC spoofing -- just as an example. I don't see how one would do that without adding rules into the mac chain *after* the rules that do the RETURN for packets with acceptable source MAC addresses and all others get logged. I would not call these 'random fields' being evaluated, but that the implementation of addRules() requires the filters to be limited in a way that may at some point become a problem for some use cases and someone has to reimplement it then.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]