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 OpenSSL 3.0 #5

Merged
merged 5 commits into from
Jun 5, 2022

Conversation

ClearlyClaire
Copy link
Contributor

Fixes #4

ECDSA::SigningKey and RSA::SigningKey are changed to wrap the OpenSSL PKey classes instead of inheriting them, because there seem to be no way with version 3.0.0 of the openssl gem to generate such PKeys without using a .generate factory method.

Fixes cedarcode#4

`ECDSA::SigningKey` and `RSA::SigningKey` are changed to wrap the OpenSSL PKey
classes instead of inheriting them, because there seem to be no way with
version 3.0.0 of the `openssl` gem to generate such PKeys without using a
`.generate` factory method.
Copy link
Member

@brauliomartinezlm brauliomartinezlm left a comment

Choose a reason for hiding this comment

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

So grateful for this contribution (and the other associated PRs)! ❤️

Left a question. I'm not used to this codebase so I'll take me a bit to get up to speed to understand in depth the proposed changes.

Again sorry for the delay, and thank you for the patience

Comment on lines 10 to 22
class SigningKey
extend Forwardable

def_delegators :@pkey, :sign, :verify
def_delegators :@pkey, :public_key, :private_key, :to_pem, :to_der, :public?, :private?, :export, :to_s
def_delegators :@pkey, :public_encrypt, :public_decrypt, :private_encrypt, :private_decrypt
def_delegators :@pkey, :sign_pss, :verify_pss
def_delegators :@pkey, :blinding_off!, :blinding_on!
def_delegators :@pkey, :params, :to_text

def initialize(*args)
@pkey = OpenSSL::PKey::RSA.generate(*args)
end
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I'm confused but wouldn't this allow us to keep inheriting from OpenSSL::PKey::RSA ?

That OpenSSL::PKey::RSA#new would get called as a result of this call and would avoid us having to maintain all these delegations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean, but the code you pointed means that when SigningKey.new(size) is called, OpenSSL::PKey::RSA#new bypasses inheritance by creating a new OpenSSL::PKey::RSA object in OpenSSL::PKey::RSA.generate. I'm not sure we can do without delegating.

A more maintainable way to delegate my be by using the Delegator class, though. Doing that.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for finding a workaround to listing delegations like before. I think this way nicer and maintainable 👍

@brauliomartinezlm brauliomartinezlm merged commit c46c9b9 into cedarcode:master Jun 5, 2022
@brauliomartinezlm
Copy link
Member

@ClearlyClaire just released this under https://rubygems.org/gems/openssl-signature_algorithm/versions/1.2.1

Thank you again for your work on 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.

openssl 3 support
2 participants