Re: [libvirt] Appending REJECT rules.

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.