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

crypto/rand: cap getrandom reads to ~32 MB per call on Linux #20877

Closed
Yawning opened this issue Jul 1, 2017 · 4 comments
Closed

crypto/rand: cap getrandom reads to ~32 MB per call on Linux #20877

Yawning opened this issue Jul 1, 2017 · 4 comments
Labels
FrozenDueToAge NeedsFix release-blocker
Milestone

Comments

@Yawning
Copy link

@Yawning Yawning commented Jul 1, 2017

What version of Go are you using (go version)?

The problem is obvious from inspection of the source code and the documented Linux getrandom semantics, I looked at master.

What operating system and processor architecture are you using (go env)?

This affects all Linux platforms with support for the getrandom() system call.

What did you do?

Looked at the code to see if crypto/rand was correct or not.

What did you expect to see?

getrandom() used correctly.

What did you see instead?

From the getrandom documentation:

When reading from the urandom source, a maximum of 33554431 bytes is returned by a single call to getrandom() on systems where int has a size of 32 bits.

From the system call implementation in drivers/char/random.c:

	if (count > INT_MAX)
		count = INT_MAX;

src/crypto/rand/rand_linux.go:getRandomLinux() will return false due to the truncated getrandom() output, and the caller will fall back to servicing the request by opening and reading from /dev/urandom.

While I would be inclined to agree that a Read that is 32 MiB - 1 bytes or larger is excessive and out of the ordinary, this still should be handled correctly or documented.

@bradfitz bradfitz changed the title crypto/rand reads excessive amounts of entropy for very large requests. crypto/rand: cap getrandom reads to ~32 MB per call on Linux Jul 1, 2017
@bradfitz bradfitz added the NeedsFix label Jul 1, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jul 1, 2017
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 1, 2017

Thanks for the report.

@mmcloughlin
Copy link
Contributor

@mmcloughlin mmcloughlin commented Jul 15, 2017

I just attended the Gophercon Contributor Workshop and thought this might be a manageable fix to take on.

I was looking around drivers/char/random.c as @Yawning pointed out, but I don't think the offending line is if (count > INT_MAX) ... because INT_MAX is probably 2^31-1. That line is here:

https://github.com/torvalds/linux/blob/486088bc4689f826b80aa317b45ac9e42e8b25ee/drivers/char/random.c#L1916

The syscall definition delegates to urandom_read which contains the line

nbytes = min_t(size_t, nbytes, INT_MAX >> (ENTROPY_SHIFT + 3));

https://github.com/torvalds/linux/blob/486088bc4689f826b80aa317b45ac9e42e8b25ee/drivers/char/random.c#L1782

And ENTROPY_SHIFT is 3 (defined near the top of the file). That explains why the limit is 2^25-1 rather than 2^31-1.

@mmcloughlin
Copy link
Contributor

@mmcloughlin mmcloughlin commented Jul 16, 2017

I have been working on a patch. I'll put it out tomorrow for review.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 17, 2017

CL https://golang.org/cl/49170 mentions this issue.

@golang golang locked and limited conversation to collaborators Aug 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix release-blocker
Projects
None yet
Development

No branches or pull requests

4 participants