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 back crypto.Signer support for ECDSA signing keys. #227

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

doryiii
Copy link
Contributor

@doryiii doryiii commented Sep 5, 2024

Currently only PubKeyAlgoRSA supports crypto.Signer. x/crypto/openpgp originally had support for ECDSA external crypto.Signer. This change adds that back.

This resolves #226 (at least for ECDSA signing keys).

@twiss
Copy link
Member

twiss commented Sep 6, 2024

Thanks! This looks reasonable to me, though the build fails because of a missing import of big :)

@doryiii
Copy link
Contributor Author

doryiii commented Sep 9, 2024

Thanks! This looks reasonable to me, though the build fails because of a missing import of big :)

Whoops, fixed. Also reolved scoping warnings.

openpgp/packet/signature.go Outdated Show resolved Hide resolved
@doryiii
Copy link
Contributor Author

doryiii commented Sep 10, 2024

Is there any documentation on how to run these regression tests locally so I can try and resolve them?

@twiss
Copy link
Member

twiss commented Sep 10, 2024

There was an issue on the main branch, should be fixed by #229. So just rebasing should fix it :) Sorry about that.

Currently only PubKeyAlgoRSA supports crypto.Signer. x/crypto/openpgp
originally had support for ECDSA external crypto.Signer. This change adds
that back.

Signed-off-by: Duy Truong <dory@dory.moe>
Copy link
Member

@twiss twiss left a comment

Choose a reason for hiding this comment

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

Thanks!

@twiss twiss merged commit 2add693 into ProtonMail:main Sep 11, 2024
8 checks passed
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.

Support crypto.Signer for signature generation
2 participants