[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[STINNER Victor]

STINNER Victor added the comment:

random-py35.patch: Patch for the 3.5 branch. My prepared commit message:
---
Issue #29157: Prefer getrandom() over getentropy()

Copy and then adapt Python/random.c from default branch. Difference between 3.5
and default branches:

* Python 3.5 only uses getrandom() in non-blocking mode: flags=GRND_NONBLOCK
* If getrandom() fails with EAGAIN: py_getrandom() immediately fails and
  remembers that getrandom() doesn't work.
* Python 3.5 has no _PyOS_URandomNonblock() function: _PyOS_URandom()
  works in non-blocking mode on Python 3.5
---

It seems like Python 3.5 is close to a release, I prefer to wait after the 
release to fix this issue. I don't think that many Linux distributions are 
affected, since the issue only occurs with glibc 2.24 which is very recent.

@Larry: Do you want this change in Python 3.5.3? The change is quite large.

--
nosy: +larry
Added file: http://bugs.python.org/file46181/random-py35.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[STINNER Victor]

STINNER Victor added the comment:

Christian Heimes: "I'm doing a review now."

Follow-up on #python-dev (IRC):

 haypo: yes, I looked at the patch and did not see any obvious problem 
with it. Didn't I tell you?
 haypo: maybe I forgot :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[Roundup Robot]

Roundup Robot added the comment:

New changeset f8e24a0a1124 by Victor Stinner in branch '3.6':
Issue #29157: Prefer getrandom() over getentropy()
https://hg.python.org/cpython/rev/f8e24a0a1124

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[STINNER Victor]

STINNER Victor added the comment:

Thanks Nick and Xiang for the review. I splitted again the giant change into 
small atomic changes, easier to review and understand.

Right now, I only applied the change to default (Python 3.7). I will now wait 
for buildbots before considering backporting the change.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[Roundup Robot]

Roundup Robot added the comment:

New changeset 140f0459fe80 by Victor Stinner in branch 'default':
Issue #29157: dev_urandom() now calls py_getentropy()
https://hg.python.org/cpython/rev/140f0459fe80

New changeset 69b23952d122 by Victor Stinner in branch 'default':
Issue #29157: Simplify dev_urandom()
https://hg.python.org/cpython/rev/69b23952d122

New changeset 46ca697c6f26 by Victor Stinner in branch 'default':
Issue #29157: getrandom() is now preferred over getentropy()
https://hg.python.org/cpython/rev/46ca697c6f26

New changeset 4c11a01fa881 by Victor Stinner in branch 'default':
py_getentropy() now supports ENOSYS, EPERM & EINTR
https://hg.python.org/cpython/rev/4c11a01fa881

New changeset 4edd6cbf9abf by Victor Stinner in branch 'default':
Issue #29157: enhance py_getrandom() documentation
https://hg.python.org/cpython/rev/4edd6cbf9abf

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[Xiang Zhang]

Xiang Zhang added the comment:

LGTM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[Christian Heimes]

Christian Heimes added the comment:

I'm doing a review now.

By the way I did not copy random.c for cryptography. I took bits and pieces out 
of it. 
https://github.com/pyca/cryptography/blob/master/src/_cffi_src/openssl/src/osrandom_engine.c
 and 
https://github.com/pyca/cryptography/blob/master/src/_cffi_src/openssl/src/osrandom_engine.h

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[STINNER Victor]

STINNER Victor added the comment:

Nick: Thanks for the review!

Since random.c is critical for security, I would prefer to have at least a 
review from another core developer. I added Serhiy and Xiang in the nosy list. 
I'm also looking at you, Christian! ;-) (Christian reused random.c code in the 
pycrytography project.)

--
nosy: +serhiy.storchaka, xiang.zhang

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[Nick Coghlan]

Nick Coghlan added the comment:

New patch looks good to me, and +1 on applying the refactoring to all supported 
branches.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[STINNER Victor]

STINNER Victor added the comment:

New patch (version 2), much larger: it refactors the code, not only fix this 
specific issue (prefer getrandom() over getentropy()). Changes since 
getentropy.patch:

* Add a lot of comments to explain in depth how each function is implemented, 
which errors are handled, etc. It should help to audit the code: this code is 
very critical for security and so should be, IMHO, well documented.

* handle also EPERM and EINTR errors in getentropy(): retry on EINTR, fallback 
on /dev/urandom on EPERM -- sadly, I don't have access to a system with 
getentropy() to test this part of the code.

* call py_getrandom() and py_getentropy() in pyurandom() to make dev_urandom() 
simpler and so easy to review: dev_urandom() looses its blocking parameter

* Document the cached file descriptor, and cached st_dev+st_ino in 
dev_urandom().

* Document explicitly that functions are retried on EINTR error. Document that 
only getrandom() supports non-blocking mode. Document why we prefer an entropy 
source over others.

I'm not sure that getentropy() can fail with EPERM or EINTR in practice, but it 
shouldn't harm to handle correctly these errors :-) At least, getentropy() can 
fail with these errors on Linux since the glibc implements the getentropy() 
function using the getrandom() syscall (and it's known that getrandom() can 
fail with these errors). But on Linux, the code now prefers getrandom() over 
getentropy().

Should we use the new shiny code on all Python versions? Or only fix the 
reported issue on all Python issues, and use the refactored code in Python 
default?

Note: Python 2.7 still supports VMS. VMS is unsupported in Python 3.3 and the 
VMS code was removed in Python 3.4 (issue 16136): see the PEP 11.

I suggest to use the same code on all maintained Python versions to ease 
maintenance.

--
Added file: http://bugs.python.org/file46157/random-2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[Nick Coghlan]

Nick Coghlan added the comment:

Aside from a couple of outdated comments and the EPERM question, the attached 
patch looks good to me.

Regarding EPERM, I think it would make sense to make py_getrandom and 
py_getentropy handle that consistently, otherwise I can see future maintainers 
readily getting confused by the discrepancy.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[Nick Coghlan]

Changes by Nick Coghlan :


--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[STINNER Victor]

STINNER Victor added the comment:

Python 2.7, 3.5, 3.6 and 3.7 are impacted: they should fail on Linux if 
compiled with a recent glibc which has getentropy().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29157] random.c: Prefer getrandom() over getentropy(), handle ENOSYS in py_getentropy()

[STINNER Victor]

New submission from STINNER Victor:

A new getentropy() function was recently added to the glibc:
https://sourceware.org/bugzilla/show_bug.cgi?id=17252

When the Python/random.c file was written (by me), the getentropy() function 
was only supported on OpenBSD. Later, random.c was modified to *not* use 
getentropy() on Solaris (Issue #25003).

The problem is that py_getentropy() doesn't handle ENOSYS, and so Python fails 
at startup with a fatal error (Python 3.6):
   Fatal Python error: failed to get random numbers to initialize Python
or (Python 3.5):
   Fatal Python error: getentropy() failed

The bug was first reported in Fedora 26 (rawhide):
https://bugzilla.redhat.com/show_bug.cgi?id=1410175

Attached patch (written for the default branch) should fix these issues:

* Prefer getrandom() syscall over getentropy() function: getrandom() supports 
blocking and non-blocking mode on Linux, whereas getentropy() doesn't
* Enhance py_getentropy() to handle ENOSYS: fallback on reading from 
/dev/urandom and remember that the function doesn't work

I'm not sure that handling ENOSYS is required, since it's no more used on 
Linux, but it shouldn't hurt. I don't know if py_getentropy() should also 
handle EPERM?

py_getrandom() catchs errors: EAGAIN, EINTR, EPERM and ENOSYS.

With the patch, py_getentropy() catchs ENOSYS error.

--
files: getentropy.patch
keywords: patch
messages: 284659
nosy: christian.heimes, haypo
priority: normal
severity: normal
status: open
title: random.c: Prefer getrandom() over getentropy(), handle ENOSYS in 
py_getentropy()
type: security
versions: Python 2.7, Python 3.5, Python 3.6, Python 3.7
Added file: http://bugs.python.org/file46144/getentropy.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com