Bug #4421
[ext/openssl] Fix RSA public key encoding
| Status: | Closed | Start date: | 02/22/2011 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: |  Martin Bosslet |
% Done: | 100% |
|
| Category: | ext | |||
| Target version: | 1.9.3 | |||
| ruby -v: | - |
Description
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
Associated revisions
Thu May 12 07:27:31 2011 Martin Bosslet <Martin.Bosslet@googlemail.com>
* 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
Thu May 12 07:27:31 2011 Martin Bosslet <Martin.Bosslet@googlemail.com>
* 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
History
Updated by Yui NARUSE about 1 year ago
- Status changed from Open to Assigned
- Assignee set to Hiroshi Nakamura
Updated by Yui NARUSE 10 months ago
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 DeveloperHowTo and CommitterHowTo.
Updated by Martin Bosslet 10 months ago
Hi Yui,
Yes, I'd love to contribute and I would really appreciate it!
Updated by Martin Bosslet 10 months ago
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
Updated by Yui NARUSE 10 months ago
Welcome to committers!
Yeah you can change the assignee and commit the patch.
Updated by Martin Bosslet 10 months ago
- Assignee changed from Hiroshi Nakamura to Martin Bosslet
Great :) Thanks for all the help so far!
Updated by Aaron Patterson 10 months ago
- 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/
Updated by Anonymous 10 months ago
- 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 <Martin.Bosslet@googlemail.com>
* 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
Updated by Hiroshi Nakamura 10 months ago
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
Updated by Martin Bosslet 10 months ago
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
Updated by Yui NARUSE 9 months ago
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 <Martin.Bosslet@googlemail.com>
* ext/openssl/ossl_asn1.c: Add documentation.
Previous revision: 31583
It should be
* ext/openssl/ossl_asn1.c: Add documentation.
--
NARUSE, Yui  <naruse@airemix.jp>
Updated by Hiroshi Nakamura 8 months ago
On Mon, May 16, 2011 at 11:47, NARUSE, Yui <naruse@airemix.jp> 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  <Martin.Bosslet@googlemail.com>
>
> Â Â * 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