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

Ensure that the generated RSA key has the desired length #1

Merged
merged 1 commit into from
Oct 26, 2014

Conversation

phillipp
Copy link
Contributor

Currently it is possible that RSAGenerate generates RSA keys with a public key of length B-1 instead of B (for example 1023 bit instead of 1024 bit). For use cases that require specific bit lengths (for example Certificate Authorities), that will not work. The patch adds a check to the bit length of the public key (n). If the length is less than B (desired bit length), p and q are chosen again.

There was another option, where the two most significant bits of both p and q would be tested (independently) and re-generated when both were not significant.

I profiled both methods and found out that the method in the commit has less overhead. I profiled all three methods with 100 keys each.

Original               22s
Method from patch      30s (+36%)
Significant-bit-method 36s (+63%)

Still, this commit increases the time to generate keys in some cases significantly. In ~33% of the key generations the run-time nearly doubled.
But it is important to notice the original library didn't generate correct keys in that cases. So actually, this fixes a serious bug in the original library that led to the generation of faulty keys in 33% of the cases but was faster.

Before, it was possible that RSAGenerate generated a RSA key with a
public key of length B-1. For use cases that require specific bit
lengths (for example Certificate Authorities), that will not work.
The patch adds a check to the public key (n) bit length. If the length
is less than B (desired bit length), p and q are chosen again.
zoloft added a commit that referenced this pull request Oct 26, 2014
Ensure that the generated RSA key has the desired length
@zoloft zoloft merged commit a2d8d9b into zoloft:master Oct 26, 2014
@zoloft
Copy link
Owner

zoloft commented Oct 26, 2014

Thank you very much @phillipp, sorry if I accept this only now but I just got back home. As soon as I can start work again on JSEncrypt I will port this to the main repo

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