homepage

I've seen an issue with using urandom on Python 3.4. I've traced down to fd being closed (not by core CPython, but by third party library code). After this, access to urandom fails.
I assume this is related to persistent fd for urandom in http://bugs.python.org/issue18756
$ python -c "import os;os.urandom(1);os.closerange(3,256);os.urandom(1)"
Traceback (most recent call last):
 File "<string>", line 1, in <module>
OSError: [Errno 9] Bad file descriptor

Well, if a third-party library decides to close fds it doesn't own, that library should have a bug reported to it.

I agree in part, but it's quite common to close fd's in some cases like in a child process after using "os.fork()". There is no way, as far as I'm aware, to identify which fd is associated with /dev/urandom to keep it open; or anyway to reopen it such that other libraries which depend on it can use it (for example "tempfile.TemporaryFile").

Just to add for those interested: a possible work around solution is using "os.path.sameopenfile" to check fds against another known fd for urandom.
And for those wish to have a bit of fun (and maybe a security consideration):
python -c "import os;os.urandom(1);os.closerange(3,256);fd = open('/dev/zero');print(os.urandom(10))"
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'

> I agree in part, but it's quite common to close fd's in some cases like in a child process after using "os.fork()"
Which project or Python module does that? Can you show me the code?

Issue where I hit this is in Fail2Ban: https://github.com/fail2ban/fail2ban/issues/687
Lines of code where this occurs: https://github.com/fail2ban/fail2ban/blob/1c65b946171c3bbc626ddcd9320ea2515018677b/fail2ban/server/server.py#L518-530
There are other examples of closing file descriptors in other packages which create daemon processes, as well as code snippets about, as it is typical behaviour to close files. (http://en.wikipedia.org/wiki/Daemon_%28computing%29#Creation)

Well, on the one hand this does sound like a valid use case. On the other hand, once the urandom file descriptor is closed by third-party code, it can very well be re-opened to point to another file, and then os.urandom() will start behaving in a very bad way.
Here is a possible solution in Python:
- when opening the urandom fd for the first time, record its st_ino and st_dev
- when calling urandom() a second time, call fstat() on the fd and check the st_ino and st_dev with the known values
- if the values have changed (or if fstat() fails with EBADF), open a new fd to /dev/urandom, again

Christian, do you see a security risk with the proposed change?

I was expecting to see such a report :-)
I'm al for the st_ino+st_dev check, it can't hurt.
But everybody must keep in mind that if another thread messes with the
FD between the check and the read, there's nothing we can do...

Here is a proposed patch (with tests).

Hmm, the patch doesn't release the GIL around the fstat() calls, I wonder if that's necessary.

Updated patch using an anonymous struct.

maybe you've thought and dismissed this already but os.close could call dev_urandom_close for the urandom_fd. Then there's no fstat calls in every random access. As a sweeping close all isn't going to occur that often and extra open probably isn't that much overhead.

> maybe you've thought and dismissed this already but os.close could
> call dev_urandom_close for the urandom_fd. Then there's no fstat calls
> in every random access.
That's fine if os.close() is indeed used to close fd, but not if some
third-party library uses the C close() function directly. I don't know
how likely that is, but I think it's better to squash the bug
completely, rather than 80% of it :-)
(also some other stdlib code might (?) also call C close()...)

fine by me. was just a thought

> Updated patch using an anonymous struct.
LGTM!

New changeset a66524ce9551 by Antoine Pitrou in branch '3.4':
Issue #21207: Detect when the os.urandom cached fd has been closed or replaced, and open it anew.
http://hg.python.org/cpython/rev/a66524ce9551
New changeset d3e8db93dc18 by Antoine Pitrou in branch 'default':
Issue #21207: Detect when the os.urandom cached fd has been closed or replaced, and open it anew.
http://hg.python.org/cpython/rev/d3e8db93dc18 

Ok, I've committed the patch. Hopefully this will also fix any similar issues.