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

Fix OpenSSL::PKey::RSA#private?, export, to_der when private exp not set #258

Merged
merged 2 commits into from
Jun 20, 2019

Conversation

thekuwayama
Copy link
Contributor

Hi!

This PR is related https://bugs.ruby-lang.org/issues/15841 .

When key is not set private exponent(=d),

  • OpenSSL::PKey::RSA#private? returns true. It should return false.
  • OpenSSL::PKey::RSA#export returns PEM string with RSA PRIVATE KEY header. It should return public key PEM.
  • OpenSSL::PKey::RSA#to_der returns neither public key format nor private key format string. It should return either.
  • OpenSSL::PKey::RSA#{en,de}crypt raises segfaults. It is fixed by Fix segfaults in OpenSSL::PKey::RSA#private_{en,de}crypt when private exp not set #255 .

For example:

irb(main):001:0> require 'openssl'
=> true
irb(main):002:0> rsa512 = OpenSSL::PKey::RSA.new(512, 3)
=> #<OpenSSL::PKey::RSA:0x00007fe944914f08>
irb(main):003:0> key = OpenSSL::PKey::RSA.new
=> #<OpenSSL::PKey::RSA:0x00007fe94492e1b0>
irb(main):004:0> key.set_factors(rsa512.p, rsa512.q)
=> #<OpenSSL::PKey::RSA:0x00007fe94492e1b0>
irb(main):005:0> key.private?
=> true
irb(main):006:0> key.export
=> "-----BEGIN RSA PRIVATE KEY-----\nMEkCAQACIQD0Zqdrl+2NjdzOCtRQEiehhZfhzC9E6joc3wPDiApcLQIhAMc74OEW\nyFbhiULEBNpeIe+4oxq3bLnTncFu5C6UQl+F\n-----END RSA PRIVATE KEY-----\n"
irb(main):007:0> OpenSSL::PKey::RSA.new(key.to_der)
Traceback (most recent call last):
        6: from /Users/thekuwayama/.rbenv/versions/2.6.3/bin/irb:23:in `<main>'
        5: from /Users/thekuwayama/.rbenv/versions/2.6.3/bin/irb:23:in `load'
        4: from /Users/thekuwayama/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        3: from (irb):8
        2: from (irb):8:in `new'
        1: from (irb):8:in `initialize'
OpenSSL::PKey::RSAError (Neither PUB key nor PRIV key: sequence length mismatch)

This PR fixes RSA_HAS_PRIVATE to check that both e and d are not NULL and RSA#export and RSA#to_der to check key, factors and crt_params.

RSA_get0_key(rsa, &n, &e, &d);
RSA_get0_factors(rsa, &p, &q);
RSA_get0_crt_params(rsa, &dmp1, &dmq1, &iqmp);
if (n && e && d && p && q && dmp1 && dmq1 && iqmp) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

followed https://tools.ietf.org/html/rfc8017#appendix-A.1.2

   An RSA private key should be represented with the ASN.1 type
   RSAPrivateKey:

         RSAPrivateKey ::= SEQUENCE {
             version           Version,
             modulus           INTEGER,  -- n
             publicExponent    INTEGER,  -- e
             privateExponent   INTEGER,  -- d
             prime1            INTEGER,  -- p
             prime2            INTEGER,  -- q
             exponent1         INTEGER,  -- d mod (p-1)
             exponent2         INTEGER,  -- d mod (q-1)
             coefficient       INTEGER,  -- (inverse of q) mod p
             otherPrimeInfos   OtherPrimeInfos OPTIONAL
         }

@ioquatix ioquatix merged commit acd4e08 into ruby:master Jun 20, 2019
@ioquatix
Copy link
Member

LGTM.

@thekuwayama
Copy link
Contributor Author

Thanks!

@thekuwayama thekuwayama deleted the fix-rsa-private branch June 20, 2019 12:44
rhenium added a commit to rhenium/ruby-openssl that referenced this pull request Feb 15, 2020
… private exp not set"

This reverts commit e30b9a2 (ruby#255)
except the added test code.

The 'd' value can be NULL when the RSA private key is backed by an
OpenSSL engine, such as an HSM. In that case, only 'n' and 'e' are
visible from the OpenSSL API.

The original issue has been fixed by Pull Request ruby#258 in another way.

Reference: ruby#255
Reference: ruby#258
ioquatix pushed a commit that referenced this pull request Feb 16, 2020
… private exp not set"

This reverts commit e30b9a2 (#255)
except the added test code.

The 'd' value can be NULL when the RSA private key is backed by an
OpenSSL engine, such as an HSM. In that case, only 'n' and 'e' are
visible from the OpenSSL API.

The original issue has been fixed by Pull Request #258 in another way.

Reference: #255
Reference: #258
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.

2 participants