crypto/rand: Panic fetching random boundary string

[gopherbot]

by birkirb@stoicviking.net:

What happened?

On creating a random boundary the following panic occurred:

panic: read /dev/urandom: resource temporarily unavailable

Which unfortunately crashed my program (since it was running in a go-routine off main
thread which had panic recovery).

Is having a panic recovery for all go-routines the Right Thing to do?

What should have happened instead?

Error in this rate case would have been nice. Not sure what causes /dev/urandom to be
unavailable.

[ianlancetaylor]

Comment 1:

What system did this happen on?
I don't understand why /dev/urandom would be unavailable.  But I also don't understand
why mime/multipart needs secure random strings.  As far as I can see mime/multipart
could reasonably use math/random instead.
Brad, you switched from math/rand to crypto/rand in
http://golang.org/cl/4532089 .  Any special reason?
It's also possible that this indicates a bug in crypto/rand.

Labels changed: added repo-main, release-go1.5.

[bradfitz]

Comment 2:

Russ told me to.
https://golang.org/cl/4530054/diff/19001/src/pkg/mime/multipart/writer.go#newcode48
"I know it seems like overkill but I think you'd be better
off importing and using crypto/rand here.  Package rand is
not great if your goal is to avoid collisions.  For example
since the generator is not typically seeded this will use
the same boundary for the first message each time it gets run.  Even if it is
seeded, you're only getting 64 bits of
randomness out of this, and you're trying to pull out 
quite a bit more."
And I still agree with that.
I also have no clue why /dev/urandom would be temporarily unavailable. 
If there was a well-seeded random mechanism available, I would use it, but I don't
believe there is.

[gopherbot]

Comment 3 by birkirb@stoicviking.net:

System was:
$ uname -a
Linux ####### 3.14.19-17.43.amzn1.x86_64 #1 SMP Wed Sep 17 22:14:52 UTC 2014 x86_64
x86_64 x86_64 GNU/Linux

[ianlancetaylor]

Comment 4:

Which version of Go did you use to build your program?

[ianlancetaylor]

Comment 5:

I don't see any way that the Linux kernel could return EAGAIN for a read of /dev/urandom
when it is in blocking mode.  I don't see any Go code that could set the file descriptor
to nonblocking mode.
Is this problem repeatable?  If so, please run it under strace -f and attach the results.

[gopherbot]

Comment 6 by birkirb@stoicviking.net:

Sorry, I thought I had pasted a complete path in the subject.
It was here:
https://code.google.com/p/go/source/browse/src/pkg/mime/multipart/writer.go?name=release-branch.go1.3#75
So compiled in go 1.3
It happened twice in two separate go-routines, that both were uploading files. Of of
those go routines was using goamz which caught the panic and returned an error, but the
other didn't which crashed my server :(
It's definitely not common, I've run the same code for over two weeks without this
issue, and probably not easily repeatable since I'm a bit clueless as to why
/dev/urandom would be unavailable.  I had recently restarted my application which uses
goagain for seemless restarts.
Would having a fallback to math/rand make sense there? Wasn't really expecting a panic
in this package.

[bradfitz]

Comment 7:

Could it be the lazy os.Open that returned EAGAIN? That would also be weird.
In Go 1.4 with a modern kernel we use the getrandom system call so no fd opens/reads are
involved.

[minux]

Comment 8:

Could it be the case that the VM has just started and there is not enough entropy
in the random pool so even /dev/urandom returns EAGAIN?
(the kernel shouldn't do this, but on VMs, this has been known to create security
issues, e.g. when generating ssh hostkeys, so perhaps the kernel contain a patch)
Also, according to this post,
https://groups.google.com/forum/#!topic/cryptopp-users/mEMkHo3Gafk
the poster asserted that it's possible for read from urandom to return EAGAIN and
EINTR.

[ianlancetaylor]

Comment 9:

OK, I guess crypto/rand should handle a read from /dev/urandom returning EAGAIN (note
that we should not see EINTR as all signals should have SA_RESTART set).

Status changed to Accepted.