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

PKey::EC does not follow API #29

Closed
pzb opened this issue Oct 12, 2015 · 10 comments
Closed

PKey::EC does not follow API #29

pzb opened this issue Oct 12, 2015 · 10 comments

Comments

@pzb
Copy link

pzb commented Oct 12, 2015

There are four classes under OpenSSL::PKey: RSA, DSA, DH, and EC. Three of the four share a common API. EC is the outlier. I am routinely monkey patching EC to support the common API. Without this, you cannot use EC keys to sign certs.

The missing methods:
::generate

private?

public?

public_key

params

Almost all of these can be filled via monkey patching, so the fix can be in lib/ or in the C extension.

@pzb
Copy link
Author

pzb commented Oct 21, 2015

To be clear, the #public_key method does exist, it just behaves completely differently from other public key methods. To make it work as expected, you have to do this:

def real_public_key(k)
    point = k.public_key
    pub = OpenSSL::PKey::EC.new(point.group)
    pub.public_key = point
    pub
end

@cognitiveflux
Copy link

is there community agreement on how to resolve this issue? Is anyone working on a fix?

@rhenium
Copy link
Member

rhenium commented Jul 4, 2016

1ed0cf0 added PKey::EC.generate, and 5588865 added PKey::EC#private? and #public?.

It's too late to change the behavior of PKey::EC#public_key, so I first though of adding a new method #public_pkey to every PKey classes, but I started wondering. I can't come up with any situations where PKey::PKey#public_key is useful, except when exporting to a PEM or DER string.

If it is the only use case, isn't it better to add an option to PKey::PKey#to_der and #export, like this?

ec_key.generate_key!
ec_key.to_der #=> PKCS#8 PrivateKeyInfo format
ec_key.to_der(format: :PUBKEY) #=> X.509 SubjectPublicKeyInfo format

What do you think?

@pzb
Copy link
Author

pzb commented Jul 4, 2016

Yes, the most common case is calling to_der and to_pem/export. I'm looking through my code to see if I have any other uses, but I think generating a key pair then exporting/saving it is the main use case.

@cognitiveflux
Copy link

OpenSSL::X509::Certificate and OpenSSL::X509::Request require the public_key method to assign the public key to the certificate or request object, which for now requires a workaround as commented by @pzb. Since certificate creation and trust chains are important functions of the library, I'd say this is a main use case.

@pzb
Copy link
Author

pzb commented Jul 6, 2016

@cognitiveflux yes, if try to set the public key on a Certificate object to the result of EC#public_key, you get the following error:

test_ecdsa_sig.rb:44:in `public_key=': wrong argument (OpenSSL::PKey::EC::Point)! (Expected kind of OpenSSL::PKey::PKey) (TypeError)
        from test_ecdsa_sig.rb:44:in `<main>'

However if you simply set the public key to the OpenSSL::PKey::EC object that contains the private key, it will do the right thing and only include the public key in the certificate.

@cognitiveflux
Copy link

Thanks for that clarification! Which release should include 5588865?

antaflos added a commit to antaflos/puppetlabs-java_ks that referenced this issue Dec 21, 2016
Add a new parameter "private_key_type" which defaults to "rsa" but can
also be set to "ec" for ECDSA keys.

Contains updated README and some spec tests for the type. Due to various
bugs in ruby-openssl it is currently difficult to properly test EC keys
(https://bugs.ruby-lang.org/issues/12348, ruby/openssl#29).
@whitehat101
Copy link

whitehat101 commented Dec 28, 2017

For making Certs and CSRs you need to supply an OpenSSL::PKey's public key.

rsa = OpenSSL::PKey::RSA.generate 2048
dsa = OpenSSL::PKey::DSA.generate 2048
ec  = OpenSSL::PKey::EC.generate "prime256v1"

cert = OpenSSL::X509::Certificate.new
cert.public_key = rsa.public_key
cert.public_key = dsa.public_key
cert.public_key = ec.public_key # in `public_key=': wrong argument (OpenSSL::PKey::EC::Point)! (Expected kind of OpenSSL::PKey::PKey) (TypeError)

req = OpenSSL::X509::Request.new
req.public_key = rsa.public_key
req.public_key = dsa.public_key
req.public_key = ec.public_key # in `public_key=': wrong argument (OpenSSL::PKey::EC::Point)! (Expected kind of OpenSSL::PKey::PKey) (TypeError)

Using a PKey::EC requires special preparation. If it's too late to change PKey::EC#public_key, we could change OpenSSL::X509::Certificate and OpenSSL::X509::Request to accept what EC#public_key produces. With the below patch, the above completes without error.

module AcceptECPublicKey
  def public_key= value
    if OpenSSL::PKey::EC::Point === value
      key = OpenSSL::PKey::EC.new value.group
      key.public_key = value
      value = key
    end
    super value
  end
end
OpenSSL::X509::Certificate.prepend AcceptECPublicKey
OpenSSL::X509::Request.prepend AcceptECPublicKey

We can also teach EC::Point to to_pem and to_der.

module QuackLikeAPKey
  def to_pem; public_key.to_pem end
  def to_der; public_key.to_der end
private
  def public_key
    key = OpenSSL::PKey::EC.new group
    key.public_key = self
    key
  end
end
OpenSSL::PKey::EC::Point.prepend QuackLikeAPKey

With these patches PKey::EC#public_key should behave like the rest of the PKey classes for key export and certificate request/creation, even though it produces a different class.

@rhenium
Copy link
Member

rhenium commented Dec 28, 2017

I forgot to close this issue. PKey::EC.generate, PKey::EC#private?, and #public? are part of our v2.0.0 release.

@whitehat101

For making Certs and CSRs you need to supply an OpenSSL::PKey's public key.

You can pass private key as-is as pointed out by @pzb.

@marek22k
Copy link

marek22k commented Aug 8, 2022

I still have this issue, when I try to set a certs public key:

`public_key=': wrong argument type OpenSSL/EC_POINT (expected OpenSSL/EVP_PKEY) (TypeError)
	from generate_cert.rb:39:in `<main>'

mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 26, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 27, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 27, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 27, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 27, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 27, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 27, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
mhashizume added a commit to mhashizume/puppet that referenced this issue Jun 27, 2023
Previously, the build_cert method in Puppet::TestCa needed special logic
for the Eliptical Curve class: ruby/openssl#29

This issue was resolved in OpenSSL 2.0, which was released in 2016,
meaning that all versions of Ruby that Puppet supports includes
Ruby/OpenSSL >= 2.0.

This commit removes special logic for EC in Puppet::TestCa.
weaselp pushed a commit to weaselp/bzed-dehydrated that referenced this issue May 26, 2024
ECC keys are weird in ruby, cf. ruby/openssl#29
bzed pushed a commit to bzed/bzed-dehydrated that referenced this issue May 26, 2024
ECC keys are weird in ruby, cf. ruby/openssl#29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants