what is the semantic meaning of find_appropriate_src()?

Hi,

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

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

 Regards,
   Krisztian KOVACS

Index: linux/net/ipv4/netfilter/ip_nat_core.c
===================================================================
@@ -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;