On Thu, Jun 23, 2011 at 11:00:28AM -0400, Laine Stump wrote:
> On 06/22/2011 08:58 PM, Stefan Berger wrote:
> >On 06/22/2011 06:01 PM, Eric Blake wrote:
> >>On 05/18/2011 03:10 PM, Stephen O'Dor wrote:
> >>>Greetings folks,
> >>Hello, and sorry for the delayed response. Looks like this fell through
> >>the cracks, because it wasn't in traditional 'git format-patch' style.
> >>
> >>>I've patched the libvirt iptables interface to append it's REJECT
> >>>rules rather than insert at the head. Idea being that I'm not the only
> >>>person who usually puts the REJECTs at the end of a chain.
> >>>
> >>>In my particular case any custom ACCEPT rules involving the bridge
> >>>interfaces would get pushed below the rules that libvirt puts in to
> >>>REJECT everything on the bridge interface.
> >>>
> >>>I'm using the routed network mode, I have no idea if this hurts any
> >>>other network mode.
> >>Stefan is probably the best person to comment on whether this
> >>makes sense.
> >>
> >>>Thanks,
> >>>
> >>>-Steve
> >>>
> >>>
> >>>--- iptables.c 2011-02-28 23:03:32.000000000 -0800
> >>>+++ iptables.c_new 2011-05-18 14:00:59.110855881 -0700
> >>>@@ -51,7 +51,8 @@
> >>>
> >>> enum {
> >>> ADD = 0,
> >>>- REMOVE
> >>>+ REMOVE,
> >>>+ APPEND
> >>> };
> >>>
> >>> typedef struct
> >>>@@ -111,7 +112,7 @@
> >>> ? IP6TABLES_PATH : IPTABLES_PATH);
> >>>
> >>> virCommandAddArgList(cmd, "--table", rules->table,
> >>>- action == ADD ? "--insert" : "--delete",
> >>>+ action == ADD ? "--insert" : action ==
> >>>REMOVE ? "--delete" : "--append",
> >>> rules->chain, arg, NULL);
> >>>
> >>> va_start(args, arg);
> >>>@@ -666,7 +667,7 @@
> >>> int family,
> >>> const char *iface)
> >>> {
> >>>- return iptablesForwardRejectOut(ctx, family, iface, ADD);
> >>>+ return iptablesForwardRejectOut(ctx, family, iface, APPEND);
> >>> }
> >>>
> >>> /**
> >>>@@ -722,7 +723,7 @@
> >>> int family,
> >>> const char *iface)
> >>> {
> >>>- return iptablesForwardRejectIn(ctx, family, iface, ADD);
> >>>+ return iptablesForwardRejectIn(ctx, family, iface, APPEND);
> >>> }
> >>>
> >'ADD' caused an 'insertion' at position 0. Now 'APPEND' appends
> >the new rule to the end. To me it makes sense per-se to put the
> >reject rules to the end. There shoudn't be any negative side
> >effects because of this. So I'd give it an ACK.
>
> This very old bug demonstrates that changing the order of the rules
> can have unintended consequences.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=453580
>
> What does this patch do to that situation? A short synopsis - what
> we really want when there are two virtual networks is that the
> guests on the two networks be completely isolated from each other.
> Instead, with the current filter scheme, a guest on network "A" can
> contact a guest on network "B", but "guest B" can't contact " guest
> A". Will changing the ordering of the reject rules make this
> behavior better, worse, or will it remain the same? That question
> needs to be answered before making a decision about this patch.
Yes, appending the REJECT rules is not safe. If there are other existing
rules in the tables, then these rules may accidentally be allowing
traffic to/from the network we just added that is not desired. Of course
the reverse may also be true, so this is a no-win situation. We thus have
to pick the conservative option and insert at the head of the tables.
So NACK to this patch
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list