Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

New AS key generation algo causes problems on JRuby #8534

Closed
bai opened this Issue Dec 17, 2012 · 19 comments

Comments

Projects
None yet

bai commented Dec 17, 2012

For generating secure keys Rails uses OpenSSL's PBKDF2 (see key_generator.rb). This causes a problem running current master on latest JRuby (1.7.1).
The whole PKCS5 is not in jruby-openssl, thus rails throws an exception: NameError (uninitialized constant OpenSSL::PKCS5).

And, according to @headius, PBKDF2 is not something that is going to be implemented soon for variety of reasons.

Being said that, the discussion on IRC came to suggesting to extract crypto stuff into a gem.

Thoughts?

/cc @headius

Member

steveklabnik commented Dec 17, 2012

/cc @NZKoz

Member

steveklabnik commented Dec 17, 2012

Lots of the tests fail on JRuby right now, but I'd like to get them all working.

Owner

rafaelfranca commented Dec 17, 2012

Could Mr. @headius enlighten us with the reasons to not implement it?

We want to ship this crypto stuff by default in Rails 4.

Member

NZKoz commented Dec 17, 2012

We won't be extracting the crypto stuff to a gem, it's on by default in rails 4 for a reason. PBKDF2 is defined in an RFC and implemented by the vast bulk of crypto libraries, including bouncycastle which is what jruby-openssl appears to use.

I'd be happy to help someone implement the necessary subset of PKCS5 if needs be, but we're not going to abandon the encrypted cookie store for 4.0

Owner

tenderlove commented Dec 17, 2012

It sounds like JRuby is going to support PBKDF2 via the krypt gem. We just need to special case our implementation to use the correct implementation depending on which Ruby we're using (e.g. on JRuby, use krypt, on others, use openssl).

Owner

tenderlove commented Dec 17, 2012

@emboss does krypt currently support PBKDF2 on JRuby?

emboss commented Dec 18, 2012

@tenderlove: I talked to @headius, our plan is to integrate parts of krypt in the next release of JRuby in January. The special focus is on PKCS5 for the exact reason we witnessed here. As I also promised to provide a patch that allows using PBKDF2 in combination with other hashes than SHA-1 (SHA-256 for example), I will also make sure that this is supported by JRuby as well.

To keep compatibility with OpenSSL there will be an OpenSSL shim for krypt. This way, JRuby can offer the PKCS5 interface of OpenSSL, but it would use krypt internally to realize it. Hopefully this means no need to special case anything at all!

Member

NZKoz commented Dec 18, 2012

Sounds good to me, we can mention the minimum version required in any
release notes for betas/RCs we ship prior to the jruby release being done.

On 18/12/2012, at 2:30 PM, Martin Bosslet notifications@github.com wrote:

@tenderlove https://github.com/tenderlove: I talked to
@headiushttps://github.com/headius,
our plan is to integrate parts of krypt in the next release of JRuby in
January. The special focus is on PKCS5 for the exact reason we witnessed
here. As I also
promisedhttps://github.com/rails/rails/pull/8112#issuecomment-10574175to
provide a patch that allows using PBKDF2 in combination with other
hashes than SHA-1 (SHA-256 for example), I will also make sure that this is
supported by JRuby as well.

To keep compatibility with OpenSSL there will be an OpenSSL shim for krypt.
This way, JRuby can offer the PKCS5 interface of OpenSSL, but it would use
krypt internally to realize it. Hopefully this means no need to special
case anything at all!


Reply to this email directly or view it on
GitHubhttps://github.com/rails/rails/issues/8534#issuecomment-11470332.

Contributor

headius commented Jan 24, 2013

I have just incorporated krypt and its pkcs5 implementation into JRuby master: jruby/jruby@cc9acba

What's the simplest way for me to confirm that it's working ok with Rails 4?

bai commented Jan 24, 2013

Run an application with session_store set to encrypted_cookie_store.

For instance:
MyApp::Application.config.session_store :encrypted_cookie_store, key: '_myapp_session'

I'm on the move right now, will also try jruby-head on my test app tomorrow morning UTC.

Contributor

johndouthat commented Jan 25, 2013

It looks like it's working, as long as you remove the "jruby-openssl" gem from your Gemfile and have the the Unlimited Strength Jurisdiction Policy files for java 7 or for java 6 installed into

`/usr/libexec/java_home`/jre/lib/security

There are unrelated issues between JRuby and Rails master, though. So I could only get the fix to work for an older version of Rails4

emboss commented Jan 25, 2013

@johndouthat: I'm on hotel wifi at the moment, so I'm having a hard time setting up a Rails 4 env right now to reproduce your findings, but it's strange since so far krypt uses nothing that should require the Unlimited Strength files - could you please provide more details about how and where it fails without them? Thanks!

Contributor

johndouthat commented Jan 25, 2013

@emboss Sure, no problem. The error is
OpenSSL::Cipher::CipherError: Illegal key size: possibly you need to install Java Cryptography Extension (JCE) Unlimited Strength Jurisdiction Policy Files for your JRE

Here's how you can reproduce it: https://gist.github.com/4632397

It happened for me using jruby 1.7.3.dev (1.9.3p327) 2013-01-24 cc9acba on both

  • OS X 10.7.5 with Java 1.7.0_12 64-bit
  • Ubuntu 12.04.1 server with Java 1.7.0_11 64-bit

This rails app should reproduce it, too.

emboss commented Jan 25, 2013

@johndouthat: Thank you! I believe that error should somehow be related to jruby-openssl, because krypt doesn't implement Cipher yet. It does use MessageDigest internally for the PKCS5 part, but MessageDigest should not require the Policy Files. Your statement in https://jira.codehaus.org/browse/JRUBY-7035 seems to support this view?

Contributor

johndouthat commented Jan 26, 2013

@emboss You are right. This bug is fixed. OpenSSL::PKCS5 is defined and working. Thanks for your work on krypt!

I should have said, "now that this issue is fixed, a new issue is exposed: using ActiveSupport::MessageEncryptor in JRuby throws a CipherError unless you have the unlimited strength policy files."

emboss commented Jan 30, 2013

@johndouthat. I have to thank you! Thanks for analyzing this, and thanks for enjoying krypt :)

Contributor

mdespuits commented Feb 20, 2013

Ping...
Can this be closed?

Member

arunagw commented Mar 22, 2013

I think this has been fixed here jruby/jruby@cc9acba

Member

arunagw commented Mar 22, 2013

But not be able to test this out. I did tried with JRuby head.

@dhh dhh closed this Mar 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment