[compat] improve DSA key parsing and errors

Author: kares

src/main/java/org/jruby/ext/openssl/PKeyDSA.java

@@ -302,6 +302,9 @@ public RubyBoolean private_p() {
 
     @JRubyMethod(name = "public_to_der")
     public RubyString public_to_der(ThreadContext context) {
+        if (publicKey == null) {
+            throw newPKeyError(context.runtime, "incompletely initialized DSA key");
+        }
         final byte[] bytes;
         try {
             bytes = toDerDSAPublicKey(publicKey);
@@ -325,6 +328,9 @@ public RubyString to_der() {
         catch (NoClassDefFoundError e) {
             throw newDSAError(getRuntime(), bcExceptionMessage(e));
         }
+        catch (IllegalArgumentException e) {
+            throw newPKeyError(getRuntime(), e.getMessage());
+        }
         catch (IOException e) {
             throw newDSAError(getRuntime(), e.getMessage(), e);
         }

src/main/java/org/jruby/ext/openssl/impl/PKey.java

@@ -119,14 +119,28 @@ public static KeyPair readPrivateKey(final Type type, final PrivateKeyInfo keyIn
                         exp1.getValue(), exp2.getValue(), crtCoef.getValue());
                 break;
             case DSA:
-                seq = (ASN1Sequence) keyInfo.parsePrivateKey();
-                ASN1Integer p = (ASN1Integer) seq.getObjectAt(1);
-                ASN1Integer q = (ASN1Integer) seq.getObjectAt(2);
-                ASN1Integer g = (ASN1Integer) seq.getObjectAt(3);
-                ASN1Integer y = (ASN1Integer) seq.getObjectAt(4);
-                ASN1Integer x = (ASN1Integer) seq.getObjectAt(5);
-                privSpec = new DSAPrivateKeySpec(x.getValue(), p.getValue(), q.getValue(), g.getValue());
-                pubSpec = new DSAPublicKeySpec(y.getValue(), p.getValue(), q.getValue(), g.getValue());
+                final ASN1Encodable parsedDSAKey = keyInfo.parsePrivateKey();
+                if (parsedDSAKey instanceof ASN1Integer) {
+                    // PKCS#8 format: private key is just x (INTEGER), params in AlgorithmIdentifier
+                    final BigInteger xVal = ((ASN1Integer) parsedDSAKey).getValue();
+                    final DSAParameter dsaParam = DSAParameter.getInstance(keyInfo.getPrivateKeyAlgorithm().getParameters());
+                    final BigInteger pVal = dsaParam.getP();
+                    final BigInteger qVal = dsaParam.getQ();
+                    final BigInteger gVal = dsaParam.getG();
+                    final BigInteger yVal = gVal.modPow(xVal, pVal);
+                    privSpec = new DSAPrivateKeySpec(xVal, pVal, qVal, gVal);
+                    pubSpec = new DSAPublicKeySpec(yVal, pVal, qVal, gVal);
+                } else {
+                    // Traditional "DSA PRIVATE KEY" format: SEQUENCE { version, p, q, g, y, x }
+                    seq = (ASN1Sequence) parsedDSAKey;
+                    ASN1Integer p = (ASN1Integer) seq.getObjectAt(1);
+                    ASN1Integer q = (ASN1Integer) seq.getObjectAt(2);
+                    ASN1Integer g = (ASN1Integer) seq.getObjectAt(3);
+                    ASN1Integer y = (ASN1Integer) seq.getObjectAt(4);
+                    ASN1Integer x = (ASN1Integer) seq.getObjectAt(5);
+                    privSpec = new DSAPrivateKeySpec(x.getValue(), p.getValue(), q.getValue(), g.getValue());
+                    pubSpec = new DSAPublicKeySpec(y.getValue(), p.getValue(), q.getValue(), g.getValue());
+                }
                 break;
             case EC:
                 return readECPrivateKey(SecurityHelper.getKeyFactory("EC"), keyInfo);
@@ -142,9 +156,8 @@ public static PublicKey readPublicKey(final byte[] input) throws IOException {
         // Try PEM first
         try (Reader in = new InputStreamReader(new ByteArrayInputStream(input))) {
             Object pemObject = new PEMParser(in).readObject();
-            if (pemObject != null) {
-                SubjectPublicKeyInfo publicKeyInfo = SubjectPublicKeyInfo.getInstance(pemObject);
-                return new JcaPEMKeyConverter().getPublicKey(publicKeyInfo);
+            if (pemObject instanceof SubjectPublicKeyInfo) {
+                return new JcaPEMKeyConverter().getPublicKey((SubjectPublicKeyInfo) pemObject);
             }
         }
         // Fall back to DER-encoded SubjectPublicKeyInfo
@@ -335,7 +348,8 @@ public static ASN1Sequence toASN1Primitive(final RSAPublicKey publicKey) {
         return new DERSequence(vec);
     }
 
-    public static byte[] toDerDSAKey(DSAPublicKey pubKey, DSAPrivateKey privKey) throws IOException {
+    public static byte[] toDerDSAKey(DSAPublicKey pubKey, DSAPrivateKey privKey)
+        throws IOException, IllegalArgumentException {
         if ( pubKey != null && privKey == null ) {
             return toDerDSAPublicKey(pubKey);
         }

src/test/ruby/dsa/test_dsa.rb

@@ -8,6 +8,7 @@ def setup
   end
 
   def test_private
+    # Traditional "DSA PRIVATE KEY" (OpenSSL legacy format)
     key = Fixtures.pkey("dsa1024")
     assert_equal true, key.private?
     key2 = OpenSSL::PKey::DSA.new(key.to_der)
@@ -18,12 +19,31 @@ def test_private
     assert_equal false, key4.private?
   end
 
+  def test_private_pkcs8
+    # PKCS#8 "PRIVATE KEY" format: private key is bare INTEGER (x),
+    # params (p, q, g) come from AlgorithmIdentifier; y must be derived.
+    key = Fixtures.pkey("dsa2048")
+    assert_equal true, key.private?
+    key2 = OpenSSL::PKey::DSA.new(key.to_der)
+    assert_equal true, key2.private?
+    key3 = key.public_key
+    assert_equal false, key3.private?
+    key4 = OpenSSL::PKey::DSA.new(key3.to_der)
+    assert_equal false, key4.private?
+  end
+
   def test_new
     key = OpenSSL::PKey::DSA.new(2048)
     pem  = key.public_key.to_pem
     OpenSSL::PKey::DSA.new pem
   end
 
+  def test_new_empty
+    key = OpenSSL::PKey::DSA.new
+    assert_nil(key.p)
+    assert_raise(OpenSSL::PKey::PKeyError) { key.to_der }
+  end
+
   def test_dup
     key = Fixtures.pkey("dsa1024")
     key2 = key.dup