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

proposal: math/rand/v2: add Rand.Read #65562

Closed
lukechampine opened this issue Feb 7, 2024 · 3 comments
Closed

proposal: math/rand/v2: add Rand.Read #65562

lukechampine opened this issue Feb 7, 2024 · 3 comments
Labels
Milestone

Comments

@lukechampine
Copy link
Contributor

Proposal Details

In #61716, Russ said:

If we do keep ChaCha8 as the global generator and commit to having some cryptographic global generator like ChaCha8 in the future, we could potentially bring back both the top-level Read function and the Rand.Read method.

In the earlier discussion, some people asked what to do about getting short byte sequences from the PRNG. We essentially sacrificed the convenience of Read for the security of forcing people over to crypto/rand. But if we make the top-level Read backed by a cryptographic generator, we could bring Read back and have both convenience and security.

I would like to formally propose this. To put it bluntly, I do not believe there is a meaningful security difference between reading from crypto/rand and reading from a ChaCha8 CSPRNG seeded from crypto/rand. But others may disagree, and if adding a top-level Read causes tools like goimports to import math/v2/rand instead of crypto/rand, they could rightfully complain. Thus I propose adding Rand.Read, but not a top-level Read.

@gopherbot gopherbot added this to the Proposal milestone Feb 7, 2024
@zephyrtronium
Copy link
Contributor

I do not believe there is a meaningful security difference between reading from crypto/rand and reading from a ChaCha8 CSPRNG seeded from crypto/rand. But others may disagree, and if adding a top-level Read causes tools like goimports to import math/v2/rand instead of crypto/rand, they could rightfully complain.

Not really about the proposal per se, but another reason not to add package-level Read is that nothing actually constrains those functions to use a CSPRNG. The fact that they use ChaCha8 is an implementation detail, and other Go implementations could make other choices. (E.g., TinyGo still uses xorshift in some cases.) We could promise to use a CSPRNG for package-level Read, but that's already what crypto/rand does.

@rsc
Copy link
Contributor

rsc commented May 8, 2024

Will close this as a duplicate of #67059.

@rsc rsc changed the title proposal: math/rand/v2: Add Rand.Read proposal: math/rand/v2: add Rand.Read May 8, 2024
@rsc
Copy link
Contributor

rsc commented May 8, 2024

This proposal is a duplicate of a previously discussed proposal, as noted above,
and there is no significant new information to justify reopening the discussion.
The issue has therefore been declined as a duplicate.
— rsc for the proposal review group

@rsc rsc closed this as completed May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

No branches or pull requests

4 participants