[ python-Bugs-1177468 ] random.py/os.urandom robustness

[SourceForge.net]

Bugs item #1177468, was opened at 2005-04-06 01:03
Message generated for change (Comment added) made by jafo
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1177468&group_id=5470

Category: Python Library
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: Fazal Majid (majid)
Assigned to: Nobody/Anonymous (nobody)
Summary: random.py/os.urandom robustness

Initial Comment:
Python 2.4.1 now uses os.urandom() to seed the random
number generator. This is mostly an improvement, but
can lead to subtle regression bugs.

os.urandom() will open /dev/urandom on demand, e.g.
when random.Random.seed() is called, and keep it alive
as os._urandomfd.

It is standard programming practice for a daemon
process to close file descriptors it has inherited from
its parent process, and if it closes the file
descriptor corresponding to os._urandomfd, the os
module is blissfully unaware and the next time
os.urandom() is called, it will try to read from a
closed file descriptor (or worse, a new one opened
since), with unpredictable results.

My recommendation would be to make os.urandom() open
/dev/urandom each time and not keep a persistent file
descripto. This will be slightly slower, but more
robust. I am not sure how I feel about a standard
library function steal a file descriptor slot forever,
specially when os.urandom() is probably going to be
called only once in the lifetime of a program, when the
random module is seeded.


----------------------------------------------------------------------

>Comment By: Sean Reifschneider (jafo)
Date: 2005-04-06 09:04

Message:
Logged In: YES 
user_id=81797

The child is a copy of the parent.  Therefore, if in the
parent you open a few file descriptors, those are the ones
you should close in the child.  That is exactly what I've
done in the past when I forked a child, and it has worked
very well.

I suspect Stevens would make an exception to his guideline
in the event that closing a file descriptor results in
library routine failures.

----------------------------------------------------------------------

Comment By: Fazal Majid (majid)
Date: 2005-04-06 07:27

Message:
Logged In: YES 
user_id=110477

Unfortunately, catching exceptions is not sufficient - the
file descriptor may have been reassigned. Fortunately in my
case, to a socket which raised ENOSYS, but if it had been a
normal file, this would have been much harder to trace
because reading from it would cause weird errors for readers
of the reassigned fd without triggering an exception in
os.urandom() itself.

As for not closing file descriptors you haven't opened
yourself, if the process is the result of a vfork/exec (in
my case Python processes started by a cluster manager, sort
of like init), the child process has no clue what file
descriptors, sockets or the like it has inherited from its
parent process, and the safest course is to close them all.
Indeed, that's what W. Richard Stevens recommends in
"Advanced Programming for the UNIX environment".

As far as I can tell, os.urandom() is used mostly to seed
the RNG in random.py, and thus is not a high-drequency
operation. It is going to be very hard to document this
adequately for coders to defend against - in my case, the
problem was being triggered by os.tempnam() from within
Webware's PSP compiler. There are so many functions that
depend on random (sometimes in non-obvious ways), you can't
flag them all so their users know they should use
urandomcleanup.

One possible solution would be for os.py to offer a
go_daemon() function that implements the fd closing, signal
masking, process group and terminal disassociation required
by true daemons. This function could take care of internal
book-keeping like calling urandomcleanup.

----------------------------------------------------------------------

Comment By: Sean Reifschneider (jafo)
Date: 2005-04-06 07:20

Message:
Logged In: YES 
user_id=81797

Yeah, I was thinking the same thing.  It doesn't address the
consumed file handle, but it does address the "robustness"
issue.  It complicates the code, but should work.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2005-04-06 07:11

Message:
Logged In: YES 
user_id=21627

To add robustness, it would be possible to catch read errors
from _urandomfd, and try reopening it if it got somehow closed.

----------------------------------------------------------------------

Comment By: Sean Reifschneider (jafo)
Date: 2005-04-06 03:11

Message:
Logged In: YES 
user_id=81797

Just providing some feedback:

I'm able to reproduce this.  Importing random will cause
this file descriptor to be called.  Opening urandom on every
call could lead to unacceptable syscall overhead for some. 
Perhaps there should be a "urandomcleanup" method that
closes the file descriptor, and then random could get the
bytes from urandom(), and clean up after itself?

Personally, I only clean up the file descriptors I have
allocated when I fork a new process.  On the one hand I
agree with you about sucking up a fd in the standard
library, but on the other hand I'm thinking that you just
shouldn't be closing file descriptors for stuff you'll be
needing.  That's my two cents on this bug.

----------------------------------------------------------------------

Comment By: Fazal Majid (majid)
Date: 2005-04-06 01:06

Message:
Logged In: YES 
user_id=110477

There are many modules that have a dependency on random, for
instance os.tempnam(), and a program could well
inadvertently use it before closing file descriptors.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1177468&group_id=5470