diff --git a/src/main/java/org/jruby/ext/openssl/PKeyDSA.java b/src/main/java/org/jruby/ext/openssl/PKeyDSA.java index 399d8c43..389a2fc0 100644 --- a/src/main/java/org/jruby/ext/openssl/PKeyDSA.java +++ b/src/main/java/org/jruby/ext/openssl/PKeyDSA.java @@ -42,6 +42,7 @@ import java.security.interfaces.DSAKey; import java.security.interfaces.DSAPrivateKey; import java.security.interfaces.DSAPublicKey; +import java.security.spec.DSAPrivateKeySpec; import java.security.spec.DSAPublicKeySpec; import java.security.spec.InvalidKeySpecException; @@ -111,6 +112,7 @@ public PKeyDSA(Ruby runtime, RubyClass type, DSAPrivateKey privKey, DSAPublicKey // a public key to be constructed incrementally, as required by the // current implementation of Net::SSH. // (see net-ssh-1.1.2/lib/net/ssh/transport/ossl/buffer.rb #read_keyblob) + private transient volatile BigInteger dsa_x; private transient volatile BigInteger dsa_y; private transient volatile BigInteger dsa_p; private transient volatile BigInteger dsa_q; @@ -368,19 +370,28 @@ public IRubyObject sysverify(IRubyObject arg, IRubyObject arg2) { return getRuntime().getNil(); } - @JRubyMethod(name = "p") - public synchronized IRubyObject get_p() { - // FIXME: return only for public? - DSAKey key; BigInteger param; - if ((key = this.publicKey) != null || (key = this.privateKey) != null) { - if ((param = key.getParams().getP()) != null) { - return BN.newBN(getRuntime(), param); - } + private DSAKey getDsaKey() { + DSAKey result; + return (result = publicKey) != null ? result : privateKey; + } + + private IRubyObject toBN(BigInteger value) { + return value == null ? getRuntime().getNil() : BN.newBN(getRuntime(), value); + } + + private synchronized BigInteger getP() { + DSAKey key = getDsaKey(); + if (key != null) { + return key.getParams().getP(); } - else if (dsa_p != null) { - return BN.newBN(getRuntime(), dsa_p); + else { + return dsa_p; } - return getRuntime().getNil(); + } + + @JRubyMethod(name = "p") + public IRubyObject get_p() { + return toBN(getP()); } @JRubyMethod(name = "p=") @@ -388,19 +399,19 @@ public synchronized IRubyObject set_p(IRubyObject p) { return setKeySpecComponent(SPEC_P, p); } - @JRubyMethod(name = "q") - public synchronized IRubyObject get_q() { - // FIXME: return only for public? - DSAKey key; BigInteger param; - if ((key = this.publicKey) != null || (key = this.privateKey) != null) { - if ((param = key.getParams().getQ()) != null) { - return BN.newBN(getRuntime(), param); - } + private synchronized BigInteger getQ() { + DSAKey key = getDsaKey(); + if (key != null) { + return key.getParams().getQ(); } - else if (dsa_q != null) { - return BN.newBN(getRuntime(), dsa_q); + else { + return dsa_q; } - return getRuntime().getNil(); + } + + @JRubyMethod(name = "q") + public IRubyObject get_q() { + return toBN(getQ()); } @JRubyMethod(name = "q=") @@ -408,19 +419,19 @@ public synchronized IRubyObject set_q(IRubyObject q) { return setKeySpecComponent(SPEC_Q, q); } - @JRubyMethod(name = "g") - public synchronized IRubyObject get_g() { - // FIXME: return only for public? - DSAKey key; BigInteger param; - if ((key = this.publicKey) != null || (key = this.privateKey) != null) { - if ((param = key.getParams().getG()) != null) { - return BN.newBN(getRuntime(), param); - } + private synchronized BigInteger getG() { + DSAKey key = getDsaKey(); + if (key != null) { + return key.getParams().getG(); } - else if (dsa_g != null) { - return BN.newBN(getRuntime(), dsa_g); + else { + return dsa_g; } - return getRuntime().getNil(); + } + + @JRubyMethod(name = "g") + public IRubyObject get_g() { + return toBN(getG()); } @JRubyMethod(name = "g=") @@ -428,25 +439,27 @@ public synchronized IRubyObject set_g(IRubyObject g) { return setKeySpecComponent(SPEC_G, g); } - @JRubyMethod(name = "pub_key") - public synchronized IRubyObject get_pub_key() { - DSAPublicKey key; - if ( ( key = this.publicKey ) != null ) { - return BN.newBN(getRuntime(), key.getY()); - } - else if (dsa_y != null) { - return BN.newBN(getRuntime(), dsa_y); - } - return getRuntime().getNil(); - } - @JRubyMethod(name = "priv_key") public synchronized IRubyObject get_priv_key() { DSAPrivateKey key; if ((key = this.privateKey) != null) { - return BN.newBN(getRuntime(), key.getX()); + return toBN(key.getX()); } - return getRuntime().getNil(); + return toBN(dsa_x); + } + + @JRubyMethod(name = "priv_key=") + public synchronized IRubyObject set_priv_key(IRubyObject priv_key) { + return setKeySpecComponent(SPEC_X, priv_key); + } + + @JRubyMethod(name = "pub_key") + public synchronized IRubyObject get_pub_key() { + DSAPublicKey key; + if ( ( key = this.publicKey ) != null ) { + return toBN(key.getY()); + } + return toBN(dsa_y); } @JRubyMethod(name = "pub_key=") @@ -458,15 +471,38 @@ private IRubyObject setKeySpecComponent(final int index, final IRubyObject value final BigInteger val = BN.getBigInteger(value); switch (index) { + case SPEC_X: this.dsa_x = val; break; case SPEC_Y: this.dsa_y = val; break; case SPEC_P: this.dsa_p = val; break; case SPEC_Q: this.dsa_q = val; break; case SPEC_G: this.dsa_g = val; break; } - if ( dsa_y != null && dsa_p != null && dsa_q != null && dsa_g != null ) { - // we now have all components. create the key : - DSAPublicKeySpec spec = new DSAPublicKeySpec(dsa_y, dsa_p, dsa_q, dsa_g); + // Don't access the dsa_p, dsa_q and dsa_g fields directly. They may + // have already been consumed and cleared. + BigInteger _dsa_p = getP(); + BigInteger _dsa_q = getQ(); + BigInteger _dsa_g = getG(); + + if ( dsa_x != null && _dsa_p != null && _dsa_q != null && _dsa_g != null ) { + // we now have all private key components. create the key : + DSAPrivateKeySpec spec = new DSAPrivateKeySpec(dsa_x, _dsa_p, _dsa_q, _dsa_g); + try { + this.privateKey = (DSAPrivateKey) SecurityHelper.getKeyFactory("DSA").generatePrivate(spec); + } + catch (InvalidKeySpecException e) { + throw newDSAError(getRuntime(), "invalid keyspec", e); + } + catch (NoSuchAlgorithmException e) { + throw newDSAError(getRuntime(), "unsupported key algorithm (DSA)", e); + } + // clear out the specValues + this.dsa_x = this.dsa_p = this.dsa_q = this.dsa_g = null; + } + + if ( dsa_y != null && _dsa_p != null && _dsa_q != null && _dsa_g != null ) { + // we now have all public key components. create the key : + DSAPublicKeySpec spec = new DSAPublicKeySpec(dsa_y, _dsa_p, _dsa_q, _dsa_g); try { this.publicKey = (DSAPublicKey) SecurityHelper.getKeyFactory("DSA").generatePublic(spec); } @@ -483,10 +519,11 @@ private IRubyObject setKeySpecComponent(final int index, final IRubyObject value return value; } - private static final int SPEC_Y = 0; - private static final int SPEC_P = 1; - private static final int SPEC_Q = 2; - private static final int SPEC_G = 3; + private static final int SPEC_X = 0; + private static final int SPEC_Y = 1; + private static final int SPEC_P = 2; + private static final int SPEC_Q = 3; + private static final int SPEC_G = 4; public static RaiseException newDSAError(Ruby runtime, String message) { return Utils.newError(runtime, _PKey(runtime).getClass("DSAError"), message); diff --git a/src/test/ruby/dsa/private_key.pem b/src/test/ruby/dsa/private_key.pem new file mode 100644 index 00000000..e8df456d --- /dev/null +++ b/src/test/ruby/dsa/private_key.pem @@ -0,0 +1,12 @@ +-----BEGIN DSA PRIVATE KEY----- +MIIBugIBAAKBgQDpdW60slBJrsrXrsputlqXFlT70CA0czpJZWbppiv4fed941TN +/v/ICLjrNcsWXMbU5hb4faPrMZpAbUuIK+tMtJzz7sWMiINtso1FlQE/sUYBFqCv +Tmkj52N0dPGsE7qmZ6ZknaJn6DbrAL569+5NIe9CR6cTtwL4IPWVXT3HQQIVAJqv +o3Yrj85paNmGTIZfVt/oymAFAoGAb+S//7DQc6S/AK6r26BpQ/C4swSx1MTSl490 +hBJw0Czns5djqz9QB6ELufshGES1gcDGrYIncxQTGw1tPoJVrA+kefPVbRaYs2qM +HasEfM1GfILfu4XDBB4xAoFryjKizOu8MwEXTsPLiTe9MdiT90NfcgSyIty1FgFP +ZSz0JLMCgYBmAYli6D4DGB05NCVXBWPiu42c78gGCgibrbvCXpozB22TdMWA41ho +7Oy7diBJLuJPUdmSsK++RE0bxlDl6QfmxTqfdb0ZUZ4u2bC9VeSM8ZtkbtxRpJGU +p6znJdL83f05H2bhkKWI6a+vj894wRbtj+ube2UZPgKHFvkgdv732QIUKPp2Kkq+ +UQDoq2xu7v84G0sFIhc= +-----END DSA PRIVATE KEY----- diff --git a/src/test/ruby/dsa/test_dsa.rb b/src/test/ruby/dsa/test_dsa.rb new file mode 100644 index 00000000..54df466c --- /dev/null +++ b/src/test/ruby/dsa/test_dsa.rb @@ -0,0 +1,64 @@ +# coding: US-ASCII +require File.expand_path('../test_helper', File.dirname(__FILE__)) + +class TestDsa < TestCase + + def setup + super + self.class.disable_security_restrictions! + require 'base64' + end + + def test_dsa_param_accessors + key_file = File.join(File.dirname(__FILE__), 'private_key.pem') + key = OpenSSL::PKey::DSA.new(File.read(key_file)) + + [:priv_key, :pub_key, :p, :q, :g].each do |param| + dsa = OpenSSL::PKey::DSA.new + assert_nil(dsa.send(param)) + value = key.send(param) + dsa.send("#{param}=", value) + assert_equal(value, dsa.send(param), param) + end + end + + def test_dsa_from_params_private_first + key_file = File.join(File.dirname(__FILE__), 'private_key.pem') + key = OpenSSL::PKey::DSA.new(File.read(key_file)) + + dsa = OpenSSL::PKey::DSA.new + dsa.priv_key, dsa.p, dsa.q, dsa.g = key.priv_key, key.p, key.q, key.g + assert(dsa.private?) + assert(!dsa.public?) + [:priv_key, :p, :q, :g].each do |param| + assert_equal(key.send(param), dsa.send(param), param) + end + + dsa.pub_key = key.pub_key + assert(dsa.private?) + assert(dsa.public?) + [:priv_key, :pub_key, :p, :q, :g].each do |param| + assert_equal(key.send(param), dsa.send(param), param) + end + end + + def test_dsa_from_params_public_first + key_file = File.join(File.dirname(__FILE__), 'private_key.pem') + key = OpenSSL::PKey::DSA.new(File.read(key_file)) + + dsa = OpenSSL::PKey::DSA.new + dsa.pub_key, dsa.p, dsa.q, dsa.g = key.pub_key, key.p, key.q, key.g + assert(!dsa.private?) + assert(dsa.public?) + [:pub_key, :p, :q, :g].each do |param| + assert_equal(key.send(param), dsa.send(param), param) + end + + dsa.priv_key = key.priv_key + assert(dsa.private?) + assert(dsa.public?) + [:priv_key, :pub_key, :p, :q, :g].each do |param| + assert_equal(key.send(param), dsa.send(param), param) + end + end +end