Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy "DROP"
Stefan Berger <stefanb(a)linux.vnet.ibm.com> wrote on 10/17/2011 10:29:08
AM:
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.
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
does
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
processing.
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
different
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
top after the address checks. In fact, for compatibility, it'd be possible
to change the address checks to a different chain and leave an empty
"ip" and "arp" chain that does only "accept". It's just
that it isn't
really all
that compatible if what's really happening is that a customer is replacing
the existing chains with modifications, rather than applying additional
rules
to it.
In the end, any modification whatsoever to the "examples"
directory
requires someone who has customized a filter to look at the customization
again. Addition of "RETURN"/"CONTINUE" makes that easier in the
future.
With
the existing filters, any customization cannot be independent of the
internals
of the existing filters (it's adding rules in the middle of it), so the
only
way to maintain compatibility is to never change them.
With "RETURN/CONTINUE" and your suggested "user-defined"
chain(s),
you
can apply the standard sets with "RETURN" for acceptable packets and then
apply any user-defined checks also to the same packets afterwards. I
think
that's the right approach, but it requires either changing the existing
chains
or introducing a completely parallel set and not using the original
filters
at all, except for sites that want the old stuff only.
+-DLS