455829 - Please use /dev/urandom instead of /dev/random on Linux

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Mike Hommey [:glandium]]

Attachment: Patch

We've go several people reporting that iceweasel would freeze at startup on some appliances of the web kiosk kind that just boot up to iceweasel almost directly, until some mouse movements occur and/or keys are pressed. This is due to the fact that Linux's /dev/random uses mouse and keys to get some entropy, and /dev/random blocks when it doesn't have enough entropy. /dev/urandom is a (weaker) version that doesn't block. /dev/urandom is actually used in NSS already.

Anyways, I see two possibilities, here, one of which I am sending a patch for. Please tell me if you'd prefer the other approach.
- Use /dev/urandom on Linux (BSD variants have already a non blocking (weak) RNG on /dev/random)
- Use clock_gettime() to get a real high resolution clock response, but that would depend on the resolution (clock_getres()) ; high resolution clock is quite recent in linux iirc.

Comment 1

[Wan-Teh Chang]

Comment on attachment 339195 [details] [diff] [review]
Patch

This fix is good.  We should make this change unconditionally,
rather than with #ifdef LINUX.  Perhaps OpenDevRandom and
fdDevRandom should be renamed accordingly.

I'm more concerned about the code that's calling PR_GetRandomNoise.
The output of PR_GetRandomNoise should only be used to seed a
pseudorandom number generator (PRNG).  PR_GetRandomNoise must not
be used as a PRNG directly.

Comment 2

[Mike Hommey [:glandium]]

Doesn't seem to be much problem...
http://mxr.mozilla.org/firefox/search?string=PR_GetRandomNoise&tree=firefox

Comment 3

[Mike Hommey [:glandium]]

Attachment: Patch v2

How about this ?

Comment 4

[Wan-Teh Chang]

Comment on attachment 339296 [details] [diff] [review]
Patch v2

r=wtc.  Thanks.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, I think there may be disagreement about whether or not the switch to 
/dev/urandom should be made "unconditionally, rather than with #ifdef LINUX."

Comment 6

[Wan-Teh Chang]

Mike, could you find the iceweasel code that calls PR_GetRandomNoise?
Unless it uses the PR_GetRandomNoise output to seed a PRNG, it is
using PR_GetRandomNoise incorrectly and it needs to be changed.

Nelson, in spite of the documentation
(http://developer.mozilla.org/en/PR_GetRandomNoise
http://mxr.mozilla.org/nspr/source/nsprpub/pr/include/prrng.h#53)
people use PR_GetRandomNoise incorrectly as a PRNG.  In fact,
I doubt there is any code that uses PR_GetRandomNoise as intended
(to seed a PRNG).  This is why switching to /dev/urandom is a good
solution.

Comment 7

[Mike Hommey [:glandium]]

(In reply to comment #6)
> Mike, could you find the iceweasel code that calls PR_GetRandomNoise?
> Unless it uses the PR_GetRandomNoise output to seed a PRNG, it is
> using PR_GetRandomNoise incorrectly and it needs to be changed.

See comment #2. (There are no differences between iceweasel and firefox introducing uses of PR_GetRandomNoise)

Comment 8

[Mike Hommey [:glandium]]

Anyways, please note that I'm waiting feedback as whether the patch really addresses the freezing issue.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

There are three callers of PR_GetRandomNoise in the Mozilla source tree outside of NSPR:

1) Use of PR_GetRandomNoise in nsUUIDGenerator.cpp was discussed in bug 279521.
2) nsMetricsService.cpp uses the "noise" to stagger update requests randomly - it doesn't really care that the numbers might not be sufficiently random.
3) nsXFormsXPathFunctions::Random passes the return value of PR_GetRandomNoise to srand().

Comment 10

[Wan-Teh Chang]

Mike, Gavin, thanks for finding the users of PR_GetRandomNoise.

Among them, only nsMetricsService.cpp is using PR_GetRandomNoise
incorrectly.  It uses PR_GetRandomNoise directly to generate a
random 32-bit number.

The problem of using PR_GetRandomNoise as a PRNG is that the
output of PR_GetRandomNoise, although random, may not be
uniformly distributed.  This is why we need to feed it to
a PRNG to "whiten" the output.

Comment 11

[Mike Hommey [:glandium]]

I can confirm the patch solved the freezing problem for the people reporting it. I don't know what codepath was used then, though.

Comment 12

[Mike Hommey [:glandium]]

And the code path leading to the freeze was through nsUUIDGenerator.

Comment 14

[Jordan Erickson]

Please also see bug 455829 (with 5 votes), as this seems to be a duplicate. A
patch would be greatly appreciated as it seems to affect LTSP setups on a
critical level (Firefox won't launch for *anyone* after 2-4 users launch it on
the network).

Comment 15

[Jordan Erickson]

Heh, there IS a patch! Ok, I'll try and test it out.

Comment 16

[Wan-Teh Chang]

I checked in the patch on the NSPR trunk (NSPR 4.7.2).

Checking in uxrng.c;
/cvsroot/mozilla/nsprpub/pr/src/md/unix/uxrng.c,v  <--  uxrng.c
new revision: 1.22; previous revision: 1.21
done