Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Martin Josefsson]

On Tue, 7 Apr 2009, Andrew Morton wrote:

>> Back in the days of 2.6.18 there was the SAME target which allowed, with the
>> option '--nodst' to SNAT internal hosts to the same address of a whole SNAT
>> range regardless of the destination address.
>>
>> In cb76c6a597350534d211ba79d92da1f9771f8226 the SAME target was removed from
>> the kernel sources due to being obsolete, since the same functionality was now
>> in nf_nat. Shortly after that a discussion Patrick McHardy proposed a patch to
>> mimic the behaviour of SAME with --nodst in nf_nat by dropping the destination
>> IP from the jhash. The patch was dropped shortly after because it apparently
>> showed some uneven distribution.
>>
>> The whole thread can be read at
>> http://thread.gmane.org/gmane.comp.security.firewalls.netfilter.devel/23275/focus=27670
>> .
>>
>> This thread went dead, I tried to revive it but did not get an answer. We're
>> getting hit by this regression because we are currently NATing some thousand IP
>> addresses (student dorms) to an external /28. It works fine with our old
>> 2.6.18+SAME setup, but tests with 2.6.25+SNAT showed massive issues with
>> connections from the same internal address to different destinations getting
>> NATed to different addresses in the pool. Which breaks, for example, ICQ quite
>> badly.

Problems like these were the reason why I wrote the SAME target in the 
first place. I NAT'ed a few hundred students behind a small range of 
ipaddresses and with normal SNAT they had extreme problems with ICQ and 
online banking sites which require you to have the same source ip even if 
you move between servers (diffrent destinations) etc.

After I wrote SAME I didn't get a single complaint.

One bonus feature that many students really liked was that they always 
had the same external ipaddress as long as they had the same internal 
ipaddress and the external range of ipaddresses remained the same.

This feature isn't as important as having the same source ip for all 
current sessions when using online banking services etc.

I didn't think anyone was still using SAME... guess I was wrong.

The uneven distribution of the patch in question can't be worse than the 
SAME distributions which, iirc, just does something like:

first_ip_in_snat_range + (client_ip % num_ips_in_snat_range)

And given many clients behind the SNAT the distribution should be quite 
even.


Patrick, how about making the ipaddress selection based on only client 
ipaddress behaviour selectable with an SNAT parameter if the problem with 
the patch is that the distribution can be uneven for a small number of 
clients?

/Martin

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Patrick McHardy]

[Resend to mailing list, I didn't notice this had left bugzilla]

Martin Josefsson wrote:
> Patrick, how about making the ipaddress selection based on only client 
> ipaddress behaviour selectable with an SNAT parameter if the problem 
> with the patch is that the distribution can be uneven for a small number 
> of clients?

Actually I think the results back then were incorrect or it was
just bad luck or something. Ideally we would just enable this
unconditionally I think. I'll do some testing of the distribution
myself during the next days and see how it goes.

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Patrick McHardy]

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

Patrick McHardy wrote:
> [Resend to mailing list, I didn't notice this had left bugzilla]
> 
> Martin Josefsson wrote:
>> Patrick, how about making the ipaddress selection based on only client 
>> ipaddress behaviour selectable with an SNAT parameter if the problem 
>> with the patch is that the distribution can be uneven for a small 
>> number of clients?
> 
> Actually I think the results back then were incorrect or it was
> just bad luck or something. Ideally we would just enable this
> unconditionally I think. I'll do some testing of the distribution
> myself during the next days and see how it goes.

The distribution did suffer noticably in some cases, so offering
this optionally seems better.

How about this patch? If the IP_NAT_RANGE_PERSISTENT flag is set
on a NAT range, we ignore the destination address in the selection.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1015 bytes --]

diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index 9dc1039..8df0b7f 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -18,6 +18,7 @@ enum nf_nat_manip_type
 #define IP_NAT_RANGE_MAP_IPS 1
 #define IP_NAT_RANGE_PROTO_SPECIFIED 2
 #define IP_NAT_RANGE_PROTO_RANDOM 4
+#define IP_NAT_RANGE_PERSISTENT 8
 
 /* NAT sequence number modifications */
 struct nf_nat_seq {
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index fe65187..3229e0a 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -211,7 +211,8 @@ find_best_ips_proto(struct nf_conntrack_tuple *tuple,
 	minip = ntohl(range->min_ip);
 	maxip = ntohl(range->max_ip);
 	j = jhash_2words((__force u32)tuple->src.u3.ip,
-			 (__force u32)tuple->dst.u3.ip, 0);
+			 range->flags & IP_NAT_RANGE_PERSISTENT ?
+				(__force u32)tuple->dst.u3.ip : 0, 0);
 	j = ((u64)j * (maxip - minip + 1)) >> 32;
 	*var_ipp = htonl(minip + j);
 }

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Jan Engelhardt]

On Wednesday 2009-04-15 13:53, Patrick McHardy wrote:

> Patrick McHardy wrote:
>> [Resend to mailing list, I didn't notice this had left bugzilla]
>>
>> Martin Josefsson wrote:
>>> Patrick, how about making the ipaddress selection based on only client
>>> ipaddress behaviour selectable with an SNAT parameter if the problem with the
>>> patch is that the distribution can be uneven for a small number of clients?
>>
>> Actually I think the results back then were incorrect or it was
>> just bad luck or something. Ideally we would just enable this
>> unconditionally I think. I'll do some testing of the distribution
>> myself during the next days and see how it goes.
>
> The distribution did suffer noticably in some cases, so offering
> this optionally seems better.
>
> How about this patch? If the IP_NAT_RANGE_PERSISTENT flag is set
> on a NAT range, we ignore the destination address in the selection.

But where do you set IP_NAT_RANGE_PERSISTENT? (It seems like a dead 
feature right now.)

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2009-04-15 13:53, Patrick McHardy wrote:
>   
>> How about this patch? If the IP_NAT_RANGE_PERSISTENT flag is set
>> on a NAT range, we ignore the destination address in the selection.
>>     
>
> But where do you set IP_NAT_RANGE_PERSISTENT? (It seems like a dead 
> feature right now.)
>   

In userspace of course :)

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Jan Engelhardt]

On Wednesday 2009-04-15 14:13, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Wednesday 2009-04-15 13:53, Patrick McHardy wrote:
>>   
>>> How about this patch? If the IP_NAT_RANGE_PERSISTENT flag is set
>>> on a NAT range, we ignore the destination address in the selection.
>>
>> But where do you set IP_NAT_RANGE_PERSISTENT? (It seems like a dead 
>> feature right now.)
>
>In userspace of course :)

Ah I hear the crisp sound of an upcoming iptables 1.4.4.

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Patrick McHardy]

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

Jan Engelhardt wrote:
> On Wednesday 2009-04-15 14:13, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> On Wednesday 2009-04-15 13:53, Patrick McHardy wrote:
>>>   
>>>> How about this patch? If the IP_NAT_RANGE_PERSISTENT flag is set
>>>> on a NAT range, we ignore the destination address in the selection.
>>> But where do you set IP_NAT_RANGE_PERSISTENT? (It seems like a dead 
>>> feature right now.)
>> In userspace of course :)
> 
> Ah I hear the crisp sound of an upcoming iptables 1.4.4.

In a while :)

This is the corresponding userspace patch:


[-- Attachment #2: nat.diff --]
[-- Type: text/x-patch, Size: 3211 bytes --]

diff --git a/extensions/libipt_DNAT.c b/extensions/libipt_DNAT.c
index b5f8028..dc79b44 100644
--- a/extensions/libipt_DNAT.c
+++ b/extensions/libipt_DNAT.c
@@ -27,12 +27,13 @@ static void DNAT_help(void)
 "DNAT target options:\n"
 " --to-destination <ipaddr>[-<ipaddr>][:port-port]\n"
 "				Address to map destination to.\n"
-"[--random]\n");
+"[--random] [--persistent]\n");
 }
 
 static const struct option DNAT_opts[] = {
 	{ "to-destination", 1, NULL, '1' },
 	{ "random", 0, NULL, '2' },
+	{ "persistent", 0, NULL, '3' },
 	{ .name = NULL }
 };
 
@@ -178,6 +179,11 @@ static int DNAT_parse(int c, char **argv, int invert, unsigned int *flags,
 		} else
 			*flags |= IPT_DNAT_OPT_RANDOM;
 		return 1;
+
+	case '3':
+		info->mr.range[0].flags |= IP_NAT_RANGE_PERSISTENT;
+		return 1;
+
 	default:
 		return 0;
 	}
@@ -222,6 +228,8 @@ static void DNAT_print(const void *ip, const struct xt_entry_target *target,
 		printf(" ");
 		if (info->mr.range[i].flags & IP_NAT_RANGE_PROTO_RANDOM)
 			printf("random ");
+		if (info->mr.range[i].flags & IP_NAT_RANGE_PERSISTENT)
+			printf("persistent ");
 	}
 }
 
@@ -236,6 +244,8 @@ static void DNAT_save(const void *ip, const struct xt_entry_target *target)
 		printf(" ");
 		if (info->mr.range[i].flags & IP_NAT_RANGE_PROTO_RANDOM)
 			printf("--random ");
+		if (info->mr.range[i].flags & IP_NAT_RANGE_PERSISTENT)
+			printf("--persistent ");
 	}
 }
 
diff --git a/extensions/libipt_SNAT.c b/extensions/libipt_SNAT.c
index 944fe67..762d8d0 100644
--- a/extensions/libipt_SNAT.c
+++ b/extensions/libipt_SNAT.c
@@ -27,12 +27,13 @@ static void SNAT_help(void)
 "SNAT target options:\n"
 " --to-source <ipaddr>[-<ipaddr>][:port-port]\n"
 "				Address to map source to.\n"
-"[--random]\n");
+"[--random] [ --persistent]\n");
 }
 
 static const struct option SNAT_opts[] = {
 	{ "to-source", 1, NULL, '1' },
 	{ "random", 0, NULL, '2' },
+	{ "perstistent", 0, NULL, '3' },
 	{ .name = NULL }
 };
 
@@ -179,6 +180,10 @@ static int SNAT_parse(int c, char **argv, int invert, unsigned int *flags,
 			*flags |= IPT_SNAT_OPT_RANDOM;
 		return 1;
 
+	case '3':
+		info->mr.range[0].flags |= IP_NAT_RANGE_PERSISTENT;
+		return 1;
+
 	default:
 		return 0;
 	}
@@ -223,6 +228,8 @@ static void SNAT_print(const void *ip, const struct xt_entry_target *target,
 		printf(" ");
 		if (info->mr.range[i].flags & IP_NAT_RANGE_PROTO_RANDOM)
 			printf("random ");
+		if (info->mr.range[i].flags & IP_NAT_RANGE_PERSISTENT)
+			printf("persistent ");
 	}
 }
 
@@ -237,6 +244,8 @@ static void SNAT_save(const void *ip, const struct xt_entry_target *target)
 		printf(" ");
 		if (info->mr.range[i].flags & IP_NAT_RANGE_PROTO_RANDOM)
 			printf("--random ");
+		if (info->mr.range[i].flags & IP_NAT_RANGE_PERSISTENT)
+			printf("--persistent ");
 	}
 }
 
diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index 094473e..c3e2060 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -18,6 +18,7 @@ enum nf_nat_manip_type
 #define IP_NAT_RANGE_MAP_IPS 1
 #define IP_NAT_RANGE_PROTO_SPECIFIED 2
 #define IP_NAT_RANGE_PROTO_RANDOM 4
+#define IP_NAT_RANGE_PERSISTENT 8
 
 /* NAT sequence number modifications */
 struct nf_nat_seq {

Re: [Bugme-new] [Bug 12954] New: SAMEIP --nodst functionality gone missing

[Patrick McHardy]

Patrick McHardy wrote:
> This is the corresponding userspace patch:
> 

I've just commited the patch to the iptables git tree. The kernel
patch is on its way upstream.