[libvirt] Appending REJECT rules.

[Laine Stump]

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.