[libvirt-users] netfilter+libvirt=(smth got broken?)
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
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
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
On 03/20/2013 09:41 AM, 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.
To save me the trouble, can you point me at a copy of the patch so I can read the commit message? 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? Bah. (This is the 2nd issue this week caused by a change in kernel ABI, so I'm not in a good mood...)
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/3860... 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@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-users
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.
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
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.
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.
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,
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
On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote: [...] 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.)
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,
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
On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote: [...] 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.
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 Thu, Mar 21, 2013 at 10:55:42AM +0100, Pablo Neira Ayuso wrote:
Hi Eric,
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
On Wed, Mar 20, 2013 at 09:18:21PM -0600, Eric Blake wrote: [...] 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,
On 03/22/2013 06:53 AM, Pablo Neira Ayuso wrote: 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" :-)
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
participants (4)
-
Eric Blake -
Laine Stump -
Nikolai Zhubr -
Pablo Neira Ayuso