netfilter+libvirt=(smth got broken?)

netfilter+libvirt=(smth got broken?)

[Nikolai Zhubr]

Hello,

I'm having problem setting up filtering traffic for a virtual machine 
managed by libvirt. Strange thing is, such a setup has been working fine 
for me on an older version of distro (namely, opensuse 11.3  w/updates, 
kernel 2.6.34, libvirt 0.8.8) but refused to work on shiny new opensuse 
12.4 (kernel 3.7.10, libvirt 1.0.2).

The definition of filter in question is pretty simple:

<filter name='some-filt' chain='ipv4'>
   <rule action='accept' direction='in'>
     <tcp dstportstart='110'/>
   </rule>
   <rule action='drop' direction='inout'>
     <all/>
   </rule>
</filter>

So basically it should allow incoming connections to the specified port 
number and nothing else. After activating this filter on a box in 
question, connections to 110 started to fail (timeout). Examining 
iptables rules manually and comparing them to the rules from my old box 
did not reveal anything suspicious to me. However, through just pure 
guesswork, I managed to ocasionally "fix" the problem by manually 
editing 3 relevant rules as follows:

--A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate 
ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
+-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate 
ESTABLISHED -m conntrack --ctdir REPLY -j RETURN

--A FO-vnet0 -p tcp -m tcp --dport 110 -m conntrack --ctstate 
NEW,ESTABLISHED -m conntrack --ctdir REPLY -j ACCEPT
+-A FO-vnet0 -p tcp -m tcp --dport 110 -m conntrack --ctstate 
NEW,ESTABLISHED -m conntrack --ctdir ORIGINAL -j ACCEPT

--A HI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate 
ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
+-A HI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate 
ESTABLISHED -m conntrack --ctdir REPLY -j RETURN

So essentially, just manually swtiching "--ctdir" values to their 
opposites makes the filter allow connections to port 110. I've also 
verified that the filter still blocks unwanted connections originating 
from port 110 from VM exactly as it should:

(in VM): netcat -v -v -n -p 110 192.168.122.1 22
(UNKNOWN) [192.168.122.1] 22 (?) : Connection timed out
  set 0, rcvd 0

I've then compared /proc/net/nf_conntrack on both (old and new) boxes. 
They look roughly the same, nothing suspicious.

This all looks to me as if "--ctdir" argument somehow magically changed 
its meaning to the opposite, but this just cannot be! I'm out of ideas 
and looking for insights. Any hints appreciated quite a lot.

Thank you.
Nikolai

Re: netfilter+libvirt=(smth got broken?)

[Nikolai Zhubr]

Hello,
20.03.2013 16:47, I wrote:
[...]
> for me on an older version of distro (namely, opensuse 11.3 w/updates,
> kernel 2.6.34, libvirt 0.8.8) but refused to work on shiny new opensuse
> 12.4 (kernel 3.7.10, libvirt 1.0.2).
^^^^^^ This was a typo: should read '12.3' of course, sorry.

Nikolai

Re: netfilter+libvirt=(smth got broken?)

[Nikolai Zhubr]

Hello,
20.03.2013 16:47, I wrote:
[...]
> This all looks to me as if "--ctdir" argument somehow magically changed
> its meaning to the opposite, but this just cannot be! I'm out of ideas
> and looking for insights. Any hints appreciated quite a lot.

Some more searching over maillists yielded this (quite astonishing):

net/netfilter/xt_conntrack.c	
diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index 2c0086a..481a86f 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -195,7 +195,7 @@ conntrack_mt(const struct sk_buff *skb, struct 
xt_action_param *par,
  		return info->match_flags & XT_CONNTRACK_STATE;
  	if ((info->match_flags & XT_CONNTRACK_DIRECTION) &&
  	    (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) ^
-	    !!(info->invert_flags & XT_CONNTRACK_DIRECTION))
+	    !(info->invert_flags & XT_CONNTRACK_DIRECTION))
  		return false;

  	if (info->match_flags & XT_CONNTRACK_ORIGSRC)

So apparently, netfilter's behaviour was indeed reversed at some point, 
therefore libvirt stopped working properly.

I'd guess libvirt needs to be adapted then? Is it a known issue or 
should I fill in bugreport at Novell/Red Hat?


Thank you.
Nikolai
>
> Thank you.
> Nikolai
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>

Re: netfilter+libvirt=(smth got broken?)

[Nikolai Zhubr]

21.03.2013 0:41, Laine Stump wrote:
[...]
>> -        !!(info->invert_flags&  XT_CONNTRACK_DIRECTION))
>> +        !(info->invert_flags&  XT_CONNTRACK_DIRECTION))
>>           return false;
>>
>>       if (info->match_flags&  XT_CONNTRACK_ORIGSRC)
>>
>> So apparently, netfilter's behaviour was indeed reversed at some
>> point, therefore libvirt stopped working properly.
>
> To save me the trouble, can you point me at a copy of the patch so I can
> read the commit message?

Maybe this
http://comments.gmane.org/gmane.comp.security.firewalls.netfilter.devel/38602
and this
http://git.opencores.org/?a=commit&p=linux&h=96120d86fe302c006259baee9061eea9e1b9e486
will be of some use.

>
> That seems a very bad thing to do :-/
>
>>
>> I'd guess libvirt needs to be adapted then? Is it a known issue or
>> should I fill in bugreport at Novell/Red Hat?
>
> I suppose it needs to be adapted, but how are we supposed to know which
> way to go? Some magic number of kernel version?

Yeah, makes me wonder.
Anyway, I've filed a bugreport at Novell (with a trivial patch proposed, 
although not taking into account possible "older" kernels hanging around 
with "historical" behaviour)

https://bugzilla.novell.com/show_bug.cgi?id=810611

Nikolai

>
> Bah. (This is the 2nd issue this week caused by a change in kernel ABI,
> so I'm not in a good mood...)
>
> _______________________________________________
> libvirt-users mailing list
> libvirt-users-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> https://www.redhat.com/mailman/listinfo/libvirt-users
>
>

Re: netfilter+libvirt=(smth got broken?)

[Pablo Neira Ayuso]

Hi Nikolai,

On Wed, Mar 20, 2013 at 05:41:37PM +0400, Nikolai Zhubr wrote:
> Hello,
> 20.03.2013 16:47, I wrote:
> [...]
> >This all looks to me as if "--ctdir" argument somehow magically changed
> >its meaning to the opposite, but this just cannot be! I'm out of ideas
> >and looking for insights. Any hints appreciated quite a lot.
> 
> Some more searching over maillists yielded this (quite astonishing):
> 
> net/netfilter/xt_conntrack.c	
> diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
> index 2c0086a..481a86f 100644
> --- a/net/netfilter/xt_conntrack.c
> +++ b/net/netfilter/xt_conntrack.c
> @@ -195,7 +195,7 @@ conntrack_mt(const struct sk_buff *skb, struct
> xt_action_param *par,
>  		return info->match_flags & XT_CONNTRACK_STATE;
>  	if ((info->match_flags & XT_CONNTRACK_DIRECTION) &&
>  	    (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) ^
> -	    !!(info->invert_flags & XT_CONNTRACK_DIRECTION))
> +	    !(info->invert_flags & XT_CONNTRACK_DIRECTION))
>  		return false;
> 
>  	if (info->match_flags & XT_CONNTRACK_ORIGSRC)
> 
> So apparently, netfilter's behaviour was indeed reversed at some
> point, therefore libvirt stopped working properly.

--ctdir was broken and it was fixed in patch:

commit 96120d86fe302c006259baee9061eea9e1b9e486
Author: Florian Westphal <fw@strlen.de>
Date:   Mon Apr 4 17:06:21 2011 +0200

    netfilter: xt_conntrack: fix inverted conntrack direction test

    --ctdir ORIGINAL matches REPLY packets, and vv:

    userspace sets "invert_flags &= ~XT_CONNTRACK_DIRECTION" in ORIGINAL
    case.

By looking at the changes you made:

>--A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
>+-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>ESTABLISHED -m conntrack --ctdir REPLY -j RETURN

The first rule looks wrong to me indeed, traffic coming in the
original direction will initiate the connection to destination port
TCP/110. Therefore, your change is correct.

It's unfortunate nobody noticed this rule was incorrect so far (even
if it was working).

> I'd guess libvirt needs to be adapted then? Is it a known issue or
> should I fill in bugreport at Novell/Red Hat?

I think you should file the bug.

Re: [libvirt-users] netfilter+libvirt=(smth got broken?)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

On 03/20/2013 08:30 PM, Pablo Neira Ayuso wrote:

>>
>> So apparently, netfilter's behaviour was indeed reversed at some
>> point, therefore libvirt stopped working properly.
> 
> --ctdir was broken and it was fixed in patch:

In other words, the kernel folks made a silent change in ABI.  Eww.

How can we reliably tell which kernels have the old behavior, and which
have the new, so that libvirt knows which sense to use?

> By looking at the changes you made:
> 
>> --A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>> ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
>> +-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>> ESTABLISHED -m conntrack --ctdir REPLY -j RETURN
> 
> The first rule looks wrong to me indeed, traffic coming in the
> original direction will initiate the connection to destination port
> TCP/110. Therefore, your change is correct.

Correct for the new kernel interpretation, but we also want to support
use of libvirt with older kernels, preferably with a runtime check so
that a binary compiled on an older kernel will still work after a kernel
upgrade.

> 
> It's unfortunate nobody noticed this rule was incorrect so far (even
> if it was working).

It's also unfortunate that the kernel folks did a silent ABI change,
without offering any witness of which behavior is in operation.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [libvirt-users] netfilter+libvirt=(smth got broken?)

[Pablo Neira Ayuso]

Hi Eric,

On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
[...]
> > By looking at the changes you made:
> > 
> >> --A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
> >> ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
> >> +-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
> >> ESTABLISHED -m conntrack --ctdir REPLY -j RETURN
> > 
> > The first rule looks wrong to me indeed, traffic coming in the
> > original direction will initiate the connection to destination port
> > TCP/110. Therefore, your change is correct.
> 
> Correct for the new kernel interpretation, but we also want to support
> use of libvirt with older kernels, preferably with a runtime check so
> that a binary compiled on an older kernel will still work after a kernel
> upgrade.

My suggestion is to relax that rule-set that you're using, ie. remove
the --ctdir. The connection tracking table and the TCP tracker already
take care for those invalid situations that you were trying to catch
with that --ctdir. You only have to add an iptables rule somewhere to
catch invalid packets.

I can also pass that patch to -stable starting 2.6.32 if that helps,
but you will still have to fix your rule-set.

Re: [libvirt-users] netfilter+libvirt=(smth got broken?)

[Pablo Neira Ayuso]

On Thu, Mar 21, 2013 at 10:55:42AM +0100, Pablo Neira Ayuso wrote:
> Hi Eric,
> 
> On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
> [...]
> > > By looking at the changes you made:
> > > 
> > >> --A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
> > >> ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
> > >> +-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
> > >> ESTABLISHED -m conntrack --ctdir REPLY -j RETURN
> > > 
> > > The first rule looks wrong to me indeed, traffic coming in the
> > > original direction will initiate the connection to destination port
> > > TCP/110. Therefore, your change is correct.
> > 
> > Correct for the new kernel interpretation, but we also want to support
> > use of libvirt with older kernels, preferably with a runtime check so
> > that a binary compiled on an older kernel will still work after a kernel
> > upgrade.
> 
> My suggestion is to relax that rule-set that you're using, ie. remove
> the --ctdir. The connection tracking table and the TCP tracker already
> take care for those invalid situations that you were trying to catch
> with that --ctdir. You only have to add an iptables rule somewhere to
> catch invalid packets.

In case you need more information, have a look at:

linux/net/netfilter/nf_conntrack_proto_tcp.c

Basically, the TCP tracker already validates that traffic is coming
from the correct direction. We have an internal state-machine for
that that will put coming in the wrong direction packets into the
INVALID state. If you have a rule-set whose default policy is drop or
you log and drop invalid packets, it will allow you to obtain the
effect you seem to be looking for. So basically, it's safe to remove
the --ctdir without having a less secure rule-set.

Regards.

Re: [libvirt-users] netfilter+libvirt=(smth got broken?)

[Laine Stump]

On 03/22/2013 06:53 AM, Pablo Neira Ayuso wrote:
> On Thu, Mar 21, 2013 at 10:55:42AM +0100, Pablo Neira Ayuso wrote:
>> Hi Eric,
>>
>> On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
>> [...]
>>>> By looking at the changes you made:
>>>>
>>>>> --A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>>>>> ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
>>>>> +-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>>>>> ESTABLISHED -m conntrack --ctdir REPLY -j RETURN
>>>> The first rule looks wrong to me indeed, traffic coming in the
>>>> original direction will initiate the connection to destination port
>>>> TCP/110. Therefore, your change is correct.
>>> Correct for the new kernel interpretation, but we also want to support
>>> use of libvirt with older kernels, preferably with a runtime check so
>>> that a binary compiled on an older kernel will still work after a kernel
>>> upgrade.
>> My suggestion is to relax that rule-set that you're using, ie. remove
>> the --ctdir. The connection tracking table and the TCP tracker already
>> take care for those invalid situations that you were trying to catch
>> with that --ctdir. You only have to add an iptables rule somewhere to
>> catch invalid packets.
> In case you need more information, have a look at:
>
> linux/net/netfilter/nf_conntrack_proto_tcp.c
>
> Basically, the TCP tracker already validates that traffic is coming
> from the correct direction. We have an internal state-machine for
> that that will put coming in the wrong direction packets into the
> INVALID state. If you have a rule-set whose default policy is drop or
> you log and drop invalid packets, it will allow you to obtain the
> effect you seem to be looking for. So basically, it's safe to remove
> the --ctdir without having a less secure rule-set.

So in effect, you're admitting that --ctdir is now more or less
unusable, since it's meaning/function can't be relied on, so everyone
should just avoid it. In retrospect then, it probably would have been a
much better decision to leave it "broken" (but at least in a
known/consistent/usable fashion). (Nothing to be done about that now,
though, since it's in the wild in both versions).

(I'm especially sensitive about this type of problem because this is the
3rd time in a week that I've been burned by incompatible changes to
kernel ABI. I feel like Joe Btfsplk.)

Re: [libvirt-users] netfilter+libvirt=(smth got broken?)

[Pablo Neira Ayuso]

On Fri, Mar 22, 2013 at 02:10:33PM -0400, Laine Stump wrote:
> On 03/22/2013 06:53 AM, Pablo Neira Ayuso wrote:
> > On Thu, Mar 21, 2013 at 10:55:42AM +0100, Pablo Neira Ayuso wrote:
> >> Hi Eric,
> >>
> >> On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
> >> [...]
> >>>> By looking at the changes you made:
> >>>>
> >>>>> --A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
> >>>>> ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
> >>>>> +-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
> >>>>> ESTABLISHED -m conntrack --ctdir REPLY -j RETURN
> >>>> The first rule looks wrong to me indeed, traffic coming in the
> >>>> original direction will initiate the connection to destination port
> >>>> TCP/110. Therefore, your change is correct.
> >>> Correct for the new kernel interpretation, but we also want to support
> >>> use of libvirt with older kernels, preferably with a runtime check so
> >>> that a binary compiled on an older kernel will still work after a kernel
> >>> upgrade.
> >> My suggestion is to relax that rule-set that you're using, ie. remove
> >> the --ctdir. The connection tracking table and the TCP tracker already
> >> take care for those invalid situations that you were trying to catch
> >> with that --ctdir. You only have to add an iptables rule somewhere to
> >> catch invalid packets.
> > In case you need more information, have a look at:
> >
> > linux/net/netfilter/nf_conntrack_proto_tcp.c
> >
> > Basically, the TCP tracker already validates that traffic is coming
> > from the correct direction. We have an internal state-machine for
> > that that will put coming in the wrong direction packets into the
> > INVALID state. If you have a rule-set whose default policy is drop or
> > you log and drop invalid packets, it will allow you to obtain the
> > effect you seem to be looking for. So basically, it's safe to remove
> > the --ctdir without having a less secure rule-set.
> 
> So in effect, you're admitting that --ctdir is now more or less
> unusable, since it's meaning/function can't be relied on, so everyone
> should just avoid it. In retrospect then, it probably would have been a
> much better decision to leave it "broken" (but at least in a
> known/consistent/usable fashion). (Nothing to be done about that now,
> though, since it's in the wild in both versions).

This option has also been broken for quite some time in user-space:

http://www.spinics.net/lists/netfilter-devel/msg15827.html

The fix was available starting iptables 1.4.11. The kernel fix went
into 2.6.39.

I'm trying to help you to find a good solution that works in all
cases, including old kernels, and to explain why that option, using it
in the broken way or not, provides no safer ruleset in the TCP case.

Regards.

Re: [libvirt-users] netfilter+libvirt=(smth got broken?)

[Laine Stump]

On 03/26/2013 10:18 AM, Pablo Neira Ayuso wrote:
> On Fri, Mar 22, 2013 at 02:10:33PM -0400, Laine Stump wrote:
>> On 03/22/2013 06:53 AM, Pablo Neira Ayuso wrote:
>>> On Thu, Mar 21, 2013 at 10:55:42AM +0100, Pablo Neira Ayuso wrote:
>>>> Hi Eric,
>>>>
>>>> On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
>>>> [...]
>>>>>> By looking at the changes you made:
>>>>>>
>>>>>>> --A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>>>>>>> ESTABLISHED -m conntrack --ctdir ORIGINAL -j RETURN
>>>>>>> +-A FI-vnet0 -p tcp -m tcp --sport 110 -m conntrack --ctstate
>>>>>>> ESTABLISHED -m conntrack --ctdir REPLY -j RETURN
>>>>>> The first rule looks wrong to me indeed, traffic coming in the
>>>>>> original direction will initiate the connection to destination port
>>>>>> TCP/110. Therefore, your change is correct.
>>>>> Correct for the new kernel interpretation, but we also want to support
>>>>> use of libvirt with older kernels, preferably with a runtime check so
>>>>> that a binary compiled on an older kernel will still work after a kernel
>>>>> upgrade.
>>>> My suggestion is to relax that rule-set that you're using, ie. remove
>>>> the --ctdir. The connection tracking table and the TCP tracker already
>>>> take care for those invalid situations that you were trying to catch
>>>> with that --ctdir. You only have to add an iptables rule somewhere to
>>>> catch invalid packets.
>>> In case you need more information, have a look at:
>>>
>>> linux/net/netfilter/nf_conntrack_proto_tcp.c
>>>
>>> Basically, the TCP tracker already validates that traffic is coming
>>> from the correct direction. We have an internal state-machine for
>>> that that will put coming in the wrong direction packets into the
>>> INVALID state. If you have a rule-set whose default policy is drop or
>>> you log and drop invalid packets, it will allow you to obtain the
>>> effect you seem to be looking for. So basically, it's safe to remove
>>> the --ctdir without having a less secure rule-set.
>> So in effect, you're admitting that --ctdir is now more or less
>> unusable, since it's meaning/function can't be relied on, so everyone
>> should just avoid it. In retrospect then, it probably would have been a
>> much better decision to leave it "broken" (but at least in a
>> known/consistent/usable fashion). (Nothing to be done about that now,
>> though, since it's in the wild in both versions).
> This option has also been broken for quite some time in user-space:
>
> http://www.spinics.net/lists/netfilter-devel/msg15827.html
>
> The fix was available starting iptables 1.4.11. The kernel fix went
> into 2.6.39.
>
> I'm trying to help you to find a good solution that works in all
> cases, including old kernels, and to explain why that option, using it
> in the broken way or not, provides no safer ruleset in the TCP case.

Understood and appreciated. I'm just pointing out the futility of
"fixing" something that's already in a published API.

Now I just need to convince Stefan that rulesets without --ctdir are
just as secure (where the limit of my "convince" is "point at your
message on the list" :-)

Re: [libvirt-users] netfilter+libvirt=(smth got broken?)

[Nikolai Zhubr]

Hello all,
21.03.2013 7:18, Eric Blake:
[...]
>> --ctdir was broken and it was fixed in patch:
>
> In other words, the kernel folks made a silent change in ABI.  Eww.
>
> How can we reliably tell which kernels have the old behavior, and which
> have the new, so that libvirt knows which sense to use?

There are so many customized kernels in the wild with whatever mix of 
patches applied and whatever custom versioning involved that IMHO no one 
can now really know without some special flag added for that (other than 
try and see)

[...]
>> It's unfortunate nobody noticed this rule was incorrect so far (even
>> if it was working).

In this case I'd say it was rather just somewhat inconsistent with it 
own documentation. Not a big deal. IMHO it would be OK to just add a 
notice to documentation saying that "for historical reasons" behaviour 
is inverted, instead of changing the code in question to make it wrose.

Alternatively, upon noticing unwanted inversion, netfilter could just 
introduce some new correct --ctdir2 and subsequently depreciate and 
remove original --ctdir, allowing some time for adaptation.

>
> It's also unfortunate that the kernel folks did a silent ABI change,
> without offering any witness of which behavior is in operation.
>

Yes, netfilter is extremely valuable and extremely respected project. 
However, breaking the work of other people so easily, for almost no 
reason, without even a word of discussion and without any proposed way 
to relaibly handle the situation seems surprising at least. Huge number 
of people depend on netfilter, really! Sorry for some rant.


Nikolai