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

Add support for SHA-224 for PKCS1 signatures #104

Closed

Conversation

joostrijneveld
Copy link
Contributor

Since version 2.2, PKCS1 also includes SHA-224. Since it's available in hashlib, adding support was straight-forward. I've omitted SHA-512/224 and SHA-512/256 for now, since the fact that they're not in hashlib would make it a bit clunky.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.848% when pulling 3f0dc88 on joostrijneveld:pkcs1-sha224 into 8affa13 on sybrenstuvel:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.848% when pulling 47ca8ab on joostrijneveld:pkcs1-sha224 into 8affa13 on sybrenstuvel:master.

@joostrijneveld
Copy link
Contributor Author

In doc/usage.rst, you mention key size recommendations. What is the source or reason behind these recommendations? I'm unsure how to make a similar recommendation for SHA-224.

@coveralls
Copy link

coveralls commented Oct 31, 2017

Coverage Status

Coverage remained the same at 92.414% when pulling 0d83823 on joostrijneveld:pkcs1-sha224 into fa9b787 on sybrenstuvel:master.

@sybrenstuvel
Copy link
Owner

That is a very good question... You can see that I wrote that before I got my Ph.D., nowadays I would certainly have added a reference ;-)

Looking at that list now, I agree with you that it's hard to make a good recommendation. Searching for "SHA-512 752" shows the Python-RSA documentation as top hit, which means that this is not a widely used recommendation. Let's just drop it, and suggest people to use modern recommendations for the key size from, for example, NIST Special Publication 800-131A. At this time they simply recommend RSA keys of at least 2048 bits.

@joostrijneveld
Copy link
Contributor Author

That sounds very reasonable. I've submitted a different pull request that removes the whole key size discussion.

sybrenstuvel added a commit that referenced this pull request Feb 5, 2018
@sybrenstuvel
Copy link
Owner

Merged in 173be1f, with some additional work in da7145a and 83e273b.

Please include unit testing and updating documentation (like the changelog) in your future pull requests.

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.

3 participants