Bug #4421: [ext/openssl] Fix RSA public key encoding

• Status: Closed
• Assignee: MartinBosslet (Martin Bosslet)
• Target version: 1.9.3
• ruby -v: -

[MartinBosslet (Martin Bosslet)]

=begin
When calling RSA#to_der and RSA#to_pem on RSA public keys, they currently
get encoded using i2d_RSAPublicKey and PEM_write_bio_RSAPublicKey. This encoding
was specified in PKCS#1 and is specific to RSA. It is also not the default
encoding used by OpenSSL itself, which rather uses the generic format generated
by i2d_RSA_PUBKEY and PEM_write_bio_RSA_PUBKEY. This format is the same that is
used in a certificate's SubjectPublicKeyInfo, the advantage being that the format
is generic and can be used to represent public keys of all kinds, including RSA,
DSA and Elliptic Curve.

The attached patch will make use of the generic format for encoding RSA keys. The
change should not cause compatibility problems, since RSA#initialize uses several
fallback scenarios that cover public keys of both formats.

The fallbacks are also re-prioritized according to these changes.

Regards,
Martin
=end

[naruse (Yui NARUSE)]

• Status changed from Open to Assigned
• Assignee set to nahi (Hiroshi Nakamura)

=begin

=end

[naruse (Yui NARUSE)]

=begin
Hi, Martin,

We don't have enough resource and knowledge about ext/openssl. So do you need a commit bit?

If you want, please express it; and after the approval of matz, you can commit to our repo.
(of course, before a commit you need to reach a consensus)

See also [[ruby:DeveloperHowTo]] and [[ruby:CommitterHowTo]].
=end

[MartinBosslet (Martin Bosslet)]

=begin
Hi Yui,

Yes, I'd love to contribute and I would really appreciate it!
=end

[MartinBosslet (Martin Bosslet)]

Hi all,

now that I have SVN access - would it be fine if I assigned the issues that I reported and that are still open to myself?
What about those already assigned?

Regards,
Martin

[naruse (Yui NARUSE)]

Welcome to committers!
Yeah you can change the assignee and commit the patch.

[MartinBosslet (Martin Bosslet)]

• Assignee changed from nahi (Hiroshi Nakamura) to MartinBosslet (Martin Bosslet)

Great :) Thanks for all the help so far!

[tenderlovemaking (Aaron Patterson)]

• ruby -v changed from ruby 1.9.2p180 (2011-02-18 revision 30909) [i686-linux] to -

On Tue, May 10, 2011 at 07:37:13AM +0900, Martin Bosslet wrote:

Issue #4421 has been updated by Martin Bosslet.

Hi all,

now that I have SVN access - would it be fine if I assigned the issues that I reported and that are still open to myself?

Yay! Congrats Martin!

--
Aaron Patterson
http://tenderlovemaking.com/

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r31520.
Martin, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

Thu May 12 07:27:31 2011 Martin Bosslet [e-mail]

* ext/openssl/ossl_pkey_rsa.c: Use generic X.509 SubjectPublicKeyInfo
  format for encoding RSA public keys. 
  [ruby-core:35327] [Bug #4421]

Previous revision: 31507

M ChangeLog
M ext/openssl/ossl_pkey_rsa.c

[nahi (Hiroshi Nakamura)]

Martin: Congrats! Go ahead.

Some comments;

• Please add a test for each commit to express the intent of the change. It must help us in the future.
• Please add some description about PKey format change to NEWS file. I think this change is OK (our older ruby should be able to read new format) but DSA might have some incompat behavior. DSA#p could be nil? Test it!

Regards,
// NaHi

[MartinBosslet (Martin Bosslet)]

First of all thanks for the kind words!

Some comments;
 * Please add a test for each commit to express the intent of the change. It must help us in the future.

The real benefit of applying this patch and
http://redmine.ruby-lang.org/issues/4422 is that
http://redmine.ruby-lang.org/issues/4424
becomes possible. You can create a PKey without knowing what kind of
key it actually is, much like what EVP_PKEY allows in OpenSSL itself.
I have included tests in 4424 that would test the changes in this
patch and 4422. If there is no objection, maybe I could commit 4424 to
add the tests and make the intent more clear? I was reluctant about
the naming in 4424, maybe you could help me out?

 * Please add some description about PKey format change to NEWS file. I think this change is OK (our older ruby should be able to read new format) but DSA might have some incompat behavior. DSA#p could be nil? Test it!

I'll add explicit tests for DSA and RSA that will use the old format
to "prove" backwards compatibility. Thanks for the hint with DSA. I'll
add the tests and fix it should any incompatibility show up. I also
added a few words to NEWS!

Regards,
Martin

[naruse (Yui NARUSE)]

Hi,

thank you for your contributions!

I have another comment: nahi and emboss's commit message has extra
header and indent like:

Mon May 16 05:13:20 2011 Martin Bosslet [e-mail]

 * ext/openssl/ossl_asn1.c: Add documentation.

Previous revision: 31583

It should be

• ext/openssl/ossl_asn1.c: Add documentation.

--
NARUSE, Yui  [e-mail]

[nahi (Hiroshi Nakamura)]

On Mon, May 16, 2011 at 11:47, NARUSE, Yui [e-mail] wrote:

I have another comment: nahi and emboss's commit message has extra
header and indent like:

Mon May 16 05:13:20 2011  Martin Bosslet  [e-mail]

   * ext/openssl/ossl_asn1.c: Add documentation.

Previous revision: 31583

It should be

• ext/openssl/ossl_asn1.c: Add documentation.

OK. I copied ChangeLog and left it. I changed my style.

Thanks for your suggestion!

Regards,
// NaHi

[ntalbott (Nathaniel Talbott)]

FYI, this does seem to break compatibility if you're using a fingerprint of the key for something, since (I just learned) key fingerprints are dependent on the format the key is stored in (since the fingerprint is based on the MD5 of the der-encoded key). Not sure there's much that can be done, but figured I'd leave this note here for anyone else that comes along later and encounters breakage due to this.

Our solution is to do a temporary hack to continue to spit out the old fingerprint, and migrate to a fingerprint based on the new format. Tricky, but doable.

[MartinBosslet (Martin Bosslet)]

Nathaniel Talbott wrote:

FYI, this does seem to break compatibility if you're using a fingerprint of the key for something, since (I just learned) key fingerprints are dependent on the format the key is stored in (since the fingerprint is based on the MD5 of the der-encoded key). Not sure there's much that can be done, but figured I'd leave this note here for anyone else that comes along later and encounters breakage due to this.

Our solution is to do a temporary hack to continue to spit out the old fingerprint, and migrate to a fingerprint based on the new format. Tricky, but doable.

Yes, unfortunately this broke things like fingerprints - someone approached me about a similar issue not that long ago. What might help in the process of migration is the fact that it is not too hard to port the pre-1.9.3 format to the newer X.509 public key format [1]. Similarly, you could also do the "downgrade" from the 1.9.3 format to the PKCS#1 format used by pre-1.9.3, allowing you to keep the old fingerprints.

If this would help you in your migration process and you need the latter instead of the former, let me know, I could also provide a code sample for that case.

-Martin

[1] https://gist.github.com/1470287

[wolfgangw (Wolfgang Woehl)]

MartinBosslet (Martin Bosslet) wrote:

[...] and you need the latter instead of the former, let me know, I could also provide a code sample for that case.

Martin, I'd appreciate an example for the latter, yes, thanks in advance. In my code I need to reach public key digests which correspond to info embedded in X.509v3 certs' CNs (generated elsewhere, not with some ruby/openssl) and 1.9.3(-p125) breaks this.

Also I'm wondering: With 1.8.7-p352 and 1.9.2-p290 I can reproduce values computed by openssl 0.9.8k. With 1.9.3-p125 I cannot. So did openssl's default encoding change?

Here's what I'm doing:

$ openssl x509 -pubkey -noout -in x509v3.pem | openssl base64 -d | dd bs=1 skip=24 2> /dev/null | openssl sha1 -binary | openssl base64
NPq2kOXj9wUCE/Q/L+YSWm8Es9k=

$ irb

RUBY_VERSION
=> "1.9.2"
cert = OpenSSL::X509::Certificate.new( open 'x509v3.pem' )

=> #<OpenSSL::X509::Certificate subject=/O=example.org/OU=csc.example.org/CN=leaf/dnQualifier=NPq2kOXj9wUCE/Q/L+YSWm8Es9k=, issuer=/O=example.org/OU=csc.example.org/CN=intermediate/dnQualifier=7OTO2EEWPx8palhiTx1VZ1adrjE=, serial=7, not_before=2012-03-19 19:57:11 UTC, not_after=2022-03-15 19:57:11 UTC>

puts cert.public_key.to_pem
-----BEGIN RSA PUBLIC KEY-----
MIIBCgKCAQEA7YZMQoS91QPXD1lMbyJQlK7jPidOMZ2hCsuq6UaJQsIyqDuu3RkJ
3Byl2xayvFmt7NSAwUEQvaCC0hoUPASB9GJJ9G/nAk4kPP1vbSmnyEjeWe+deb+m
6BB9/4GvRnacoHYw2MoOXScqrLVBJ2JoNvSXcPjqxZ266bb8b0mznuDubGACOH8L
luATgTdomeBmh80hl+Kpb4mfFKoyNGoIQPSybwoFDzTxgDo1YHD/rUgCF8Djim9W
c+/Rllz5q+Fhxsz9VgxlY0E2yV6vdUML+n4fqK9QmM9Z0e9X5TOz5Ntj6lZFCjmE
Hot18W+HNhncghiPkfEMDwyldP+/797ruwIDAQAB
-----END RSA PUBLIC KEY-----
=> nil

asn1 = Base64.decode64( cert.public_key.to_pem.split( "\n" )[ 1 .. -2 ].join )
=> "0\x82\x01\n\x02\x82\x01\x01\x00\xED\x86LB\x84\xBD\xD5\x03\xD7\x0FYLo"P\x94\xAE\xE3>'N1\x9D\xA1\n\xCB\xAA\xE9F\x89B\xC22\xA8;\xAE\xDD\x19\t\xDC\x1C\xA5\xDB\x16\xB2\xBCY\xAD\xEC\xD4\x80\xC1A\x10\xBD\xA0\x82\xD2\x1A\x14<\x04\x81\xF4bI\xF4o\xE7\x02N$<\xFDom)\xA7\xC8H\xDEY\xEF\x9Dy\xBF\xA6\xE8\x10}\xFF\x81\xAFFv\x9C\xA0v0\xD8\xCA\x0E]'*\xAC\xB5A'bh6\xF4\x97p\xF8\xEA\xC5\x9D\xBA\xE9\xB6\xFCoI\xB3\x9E\xE0\xEEl\x028\x7F\v\x96\xE0\x13\x817h\x99\xE0f\x87\xCD!\x97\xE2\xA9o\x89\x9F\x14\xAA24j\b@\xF4\xB2o\n\x05\x0F4\xF1\x80:5p\xFF\xADH\x02\x17\xC0\xE3\x8AoVs\xEF\xD1\x96\\xF9\xAB\xE1a\xC6\xCC\xFDV\fecA6\xC9^\xAFuC\v\xFA~\x1F\xA8\xAFP\x98\xCFY\xD1\xEFW\xE53\xB3\xE4\xDBc\xEAVE\n9\x84\x1E\x8Bu\xF1o\x876\x19\xDC\x82\x18\x8F\x91\xF1\f\x0F\f\xA5t\xFF\xBF\xEF\xDE\xEB\xBB\x02\x03\x01\x00\x01"

dnq_calc = Base64.encode64( OpenSSL::Digest.new( 'sha1', asn1 ).digest ).chomp
=> "NPq2kOXj9wUCE/Q/L+YSWm8Es9k="
exit

$ rbenv global 1.9.3-p125
$ irb

RUBY_VERSION
=> "1.9.3"
cert = OpenSSL::X509::Certificate.new( open 'x509v3.pem' )

=> #<OpenSSL::X509::Certificate subject=/O=example.org/OU=csc.example.org/CN=leaf/dnQualifier=NPq2kOXj9wUCE/Q/L+YSWm8Es9k=, issuer=/O=example.org/OU=csc.example.org/CN=intermediate/dnQualifier=7OTO2EEWPx8palhiTx1VZ1adrjE=, serial=7, not_before=2012-03-19 19:57:11 UTC, not_after=2022-03-15 19:57:11 UTC>

puts cert.public_key.to_pem
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA7YZMQoS91QPXD1lMbyJQ
lK7jPidOMZ2hCsuq6UaJQsIyqDuu3RkJ3Byl2xayvFmt7NSAwUEQvaCC0hoUPASB
9GJJ9G/nAk4kPP1vbSmnyEjeWe+deb+m6BB9/4GvRnacoHYw2MoOXScqrLVBJ2Jo
NvSXcPjqxZ266bb8b0mznuDubGACOH8LluATgTdomeBmh80hl+Kpb4mfFKoyNGoI
QPSybwoFDzTxgDo1YHD/rUgCF8Djim9Wc+/Rllz5q+Fhxsz9VgxlY0E2yV6vdUML
+n4fqK9QmM9Z0e9X5TOz5Ntj6lZFCjmEHot18W+HNhncghiPkfEMDwyldP+/797r
uwIDAQAB
-----END PUBLIC KEY-----
=> nil

asn1 = Base64.decode64( cert.public_key.to_pem.split( "\n" )[ 1 .. -2 ].join )
=> "0\x82\x01"0\r\x06\t*\x86H\x86\xF7\r\x01\x01\x01\x05\x00\x03\x82\x01\x0F\x000\x82\x01\n\x02\x82\x01\x01\x00\xED\x86LB\x84\xBD\xD5\x03\xD7\x0FYLo"P\x94\xAE\xE3>'N1\x9D\xA1\n\xCB\xAA\xE9F\x89B\xC22\xA8;\xAE\xDD\x19\t\xDC\x1C\xA5\xDB\x16\xB2\xBCY\xAD\xEC\xD4\x80\xC1A\x10\xBD\xA0\x82\xD2\x1A\x14<\x04\x81\xF4bI\xF4o\xE7\x02N$<\xFDom)\xA7\xC8H\xDEY\xEF\x9Dy\xBF\xA6\xE8\x10}\xFF\x81\xAFFv\x9C\xA0v0\xD8\xCA\x0E]'*\xAC\xB5A'bh6\xF4\x97p\xF8\xEA\xC5\x9D\xBA\xE9\xB6\xFCoI\xB3\x9E\xE0\xEEl\x028\x7F\v\x96\xE0\x13\x817h\x99\xE0f\x87\xCD!\x97\xE2\xA9o\x89\x9F\x14\xAA24j\b@\xF4\xB2o\n\x05\x0F4\xF1\x80:5p\xFF\xADH\x02\x17\xC0\xE3\x8AoVs\xEF\xD1\x96\\xF9\xAB\xE1a\xC6\xCC\xFDV\fecA6\xC9^\xAFuC\v\xFA~\x1F\xA8\xAFP\x98\xCFY\xD1\xEFW\xE53\xB3\xE4\xDBc\xEAVE\n9\x84\x1E\x8Bu\xF1o\x876\x19\xDC\x82\x18\x8F\x91\xF1\f\x0F\f\xA5t\xFF\xBF\xEF\xDE\xEB\xBB\x02\x03\x01\x00\x01"

dnq_calc = Base64.encode64( OpenSSL::Digest.new( 'sha1', asn1 ).digest ).chomp
=> "7pxBugGtDPy/CAbe8IDHuj4LUY4="

[MartinBosslet (Martin Bosslet)]

Sorry Wolfgang, I just saw your request. Better late than never, here's the example for what you asked for: https://gist.github.com/2902696

To reproduce the same digests as you got in pre-1.9.3, you need to create the format as in the gist, then compute the hash on that value. The default format has indeed changed with 1.9.3 (note the "BEGIN RSA PUBLIC KEY" vs. "BEGIN PUBLIC KEY"). We now use the more generic X.509 encoding of public keys instead of the RSA-specific PKCS#1 encoding.

[davidw (David Welton)]

Hi,

It certainly would have been useful to have the code available in gist https://gist.github.com/2902696 available in some form, because this patch broke backwards compatibility for a system I was working on.