All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [HELP] what is the semantic meaning of find_appropriate_src()?
       [not found] <OF87010E92.B8DC3ACD-ON48256E4B.002B8F90@legend.com.cn>
@ 2004-03-02 11:20 ` KOVACS Krisztian
  2004-03-07 12:24   ` [PATCH] find_appropriate_src() fix Kovacs Krisztian
  0 siblings, 1 reply; 2+ messages in thread
From: KOVACS Krisztian @ 2004-03-02 11:20 UTC (permalink / raw)
  To: wanghtb; +Cc: netfilter-devel


  Hi,

On Tue, 2004-03-02 at 08:56, wanghtb@legend.com.cn wrote:
> I tried  the UDP SNAT, but it still  do not work: find_appropriate_src() 
> do not
> find the proper manip.
> 
>  [Client A ] -----> [Box B ]   -----> [Server C]
>  
>  and the rule is: iptables -t nat -A POSTROUTING -p udp -j SNAT 
> --to-source B
>  
> consider two udp sessions: 
> i)  A.1 ---> C.1 
> ii) A.1 ----> C.2
> 
> when first udp session packet passes through Box B,
> ct1: ORIG A.1--->C.1  REPLY C.1--->B.1
> all is ok as we expected:-)
> 
> However, when the second udp session packet arrives Box B:
> at PRE_ROUTING hook, new ct is builded for it:
> ct2: ORIG A.1---->C.2 REPLY C.2 ---> A.1
> and at POST_ROUTING, find_appropriate_src()is called, and
> the content of mr will be to set new src_ip as  B.
> 
> Just a quick check of code, the result coincides with 
> the implementation of the function: the in_range() will
> always returnes false in the example.

  Oops, you're right. However, I don't think this is the intended
operation of find_appropriate_src(), I suspect this is a bug. The
problem seems to be the following:

1. ip_nat_setup_info() passes the inverse of the reply tuple to
get_unique_tuple() as orig_tuple, in our case this will be the same as
the tuple in the original direction (A.1 -> C.2). mr will be set to
MANIP_SRC to B. 

2. get_unique_tuple() calls find_appropriate_src() with orig_tuple and
mr.

3. find_appropriate_src() traverses the appropriate hash chain,
returning the first list entry for which src_cmp() returns true.

4. src_cmp() gets the following arguments: the current nat hash entry,
orig_tuple, and mr. It compares the list entry's ORIG_DIR source to
orig_tuple's source. In our case this matches. Then it calls in_range()

5. in_range() gets the following arguments: orig_tuple, ORIG_DIR source
of the current list entry as the manip, and mr. It constructs newtuple:
the source will be current conntrack's ORIG_DIR's source, the
destination that of orig_tuple. Note that orig_tuple is the same as the
to-be-NAT-ed connection's ORIG_DIR tuple, and the conntrack's source
must be equal, if in_range() is called by src_cmp(). However, in this
case, in_range returns false, because newtuple's source IP (A) won't be
the same as that given by mr (B).

  So, in our case in_range() does not apply the source manip to
orig_tuple, because src_cmp() passes the wrong manip to it. Maybe
src_cmp() should pass

&i->conntrack->tuplehash[IP_CT_DIR_REPLY].tuple.dst
instead of
&i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src

as manip to in_range()? I think it would make more sense, since we are
interested in the probably matching connection's NAT-ed source.

  This change is still not enough in our example. The following patch
was proposed for inclusion into the Linux kernel before the 2.4.23
release, however, it was rolled back since there were problems reported
when it was applied. (See 
http://lists.netfilter.org/pipermail/netfilter-devel/2003-November/013099.html) As a side note: this patch went into Linux 2.6, it was rolled back only in Linux 2.4. Doesn't it cause stability problems there?

--- linux-2.4.23-pre3/net/ipv4/netfilter/ip_nat_core.c	2003-11-08 
03:01:59.000000000 -1000
+++ linux-2.4.23-pre4/net/ipv4/netfilter/ip_nat_core.c	2003-11-08 
03:00:47.000000000 -1000
@@ -157,8 +157,8 @@
 				continue;
 		}
 
-		if ((mr->range[i].flags & IP_NAT_RANGE_PROTO_SPECIFIED)
-		    && proto->in_range(&newtuple, IP_NAT_MANIP_SRC,
+		if (!(mr->range[i].flags & IP_NAT_RANGE_PROTO_SPECIFIED)
+		    || proto->in_range(&newtuple, IP_NAT_MANIP_SRC,
 				       &mr->range[i].min, &mr->range[i].max))
 			return 1;
 	}

  The patch is obviously correct, as the old code fails to return 1 if
the IP_NAT_RANGE_PROTO_SPECIFIED flag is not set in the range flags.
However, for this code to be run, there must be some case when in_range
finds matching IP addresses, and goes on to check the per-protocol part.
This can happen only when a connection is NAT-ed to the same address as
its original address. As Karlis Peisenieks reported
(http://lists.netfilter.org/pipermail/netfilter-devel/2003-September/012368.html), this is the case for null-mappings. Because in this case the NAT entry maps the original source/dest IP to the same IP, src_cmp() will pass the correct manip to in_range(), and thus the code above will decide if find_appropriate_src() will find a matching NAT entry. Without the in_range() fix, it won't match, since IP_NAT_RANGE_PROTO_SPECIFIED is not set for null-mappings. However, with the fix, it will, thus possibly creating problems. (As in Karlis' case.)

  So I'm a bit confused. I think src_cmp() is buggy. However, I don't
understand how the in_range() fix could cause problems in 2.4, if it
works in 2.6...

-- 
 Regards,
   Krisztian KOVACS

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH] find_appropriate_src() fix
  2004-03-02 11:20 ` [HELP] what is the semantic meaning of find_appropriate_src()? KOVACS Krisztian
@ 2004-03-07 12:24   ` Kovacs Krisztian
  0 siblings, 0 replies; 2+ messages in thread
From: Kovacs Krisztian @ 2004-03-07 12:24 UTC (permalink / raw)
  To: netfilter-devel

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


  Hi,

  I've created a patch for the (suspected) problem described below. I've
test-booted the patch, and it seems to work in a simple MASQ setup, but
since this is my small home-network, I can't tell if it really does things
right.

  Could someone give it a try, and see if SNAT still works after applying
these changes? (The patch is for 2.6, won't do any good on 2.4 since the
in_range() function in 2.4 is seriously broken.)

On Tue, Mar 02, 2004 at 12:20:07PM +0100, KOVACS Krisztian wrote:
>   Oops, you're right. However, I don't think this is the intended
> operation of find_appropriate_src(), I suspect this is a bug. The
> problem seems to be the following:
> 
> 1. ip_nat_setup_info() passes the inverse of the reply tuple to
> get_unique_tuple() as orig_tuple, in our case this will be the same as
> the tuple in the original direction (A.1 -> C.2). mr will be set to
> MANIP_SRC to B. 
> 
> 2. get_unique_tuple() calls find_appropriate_src() with orig_tuple and
> mr.
> 
> 3. find_appropriate_src() traverses the appropriate hash chain,
> returning the first list entry for which src_cmp() returns true.
> 
> 4. src_cmp() gets the following arguments: the current nat hash entry,
> orig_tuple, and mr. It compares the list entry's ORIG_DIR source to
> orig_tuple's source. In our case this matches. Then it calls in_range()
> 
> 5. in_range() gets the following arguments: orig_tuple, ORIG_DIR source
> of the current list entry as the manip, and mr. It constructs newtuple:
> the source will be current conntrack's ORIG_DIR's source, the
> destination that of orig_tuple. Note that orig_tuple is the same as the
> to-be-NAT-ed connection's ORIG_DIR tuple, and the conntrack's source
> must be equal, if in_range() is called by src_cmp(). However, in this
> case, in_range returns false, because newtuple's source IP (A) won't be
> the same as that given by mr (B).
> 
>   So, in our case in_range() does not apply the source manip to
> orig_tuple, because src_cmp() passes the wrong manip to it. Maybe
> src_cmp() should pass
> 
> &i->conntrack->tuplehash[IP_CT_DIR_REPLY].tuple.dst
> instead of
> &i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src
> 
> as manip to in_range()? I think it would make more sense, since we are
> interested in the probably matching connection's NAT-ed source.

-- 
 Regards,
   Krisztian KOVACS


[-- Attachment #2: find_appropriate_src_fix.patch --]
[-- Type: text/plain, Size: 2550 bytes --]

Index: linux/net/ipv4/netfilter/ip_nat_core.c
===================================================================
--- linux.orig/net/ipv4/netfilter/ip_nat_core.c	2004-03-06 18:52:05.000000000 +0100
+++ linux/net/ipv4/netfilter/ip_nat_core.c	2004-03-07 13:05:42.000000000 +0100
@@ -176,20 +176,25 @@
 	const struct ip_conntrack_tuple *tuple,
 	const struct ip_nat_multi_range *mr)
 {
-	return (i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum
-		== tuple->dst.protonum
-		&& i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip
-		== tuple->src.ip
-		&& i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u.all
-		== tuple->src.u.all
-		&& in_range(tuple,
-			    &i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL]
-			    .tuple.src,
-			    mr));
+	struct ip_conntrack_tuple reverse;
+
+	if (i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum
+	    == tuple->dst.protonum
+	    && i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip
+	    == tuple->src.ip
+	    && i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u.all
+	    == tuple->src.u.all) {
+		/* the original source is the same, check if the same manip
+		 * would be appropriate for the given multi-range */
+		invert_tuplepr(&reverse, &i->conntrack->tuplehash[IP_CT_DIR_REPLY].tuple);
+		return in_range(tuple, &reverse.src, mr);
+	}
+
+	return 0;
 }
 
 /* Only called for SRC manip */
-static struct ip_conntrack_manip *
+static struct ip_conntrack_tuple *
 find_appropriate_src(const struct ip_conntrack_tuple *tuple,
 		     const struct ip_nat_multi_range *mr)
 {
@@ -199,7 +204,7 @@
 	MUST_BE_READ_LOCKED(&ip_nat_lock);
 	i = LIST_FIND(&bysource[h], src_cmp, struct ip_nat_hash *, tuple, mr);
 	if (i)
-		return &i->conntrack->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src;
+		return &i->conntrack->tuplehash[IP_CT_DIR_REPLY].tuple;
 	else
 		return NULL;
 }
@@ -419,13 +424,14 @@
 	   So far, we don't do local source mappings, so multiple
 	   manips not an issue.  */
 	if (hooknum == NF_IP_POST_ROUTING) {
-		struct ip_conntrack_manip *manip;
+		struct ip_conntrack_tuple *srctuple, mtuple;
 
-		manip = find_appropriate_src(orig_tuple, mr);
-		if (manip) {
+		srctuple = find_appropriate_src(orig_tuple, mr);
+		if (srctuple) {
 			/* Apply same source manipulation. */
+			invert_tuplepr(&mtuple, srctuple);
 			*tuple = ((struct ip_conntrack_tuple)
-				  { *manip, orig_tuple->dst });
+				  { mtuple.src, orig_tuple->dst });
 			DEBUGP("get_unique_tuple: Found current src map\n");
 			if (!ip_nat_used_tuple(tuple, conntrack))
 				return 1;

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2004-03-07 12:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <OF87010E92.B8DC3ACD-ON48256E4B.002B8F90@legend.com.cn>
2004-03-02 11:20 ` [HELP] what is the semantic meaning of find_appropriate_src()? KOVACS Krisztian
2004-03-07 12:24   ` [PATCH] find_appropriate_src() fix Kovacs Krisztian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.