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

support prehashing in RSA sign #3238

Merged
merged 3 commits into from
Nov 20, 2016
Merged

support prehashing in RSA sign #3238

merged 3 commits into from
Nov 20, 2016

Conversation

reaperhulk
Copy link
Member

Support supplying previously hashed data to the RSAPrivateKey.sign method. Support for RSA verification and sign/verify for DSA/ECDSA will come in future PRs.

This addresses part of #1648

@mention-bot
Copy link

@reaperhulk, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alex, @cmurphy and @gorisaka to be potential reviewers.

@alex
Copy link
Member

alex commented Nov 13, 2016

It feels like there was a substantial refactoring here, any chance of splitting that out?

@alex
Copy link
Member

alex commented Nov 19, 2016

Ready for a rebase now.

@alex
Copy link
Member

alex commented Nov 19, 2016

Needs a test for what happens if len(data) != algorith.digest_size.

@@ -167,6 +167,25 @@ There is a shortcut to sign sufficiently short messages directly:
... hashes.SHA256()
... )


If you have pre-hashed your data you can sign using that as well:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this shouldn't be a part of the primary narrative, since it's relatively obscure. What do you think?

@alex
Copy link
Member

alex commented Nov 20, 2016

Needs a changelog entry

@alex alex merged commit 033bd71 into pyca:master Nov 20, 2016
@reaperhulk reaperhulk deleted the prehash branch November 20, 2016 13:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants