PEP 524: Make os.urandom() blocking on Linux

[vstinner]

BPO	27776
Nosy	@ncoghlan, @vstinner, @AraHaan
Files	urandom_nonblock.patch urandom_nonblock-2.patch

^{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:

assignee = None
closed_at = <Date 2016-09-06.23:43:59.794>
created_at = <Date 2016-08-16.12:55:06.620>
labels = ['type-security']
title = 'PEP 524: Make os.urandom() blocking on Linux'
updated_at = <Date 2016-09-07.04:30:09.790>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2016-09-07.04:30:09.790>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2016-09-06.23:43:59.794>
closer = 'vstinner'
components = []
creation = <Date 2016-08-16.12:55:06.620>
creator = 'vstinner'
dependencies = []
files = ['44126', '44150']
hgrepos = []
issue_num = 27776
keywords = ['patch']
message_count = 14.0
messages = ['272852', '272853', '272855', '272864', '272865', '273025', '273111', '273112', '273232', '274668', '274670', '274680', '274707', '274755']
nosy_count = 4.0
nosy_names = ['ncoghlan', 'vstinner', 'python-dev', 'Decorater']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue27776'
versions = ['Python 3.6']

[vstinner]

Issue to track the implementation of the PEP-524.

[AraHaan]

Wow. ( linux only pep? inb4 a windows thing gets wedged in)

[python-dev]

New changeset 980e2c781810 by Victor Stinner in branch 'default':
Issue bpo-27776: Cleanup random.c
https://hg.python.org/cpython/rev/980e2c781810

New changeset 265644bad99e by Victor Stinner in branch 'default':
Issue bpo-27776: _PyRandom_Init() doesn't call PyErr_CheckSignals() anymore
https://hg.python.org/cpython/rev/265644bad99e

[python-dev]

New changeset 86d0d74bc2e1 by Victor Stinner in branch 'default':
Issue bpo-27776: Cleanup random.c
https://hg.python.org/cpython/rev/86d0d74bc2e1

New changeset ad141164c792 by Victor Stinner in branch 'default':
Issue bpo-27776: dev_urandom(raise=0) now closes the file descriptor on error
https://hg.python.org/cpython/rev/ad141164c792

[vstinner]

Patch to make os.urandom() blocking on Linux 3.17+, but use non-blocking urandom in _random.Random constructor and _random.Random.seed() with no seed is set.

[ncoghlan]

I have a few requests for clarification and confirmation as review comments, but overall +1 from me.

(I'd still like a warning when we need to block in order to make life easier for system administrators attempting to debug any apparent system hangs, but as per the security-sig discussion, I can pursue that in a follow-up RFE and a separate patch, while this patch implements the PEP precisely as accepted)

[vstinner]

Enhanced patch to address Nick's comments and fix mistakes. The new patch now also updates the documentation.

I restored the code in _random.Random.seed() to fallback on the system clock: _PyOS_URandomNonblock() *can* fail is /dev/urandom is missing or not readable. I enhanced this part to not only read the system clock, but also use the current process identifier and get also the monotonic clock. Moreover, 64 bits are now used instead of 32 bits from the system clock (use a resolution of 1 nanoscond, not only 1 second).

I didn't test yet the fall back on clocks/pid. It should be tested manually by modifying _PyOS_URandomNonblock() to always fail.

[vstinner]

_PyOS_URandomNonblock() *can* fail is /dev/urandom is missing or not readable

Oh. It looks like Python initialization currently fails with a fatal error in this case, see _PyRandom_Init().

Maybe we should also fall back on clocks/pid in _PyRandom_Init()?

[ncoghlan]

+1 for a fallback in the SIPHash initialisation as well.

That's the case where Nathaniel Smith suggested we may want to issue a warning that the process shouldn't be used to handle untrusted inputs (since that particular remote DoS defence won't be working properly), but the monotonic time + the PID should be sufficiently unpredictable seeding for that case (since there are plenty of lower hanging fruit for attackers to go after).

For testing, is there some way we could integrate an automated test of the deliberately misbehaving _PyOS_UrandomNonBlock into the testembed helper? If we can come up with a sensible way to do that, it could potentially help with testing the os.getrandom() BlockingIOError generation as well.

[python-dev]

New changeset 45fc0c83ed42 by Victor Stinner in branch 'default':
os.urandom() now blocks on Linux
https://hg.python.org/cpython/rev/45fc0c83ed42

[vstinner]

Nick: "+1 for a fallback in the SIPHash initialisation as well."

Sorry but I don't know a simple function to implement this. We might use the LCG RNG, but it's not really designed to be "secure". I don't think that it makes sense to initialize a shiny SIPHash with a crappy LCG RNG :-)

So I skip my turn on this idea and let others implement them if anyone consider that it's worth it.

To be clear: Python 3 doesn't start when getrandom() and /dev/urandom are not available or don't work, but it's not something new. Python 3.1 already starts with:

    fd = open("/dev/urandom", O_RDONLY);
    if (fd < 0)
        Py_FatalError("Failed to open /dev/urandom");

--

os.urandom() is now blocking, I close the issue.

[python-dev]

New changeset ebbfc053360a by Victor Stinner in branch 'default':
Issue bpo-27776: include process.h on Windows for getpid()
https://hg.python.org/cpython/rev/ebbfc053360a

[ncoghlan]

If /dev/urandom isn't available, Python refusing to start is likely to be one of the least of the system's problems, so Py_FatalError sounds reasonable to me - my +1 for a fallback above was a matter of "sounds good if you can find a way to make it work".

Thanks for all your work on getting this designed and implemented, Victor!

[ncoghlan]

I've reviewed all the open issues that come up when searching for "getrandom" on the issue tracker, and closed all the ones that were either out of date or rejected based on PEP-524 being accepted and implemented.

For the remainder, I either wasn't clear on whether they could be closed or not (in which case I posted a comment asking Victor to take a look at them), or else they were clearly still valid and I posted a relevant status update.

Doing a similar search for "urandom", I checked the ones where the titles seemed relevant and commented where a status update and possible closure seemed appropriate.