netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* get_random_int() should use hash[1]
@ 2011-08-16  9:07 George Spelvin
  2011-08-16  9:09 ` David Miller
  0 siblings, 1 reply; 13+ messages in thread
From: George Spelvin @ 2011-08-16  9:07 UTC (permalink / raw)
  To: davem; +Cc: linux, netdev

Re: commit e997d47bff5a467262ef224b4cf8cbba2d3eceea

As long as you're using MD5, you should know that each round only
modifies one word of the state.  The order is [0], [3], [2], [1],
repeating 64 times.  Thus, on output, word [1] is the "most hashed"
word.  If you really wanted word [0], you could just skip the last
3 rounds.

It's not really critical, but as long as you're performing the
rounds, you might as well use them...

--- drivers/char/random.c.1	2011-08-16 05:02:09.000000000 -0400
+++ drivers/char/random.c.2	2011-08-16 05:02:43.000000000 -0400
@@ -1323,7 +1323,7 @@
 
 	hash[0] += current->pid + jiffies + get_cycles();
 	md5_transform(hash, random_int_secret);
-	ret = hash[0];
+	ret = hash[1];
 	put_cpu_var(get_random_int_hash);
 
 	return ret;

Me, I'd also put jiffies and get_cycles into different words
just on general principles, but that's up to you:

- 	hash[0] += current->pid + jiffies + get_cycles();
+ 	hash[1] += current->pid + jiffies;
+ 	hash[2] += get_cycles();

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH 0/2] Improve sequence number generation.
@ 2011-08-07  1:50 David Miller
  0 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2011-08-07  1:50 UTC (permalink / raw)
  To: netdev; +Cc: dan, w, torvalds, linux-kernel, dccp, mpm, herbert, gerrit


Dan Kaminsky pointed out that using partial MD4 and using that to
generate a sequence number, of which only 24-bits are truly
unguessable, seriously undermine the goals of random sequence number
generation.

In particular, with only 24-bits being truly unguessable, packet
injection into a session using even something like brute force is a
real potential possibility.

We only use 24-bits because we regenerate the random number every 5
minutes "just in case."  But what does is trade a "we don't know" kind
of theoretical issue for a provably real one (brute force attack).

Therefore I'm moving us more in line with RFC1948 (as well as OpenBSD
and Solaris), to use MD5 and a full 32-bit result in the generated
sequence number.

MD5 was selected as a compromise between performance loss and
theoretical ability to be compromised.  Willy Tarreau did extensive
testing and SHA1 was found to harm performance too much to be
considered seriously at this time.

We may later add a sysctl for various modes (ie. a "super secure" mode
that uses SHA1 if people want that, and an "insecure" mode that doesn't
use cryptographic hashing at all for people in protected environments
where that might be safe to do).

I've also moved the sequence number generators out of random.c (they
never really belonged there, and are only there due to historical
artifacts), and fixed a bug in DCCP sequence number generation
(on ipv6 the 43-bit sequence number was truncated to 32-bits).

This work is already pushed out to the net GIT tree and I'll ask
Linus to pull it in a moment, we've been discussing how to handle
this issue for weeks on security@kernel.org already.

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

end of thread, other threads:[~2011-08-22  2:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-16  9:07 get_random_int() should use hash[1] George Spelvin
2011-08-16  9:09 ` David Miller
2011-08-16  9:10   ` David Miller
2011-08-16 10:30     ` George Spelvin
2011-08-20 23:39       ` [PATCH 0/2] Improve sequence number generation George Spelvin
2011-08-20 23:44         ` David Miller
2011-08-21  0:49           ` George Spelvin
2011-08-21  1:28         ` Willy Tarreau
2011-08-21  3:04           ` George Spelvin
2011-08-21  3:27             ` Ted Ts'o
2011-08-21  4:02               ` Herbert Xu
2011-08-22  2:06               ` George Spelvin
  -- strict thread matches above, loose matches on Subject: below --
2011-08-07  1:50 David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).