[issue21305] PEP 466: update os.urandom

[STINNER Victor]

STINNER Victor added the comment:

See also the issue #22181: "os.urandom() should use Linux 3.17 getrandom() 
syscall".

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Alex Gaynor]

Alex Gaynor added the comment:

Attached patch backports the persistent FD for urandom.

--
components: +Extension Modules, Interpreter Core
keywords: +needs review, patch
Added file: http://bugs.python.org/file36437/backport-urandom.diff

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Benjamin Peterson]

Benjamin Peterson added the comment:

You should probably backport _PyRandom_Fini and cleanup the FD like a good 
citizen.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Alex Gaynor]

Alex Gaynor added the comment:

This patch adds the finalizer to the backport -- not sure how I missed this the 
first time.

--
Added file: http://bugs.python.org/file36496/backport-urandom.diff

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[STINNER Victor]

STINNER Victor added the comment:

@alex: please disable git format in your hgrc, so the bug tracker can create a 
"review" link to Rietveld for your patches.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Alex Gaynor]

Alex Gaynor added the comment:

Victor -- new patch is in `hg` format.

--
Added file: http://bugs.python.org/file36497/backport-urandom.diff

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[STINNER Victor]

STINNER Victor added the comment:

The third  backport-urandom.diff  (the one with the review link) looks good to 
me.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Roundup Robot]

Roundup Robot added the comment:

New changeset 3e7f88550788 by Benjamin Peterson in branch '2.7':
PEP 466: backport persistent urandom fd (closes #21305)
http://hg.python.org/cpython/rev/3e7f88550788

--
nosy: +python-dev
resolution:  -> fixed
stage: needs patch -> resolved
status: open -> closed

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Nick Coghlan]

New submission from Nick Coghlan:

Tracker issue for the os.urandom persistent file descriptor backport to 2.7 
described in PEP 466.

--
messages: 216824
nosy: alex, benjamin.peterson, christian.heimes, dstufft, giampaolo.rodola, 
janssen, ncoghlan, pitrou
priority: normal
severity: normal
stage: needs patch
status: open
title: PEP 466: update os.urandom
type: enhancement
versions: Python 2.7

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Josh Rosenberg]

Changes by Josh Rosenberg :


--
nosy: +josh.rosenberg

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Antoine Pitrou]

Antoine Pitrou added the comment:

Why exactly does it need to be backported? os.urandom under 2.7 works fine.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Nick Coghlan]

Nick Coghlan added the comment:

It was in the list of security fixes Alex and Donald wanted to reduce
vulnerabilities in 2.x network services, and Guido was OK with backporting
it.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Antoine Pitrou]

Antoine Pitrou added the comment:

> It was in the list of security fixes Alex and Donald wanted to reduce
> vulnerabilities in 2.x network services, and Guido was OK with backporting
> it.

What security issue is there exactly? os.urandom() does a similar thing
in 2.7 and 3.x (it reads from  /dev/urandom).

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Alex Gaynor]

Alex Gaynor added the comment:

It's not a security issue per-se, but if you're doing many small reads, there's 
such an enormous performance and scalability difference that if users run into 
an issue, they're likely to work around it by using a non-CS PRNG, and 
compromising their application security.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Antoine Pitrou]

Antoine Pitrou added the comment:

> It's not a security issue per-se, but if you're doing many small
> reads, there's such an enormous performance and scalability difference
> that if users run into an issue, they're likely to work around it by
> using a non-CS PRNG, and compromising their application security.

Do you have any examples of this or is it just a gratuitous inference?

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Antoine Pitrou]

Antoine Pitrou added the comment:

Note that the 3.4 scheme is not fully debugged yet: issue21207. There is a 
reason we don't backport new features!

Regardless, I'm not interested in this, so I'll let you take the risk of 
regressions if you want to.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
nosy:  -pitrou

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Tshepang Lekhonkhobe]

Changes by Tshepang Lekhonkhobe :


--
nosy: +tshepang

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Charles-François Natali]

Charles-François Natali added the comment:

Like Antoine, I'm really skeptical about the backport: honestly, this change 
doesn't bring much in a normal application. To run into the number of open file 
descriptors limit (so the "scalability" aspect), one would need to have *many* 
concurrent threads reading from /dev/urandom. For the "performance" aspect, I 
have a hard time believing that the overhead of the extra open() + close() 
syscalls is significant in a realistic workload. If reading from /dev/urandom 
becomes a bottleneck, this means that you're depleting your entropy pool 
anyway, so you're in for some potential trouble...

> There is a reason we don't backport new features!

Couldn't agree more. This whole "let's backport security enhancements" sounds 
scary to me.

--
nosy: +neologix

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Nick Coghlan]

Nick Coghlan added the comment:

Yep, it's scary indeed, but such a long lived feature release is a novel
situation that may require some adjustments to our risk management.

However, we can still decide to defer some of the changes until 2.7.8, even
though the notion of backporting them has been approved in principle.

For 2.7.7, we should probably focus on the more essential SSL enhancements.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Donald Stufft]

Donald Stufft added the comment:

"Depleting" /dev/urandom isn't actually a thing. /dev/urandom on all modern 
*nix OSs uses a fast PRNG which is secure as long as it has received enough 
bytes of initial entropy.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Charles-François Natali]

Charles-François Natali added the comment:

> "Depleting" /dev/urandom isn't actually a thing. /dev/urandom on all modern 
> *nix OSs uses a fast PRNG which is secure as long as it has received enough 
> bytes of initial entropy.

I didn't say "deplete /dev/urandom", I said that when reading from
/dev/urandom "you're depleting your entropy pool". So reading from
/dev/urandom won't block, but it can starve processes that read from
/dev/random, and that's a problem.

See 
https://groups.google.com/forum/#!msg/fa.linux.kernel/Ocl01d8TzT0/KDCon2ZUm1AJ

I think since 2.6 Linux uses two different entropy pools for
/dev/random and /dev/urandom, but that might not be true for every OS.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Donald Stufft]

Donald Stufft added the comment:

I don't think what you're worrying about here is something that has a high 
chance of happening, if it even occurs in the wild at all. To be clear in order 
for that to matter at all in the context of this ticket, some software would 
need to be reading from /dev/random (which almost zero software should be 
doing) on a system where you have a high number of threads or async handlers 
all reading from /dev/urandom at the same time and reading enough data off of 
it to drop the entropy estimation in the primary pool down below whatever 
amount of data that the other process is attempting to read from /dev/random. 
In that case no "trouble" will occur and the process reading from /dev/random 
will just block waiting on additional entropy to be collected so that the 
entropy estimation is high enough to fulfill the request.

AFAIK there are zero practical concerns from reading as much as you want off of 
/dev/urandom as often as you want.

What this change does is make it "safe" to just simply use os.urandom in order 
to generate random bytes in a Python application. The current situation is such 
that any use of os.urandom is potentially a place where your application will 
crash in highly concurrent environments. This will drive people to either:

A) Poorly reimplement the persistent FD option, especially troublesome on 
Windows because the simple approach to doing so will flat out not work on 
Windows
B) Use a userspace CSPRNG, this is considered ill advised by most reputable 
cryptographer's as most of them have had issues at one point in time or 
another, and they all generally depend on the security of /dev/urandom anyways 
so if /dev/urandom fall they fall as well.

Using os.urandom is the *right* thing to do for getting random in an 
application, but the current implementation effectively punishes people who use 
it if their application is highly concurrent.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Charles-François Natali]

Charles-François Natali added the comment:

> Using os.urandom is the *right* thing to do for getting random in an 
> application, but the current implementation effectively punishes people who 
> use it if their application is highly concurrent.

And I argue that this scenario is almost as likely as the one you
depict above: we never had a bug report before, and if you have a look
at the the bug report which led to the change in question, it's not
clear at all that all threads were indeed reading from /dev/urandom
when EMFILE was raised. Since reading from /dev/urandom shouldn't
block, it's not clear at all how a realistic workload would actually
hit the file descriptor limit because RLIMIT_NOFILE threads are
reading from /dev/urandom.

But don't get me wrong, I'm not saying this change is useless, it
actually makes sense to use a persistent FD. But backporting always
has a risk, which has to be balanced.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Donald Stufft]

Donald Stufft added the comment:

> But backporting always has a risk, which has to be balanced.

Sure, which is why a PEP was written, discussed and accepted to find that 
balance.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[STINNER Victor]

STINNER Victor added the comment:

Please don't backport this "feature". We had to wait 20 years before someone 
requested the feature, but only a few months before the first user reported an 
issue ("regression"?).

IMO it would be much better to use explicitly a random.SystemRandom instance 
which would keep the fd open, and only use it if you hit the bug (*many* 
threads calling os.urandom in parallel.

--
nosy: +haypo

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Donald Stufft]

Donald Stufft added the comment:

Well except random.SystemRandom doesn't keep the file open (At least in 2.7) 
and actually it just calls os.urandom under the covers, also it doesn't make it 
very nice to get a glob of random bytes.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Donald Stufft]

Donald Stufft added the comment:

Just verified that 3.x also does not exhibit this behavior with 
random.SystemRandom (except implicitly through os.urandom doing it).

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[STINNER Victor]

STINNER Victor added the comment:

Le 29 avr. 2014 00:22, "Donald Stufft"  a écrit :
> Well except random.SystemRandom doesn't keep the file open (At least in
2.7) and actually it just calls os.urandom under the covers, also it
doesn't make it very nice to get a glob of random bytes.

Yes, I'm proposing to enhance this class.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Charles-François Natali]

Charles-François Natali added the comment:

> Yes, I'm proposing to enhance this class.

The problem is AFAICT there's currently no way to get a file
descriptor to the underlying /dev/urandom (and I don't know how it
works on Windows).

Also, this would duplicate the work which has already been done.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Donald Stufft]

Donald Stufft added the comment:

One of the reasons the PEP was done the way it was done was it allowed you to 
write 2/3 compatible code without version checks. Enhancing that class won't 
land until 3.5 which is 18+ months away. Further more the os.urandom persistent 
FD's already exists and is a complete superset of functionality that a 
random.SystemRandom random would have.

This conversation would have been a lot more useful when the PEP was being 
discussed on python-dev.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[Nick Coghlan]

Nick Coghlan added the comment:

Note that the discussion of this PEP *did* suffer from the "language summit
effect" where folks that couldn't make it to the summit are missing some of
the context. I believe I included all of the key motivating points in the
PEP itself, but it's still not the same as being there from a "why take the
risk?" perspective.

Of the approved feature backports, this one was the most borderline, and as
noted above, I think it could comfortably wait until 2.7.8. The SSL changes
are most significant, followed by the constant time comparison function,
and only then the other updates.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[STINNER Victor]

STINNER Victor added the comment:

> The problem is AFAICT there's currently no way to get a file
> descriptor to the underlying /dev/urandom (and I don't know how it
> works on Windows).

We can reimplement os.urandom in SystemRandom on UNIX to keep the file (fd)
open. The code is very simple, basically it's just a call to file.read(n).

Adding a randbytes() method in Python 3.5 would be nice.

The io module can handle boring things for you, like calling read in a loop
until you get enough bytes and handle InterruptError.

Except if you would prefer to use os.read or FileIO.read to avoid readahead.

--

___
Python tracker 

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

[issue21305] PEP 466: update os.urandom

[STINNER Victor]

STINNER Victor added the comment:

> (and I don't know how it
> works on Windows).

On Windows, the OS CryptoAPI is used and a handle is kept open between
calls to os.urandom. On Windows, I don't think that it's a an issue to keep
a handle open. Handle are not sequential numbers and users don't manipulate
them directly.

--

___
Python tracker 

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