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

make OpenSSL::PKey::EC::{Group,Point} directly wrap EC_{GROUP,POINT} object #71

Merged
merged 3 commits into from
Sep 7, 2016

Conversation

rhenium
Copy link
Member

@rhenium rhenium commented Sep 7, 2016

No description provided.

Make ossl_pkey_ec.c follow the general convension on macro names. Prefer
CamelCase to Snake_Case and unify Require_*() and Get_*() macros into
Get*() macros. There is nothing wrong with the style itself but it's
hard to read if two different styles are mixed.
Currently an OpenSSL::PKey::EC::Point wraps an ossl_ec_point struct
which has a pointer for EC_POINT. This commit make EC::Point wrap an
EC_POINT directly in order to simplify the source code. There should be
no changes on behavior seen from Ruby.
As done for EC::Point, remove ossl_ec_group struct. This contains a
breaking change. Modifications to an EC::Group returned by EC#group
no longer affects the EC object unless set to the key explicitly using
EC#group=. This is the common behavior in Ruby/OpenSSL, including other
getter methods of EC such as EC#public_key.

EC#group currently returns a EC::Group linked with the key, i.e. the
EC::Group object holds a reference to an EC_GROUP that the EC_KEY owns.
We use some ugly workaround - the ossl_ec_group struct has a flag
'dont_free' that indicates we must not free the EC_GROUP. But it is
still not possible to control OpenSSL of free'ing the EC_GROUP, so,
for example, the following code behaves strangely:

  ec = OpenSSL::PKey::EC.generate("prime256v1")
  group = ec.group
  p group.curve_name #=> "prime256v1"
  ec.group = OpenSSL::PKey::EC::Group.new("prime256v1")
  p group.curve_name #=> nil
@rhenium rhenium merged commit 9435c8b into ruby:master Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant