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.

Sign up for GitHub

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

Jump to bottom

Crypto Rand #215

Closed
s2t2 opened this issue Feb 27, 2018 · 2 comments
Closed

Crypto Rand #215

s2t2 opened this issue Feb 27, 2018 · 2 comments
Assignees
@adamo57

Comments

@s2t2
Copy link
Contributor

s2t2 commented Feb 27, 2018

I read at https://github.com/golang/go/wiki/CodeReviewComments#crypto-rand that crypto/rand should be used instead of math/rand for security reasons.

I notice that gothicRand uses math/rand and I'm wondering how secure this is during nonce generation, or if it needs to be hardened to use crypto/rand instead.
@adamo57
Copy link
Collaborator

adamo57 commented Feb 28, 2018

hmm, good catch. We should totally change that.

@adamo57 adamo57 self-assigned this Feb 28, 2018
@s2t2 s2t2 mentioned this issue Feb 28, 2018
Closed
bentranter added a commit to bentranter/goth that referenced this issue Mar 16, 2018
@bentranter
Use crypto/rand for nonce generation
4cb4f94 
Solves issue:

* markbates#215

Previously, gothic would use math/rand for secure nonce generation.
Since math/rand is not considered secure, it was suggested that we
switch to crypto/rand. This PR does that.

I chose to panic on the error because I believe that not being able to
read from crypto/rand suggests there's something seriously wrong with
your operating system, but I'm curious to see what others think.
@bentranter bentranter mentioned this issue Mar 16, 2018
Merged
@bentranter
Copy link
Collaborator

Fixed in #220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adamo57 adamo57

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@s2t2 @bentranter @adamo57