[libvirt] Appending REJECT rules.
Greetings folks, 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. 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); } /**
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); }
/**
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Greetings folks, Hello, and sorry for the delayed response. Looks like this fell through
On 05/18/2011 03:10 PM, Stephen O'Dor wrote: 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
On 06/22/2011 06:01 PM, Eric Blake wrote: the end. There shoudn't be any negative side effects because of this. So I'd give it an ACK. Stefan
/**
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 06/22/2011 08:58 PM, Stefan Berger wrote:
On 06/22/2011 06:01 PM, Eric Blake wrote:
Greetings folks, Hello, and sorry for the delayed response. Looks like this fell through
On 05/18/2011 03:10 PM, Stephen O'Dor wrote: 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.
On Thu, Jun 23, 2011 at 11:00:28AM -0400, Laine Stump wrote:
On 06/22/2011 08:58 PM, Stefan Berger wrote:
Greetings folks, Hello, and sorry for the delayed response. Looks like this fell through
On 05/18/2011 03:10 PM, Stephen O'Dor wrote: 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
On 06/22/2011 06:01 PM, Eric Blake wrote: 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 :|
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Laine Stump -
Stefan Berger -
Stephen O'Dor