-
Notifications
You must be signed in to change notification settings - Fork 909
GODRIVER-2349 Seed all pseudorandom number generators with a crypto-secure random number. #889
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
GODRIVER-2349 Seed all pseudorandom number generators with a crypto-secure random number. #889
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
c6335ff
to
250cfb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just requesting more licensing.
The new test looks great; thank you. I only wonder if we should increase the number of goroutines? Did TestGlobalSource
as is ever fail when we were seeding with time.Now()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benjirewis Responding to your question: Yes, TestGlobalSource
does fail reliably on my workstation with 1,000 goroutines using time.Now().UnixNano()
as a seed value. However, the test is not deterministic because the timing depends on runtime conditions, which may be different on the Evergreen hosts. I can bump the number of goroutines to 10,000 to increase the probability that the test will expose any issues. Edit: Increasing the number of goroutines actually causes issues because of the issue described here, so I'll leave it at 1,000.
740fdd6
to
a89b49a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; thanks for filing GODRIVER-2361, too. This stuff is very interesting 🤔
…ecure random number. (#889)
GODRIVER-2349
On some versions of Go and on some operating systems,
time.Now()
returns a time with lower-than-expected resolution (updates every 500μs to 15ms). The Go driver usestime.Now().UnixNano()
to seed some pseudo-random number generators, including the one for generating session IDs here. Due to that, it's possible, under the right conditions, to start two processes that reproduce the same sequence of session IDs if they are started at almost the same time.To remove that possibility, read an
int64
value from the"crypto/rand".Reader
and use it to seed all pseudo-random number generators in the driver.