OpenSSL::PKey::RSA.new returns the wrong value #357

Closed
queso opened this Issue Oct 24, 2012 · 7 comments

Comments

Projects
None yet
4 participants
@queso

queso commented Oct 24, 2012

I've created a gist that shows code that works in 1.6.8, but returns the wrong value in 1.7.0.

https://gist.github.com/3947534

This leads to this line throwing an error because privKey is null: https://github.com/jruby/jruby/blob/master/src/org/jruby/ext/openssl/PKeyRSA.java#L469

@queso

This comment has been minimized.

Show comment
Hide comment
@queso

queso Oct 24, 2012

Submitted here as suggested by @headius. /cc @bcurren

queso commented Oct 24, 2012

Submitted here as suggested by @headius. /cc @bcurren

@queso

This comment has been minimized.

Show comment
Hide comment
@queso

queso Oct 24, 2012

I tried to just read the private.pem key by itself and that errors out here: https://github.com/jruby/jruby/blob/master/src/org/jruby/ext/openssl/PKeyRSA.java#L278

I am just not sure where in that long if chain it is erroring out.

queso commented Oct 24, 2012

I tried to just read the private.pem key by itself and that errors out here: https://github.com/jruby/jruby/blob/master/src/org/jruby/ext/openssl/PKeyRSA.java#L278

I am just not sure where in that long if chain it is erroring out.

@pmahoney

This comment has been minimized.

Show comment
Hide comment
@pmahoney

pmahoney Oct 24, 2012

Contributor

Some work with the (Eclipse) debugger shows an exception in line 214 of PKeyRSA.java

val = PEMInputOutput.readPrivateKey(new StringReader(str.toString()), passwd);

the initializer then falls to the next case, which apparently only considers the public key portion.

The original exception in that method is java.security.InvalidKeyException "Wrong algorithm: DESede or TripleDES required". I think the "alrogithm" arg is "DESede/CBC/PKCS5Padding" while Java is perhaps expecting one of "DESede" or "TripleDES" (from this link which hopefully isn't horribly out of date).

Will hopefully post more details later.

Contributor

pmahoney commented Oct 24, 2012

Some work with the (Eclipse) debugger shows an exception in line 214 of PKeyRSA.java

val = PEMInputOutput.readPrivateKey(new StringReader(str.toString()), passwd);

the initializer then falls to the next case, which apparently only considers the public key portion.

The original exception in that method is java.security.InvalidKeyException "Wrong algorithm: DESede or TripleDES required". I think the "alrogithm" arg is "DESede/CBC/PKCS5Padding" while Java is perhaps expecting one of "DESede" or "TripleDES" (from this link which hopefully isn't horribly out of date).

Will hopefully post more details later.

@pmahoney

This comment has been minimized.

Show comment
Hide comment
@pmahoney

pmahoney Oct 24, 2012

Contributor

Some data copied from debugging session, at the point the InvalidKeyException is thrown.

Contributor

pmahoney commented Oct 24, 2012

Some data copied from debugging session, at the point the InvalidKeyException is thrown.

@pmahoney

This comment has been minimized.

Show comment
Hide comment
@pmahoney

pmahoney Oct 25, 2012

Contributor

I cannot reproduce (I get the expected private key) on trunk (approx. 1.7.0: 2547a3a) on Linux using

$ java -version
java version "1.7.0_07"
OpenJDK Runtime Environment (IcedTea7 2.3.2) (7u7-2.3.2a-1ubuntu1)
OpenJDK 64-Bit Server VM (build 23.2-b09, mixed mode)

I verified that realName is also "DESede/CBC/PKCS5Padding" in this case. Will have to wait for tomorrow to check Java versions on my other machine, but it is a Mac, probably using the Apple provided Java 1.6.

Also failed to reproduce on Linux using jruby 1.7.0 (1.9.3p203) 2012-10-24 2547a3a on OpenJDK 64-Bit Server VM 1.6.0_24-b24 [linux-amd64]

Contributor

pmahoney commented Oct 25, 2012

I cannot reproduce (I get the expected private key) on trunk (approx. 1.7.0: 2547a3a) on Linux using

$ java -version
java version "1.7.0_07"
OpenJDK Runtime Environment (IcedTea7 2.3.2) (7u7-2.3.2a-1ubuntu1)
OpenJDK 64-Bit Server VM (build 23.2-b09, mixed mode)

I verified that realName is also "DESede/CBC/PKCS5Padding" in this case. Will have to wait for tomorrow to check Java versions on my other machine, but it is a Mac, probably using the Apple provided Java 1.6.

Also failed to reproduce on Linux using jruby 1.7.0 (1.9.3p203) 2012-10-24 2547a3a on OpenJDK 64-Bit Server VM 1.6.0_24-b24 [linux-amd64]

@pmahoney

This comment has been minimized.

Show comment
Hide comment
@pmahoney

pmahoney Oct 25, 2012

Contributor

Ok, back on the Mac. I've created a small Java program that I think reproduces this problem.

$ cat CryptoBug.java 
import javax.crypto.Cipher;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;

public class CryptoBug {

    public static void main(String[] args) throws Exception {
        final String algo = "DESede/CBC/PKCS5Padding";
        final IvParameterSpec spec = new IvParameterSpec(new byte[8]);
        final SecretKeySpec secretKey = new SecretKeySpec(new byte[100], algo);

        Cipher.getInstance(algo).init(Cipher.DECRYPT_MODE, secretKey, spec);
    }

}

It simply attempt to initialize a cipher instance. Expected behavior is failure, either because the algorithm is "wrong" (this is the cause of this bug I believe), or because the algorithm was right but the bogus parameters are invalid (the ruby code above should work in JVMs where this failure mode occurs).

$ java -version && javac CryptoBug.java && java CryptoBug
java version "1.6.0_35"
Java(TM) SE Runtime Environment (build 1.6.0_35-b10-428-10M3811)
Java HotSpot(TM) 64-Bit Server VM (build 20.10-b01-428, mixed mode)
Exception in thread "main" java.security.InvalidKeyException: Wrong algorithm: DESede or TripleDES required
    ...
    at CryptoBug.main(CryptoBug.java:12)

And with OpenJDK 7 on the Mac:

$ java7 -version && javac CryptoBug.java && java7 CryptoBug
openjdk version "1.7.0-u10-b09"
OpenJDK Runtime Environment (build 1.7.0-u10-b09-20120927)
OpenJDK 64-Bit Server VM (build 23.6-b03, mixed mode)
Exception in thread "main" java.security.InvalidKeyException: Invalid key length: 100 bytes
    ....
    at CryptoBug.main(CryptoBug.java:12)
Contributor

pmahoney commented Oct 25, 2012

Ok, back on the Mac. I've created a small Java program that I think reproduces this problem.

$ cat CryptoBug.java 
import javax.crypto.Cipher;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;

public class CryptoBug {

    public static void main(String[] args) throws Exception {
        final String algo = "DESede/CBC/PKCS5Padding";
        final IvParameterSpec spec = new IvParameterSpec(new byte[8]);
        final SecretKeySpec secretKey = new SecretKeySpec(new byte[100], algo);

        Cipher.getInstance(algo).init(Cipher.DECRYPT_MODE, secretKey, spec);
    }

}

It simply attempt to initialize a cipher instance. Expected behavior is failure, either because the algorithm is "wrong" (this is the cause of this bug I believe), or because the algorithm was right but the bogus parameters are invalid (the ruby code above should work in JVMs where this failure mode occurs).

$ java -version && javac CryptoBug.java && java CryptoBug
java version "1.6.0_35"
Java(TM) SE Runtime Environment (build 1.6.0_35-b10-428-10M3811)
Java HotSpot(TM) 64-Bit Server VM (build 20.10-b01-428, mixed mode)
Exception in thread "main" java.security.InvalidKeyException: Wrong algorithm: DESede or TripleDES required
    ...
    at CryptoBug.main(CryptoBug.java:12)

And with OpenJDK 7 on the Mac:

$ java7 -version && javac CryptoBug.java && java7 CryptoBug
openjdk version "1.7.0-u10-b09"
OpenJDK Runtime Environment (build 1.7.0-u10-b09-20120927)
OpenJDK 64-Bit Server VM (build 23.6-b03, mixed mode)
Exception in thread "main" java.security.InvalidKeyException: Invalid key length: 100 bytes
    ....
    at CryptoBug.main(CryptoBug.java:12)
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Oct 29, 2012

Member

Perhaps we can just check the algorithm string for DESede or TripleDES and specify them directly?

Member

headius commented Oct 29, 2012

Perhaps we can just check the algorithm string for DESede or TripleDES and specify them directly?

kares added a commit to kares/jruby that referenced this issue Apr 23, 2014

add a spec for `OpenSSL::PKey::RSA.new` regression - working fine as …
…is (fixes #357)

not tested on Mac (only Linux with Java 1.7.0_51) but we're now not simply doing `Cipher.getInstance(String)` ... we're first attempt to consult (BC) provider directly

kares added a commit to kares/jruby that referenced this issue Apr 23, 2014

add a spec for `OpenSSL::PKey::RSA.new` regression - working fine as …
…is (fixes #357)

not tested on Mac (only Linux with Java 1.7.0_51) but we're now not simply doing `Cipher.getInstance(String)` ... we're first attempt to consult (BC) provider directly

mkristian added a commit that referenced this issue Apr 23, 2014

add a spec for `OpenSSL::PKey::RSA.new` regression - working fine as …
…is (fixes #357)

not tested on Mac (only Linux with Java 1.7.0_51) but we're now not simply doing `Cipher.getInstance(String)` ... we're first attempt to consult (BC) provider directly

mkristian added a commit that referenced this issue Apr 29, 2014

add a spec for `OpenSSL::PKey::RSA.new` regression - working fine as …
…is (fixes #357)

not tested on Mac (only Linux with Java 1.7.0_51) but we're now not simply doing `Cipher.getInstance(String)` ... we're first attempt to consult (BC) provider directly

@mkristian mkristian closed this in d9b0d95 Apr 29, 2014

@enebo enebo added this to the JRuby 1.7.13 milestone Jun 24, 2014

eregon added a commit that referenced this issue Nov 27, 2016

Squashed 'spec/ruby/' changes from a2e5952..852254a
852254a Fixed setting ThreadGroup for signal handler Thread.
0efde43 Fix flock spec: it only applies on Solaris and needs some setup
29b472c Split an example to get guards outside the example
4859bbb [Truffle] Compile c specs with extconf make
621ce1b spawn.rb: Pass PATH variable to started process.
7727dac setuid_spec.rb: Set execute bit before setting setuid bit on Solaris.
78a81b4 Handle no supplementary groups in grpowned? specs
8901448 Solaris returns -1 on rdev on ordinary files; update test to use /dev/zfs on Solaris.
30ae2c3 Fix UDPSocket.new spec, Solaris reports a different error on unknown protocol family.
8b352c3 Move Solaris File#flock specs in a different describe
9cd374e [Truffle] Show command if compilation fails.
4fc62b4 Improve Thread::Backtrace::Location#label specs
6fb234b Add spec for remove_method against cloned singleton.
9755210 Add spec for replacing singleton method in a cloned singleton.
327594e Add specs for UNIXSocket#inspect
5e79514 Add spec for IO#inspect's owner. See jruby/jruby#4262
51f80aa /regexp/.source should not display escape characters
7bda8e3 Require squiggly_heredoc only in examples so they can easily be tagged and the file be loaded
389f951 use SEPARATOR and ALT_SEPARATOR in File.basename
c594305 Fix File.dirname on Windows (including UNC)
5a15ff7 fix File.dirname with backslashes in unix
4ba6827 No TypeError on dup (#360)
3f3b283 Home directory is taken from system (#359)
913edd4 Merge pull request #357 from eregon/share_delete
85b1e0e Add spec for File::SHARE_DELETE
b61fdb5 Make sure all spawn specs with minimal environment have a successful exit status
71152a1 Always disable rubygems for spawn specs with minimal environment
b182988 Fix spec to use absolute path in unlink specs
5c33417 Use paths inside the temp directory for the unlink spec
3a1b57c Restrict testing with Ruby 2.3 64-bit on AppVeyor
772254d Add spec for File::TMPFILE
e1ea963 Add spec for define_method and define_singleton_method requiring an explicit block
66daf29 Merge pull request #355 from ruby/shugo/fix-unsetenv_others_on_icc
74b4241 Add --disable-gems if CC is icc.
be0bf17 Merge pull request #354 from nobu/fix-complex_equal_value
43b176b The result of `==` should be truthy or falsy
22ee32d The result of `==` should be truthy or falsy
2aea831 Try fixing Process.groups by ignoring the primary group id
97f987f Fix Process#groups spec
27960d0 Merge pull request #352 from ruby/revert-351-did_you_mean_spec
dabb1fe Revert "DidYouMean specs for ruby 2.3"
0c53f4c Fix fixture path on windows
b6ddea7 Add debug_info.rb fixture
a0e55db Add spec to test frozen string debug generation
0391aea Add String#+@ in conjunction  with freeze-magic-comment spec
a6edbbf Merge pull request #351 from mjago/did_you_mean_spec
2d79878 Fix Ruby Version
e075552 DidYouMean specs for ruby 2.3
3ce9295 Merge pull request #350 from mjago/aix_invalid_syslog_constants
9757670 As with solaris avoid testing aix unknown syslog constants
d152cba Fix Ruby version.
7783c4c Module#include and Module#prepend don't accept no-arguments in 2.4.
5eb8b40 Require round-to-nearest-even behavior for Ruby 2.4+ (#322)
39f89d8 Merge pull request #348 from nobu/bug/fork-feature-fix
45efa58 Check fork feature by respond_to?
fe4673a Merge pull request #347 from mjago/fix_remaining_encodings
b53a488 Fix encoding in optional/capi/string_spec.rb
abfba52 Fix encoding in optional/capi/encoding_spec.rb
c43ea57 Fix encoding in library/zlib/inflate/set_dictionary_spec.rb
1c54447 Fix encoding in library/socket/socket/gethostbyname_spec.rb
8113f43 Fix encoding in library/openssl/shared/constants.rb
1551379 Fix encoding in language/string_spec.rb
c5d6ec9 Fix encoding in language/regexp/escapes_spec.rb
9631f02 Fix encoding in language/regexp/encoding_spec.rb
8dfae6c Fix encoding in core/time/_load_spec.rb
c3f1719 Fix encoding in core/time/_dump_spec.rb
99af163 Fix encoding in core/symbol/casecmp_spec.rb
854f7b2 Fix encoding in core/string/valid_encoding_spec.rb
182b382 Fix encoding in core/string/unicode_normalize_spec.rb
738584e Fix encoding in core/string/squeeze_spec.rb
b12b094 Fix encoding in core/string/slice_spec.rb
cd5ced3 Fix encoding in core/string/shared/succ.rb
bf3d210 Fix encoding in core/string/shared/eql.rb
1ddfd79 Fix encoding in core/string/shared/encode.rb
990e4bc Fix encoding in core/string/shared/each_codepoint_without_block.rb

git-subtree-dir: spec/ruby
git-subtree-split: 852254a3371e3ce3e3007c49d0c23cf615e0409b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment