New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
urandom persistent fd - not re-openned after fd close #65406
Comments
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): |
Which project or Python module does that? Can you show me the code? |
Issue where I hit this is in Fail2Ban: fail2ban/fail2ban#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:
|
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 |
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. |
That's fine if os.close() is indeed used to close fd, but not if some (also some other stdlib code might (?) also call C close()...) |
fine by me. was just a thought |
LGTM! |
New changeset a66524ce9551 by Antoine Pitrou in branch '3.4': New changeset d3e8db93dc18 by Antoine Pitrou in branch 'default': |
Ok, I've committed the patch. Hopefully this will also fix any similar issues. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: