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

ca doesn't read fully from /dev/urandom #911

Closed
jmhodges opened this issue Oct 4, 2015 · 1 comment
Closed

ca doesn't read fully from /dev/urandom #911

jmhodges opened this issue Oct 4, 2015 · 1 comment
Milestone
General Availability

Comments

@jmhodges
Copy link
Contributor

jmhodges commented Oct 4, 2015

The serial numbers from the ca are sometimes full of trailing zeros causing almost duplicate serial numbers (duplicates error out when attempted to be inserted out in the database, so no exploit is available) because rand/Reader.Read was used instead of rand.Read.

This ought to be a GA fix for our sanity, but is not actively exploitable.
@jmhodges
Copy link
Contributor Author

jmhodges commented Oct 4, 2015

cc @bdaehlie

@jmhodges jmhodges mentioned this issue Oct 4, 2015
Merged
jmhodges added a commit that referenced this issue Oct 4, 2015
@jmhodges
read fully from urandom in the ca
b5e989e 
This didn't cause any certificates to be made with duplicate serial
numbers because the primary key in the certificates table is the serial
number and so attempts to insert duplicates fail.

The cause is the use of rand/Reader.Read. rand/Reader.Read does not
perform the io.ReadFull that rand.Read does. Without the io.ReadFull,
less than all of the random part of the serial number would be filled
in.

Fixes #911.
@jsha jsha closed this as completed in #912 Oct 4, 2015
@bdaehlie bdaehlie added this to the General Availability milestone Oct 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
General Availability
Development

No branches or pull requests
2 participants
@jmhodges @bdaehlie