netfilter+libvirt=(smth got broken?)

DISCUSSION Nikolai Zhubr 5 participants 12 messages Mar 20, 2013 – Mar 27, 2013 2 messages not in the archive

Subsystems (2)

Networking netfilter

lore

Share & tools

lore

Nikolai Zhubr

author

2013-03-20 12:47 UTC #0 lore raw

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:

    <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

Nikolai Zhubr

author

2013-03-20 13:06 UTC #1 lore raw

Hello,
20.03.2013 16:47, I wrote:
[...]

Quoted 3 lines

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

Nikolai Zhubr

author

2013-03-20 13:41 UTC #2 lore raw

In reply to Nikolai Zhubr

Hello,
20.03.2013 16:47, I wrote:
[...]

Quoted 3 lines

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

net/netfilter/xt_conntrack.c +1 -1

index 2c0086a..481a86f 100644
@@ -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

Quoted 11 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 514A1F0A.4090402-k/[email protected]

Nikolai Zhubr

2013-03-20 23:01 UTC #5 lore raw

21.03.2013 0:41, Laine Stump wrote:
[...]

Quoted 11 lines

Quoted 8 lines
(quoted diff) +1 -1
-        !!(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.

Quoted 9 lines

That seems a very bad thing to do :-/
Quoted 3 lines
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

Quoted 10 lines

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 [email protected]
https://www.redhat.com/mailman/listinfo/libvirt-users

Pablo Neira Ayuso

maintainer

2013-03-21 02:30 UTC #6 lore raw

In reply to Nikolai Zhubr

Hi Nikolai,

Quoted 25 lines · Nikolai Zhubr

Hello,
20.03.2013 16:47, I wrote:
[...]
Quoted 3 lines
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
net/netfilter/xt_conntrack.c +1 -1
index 2c0086a..481a86f 100644
@@ -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)

Quoted 2 lines

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 <[email protected]>
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:

Quoted 4 lines

--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).

Quoted 2 lines

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.

Eric Blake

2013-03-21 03:18 UTC #7 lore raw

Quoted 5 lines · Pablo Neira Ayuso

Quoted 3 lines
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?

Quoted 10 lines

By looking at the changes you made:
Quoted 4 lines
--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.

Quoted 3 lines

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.

Signature · 2 lines

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

Pablo Neira Ayuso

maintainer

2013-03-21 09:55 UTC #8 lore raw

Hi Eric,

On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
[...]

Quoted 7 lines

Quoted 7 lines
By looking at the changes you made:
Quoted 4 lines
--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

Quoted 8 lines

Quoted 3 lines
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.

Pablo Neira Ayuso

maintainer

2013-03-22 10:53 UTC #9 lore raw

Quoted 20 lines · Pablo Neira Ayuso

Hi Eric,
On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
[...]
Quoted 7 lines
Quoted 7 lines
By looking at the changes you made:
Quoted 4 lines
--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
Quoted 8 lines
Quoted 3 lines
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.

Quoted 5 lines

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.

Laine Stump

2013-03-22 18:10 UTC #10 lore raw

Quoted 27 lines · Pablo Neira Ayuso

Quoted 17 lines · Pablo Neira Ayuso
Hi Eric,
On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
[...]
Quoted 6 lines
Quoted 6 lines
By looking at the changes you made:
Quoted 4 lines
--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
Quoted 7 lines
Quoted 3 lines
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.
Quoted 5 lines
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

Quoted 7 lines

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.)

Pablo Neira Ayuso

maintainer

2013-03-26 14:18 UTC #11 lore raw

Quoted 36 lines · Laine Stump

Quoted 27 lines · Pablo Neira Ayuso
Quoted 17 lines · Pablo Neira Ayuso
Hi Eric,
On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
[...]
Quoted 6 lines
Quoted 6 lines
By looking at the changes you made:
Quoted 4 lines
--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
Quoted 7 lines
Quoted 3 lines
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.
Quoted 5 lines
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
Quoted 7 lines
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.

Quoted 6 lines

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.

Laine Stump

2013-03-27 18:22 UTC #12 lore raw

Quoted 44 lines · Pablo Neira Ayuso

Quoted 35 lines · Laine Stump
Quoted 27 lines · Pablo Neira Ayuso
Quoted 17 lines · Pablo Neira Ayuso
Hi Eric,
On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote:
[...]
Quoted 6 lines
Quoted 6 lines
By looking at the changes you made:
Quoted 4 lines
--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
Quoted 7 lines
Quoted 3 lines
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.
Quoted 5 lines
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
Quoted 7 lines
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.
Quoted 6 lines
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:

Quoted 8 lines

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" :-)

Nikolai Zhubr

author

2013-03-21 10:32 UTC #13 lore raw

In reply to Eric Blake

Hello all,
21.03.2013 7:18, Eric Blake:
[...]

Quoted 6 lines

Quoted 1 line
--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)

[...]

Quoted 2 lines

Quoted 2 lines
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.

Quoted 4 lines

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