Skip to content

feat: create an EC::PKey from an EC::Point #916

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mcr
Copy link
Contributor

@mcr mcr commented Jul 26, 2025

It is implied that OpenSSL::PKey::EC.new can take a public key/EC::Point to form a key, but I could not make it work.
Since 3.x wants keys immutable, they have to be initialized at creation, and this allows for an EC::Point to be provided.
RSA keys do not have the same division between PKey and Point.

Copy link
Member

@rhenium rhenium left a comment

Choose a reason for hiding this comment

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

Please add a test case.

@@ -125,7 +159,7 @@ ossl_ec_key_s_generate(VALUE klass, VALUE arg)
/*
* call-seq:
* OpenSSL::PKey::EC.new
* OpenSSL::PKey::EC.new(ec_key)
* OpenSSL::PKey::EC.new(ec_key) # ec_key is PKey::EC, or PKey::EC::Point.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* OpenSSL::PKey::EC.new(ec_key) # ec_key is PKey::EC, or PKey::EC::Point.
* OpenSSL::PKey::EC.new(ec_key)
* OpenSSL::PKey::EC.new(ec_point)

{
EC_KEY *ec = NULL;

if (rb_obj_is_kind_of(arg, cEC_POINT)) {
Copy link
Member

Choose a reason for hiding this comment

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

This if seems redundant.

ossl_raise(eECError, NULL);
}

GetECPoint(arg, point);
Copy link
Member

Choose a reason for hiding this comment

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

GetECPoint() and GetECPointGroup() should be called before ec is allocated. They can potentially raise an exception, and in that case ec could leak.

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