[NETFILTER -stable]: nf_conntrack: don't track locally generated special ICMP error

[NETFILTER -stable]: nf_conntrack: don't track locally generated special ICMP error

[Patrick McHardy]

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

Attached are two patches (stable.diff, applies to stable-2.6.21 and
stable-2.6.22 and 2.6.16.diff for stable-2.6.16) fixing incorrect
conntrack association of ICMP errors generated in response to INVALID
packets, causing incorrect address translation in combination with NAT.

Please apply, thanks.

[-- Attachment #2: stable.diff --]
[-- Type: text/x-diff, Size: 2563 bytes --]

[NETFILTER]: nf_conntrack: don't track locally generated special ICMP error

The conntrack assigned to locally generated ICMP error is usually the one
assigned to the original packet which has caused the error. But if
the original packet is handled as invalid by nf_conntrack, no conntrack
is assigned to the original packet. Then nf_ct_attach() cannot assign
any conntrack to the ICMP error packet. In that case the current
nf_conntrack_icmp assigns appropriate conntrack to it. But the current
code mistakes the direction of the packet. As a result, NAT code mistakes
the address to be mangled.

To fix the bug, this changes nf_conntrack_icmp not to assign conntrack
to such ICMP error. Actually no address is necessary to be mangled
in this case.

Spotted by Jordan Russell.

Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>

Upstream commit ID: 130e7a83d7ec8c5c673225e0fa8ea37b1ed507a5

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 0fea4ce3bbdda1442282bdff5657d0f91294c637
tree 94d6fc8768d4071ad33b46edf9bf8e151cc341e9
parent f44bba1a92e01bbab6ca9817b86ddf9e1744a616
author Patrick McHardy <kaber@trash.net> Tue, 17 Jul 2007 17:18:38 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 17 Jul 2007 17:18:38 +0200

 net/ipv4/netfilter/nf_conntrack_proto_icmp.c |   22 +++++-----------------
 1 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
index f4fc657..474b4ce 100644
--- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
@@ -189,25 +189,13 @@ icmp_error_message(struct sk_buff *skb,
 
 	h = nf_conntrack_find_get(&innertuple, NULL);
 	if (!h) {
-		/* Locally generated ICMPs will match inverted if they
-		   haven't been SNAT'ed yet */
-		/* FIXME: NAT code has to handle half-done double NAT --RR */
-		if (hooknum == NF_IP_LOCAL_OUT)
-			h = nf_conntrack_find_get(&origtuple, NULL);
-
-		if (!h) {
-			DEBUGP("icmp_error_message: no match\n");
-			return -NF_ACCEPT;
-		}
-
-		/* Reverse direction from that found */
-		if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
-			*ctinfo += IP_CT_IS_REPLY;
-	} else {
-		if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
-			*ctinfo += IP_CT_IS_REPLY;
+		DEBUGP("icmp_error_message: no match\n");
+		return -NF_ACCEPT;
 	}
 
+	if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
+		*ctinfo += IP_CT_IS_REPLY;
+
 	/* Update skb to refer to this connection */
 	skb->nfct = &nf_ct_tuplehash_to_ctrack(h)->ct_general;
 	skb->nfctinfo = *ctinfo;

[-- Attachment #3: 2.6.16.diff --]
[-- Type: text/x-diff, Size: 2584 bytes --]

[NETFILTER]: nf_conntrack: don't track locally generated special ICMP error

The conntrack assigned to locally generated ICMP error is usually the one
assigned to the original packet which has caused the error. But if
the original packet is handled as invalid by nf_conntrack, no conntrack
is assigned to the original packet. Then nf_ct_attach() cannot assign
any conntrack to the ICMP error packet. In that case the current
nf_conntrack_icmp assigns appropriate conntrack to it. But the current
code mistakes the direction of the packet. As a result, NAT code mistakes
the address to be mangled.

To fix the bug, this changes nf_conntrack_icmp not to assign conntrack
to such ICMP error. Actually no address is necessary to be mangled
in this case.

Spotted by Jordan Russell.

Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>

Upstream commit ID: 130e7a83d7ec8c5c673225e0fa8ea37b1ed507a5

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 935cfadcd1b69e828ecad5794319dd418a004793
tree 22962f7a436a7d2cf0934f78b178a9389fd70b6d
parent 6b99a1744ab187073bca84a9fd3ccbf091865ca6
author Patrick McHardy <kaber@trash.net> Tue, 17 Jul 2007 17:21:23 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 17 Jul 2007 17:21:23 +0200

 net/ipv4/netfilter/nf_conntrack_proto_icmp.c |   22 +++++-----------------
 1 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
index db9e7c4..8ef5c33 100644
--- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
@@ -193,25 +193,13 @@ icmp_error_message(struct sk_buff *skb,
 
 	h = nf_conntrack_find_get(&innertuple, NULL);
 	if (!h) {
-		/* Locally generated ICMPs will match inverted if they
-		   haven't been SNAT'ed yet */
-		/* FIXME: NAT code has to handle half-done double NAT --RR */
-		if (hooknum == NF_IP_LOCAL_OUT)
-			h = nf_conntrack_find_get(&origtuple, NULL);
-
-		if (!h) {
-			DEBUGP("icmp_error_message: no match\n");
-			return -NF_ACCEPT;
-		}
-
-		/* Reverse direction from that found */
-		if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
-			*ctinfo += IP_CT_IS_REPLY;
-	} else {
-		if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
-			*ctinfo += IP_CT_IS_REPLY;
+		DEBUGP("icmp_error_message: no match\n");
+		return -NF_ACCEPT;
 	}
 
+	if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
+		*ctinfo += IP_CT_IS_REPLY;
+
         /* Update skb to refer to this connection */
         skb->nfct = &nf_ct_tuplehash_to_ctrack(h)->ct_general;
         skb->nfctinfo = *ctinfo;

Re: [NETFILTER -stable]: nf_conntrack: don't track locally generated special ICMP error

[Adrian Bunk]

On Tue, Jul 17, 2007 at 05:25:10PM +0200, Patrick McHardy wrote:
> Attached are two patches (stable.diff, applies to stable-2.6.21 and
> stable-2.6.22 and 2.6.16.diff for stable-2.6.16) fixing incorrect
> conntrack association of ICMP errors generated in response to INVALID
> packets, causing incorrect address translation in combination with NAT.
> 
> Please apply, thanks.

Thanks, applied to 2.6.16.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

patch nf_conntrack-don-t-track-locally-generated-special-icmp-error.patch queued to -stable tree

[gregkh]

This is a note to let you know that we have just queued up the patch titled

     Subject: nf_conntrack: don't track locally generated special ICMP error

to the 2.6.22-stable tree.  Its filename is

     nf_conntrack-don-t-track-locally-generated-special-icmp-error.patch

A git repo of this tree can be found at 
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary


>From stable-bounces@linux.kernel.org Tue Jul 17 08:25:30 2007
From: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>
Date: Tue, 17 Jul 2007 17:25:10 +0200
Subject: nf_conntrack: don't track locally generated special ICMP error
To: stable@kernel.org
Cc: Netfilter Development Mailinglist <netfilter-devel@lists.netfilter.org>, "David S. Miller" <davem@davemloft.net>, Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp>, Adrian Bunk <bunk@stusta.de>
Message-ID: <469CDF56.80600@trash.net>

From: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>

[NETFILTER]: nf_conntrack: don't track locally generated special ICMP error

The conntrack assigned to locally generated ICMP error is usually the one
assigned to the original packet which has caused the error. But if
the original packet is handled as invalid by nf_conntrack, no conntrack
is assigned to the original packet. Then nf_ct_attach() cannot assign
any conntrack to the ICMP error packet. In that case the current
nf_conntrack_icmp assigns appropriate conntrack to it. But the current
code mistakes the direction of the packet. As a result, NAT code mistakes
the address to be mangled.

To fix the bug, this changes nf_conntrack_icmp not to assign conntrack
to such ICMP error. Actually no address is necessary to be mangled
in this case.

Spotted by Jordan Russell.

Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>

Upstream commit ID: 130e7a83d7ec8c5c673225e0fa8ea37b1ed507a5

Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

--- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
@@ -189,25 +189,13 @@ icmp_error_message(struct sk_buff *skb,
 
 	h = nf_conntrack_find_get(&innertuple, NULL);
 	if (!h) {
-		/* Locally generated ICMPs will match inverted if they
-		   haven't been SNAT'ed yet */
-		/* FIXME: NAT code has to handle half-done double NAT --RR */
-		if (hooknum == NF_IP_LOCAL_OUT)
-			h = nf_conntrack_find_get(&origtuple, NULL);
-
-		if (!h) {
-			DEBUGP("icmp_error_message: no match\n");
-			return -NF_ACCEPT;
-		}
-
-		/* Reverse direction from that found */
-		if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
-			*ctinfo += IP_CT_IS_REPLY;
-	} else {
-		if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
-			*ctinfo += IP_CT_IS_REPLY;
+		DEBUGP("icmp_error_message: no match\n");
+		return -NF_ACCEPT;
 	}
 
+	if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
+		*ctinfo += IP_CT_IS_REPLY;
+
 	/* Update skb to refer to this connection */
 	skb->nfct = &nf_ct_tuplehash_to_ctrack(h)->ct_general;
 	skb->nfctinfo = *ctinfo;



Patches currently in stable-queue which might be from yasuyuki.kozakai@toshiba.co.jp are

queue-2.6.22/nf_conntrack-don-t-track-locally-generated-special-icmp-error.patch