SNAT --random & fully is not actually random for ips

SNAT --random & fully is not actually random for ips

[Denys Fedoryshchenko]

Hello,

I noticed that if i specify -j SNAT with options --random --random-fully 
still it keeps persistence for source IP.
Actually truly random src ip required in some scenarios like links 
balanced by IPs, but seems since 2012 at least it is not possible.

But actually if i do something like:
--- nf_nat_core.c.new	2016-11-28 09:55:54.000000000 +0000
+++ nf_nat_core.c	2016-11-21 09:11:59.000000000 +0000
@@ -282,13 +282,9 @@
  	 * client coming from the same IP (some Internet Banking sites
  	 * like this), even across reboots.
  	 */
-	if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
-	    j = prandom_u32();
-	} else {
-	    j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / 
sizeof(u32),
+	j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
  		   range->flags & NF_NAT_RANGE_PERSISTENT ?
  			0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
-	}

  	full_range = false;
  	for (i = 0; i <= max; i++) {

It works as intended. But i guess to not break compatibility it is 
better should be introduced as new option?
Or maybe there is no really need for such option?

Re: SNAT --random & fully is not actually random for ips

[Pablo Neira Ayuso]

On Mon, Nov 28, 2016 at 12:45:59PM +0200, Denys Fedoryshchenko wrote:
> Hello,
> 
> I noticed that if i specify -j SNAT with options --random --random-fully
> still it keeps persistence for source IP.

So you specify both?

> Actually truly random src ip required in some scenarios like links balanced
> by IPs, but seems since 2012 at least it is not possible.
>
> But actually if i do something like:
> --- nf_nat_core.c.new	2016-11-28 09:55:54.000000000 +0000
> +++ nf_nat_core.c	2016-11-21 09:11:59.000000000 +0000
> @@ -282,13 +282,9 @@
>  	 * client coming from the same IP (some Internet Banking sites
>  	 * like this), even across reboots.
>  	 */
> -	if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
> -	    j = prandom_u32();
> -	} else {
> -	    j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
> +	j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
>  		   range->flags & NF_NAT_RANGE_PERSISTENT ?
>  			0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
> -	}
> 
>  	full_range = false;
>  	for (i = 0; i <= max; i++) {
> 
> It works as intended. But i guess to not break compatibility it is better
> should be introduced as new option?
> Or maybe there is no really need for such option?

Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?

Re: SNAT --random & fully is not actually random for ips

[Denys Fedoryshchenko]

On 2016-11-28 13:06, Pablo Neira Ayuso wrote:
> On Mon, Nov 28, 2016 at 12:45:59PM +0200, Denys Fedoryshchenko wrote:
>> Hello,
>> 
>> I noticed that if i specify -j SNAT with options --random 
>> --random-fully
>> still it keeps persistence for source IP.
> 
> So you specify both?
> 
>> Actually truly random src ip required in some scenarios like links 
>> balanced
>> by IPs, but seems since 2012 at least it is not possible.
>> 
>> But actually if i do something like:
>> --- nf_nat_core.c.new	2016-11-28 09:55:54.000000000 +0000
>> +++ nf_nat_core.c	2016-11-21 09:11:59.000000000 +0000
>> @@ -282,13 +282,9 @@
>>  	 * client coming from the same IP (some Internet Banking sites
>>  	 * like this), even across reboots.
>>  	 */
>> -	if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
>> -	    j = prandom_u32();
>> -	} else {
>> -	    j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / 
>> sizeof(u32),
>> +	j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / 
>> sizeof(u32),
>>  		   range->flags & NF_NAT_RANGE_PERSISTENT ?
>>  			0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
>> -	}
>> 
>>  	full_range = false;
>>  	for (i = 0; i <= max; i++) {
>> 
>> It works as intended. But i guess to not break compatibility it is 
>> better
>> should be introduced as new option?
>> Or maybe there is no really need for such option?
> 
> Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?
Ops, sorry i just did mistake with files, actually it is in reverse ( 
did this patch, and it worked properly with it, with random source ip).
--- nf_nat_core.c	2016-11-21 09:11:59.000000000 +0000
+++ nf_nat_core.c.new	2016-11-28 09:55:54.000000000 +0000
@@ -282,9 +282,13 @@
  	 * client coming from the same IP (some Internet Banking sites
  	 * like this), even across reboots.
  	 */
-	j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
+	if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
+	    j = prandom_u32();
+	} else {
+	    j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / 
sizeof(u32),
  		   range->flags & NF_NAT_RANGE_PERSISTENT ?
  			0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
+	}

  	full_range = false;
  	for (i = 0; i <= max; i++) {

This is current situation, RANDOM_FULLY actually does prandom_u32 for 
source port only, but not for IP.
IP kept as persistent and kind of predictable, because hash function 
based on source ip.

Sure i did tried to specify any combination of flags, but looking to 
"find_best_ips_proto" function, it wont have any effect.

Re: SNAT --random & fully is not actually random for ips

[Pablo Neira Ayuso]

On Mon, Nov 28, 2016 at 01:12:07PM +0200, Denys Fedoryshchenko wrote:
> On 2016-11-28 13:06, Pablo Neira Ayuso wrote:
> >Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?
>
> Ops, sorry i just did mistake with files, actually it is in reverse ( did
> this patch, and it worked properly with it, with random source ip).

Oh, I see 8)

> --- nf_nat_core.c	2016-11-21 09:11:59.000000000 +0000
> +++ nf_nat_core.c.new	2016-11-28 09:55:54.000000000 +0000
> @@ -282,9 +282,13 @@
>  	 * client coming from the same IP (some Internet Banking sites
>  	 * like this), even across reboots.
>  	 */
> -	j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
> +	if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
> +	    j = prandom_u32();
> +	} else {
> +	    j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
>  		   range->flags & NF_NAT_RANGE_PERSISTENT ?
>  			0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
> +	}
> 
>  	full_range = false;
>  	for (i = 0; i <= max; i++) {
> 
> This is current situation, RANDOM_FULLY actually does prandom_u32 for source
> port only, but not for IP.
> IP kept as persistent and kind of predictable, because hash function based
> on source ip.
> 
> Sure i did tried to specify any combination of flags, but looking to
> "find_best_ips_proto" function, it wont have any effect.

IIRC the original intention on random-fully was to cover only ports.
Did you interpret from git history otherwise? Otherwise, safe
procedure is to add a new flag.

Re: SNAT --random & fully is not actually random for ips

[Denys Fedoryshchenko]

On 2016-11-28 13:29, Pablo Neira Ayuso wrote:
> On Mon, Nov 28, 2016 at 01:12:07PM +0200, Denys Fedoryshchenko wrote:
>> On 2016-11-28 13:06, Pablo Neira Ayuso wrote:
>> >Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?
>> 
>> Ops, sorry i just did mistake with files, actually it is in reverse ( 
>> did
>> this patch, and it worked properly with it, with random source ip).
> 
> Oh, I see 8)
> 
>> --- nf_nat_core.c	2016-11-21 09:11:59.000000000 +0000
>> +++ nf_nat_core.c.new	2016-11-28 09:55:54.000000000 +0000
>> @@ -282,9 +282,13 @@
>>  	 * client coming from the same IP (some Internet Banking sites
>>  	 * like this), even across reboots.
>>  	 */
>> -	j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / 
>> sizeof(u32),
>> +	if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
>> +	    j = prandom_u32();
>> +	} else {
>> +	    j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / 
>> sizeof(u32),
>>  		   range->flags & NF_NAT_RANGE_PERSISTENT ?
>>  			0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
>> +	}
>> 
>>  	full_range = false;
>>  	for (i = 0; i <= max; i++) {
>> 
>> This is current situation, RANDOM_FULLY actually does prandom_u32 for 
>> source
>> port only, but not for IP.
>> IP kept as persistent and kind of predictable, because hash function 
>> based
>> on source ip.
>> 
>> Sure i did tried to specify any combination of flags, but looking to
>> "find_best_ips_proto" function, it wont have any effect.
> 
> IIRC the original intention on random-fully was to cover only ports.
> Did you interpret from git history otherwise? Otherwise, safe
> procedure is to add a new flag.
No, seems i didnt read man page well, sorry.
I will check it, maybe will try to add new option and submit a patch, 
still studying impact on "balancing" with this change, seems it works 
great.
But not really sure such thing needed for someone else, actually some 
might have privacy concerns as well, and can use such option for 
privacy.