Skip to content
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

Use arc4random under OpenBSD for os.urandom() if /dev/urandom is not present #66732

Closed
700eb415 mannequin opened this issue Oct 2, 2014 · 11 comments
Closed

Use arc4random under OpenBSD for os.urandom() if /dev/urandom is not present #66732

700eb415 mannequin opened this issue Oct 2, 2014 · 11 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@700eb415
Copy link
Mannequin

700eb415 mannequin commented Oct 2, 2014

BPO 22542
Nosy @ncoghlan, @vstinner, @alex

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-20.21:50:49.464>
created_at = <Date 2014-10-02.17:47:40.174>
labels = ['interpreter-core', 'type-feature']
title = 'Use arc4random under OpenBSD for os.urandom() if /dev/urandom is not present'
updated_at = <Date 2016-09-20.21:50:49.462>
user = 'https://bugs.python.org/700eb415'

bugs.python.org fields:

activity = <Date 2016-09-20.21:50:49.462>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2016-09-20.21:50:49.464>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2014-10-02.17:47:40.174>
creator = '700eb415'
dependencies = []
files = []
hgrepos = []
issue_num = 22542
keywords = []
message_count = 11.0
messages = ['228246', '228249', '228803', '228849', '228851', '229320', '230316', '231865', '274728', '274731', '277074']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'vstinner', 'alex', 'rpointel', '700eb415']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue22542'
versions = ['Python 3.5']

@700eb415
Copy link
Mannequin Author

700eb415 mannequin commented Oct 2, 2014

Trying to run the python interpreter in a chroot fails if /dev/urandom is not present. Removing the "nodev" flag from the filesystem is not ideal in many situations.

Instead, we should utilize functions such as OpenBSD's arc4random(3) and the new potential getentropy() Linux syscall. Alternatively, libevent provides a portable version of arc4random(3) as a workaround.

This issue has been discussed extensively when forking LibreSSL. Since we're already providing win32 exceptions, we should at least use the syscall rather than device if it's defined.

@700eb415 700eb415 mannequin added the build The build process and cross-build label Oct 2, 2014
@alex
Copy link
Member

alex commented Oct 2, 2014

arc4random() should be avoided IMO, on many systems (including OS X) it really is still arc4; this is basically a dupe of http://bugs.python.org/issue22181

@alex alex closed this as completed Oct 2, 2014
@700eb415
Copy link
Mannequin Author

700eb415 mannequin commented Oct 8, 2014

I'm reopening this for now as advised from the Linux getrandom() thread.

I agree we should not be using arc4random() blindly. However, in the long run it is a necessary change at least on OpenBSD. OpenBSD is not likely to create another syscall to avoid portability problems with OS X, so IMO it's best to utilize the existing calls on the system.

I'll work on some portable way of deterministically enabling it when needed and put a patch out for review. I think the payoff would be good when taking into account the security implications and cases where there are no available file descriptors.

Hopefully this could then be used as a template for getrandom() when implemented on Linux.

@700eb415 700eb415 mannequin reopened this Oct 8, 2014
@pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement and removed build The build process and cross-build labels Oct 8, 2014
@pitrou pitrou changed the title Use syscall (eg. arc4random or getentropy) rather than /dev/urandom when possible Use arc4random under OpenBSD for os.urandom() Oct 8, 2014
@vstinner vstinner changed the title Use arc4random under OpenBSD for os.urandom() Use arc4random under OpenBSD for os.urandom() if /dev/urandom is not present Oct 9, 2014
@vstinner
Copy link
Member

vstinner commented Oct 9, 2014

title: Use syscall (eg. arc4random or getentropy) rather than /dev/urandom when possible -> Use arc4random under OpenBSD for os.urandom()

For the usage getentropy(), I created a dedicated issue: bpo-22585.

arc4random() should be avoided IMO, on many systems (including OS X) it really is still arc4

RC4 has known weaknesses since 1995, the most severe was reported the last year:
https://en.wikipedia.org/wiki/RC4#Royal_Holloway_attack

http://www.theregister.co.uk/2013/11/14/ms_moves_off_rc4/
"Microsoft, Cisco: RC4 encryption considered harmful, avoid at all costs"
and
"RC4 is broken in real-time by the ‪NSA‬ – stop using it."

FYI On OpenBSD 5.5 and newer, arc4random() now uses ChaCha20:
http://www.openbsd.org/cgi-bin/man.cgi?query=arc4random&sektion=3&arch=&manpath=OpenBSD+Current

If I understand correctly, arc4random() uses the ARC4 algorithm on all platforms except OpenBSD 5.5 and newer.

OpenBSD 5.6 with its getentropy() will be available when Python 3.5 will be released (PEP-478): OpenBSD 5.6 is scheduled for one month (november 2014), whereas Python 3.5 is scheduled for September 2015.

I don't think that we need to add a very specific code in Python 3.5 only for the exact platform OpenBSD 5.5, for a very specific case: chroot without /dev/urandom. What's the point of a chroot without /dev/urandom? Other applications will also have security issues if /dev/urandom is not available.

Current before of Python 3.5 if /dev/urandom is not available:

$ ~haypo/prog/python/default/python
Fatal Python error: Failed to open /dev/urandom
Abandon (core dumped)

The error is reported immediatly at startup. So you can immediatly detect the issue with your chroot.

I don't trust a function from the user space.

os.urandom() is documented as providing entropy from the "OS":
"Return a string of n random bytes suitable for cryptographic use. This function returns random bytes from an OS-specific randomness source."
https://docs.python.org/dev/library/os.html#os.urandom

I'm not sure that arc4random() can be considered as coming from the "OS".

If you really want arc4random(), IMO you should add a *new* function, but it would not be portable: only available on OpenBSD (and maybe other BSD including Mac OS X), not available on Windows nor Linux. I'm not sure that it fits Python portability policy, even if we have many functions which are only available on some recent platforms, like many Linux-specific functions (in the os module).

@vstinner
Copy link
Member

vstinner commented Oct 9, 2014

Hopefully this could then be used as a template for getrandom() when implemented on Linux.

Sorry, what is getrandom()?

Linux 3.17 has a new getrandom() syscall, but the C API is not defined yet (see the issue bpo-22181). OpenBSD 5.6 will have a getentropy() syscall and a gentropy() function in the C libray.

If if you are discussing about an hypothetical function in the C library, it's out of the scope if this bug tracker. Python don't call directly syscalls (ok, there is only one place in _posixsubprocess to avoid a race condition).

@700eb415
Copy link
Mannequin Author

700eb415 mannequin commented Oct 14, 2014

I'm not sure that arc4random() can be considered as coming from the "OS".

We really have a couple options here. (1)Include a high quality pseudorandom number function for every platform that doesn't provide the proper call (very tedious and lots of places of mistakes - see: OpenSSL failing at this horribly), (2)get the OS venders to fix their software (not likely on this timescale), (3)use lots of #ifdef's to do platform detection (yuck), or (4)ignore broken calls provided by the system in hopes that the developers fix the issue.

Bob beck has a nice dialog on this problem in FreeBSD here: libressl/portable#17

It's ugly, but I'm leaning towards an "#ifdef __OpenBSD__" in this case since it seems to be the only platform at this time with a sane implementation other than the latest Linux kernel.

If you really want arc4random(), IMO you should add a *new* function, but it would not be portable: only available on OpenBSD (and maybe other BSD including Mac OS X), not available on Windows nor Linux. I'm not sure that it fits Python portability policy, even if we have many functions which are only available on some recent platforms, like many Linux-specific functions (in the os module).

I think this would be a bad idea based on how easy this is to get wrong. The logic:

if /dev/urandom
...
else if os_has_proper_rand()
...
else
fail

seems to be the best way to handle this IMO until OS venders provide viable fixes.

Alternatively if the consensus is to reconvene at a later time, I could work on a patch for the OpenBSD port and we can ignore the problem here for now. However, I think the Python community is a great place to bring this issue to light much as was done with LibreSSL.

@vstinner
Copy link
Member

The issue is about the base "if /dev/urandom is not present". How is arc4random() PRNG/CPRNG initialized if /dev/urandom is *not* present?

Can we rely on it if it only uses a poor seed?

@700eb415
Copy link
Mannequin Author

700eb415 mannequin commented Nov 29, 2014

From the OpenBSD random(4) man page:
"The arc4random(3) function in userland libraries should be used instead, as it works without the need to access these devices every time."

Theo just had a good talk on this issue here about why /dev/random needs replacing here: http://www.openbsd.org/papers/hackfest2014-arc4random/index.html . There's also a videon on YouTube.

At this point, I should probably have a patch ready sometime towards the middle of the week. I had a conversation with Ted Unangst off list, and think the best place for me to push it would first be a patch to the OpenBSD ports. After the OpenBSD guys review it, I'll then push it here.

@ncoghlan
Copy link
Contributor

ncoghlan commented Sep 7, 2016

Victor, can this be closed following the changes to os.urandom() in 3.5 and 3.6 to avoid using a file descriptor in os.urandom() where feasible?

@vstinner
Copy link
Member

vstinner commented Sep 7, 2016

I'm not sure that os.urandom() is correct on OpenBSD. I'm not sure that using getentropy() is correct. getentropy() seems to be high quality but I understand that there is a low quantity of entropy and it can block.

I don't know if arc4random() is better: it's no more RC4 on recent OpenBSD, and it has an efficient implementation.

@vstinner
Copy link
Member

"Trying to run the python interpreter in a chroot fails if /dev/urandom is not present."

The workaround is simple: fix your chroot to correctly expose /dev/urandom in the chroot. It's a common and known issue, no?

Since the issue is almost dead since 2 years and I don't know if arc4random() is suitable for os.urandom(), I close the issue. If you want to reopen it, please come back with more information on arc4random() security ;-)

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants