[fix] work-around CSR failing with EC key (#294)

jruby · Apr 10, 2024 · ab5d342 · ab5d342
1 parent 3518d8e
commit ab5d342
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 24 deletions.
diff --git a/src/main/java/org/jruby/ext/openssl/X509Request.java b/src/main/java/org/jruby/ext/openssl/X509Request.java
@@ -42,7 +42,6 @@
 import org.bouncycastle.asn1.ASN1Encoding;
 import org.bouncycastle.asn1.ASN1Set;
 import org.bouncycastle.asn1.ASN1ObjectIdentifier;
-import org.bouncycastle.asn1.ASN1Sequence;
 import org.bouncycastle.asn1.pkcs.Attribute;
 import org.bouncycastle.asn1.x500.X500Name;
 
@@ -61,6 +60,7 @@
 import org.jruby.runtime.Visibility;
 
 import org.jruby.ext.openssl.impl.PKCS10Request;
+
 import static org.jruby.ext.openssl.OpenSSL.*;
 import static org.jruby.ext.openssl.PKey._PKey;
 import static org.jruby.ext.openssl.X509._X509;
@@ -319,7 +319,7 @@ public IRubyObject sign(final ThreadContext context,
             request = null; getRequest().sign( privateKey, digAlg );
         }
         catch (InvalidKeyException e) {
-            debug(runtime, "X509Request#sign failed", e);
+            debug(runtime, "X509Request#sign invalid key:", e);
             throw newRequestError(runtime, e);
         }
         catch (Exception e) {
@@ -344,23 +344,25 @@ private Attribute newAttributeImpl(final ThreadContext context,
 
     @JRubyMethod
     public IRubyObject verify(final ThreadContext context, IRubyObject key) {
-        final Ruby runtime = context.runtime; final PublicKey publicKey;
+        final Ruby runtime = context.runtime;
         try {
-            publicKey = ( (PKey) key.callMethod(context, "public_key") ).getPublicKey();
-            return runtime.newBoolean( getRequest().verify(publicKey) );
+            if (!(key instanceof PKey)) { // due PKeyEC
+                key = key.callMethod(context, "public_key");
+                if (!(key instanceof PKey)) {
+                    throw context.runtime.newTypeError(key, _PKey(runtime));
+                }
+            }
+            boolean signatureValid = getRequest().verify( ((PKey) key).getPublicKey() );
+            return runtime.newBoolean(signatureValid);
         }
         catch (InvalidKeyException e) {
-            debug(runtime, "X509::Request#verify invalid key", e);
+            debug(runtime, "X509Request#verify invalid key:", e);
             throw newRequestError(runtime, "invalid key supplied", e);
         }
-        //catch (IOException e) {
-        //    debug(runtime, "X509::Request.verify failed", e);
-        //    return runtime.getFalse();
-        //}
-        //catch (RuntimeException e) {
-        //    debug(runtime, "X509::Request.verify failed", e);
-        //    return runtime.getFalse();
-        //}
+        catch (RuntimeException e) {
+            debugStackTrace(runtime, "X509Request#verify", e);
+            return context.nil;
+        }
     }
 
     @JRubyMethod

diff --git a/src/main/java/org/jruby/ext/openssl/impl/PKCS10Request.java b/src/main/java/org/jruby/ext/openssl/impl/PKCS10Request.java
@@ -51,6 +51,8 @@
 import org.bouncycastle.asn1.DLSequence;
 import org.bouncycastle.asn1.pkcs.CertificationRequestInfo;
 import org.bouncycastle.asn1.x500.X500Name;
+import org.bouncycastle.asn1.x9.X9ObjectIdentifiers;
+import org.bouncycastle.operator.DefaultAlgorithmNameFinder;
 import org.bouncycastle.pkcs.PKCS10CertificationRequest;
 import org.bouncycastle.pkcs.PKCS10CertificationRequestBuilder;
 import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
@@ -136,16 +138,14 @@ public PKCS10CertificationRequest sign(final PrivateKey privateKey,
         final String digestAlg)
         throws NoSuchAlgorithmException, InvalidKeyException {
         String sigAlg = digestAlg + "WITH" + getPublicKeyAlgorithm();
-        return sign( privateKey,
-            new DefaultSignatureAlgorithmIdentifierFinder().find( sigAlg )
-        );
+        return sign(privateKey, new DefaultSignatureAlgorithmIdentifierFinder().find(sigAlg));
     }
 
     // verify
 
     public boolean verify(final PublicKey publicKey) throws InvalidKeyException {
         if ( signedRequest == null ) {
-            if ( true ) throw new IllegalStateException("no signed request");
+            assert false : "no signed request";
             return false;
         }
 
@@ -206,16 +206,23 @@ public void setPublicKey(final PublicKey publicKey) {
         resetSignedRequest();
     }
 
+    /**
+     * @return e.g. "RSA" or "ECDSA"
+     */
     private String getPublicKeyAlgorithm() {
-        //if ( publicKeyAlgorithm == null ) {
-        //    throw new IllegalStateException("no public key info");
-        //}
-        //return publicKeyAlgorithm;
         if ( publicKeyInfo == null ) {
             throw new IllegalStateException("no public key info");
         }
-        AlgorithmIdentifier algId = publicKeyInfo.getAlgorithm();
-        return ASN1Registry.oid2sym( algId.getAlgorithm() );
+
+        final AlgorithmIdentifier algId = publicKeyInfo.getAlgorithm();
+        // NOTE: BC's DefaultAlgorithmNameFinder does not handle the EC oid
+        if (X9ObjectIdentifiers.id_ecPublicKey.equals(algId.getAlgorithm())) {
+            return "ECDSA";
+        }
+        // e.g. PKCSObjectIdentifiers.rsaEncryption -> "RSA"
+        final String algName = new DefaultAlgorithmNameFinder().getAlgorithmName(algId);
+        assert algId.getAlgorithm().getId() != algName : "could not resolve name for oid: " + algId.getAlgorithm();
+        return algName;
     }
 
     public PublicKey generatePublicKey() throws NoSuchAlgorithmException,

diff --git a/src/test/ruby/x509/test_x509req.rb b/src/test/ruby/x509/test_x509req.rb
@@ -28,6 +28,18 @@ def test_csr_request_extensions
     assert_equal 0, csr.version
   end
 
+  def test_csr_request_ec_key
+    key = OpenSSL::PKey::EC.generate('secp384r1')
+
+    csr = OpenSSL::X509::Request.new
+    csr.public_key = key
+    csr.subject = OpenSSL::X509::Name.new([['CN', 'foo.bar.cat', OpenSSL::ASN1::UTF8STRING]])
+    csr.version = 2
+    csr.sign key, OpenSSL::Digest::SHA256.new # does not raise
+
+    assert_true csr.verify(key)
+  end
+
   def test_version
     csr = OpenSSL::X509::Request.new
     assert_equal 0, csr.version