All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] tcp: randomize TCP source ports
@ 2013-11-08  0:54 Eric Dumazet
  2013-11-08  1:07 ` Rick Jones
  2013-11-08 13:02 ` Hannes Frederic Sowa
  0 siblings, 2 replies; 15+ messages in thread
From: Eric Dumazet @ 2013-11-08  0:54 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

TCP does proper randomization of ports on active connections only if
bind() is used between socket() and connect()

If bind() is not specifically used, kernel performs autobind, and TCP
autobind typically uses a sequential allocation for a given (dst
address, dst port, src address) tuple.

UDP autobind does a randomization, as part of the effort to make DNS
more secure.

TCP autobind uses a global sequential number (called @hint in source
code) with a perturbation done by secure_ipv4_port_ephemeral(),
 so that the 'hint' of the next port is per (saddr, daddr, dport) tuple

This was probably done to maximize port use and avoid hitting timewait
sockets, but I think it should be OK to replace this stuff by a random
selection to have more entropy in the various flow hashing functions,
and in general higher security levels. TCP timestamps are now well
deployed.

Patch would be trivial, but I'd like to get some comments if
you think this idea is wrong.

Thanks !

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08  0:54 [RFC] tcp: randomize TCP source ports Eric Dumazet
@ 2013-11-08  1:07 ` Rick Jones
  2013-11-08  2:04   ` Eric Dumazet
  2013-11-08 13:02 ` Hannes Frederic Sowa
  1 sibling, 1 reply; 15+ messages in thread
From: Rick Jones @ 2013-11-08  1:07 UTC (permalink / raw)
  To: Eric Dumazet, David Miller; +Cc: netdev

On 11/07/2013 04:54 PM, Eric Dumazet wrote:
> TCP does proper randomization of ports on active connections only if
> bind() is used between socket() and connect()
>
> If bind() is not specifically used, kernel performs autobind, and TCP
> autobind typically uses a sequential allocation for a given (dst
> address, dst port, src address) tuple.
>
> UDP autobind does a randomization, as part of the effort to make DNS
> more secure.
>
> TCP autobind uses a global sequential number (called @hint in source
> code) with a perturbation done by secure_ipv4_port_ephemeral(),
>   so that the 'hint' of the next port is per (saddr, daddr, dport) tuple
>
> This was probably done to maximize port use and avoid hitting timewait
> sockets, but I think it should be OK to replace this stuff by a random
> selection to have more entropy in the various flow hashing functions,
> and in general higher security levels. TCP timestamps are now well
> deployed.

For perhaps most definitions of well deployed.  There is at least one 
load balancer which, while it offers TCP Window Scaling, does not also 
offer TCP Time Stamps...

By rights they should (must) be offering TCP Time Stamps, and they are, 
I am told, "working on it."

Is all going to be "well" when it is the (non-Linux) remote system which 
has the TIME_WAIT endpoint?

rick jones

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08  1:07 ` Rick Jones
@ 2013-11-08  2:04   ` Eric Dumazet
  2013-11-08 23:26     ` Rick Jones
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Dumazet @ 2013-11-08  2:04 UTC (permalink / raw)
  To: Rick Jones; +Cc: David Miller, netdev

On Thu, 2013-11-07 at 17:07 -0800, Rick Jones wrote:

> For perhaps most definitions of well deployed.  There is at least one 
> load balancer which, while it offers TCP Window Scaling, does not also 
> offer TCP Time Stamps...
> 
> By rights they should (must) be offering TCP Time Stamps, and they are, 
> I am told, "working on it."
> 
> Is all going to be "well" when it is the (non-Linux) remote system which 
> has the TIME_WAIT endpoint?

Hey, tell us why netperf does a bind(port=0, addr=ANY) and SO_REUSEADDR
tricks before connect()

It seems you do request randomization, but you do not want it for
applications written by innocent people...

Current implementation is lazy at best, as a single @hint variable is
shared for all cpus, all users, so at moderate load there is actually no
guarantee of sequential allocations.

RFC 6056 has an interesting list of alternatives

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08  0:54 [RFC] tcp: randomize TCP source ports Eric Dumazet
  2013-11-08  1:07 ` Rick Jones
@ 2013-11-08 13:02 ` Hannes Frederic Sowa
  2013-11-08 14:03   ` Eric Dumazet
  1 sibling, 1 reply; 15+ messages in thread
From: Hannes Frederic Sowa @ 2013-11-08 13:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev

On Thu, Nov 07, 2013 at 04:54:09PM -0800, Eric Dumazet wrote:
> TCP does proper randomization of ports on active connections only if
> bind() is used between socket() and connect()
> 
> If bind() is not specifically used, kernel performs autobind, and TCP
> autobind typically uses a sequential allocation for a given (dst
> address, dst port, src address) tuple.
> 
> UDP autobind does a randomization, as part of the effort to make DNS
> more secure.

If I understand the code correctly the UDP ports are fully randomized? This
is good as per-peer randomization and then incrementation seems to be
theoretically broken:

<https://sites.google.com/site/hayashulman/files/NIC-derandomisation.pdf>

Looking at the code I somehow would like to check the use of net_random there.
The prandom function is reseeded as late_initcall and then only seeded by some
network addresses.

At the time the late_initcall reseeds the PRNG my tests have shown that
the nonblockingpool was still not fully initialized where the PRNG gets
reseeded from.

Hm, I propose a patch which does reseed the pool as soon as the nonblocking
pool got credited enough entropy in credit_entropy_bits. This should help
later binds().

> TCP autobind uses a global sequential number (called @hint in source
> code) with a perturbation done by secure_ipv4_port_ephemeral(),
>  so that the 'hint' of the next port is per (saddr, daddr, dport) tuple
> 
> This was probably done to maximize port use and avoid hitting timewait
> sockets, but I think it should be OK to replace this stuff by a random
> selection to have more entropy in the various flow hashing functions,
> and in general higher security levels. TCP timestamps are now well
> deployed.

We recently had a thread that Windows (since Vista?) disabled tcp
timestamps by default. But I don't see how this should make a great
difference (and still wonder why they give up PAWS.)

> Patch would be trivial, but I'd like to get some comments if
> you think this idea is wrong.

I would like to see this happening.

Thanks,

  Hannes

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08 13:02 ` Hannes Frederic Sowa
@ 2013-11-08 14:03   ` Eric Dumazet
  2013-11-08 14:28     ` Hannes Frederic Sowa
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Dumazet @ 2013-11-08 14:03 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: David Miller, netdev

On Fri, 2013-11-08 at 14:02 +0100, Hannes Frederic Sowa wrote:

> If I understand the code correctly the UDP ports are fully randomized? This
> is good as per-peer randomization and then incrementation seems to be
> theoretically broken:
> 
> <https://sites.google.com/site/hayashulman/files/NIC-derandomisation.pdf>
> 
> Looking at the code I somehow would like to check the use of net_random there.
> The prandom function is reseeded as late_initcall and then only seeded by some
> network addresses.
> 
> At the time the late_initcall reseeds the PRNG my tests have shown that
> the nonblockingpool was still not fully initialized where the PRNG gets
> reseeded from.
> 
> Hm, I propose a patch which does reseed the pool as soon as the nonblocking
> pool got credited enough entropy in credit_entropy_bits. This should help
> later binds().

Or even better have a reseed every XX seconds.

Something like :

diff --git a/lib/random32.c b/lib/random32.c
index 52280d5526be..4af2d72281e3 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -60,6 +60,8 @@ u32 prandom_u32_state(struct rnd_state *state)
 }
 EXPORT_SYMBOL(prandom_u32_state);
 
+static DEFINE_PER_CPU(unsigned long, net_rand_shuffle);
+
 /**
  *	prandom_u32 - pseudo random number generator
  *
@@ -71,6 +73,19 @@ u32 prandom_u32(void)
 {
 	unsigned long r;
 	struct rnd_state *state = &get_cpu_var(net_rand_state);
+
+	if (unlikely(time_after(jiffies, net_rand_shuffle))) {
+		unsigned long delay;
+		u32 entropy;
+
+		get_random_bytes_arch(&entropy, sizeof(entropy));
+
+		/* reseed every ~20 seconds, in [10 .. 30] interval */
+		delay = 10 * HZ + (entropy % (20 * HZ));
+		__this_cpu_write(net_rand_shuffle, jiffies + delay);
+
+		state->s1 = __seed(state->s1 ^ entropy, 1);
+	}
 	r = prandom_u32_state(state);
 	put_cpu_var(state);
 	return r;
@@ -169,6 +184,7 @@ static int __init prandom_init(void)
 		prandom_u32_state(state);
 		prandom_u32_state(state);
 		prandom_u32_state(state);
+		per_cpu(net_rand_shuffle, i) = jiffies;
 	}
 	return 0;
 }

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08 14:03   ` Eric Dumazet
@ 2013-11-08 14:28     ` Hannes Frederic Sowa
  2013-11-08 15:11       ` Eric Dumazet
  0 siblings, 1 reply; 15+ messages in thread
From: Hannes Frederic Sowa @ 2013-11-08 14:28 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev

On Fri, Nov 08, 2013 at 06:03:38AM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 14:02 +0100, Hannes Frederic Sowa wrote:
> 
> > If I understand the code correctly the UDP ports are fully randomized? This
> > is good as per-peer randomization and then incrementation seems to be
> > theoretically broken:
> > 
> > <https://sites.google.com/site/hayashulman/files/NIC-derandomisation.pdf>
> > 
> > Looking at the code I somehow would like to check the use of net_random there.
> > The prandom function is reseeded as late_initcall and then only seeded by some
> > network addresses.
> > 
> > At the time the late_initcall reseeds the PRNG my tests have shown that
> > the nonblockingpool was still not fully initialized where the PRNG gets
> > reseeded from.
> > 
> > Hm, I propose a patch which does reseed the pool as soon as the nonblocking
> > pool got credited enough entropy in credit_entropy_bits. This should help
> > later binds().
> 
> Or even better have a reseed every XX seconds.

Yes, that's even better.

> 
> Something like :
> 
> diff --git a/lib/random32.c b/lib/random32.c
> index 52280d5526be..4af2d72281e3 100644
> --- a/lib/random32.c
> +++ b/lib/random32.c
> @@ -60,6 +60,8 @@ u32 prandom_u32_state(struct rnd_state *state)
>  }
>  EXPORT_SYMBOL(prandom_u32_state);
>  
> +static DEFINE_PER_CPU(unsigned long, net_rand_shuffle);
> +
>  /**
>   *	prandom_u32 - pseudo random number generator
>   *
> @@ -71,6 +73,19 @@ u32 prandom_u32(void)
>  {
>  	unsigned long r;
>  	struct rnd_state *state = &get_cpu_var(net_rand_state);
> +
> +	if (unlikely(time_after(jiffies, net_rand_shuffle))) {
> +		unsigned long delay;
> +		u32 entropy;
> +
> +		get_random_bytes_arch(&entropy, sizeof(entropy));

What do you think about using a timer to keep the reseed out of fast-path
and switch to the non-arch get_random_bytes instead?

Thanks,

  Hannes

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08 14:28     ` Hannes Frederic Sowa
@ 2013-11-08 15:11       ` Eric Dumazet
  2013-11-08 17:39         ` Hannes Frederic Sowa
  2013-11-09  4:47         ` Hannes Frederic Sowa
  0 siblings, 2 replies; 15+ messages in thread
From: Eric Dumazet @ 2013-11-08 15:11 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: David Miller, netdev

On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:

> What do you think about using a timer to keep the reseed out of fast-path
> and switch to the non-arch get_random_bytes instead?

Well, the initial seed value is get_random_bytes(). I felt that using a
xor with the _arch() version would be safe enough.

For the timer, I do not think its worth the pain : Do you want a per cpu
timer, or a global one ?

The unlikely() clause makes the test very small and test is well
predicted.

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08 15:11       ` Eric Dumazet
@ 2013-11-08 17:39         ` Hannes Frederic Sowa
  2013-11-09  4:47         ` Hannes Frederic Sowa
  1 sibling, 0 replies; 15+ messages in thread
From: Hannes Frederic Sowa @ 2013-11-08 17:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, tytso, linux-wireless

On Fri, Nov 08, 2013 at 07:11:18AM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:
> 
> > What do you think about using a timer to keep the reseed out of fast-path
> > and switch to the non-arch get_random_bytes instead?
> 
> Well, the initial seed value is get_random_bytes(). I felt that using a
> xor with the _arch() version would be safe enough.

Ted Ts'o talked about how we seed the prng on another mailing list. I
used his patch
<https://git.kernel.org/cgit/linux/kernel/git/tytso/random.git/patch/?id=392a546dc8368d1745f9891ef3f8f7c380de8650>
to demonstrate why I don't think get_random_bytes provides enough entropy
at that point so we can use it for port randomization.

He also raised the point that maybe the minstrel algorithm does slurp the
entropy too early and also too much, becasue it calls get_random_once
repeatedly [so I also added wireless to Cc]. It seems fine, but maybe
it could be changed to net_get_random_once, too. I have not found an
easy way to do that, yet. I just leave it here for discussion.

Using this patch my box starts up with this trace:

# dmesg | grep -i random:
[    0.000000] random: start_kernel+0x2c/0x44e get_random_bytes called with 0 bits of entropy available
[    0.078516] random: start_secondary+0x1e4/0x2e0 get_random_bytes called with 3 bits of entropy available
[    0.078516] random: cpu_startup_entry+0x24/0x410 get_random_bytes called with 3 bits of entropy available
[    0.096357] random: cpu_startup_entry+0x24/0x410 get_random_bytes called with 3 bits of entropy available
[    1.003745] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    1.010271] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    1.029952] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    1.034723] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    1.041257] random: rt_genid_init+0x2b/0x30 get_random_bytes called with 3 bits of entropy available
[    3.134969] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    3.147144] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    3.154366] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    3.159000] random: neigh_hash_alloc+0x8c/0xd0 get_random_bytes called with 3 bits of entropy available
[    3.173849] random: init_oops_id+0x35/0x40 get_random_bytes called with 3 bits of entropy available
[    3.185702] random: prandom_reseed+0x5a/0x9e get_random_bytes called with 3 bits of entropy available
[    3.191982] random: prandom_reseed+0x5a/0x9e get_random_bytes called with 3 bits of entropy available

I don't think this does look too good for initializing a PRNG. We
consider the entropy pool initialized when entropy_total reaches 128
bits of entropy (this is a not so large kernel config running in kvm).

IMHO we should be more conservative here.

Further output just FYI:

[    3.213113] random: random_int_secret_init+0x1a/0x20 get_random_bytes called with 3 bits of entropy available
[    3.342258] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 3 bits of entropy available
[    3.476120] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 4 bits of entropy available
[    3.611206] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 5 bits of entropy available
[    3.667358] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 5 bits of entropy available
[    3.734871] random: generate_random_uuid+0x17/0x40 get_random_bytes called with 5 bits of entropy available
[    3.757351] random: nl_portid_hash_rehash+0xa9/0x1c3 get_random_bytes called with 5 bits of entropy available
[    3.785217] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 5 bits of entropy available
[    3.831928] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 5 bits of entropy available
[    3.954606] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.021579] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.184380] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.212022] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.266360] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.349230] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.360435] random: nl_portid_hash_rehash+0xa9/0x1c3 get_random_bytes called with 6 bits of entropy available
[    4.503675] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.521476] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 6 bits of entropy available
[    4.730249] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 7 bits of entropy available
[    4.793874] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 7 bits of entropy available
[    4.853621] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 7 bits of entropy available
[    4.929740] random: nl_portid_hash_rehash+0xa9/0x1c3 get_random_bytes called with 7 bits of entropy available
[    5.004383] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 7 bits of entropy available
[    5.034793] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 7 bits of entropy available
[    5.268004] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 7 bits of entropy available
[    5.551956] random: __ipv6_regen_rndid+0x2d/0xc0 get_random_bytes called with 7 bits of entropy available

I put this on my todo list, too.

[    5.644543] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 7 bits of entropy available
[    5.676996] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 8 bits of entropy available
[    5.960995] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 13 bits of entropy available
[    5.974738] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 13 bits of entropy available
[    6.001795] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 13 bits of entropy available
[    6.018472] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 13 bits of entropy available
[    6.044597] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 13 bits of entropy available
[    6.091984] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 13 bits of entropy available
[    6.119498] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 13 bits of entropy available
[    6.212952] random: ext4_fill_super+0x1681/0x2f80 get_random_bytes called with 14 bits of entropy available
[    6.273980] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.274730] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.340308] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.420832] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.431657] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.440941] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.480322] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.494260] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.525949] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.582898] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.638663] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.675789] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 14 bits of entropy available
[    6.796919] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    6.816349] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    6.901432] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    6.969880] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    6.987195] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    7.030526] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    7.056945] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    7.060785] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[    7.227736] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 15 bits of entropy available
[   17.086529] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 122 bits of entropy available
[   17.105672] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 122 bits of entropy available
[   17.148562] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 122 bits of entropy available
[   17.152420] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 122 bits of entropy available
[   17.193339] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 122 bits of entropy available
[   17.193456] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 122 bits of entropy available
[   17.793175] random: load_elf_binary+0xad3/0x1890 get_random_bytes called with 126 bits of entropy available

> 
> For the timer, I do not think its worth the pain : Do you want a per cpu
> timer, or a global one ?

I would use a global one and just do the same thing as prandom_reseed. I
am not sure if we should discard the previous prng state.

IMHO reseeding must not take place very often.

> The unlikely() clause makes the test very small and test is well
> predicted.

I agree, this should not really hurt. I think your patch is fine and
added a fine safety net when we also reseed the prng after the
nonblocking_pool is fully available.

Greetings,

  Hannes


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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08  2:04   ` Eric Dumazet
@ 2013-11-08 23:26     ` Rick Jones
  2013-11-08 23:42       ` Eric Dumazet
  0 siblings, 1 reply; 15+ messages in thread
From: Rick Jones @ 2013-11-08 23:26 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev

On 11/07/2013 06:04 PM, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 17:07 -0800, Rick Jones wrote:
>
>> For perhaps most definitions of well deployed.  There is at least one
>> load balancer which, while it offers TCP Window Scaling, does not also
>> offer TCP Time Stamps...
>>
>> By rights they should (must) be offering TCP Time Stamps, and they are,
>> I am told, "working on it."
>>
>> Is all going to be "well" when it is the (non-Linux) remote system which
>> has the TIME_WAIT endpoint?
>
> Hey, tell us why netperf does a bind(port=0, addr=ANY) and SO_REUSEADDR
> tricks before connect()
>
> It seems you do request randomization, but you do not want it for
> applications written by innocent people...

That bind() call is not there to request randomization of the TCP source 
port.

The bind() call in src/nettest_bsd.c/create_data_socket() is so 
netserver can report a port number back to netperf.  It is also there so 
a TCP_CRR test can explicitly use more than the configured ephemeral 
port range.  It is also used when setting explicit port numbers for 
getting through firewalls.

In the establish_control() path the bind() call is also there to allow 
specifying an explicit port number for the control connection.  I just 
don't bother avoiding the call when someone hasn't selected an explicit 
client-side port number for the control connection.

happy benchmarking,

rick jones

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08 23:26     ` Rick Jones
@ 2013-11-08 23:42       ` Eric Dumazet
  2013-11-08 23:57         ` Rick Jones
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Dumazet @ 2013-11-08 23:42 UTC (permalink / raw)
  To: Rick Jones; +Cc: David Miller, netdev

On Fri, 2013-11-08 at 15:26 -0800, Rick Jones wrote:
> On 11/07/2013 06:04 PM, Eric Dumazet wrote:
> >
> > It seems you do request randomization, but you do not want it for
> > applications written by innocent people...
> 
> That bind() call is not there to request randomization of the TCP source 
> port.

I was actually doing some kind of humor, I should have added a ;)

Thanks

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08 23:42       ` Eric Dumazet
@ 2013-11-08 23:57         ` Rick Jones
  0 siblings, 0 replies; 15+ messages in thread
From: Rick Jones @ 2013-11-08 23:57 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev

On 11/08/2013 03:42 PM, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 15:26 -0800, Rick Jones wrote:
>> On 11/07/2013 06:04 PM, Eric Dumazet wrote:
>>>
>>> It seems you do request randomization, but you do not want it for
>>> applications written by innocent people...
>>
>> That bind() call is not there to request randomization of the TCP source
>> port.
>
> I was actually doing some kind of humor, I should have added a ;)

Sorry - been watching one of my kids most of the day, my humor/snark 
detectors were offline :)

rick

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-08 15:11       ` Eric Dumazet
  2013-11-08 17:39         ` Hannes Frederic Sowa
@ 2013-11-09  4:47         ` Hannes Frederic Sowa
  2013-11-09 15:26           ` Loganaden Velvindron
  2013-11-09 18:16           ` Daniel Borkmann
  1 sibling, 2 replies; 15+ messages in thread
From: Hannes Frederic Sowa @ 2013-11-09  4:47 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev

On Fri, Nov 08, 2013 at 07:11:18AM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:
> 
> > What do you think about using a timer to keep the reseed out of fast-path
> > and switch to the non-arch get_random_bytes instead?
> 
> Well, the initial seed value is get_random_bytes(). I felt that using a
> xor with the _arch() version would be safe enough.
> 
> For the timer, I do not think its worth the pain : Do you want a per cpu
> timer, or a global one ?

This untested diff came to my mind (it is based on the random tree). I
actually consider to propose something like this for 3.13. UDP port
randomization is really critical.

In 3.14 timeframe I suggest abandon net_random and use prandom_u32
directly so code gets easier to audit.

Would it hurt to use "proper" get_random_byte calls for port randomization?

diff --git a/drivers/char/random.c b/drivers/char/random.c
index cdf4cfb..e9d0136 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -657,9 +657,11 @@ retry:
 	r->entropy_total += nbits;
 	if (!r->initialized && nbits > 0) {
 		if (r->entropy_total > 128) {
-			if (r == &nonblocking_pool)
+			if (r == &nonblocking_pool) {
 				pr_notice("random: %s pool is initialized\n",
 					  r->name);
+				prandom_reseed();
+			}
 			r->initialized = 1;
 			r->entropy_total = 0;
 		}
diff --git a/include/linux/random.h b/include/linux/random.h
index 6312dd9..4f878c0 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -29,6 +29,7 @@ unsigned long randomize_range(unsigned long start, unsigned long end, unsigned l
 u32 prandom_u32(void);
 void prandom_bytes(void *buf, int nbytes);
 void prandom_seed(u32 seed);
+void prandom_reseed(void);
 
 u32 prandom_u32_state(struct rnd_state *);
 void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
diff --git a/lib/random32.c b/lib/random32.c
index 52280d5..1ee611f 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -174,11 +174,31 @@ static int __init prandom_init(void)
 }
 core_initcall(prandom_init);
 
+static void __prandom_timer(unsigned long dontcare);
+static DEFINE_TIMER(seed_timer, __prandom_timer, 0, 0);
+
+static void __prandom_timer(unsigned long dontcare)
+{
+	u32 entropy;
+	get_random_bytes(&entropy, sizeof(entropy));
+	prandom_seed(entropy);
+	seed_timer.expires = jiffies + 60 * HZ;
+	add_timer(&seed_timer);
+}
+
+static int prandom_start_seed_timer(void)
+{
+	seed_timer.expires = jiffies + 60 * HZ;
+	add_timer(&seed_timer);
+	return 0;
+}
+late_initcall(prandom_start_seed_timer);
+
 /*
  *	Generate better values after random number generator
  *	is fully initialized.
  */
-static int __init prandom_reseed(void)
+void prandom_reseed(void)
 {
 	int i;
 
@@ -196,4 +216,3 @@ static int __init prandom_reseed(void)
 	}
 	return 0;
 }
-late_initcall(prandom_reseed);

Greetings,

  Hannes

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-09  4:47         ` Hannes Frederic Sowa
@ 2013-11-09 15:26           ` Loganaden Velvindron
  2013-11-09 18:16           ` Daniel Borkmann
  1 sibling, 0 replies; 15+ messages in thread
From: Loganaden Velvindron @ 2013-11-09 15:26 UTC (permalink / raw)
  To: Eric Dumazet, David Miller, netdev

On Sat, Nov 9, 2013 at 8:47 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Fri, Nov 08, 2013 at 07:11:18AM -0800, Eric Dumazet wrote:
>> On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:
>>
>> > What do you think about using a timer to keep the reseed out of fast-path
>> > and switch to the non-arch get_random_bytes instead?
>>
>> Well, the initial seed value is get_random_bytes(). I felt that using a
>> xor with the _arch() version would be safe enough.
>>
>> For the timer, I do not think its worth the pain : Do you want a per cpu
>> timer, or a global one ?
>
> This untested diff came to my mind (it is based on the random tree). I
> actually consider to propose something like this for 3.13. UDP port
> randomization is really critical.

I'll try to test the diff, as the idea seems interesting.

I guess that it would help for DNS.

>
> In 3.14 timeframe I suggest abandon net_random and use prandom_u32
> directly so code gets easier to audit.
>
> Would it hurt to use "proper" get_random_byte calls for port randomization?
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index cdf4cfb..e9d0136 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -657,9 +657,11 @@ retry:
>         r->entropy_total += nbits;
>         if (!r->initialized && nbits > 0) {
>                 if (r->entropy_total > 128) {
> -                       if (r == &nonblocking_pool)
> +                       if (r == &nonblocking_pool) {
>                                 pr_notice("random: %s pool is initialized\n",
>                                           r->name);
> +                               prandom_reseed();
> +                       }
>                         r->initialized = 1;
>                         r->entropy_total = 0;
>                 }
> diff --git a/include/linux/random.h b/include/linux/random.h
> index 6312dd9..4f878c0 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -29,6 +29,7 @@ unsigned long randomize_range(unsigned long start, unsigned long end, unsigned l
>  u32 prandom_u32(void);
>  void prandom_bytes(void *buf, int nbytes);
>  void prandom_seed(u32 seed);
> +void prandom_reseed(void);
>
>  u32 prandom_u32_state(struct rnd_state *);
>  void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
> diff --git a/lib/random32.c b/lib/random32.c
> index 52280d5..1ee611f 100644
> --- a/lib/random32.c
> +++ b/lib/random32.c
> @@ -174,11 +174,31 @@ static int __init prandom_init(void)
>  }
>  core_initcall(prandom_init);
>
> +static void __prandom_timer(unsigned long dontcare);
> +static DEFINE_TIMER(seed_timer, __prandom_timer, 0, 0);
> +
> +static void __prandom_timer(unsigned long dontcare)
> +{
> +       u32 entropy;
> +       get_random_bytes(&entropy, sizeof(entropy));
> +       prandom_seed(entropy);
> +       seed_timer.expires = jiffies + 60 * HZ;
> +       add_timer(&seed_timer);
> +}
> +
> +static int prandom_start_seed_timer(void)
> +{
> +       seed_timer.expires = jiffies + 60 * HZ;
> +       add_timer(&seed_timer);
> +       return 0;
> +}
> +late_initcall(prandom_start_seed_timer);
> +
>  /*
>   *     Generate better values after random number generator
>   *     is fully initialized.
>   */
> -static int __init prandom_reseed(void)
> +void prandom_reseed(void)
>  {
>         int i;
>
> @@ -196,4 +216,3 @@ static int __init prandom_reseed(void)
>         }
>         return 0;
>  }
> -late_initcall(prandom_reseed);
>
> Greetings,
>
>   Hannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
This message is strictly personal and the opinions expressed do not
represent those of my employers, either past or present.

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-09  4:47         ` Hannes Frederic Sowa
  2013-11-09 15:26           ` Loganaden Velvindron
@ 2013-11-09 18:16           ` Daniel Borkmann
  2013-11-09 20:54             ` Hannes Frederic Sowa
  1 sibling, 1 reply; 15+ messages in thread
From: Daniel Borkmann @ 2013-11-09 18:16 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Eric Dumazet, David Miller, netdev

On 11/09/2013 05:47 AM, Hannes Frederic Sowa wrote:
> On Fri, Nov 08, 2013 at 07:11:18AM -0800, Eric Dumazet wrote:
>> On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:
>>
>>> What do you think about using a timer to keep the reseed out of fast-path
>>> and switch to the non-arch get_random_bytes instead?
>>
>> Well, the initial seed value is get_random_bytes(). I felt that using a
>> xor with the _arch() version would be safe enough.
>>
>> For the timer, I do not think its worth the pain : Do you want a per cpu
>> timer, or a global one ?
>
> This untested diff came to my mind (it is based on the random tree). I
> actually consider to propose something like this for 3.13. UDP port
> randomization is really critical.
>
> In 3.14 timeframe I suggest abandon net_random and use prandom_u32
> directly so code gets easier to audit.
>
> Would it hurt to use "proper" get_random_byte calls for port randomization?
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index cdf4cfb..e9d0136 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -657,9 +657,11 @@ retry:
>   	r->entropy_total += nbits;
>   	if (!r->initialized && nbits > 0) {
>   		if (r->entropy_total > 128) {
> -			if (r == &nonblocking_pool)
> +			if (r == &nonblocking_pool) {
>   				pr_notice("random: %s pool is initialized\n",
>   					  r->name);
> +				prandom_reseed();
> +			}
>   			r->initialized = 1;
>   			r->entropy_total = 0;
>   		}
> diff --git a/include/linux/random.h b/include/linux/random.h
> index 6312dd9..4f878c0 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -29,6 +29,7 @@ unsigned long randomize_range(unsigned long start, unsigned long end, unsigned l
>   u32 prandom_u32(void);
>   void prandom_bytes(void *buf, int nbytes);
>   void prandom_seed(u32 seed);
> +void prandom_reseed(void);
>
>   u32 prandom_u32_state(struct rnd_state *);
>   void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
> diff --git a/lib/random32.c b/lib/random32.c
> index 52280d5..1ee611f 100644
> --- a/lib/random32.c
> +++ b/lib/random32.c
> @@ -174,11 +174,31 @@ static int __init prandom_init(void)
>   }
>   core_initcall(prandom_init);
>
> +static void __prandom_timer(unsigned long dontcare);
> +static DEFINE_TIMER(seed_timer, __prandom_timer, 0, 0);
> +
> +static void __prandom_timer(unsigned long dontcare)
> +{
> +	u32 entropy;
> +	get_random_bytes(&entropy, sizeof(entropy));
> +	prandom_seed(entropy);
> +	seed_timer.expires = jiffies + 60 * HZ;
> +	add_timer(&seed_timer);
> +}
> +
> +static int prandom_start_seed_timer(void)

       ^^^^^^ __init

> +{
	prandom_reseed();

What are the objectives against initializing prandom here in
the late initcall [instead of doing so in drivers/char/random.c]
as it was the case before?

Probably for security reasons, I think you actually don't want
anyone (incl. external 3rd party modules) to call prandom_reseed()
again after this has been done once initially. So I think it
would be better to make this function not visible to anyone
outside of random32.c.

> +	seed_timer.expires = jiffies + 60 * HZ;
> +	add_timer(&seed_timer);
> +	return 0;
> +}
> +late_initcall(prandom_start_seed_timer);
> +
>   /*
>    *	Generate better values after random number generator
>    *	is fully initialized.
>    */
> -static int __init prandom_reseed(void)
> +void prandom_reseed(void)
>   {
>   	int i;
>
> @@ -196,4 +216,3 @@ static int __init prandom_reseed(void)
>   	}
>   	return 0;
>   }
> -late_initcall(prandom_reseed);
>
> Greetings,
>
>    Hannes

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

* Re: [RFC] tcp: randomize TCP source ports
  2013-11-09 18:16           ` Daniel Borkmann
@ 2013-11-09 20:54             ` Hannes Frederic Sowa
  0 siblings, 0 replies; 15+ messages in thread
From: Hannes Frederic Sowa @ 2013-11-09 20:54 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Eric Dumazet, David Miller, netdev

On Sat, Nov 09, 2013 at 07:16:10PM +0100, Daniel Borkmann wrote:
> On 11/09/2013 05:47 AM, Hannes Frederic Sowa wrote:
> >On Fri, Nov 08, 2013 at 07:11:18AM -0800, Eric Dumazet wrote:
> >>On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:
> >>
> >>>What do you think about using a timer to keep the reseed out of fast-path
> >>>and switch to the non-arch get_random_bytes instead?
> >>
> >>Well, the initial seed value is get_random_bytes(). I felt that using a
> >>xor with the _arch() version would be safe enough.
> >>
> >>For the timer, I do not think its worth the pain : Do you want a per cpu
> >>timer, or a global one ?
> >
> >This untested diff came to my mind (it is based on the random tree). I
> >actually consider to propose something like this for 3.13. UDP port
> >randomization is really critical.
> >
> >In 3.14 timeframe I suggest abandon net_random and use prandom_u32
> >directly so code gets easier to audit.
> >
> >Would it hurt to use "proper" get_random_byte calls for port randomization?
> >
> >diff --git a/drivers/char/random.c b/drivers/char/random.c
> >index cdf4cfb..e9d0136 100644
> >--- a/drivers/char/random.c
> >+++ b/drivers/char/random.c
> >@@ -657,9 +657,11 @@ retry:
> >  	r->entropy_total += nbits;
> >  	if (!r->initialized && nbits > 0) {
> >  		if (r->entropy_total > 128) {
> >-			if (r == &nonblocking_pool)
> >+			if (r == &nonblocking_pool) {
> >  				pr_notice("random: %s pool is initialized\n",
> >  					  r->name);
> >+				prandom_reseed();
> >+			}
> >  			r->initialized = 1;
> >  			r->entropy_total = 0;

I rearranged the code so get_random_bytes does not emit a warning when called
from prandom_reseed().

> >  		}
> >diff --git a/include/linux/random.h b/include/linux/random.h
> >index 6312dd9..4f878c0 100644
> >--- a/include/linux/random.h
> >+++ b/include/linux/random.h
> >@@ -29,6 +29,7 @@ unsigned long randomize_range(unsigned long start, 
> >unsigned long end, unsigned l
> >  u32 prandom_u32(void);
> >  void prandom_bytes(void *buf, int nbytes);
> >  void prandom_seed(u32 seed);
> >+void prandom_reseed(void);
> >
> >  u32 prandom_u32_state(struct rnd_state *);
> >  void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
> >diff --git a/lib/random32.c b/lib/random32.c
> >index 52280d5..1ee611f 100644
> >--- a/lib/random32.c
> >+++ b/lib/random32.c
> >@@ -174,11 +174,31 @@ static int __init prandom_init(void)
> >  }
> >  core_initcall(prandom_init);
> >
> >+static void __prandom_timer(unsigned long dontcare);
> >+static DEFINE_TIMER(seed_timer, __prandom_timer, 0, 0);
> >+
> >+static void __prandom_timer(unsigned long dontcare)
> >+{
> >+	u32 entropy;
> >+	get_random_bytes(&entropy, sizeof(entropy));
> >+	prandom_seed(entropy);
> >+	seed_timer.expires = jiffies + 60 * HZ;
> >+	add_timer(&seed_timer);
> >+}
> >+
> >+static int prandom_start_seed_timer(void)
> 
>       ^^^^^^ __init

Also fixed in my commit. Thanks!

> >+{
> 	prandom_reseed();
> 
> What are the objectives against initializing prandom here in
> the late initcall [instead of doing so in drivers/char/random.c]
> as it was the case before?

IMHO even the late initcall is way too early to seed the prng properly.
Later reseeds never touch s2 and s3 of rnd_state again, so I want to
make sure we have a proper initialized entropy pool when we do the
initial prandom_reseed().

> Probably for security reasons, I think you actually don't want
> anyone (incl. external 3rd party modules) to call prandom_reseed()
> again after this has been done once initially. So I think it
> would be better to make this function not visible to anyone
> outside of random32.c.

It would be nice but a later call to prandom_reseed would not hurt that
much as a too early one. So I guess the tradeoff is worth it.

Maybe we can add runtime protection so it only will get called once.

I am also thinking about leaving the late_initcall in place and just
add the additional reseed from entropy_credit_bits.

I would also repace the net_random() calls with secure_ipv4/6_port_ephemeral.
Still need to check if I have all the needed input available to those
functions available.

Greetings,

  Hannes

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

end of thread, other threads:[~2013-11-09 20:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-08  0:54 [RFC] tcp: randomize TCP source ports Eric Dumazet
2013-11-08  1:07 ` Rick Jones
2013-11-08  2:04   ` Eric Dumazet
2013-11-08 23:26     ` Rick Jones
2013-11-08 23:42       ` Eric Dumazet
2013-11-08 23:57         ` Rick Jones
2013-11-08 13:02 ` Hannes Frederic Sowa
2013-11-08 14:03   ` Eric Dumazet
2013-11-08 14:28     ` Hannes Frederic Sowa
2013-11-08 15:11       ` Eric Dumazet
2013-11-08 17:39         ` Hannes Frederic Sowa
2013-11-09  4:47         ` Hannes Frederic Sowa
2013-11-09 15:26           ` Loganaden Velvindron
2013-11-09 18:16           ` Daniel Borkmann
2013-11-09 20:54             ` Hannes Frederic Sowa

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.