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

Increase # of bits for random serial numbers of certificates with PyOpenSSL backend #90

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Fixes part of #76. Instead of using a random number betwen 1000 and 99999 (both inclusive), selects a 160 bit random number between 2^160 and 2^161 - 1 (both inclusive). No special random input source is used, so the quality is not very high. I think that's ok for serial numbers of certificates (and already a lot better than what we had before).

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

openssl_certificate

@felixfontein
Copy link
Contributor Author

ready_for_review

@felixfontein felixfontein mentioned this pull request Aug 9, 2020
@MarkusTeufelberger
Copy link
Contributor

LGTM, though I wonder why one would set the lower limit at 2^160 instead of 0 for serial numbers? Seems to cut the whole possible range in half...

@felixfontein
Copy link
Contributor Author

It makes sure that the bitlength is always 161 bits ;-) It's more to make it similar to the old code (which never went below 1000).

I don't mind removing that part. The chance that this is actually 0 is pretty low anyway.

@felixfontein
Copy link
Contributor Author

(Also always setting bit 160 is less bad as always setting bit 0 IMO, which is another common choice to make sure the serial cannot be 0 ;) )

@felixfontein
Copy link
Contributor Author

I've adjusted the algorithm to return a <= 160bit number between 1000 and 2^160-1.

@felixfontein felixfontein merged commit 430c6d0 into ansible-collections:main Aug 18, 2020
@felixfontein felixfontein deleted the improve-serial-number-generation branch August 18, 2020 14:34
@felixfontein
Copy link
Contributor Author

@MarkusTeufelberger thanks for reviewing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants