Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PKey::RSA.initialize does not yield generated bytes to the block #201

Open
headius opened this issue Jun 1, 2020 · 2 comments
Open

PKey::RSA.initialize does not yield generated bytes to the block #201

headius opened this issue Jun 1, 2020 · 2 comments

Comments

@headius
Copy link
Member

headius commented Jun 1, 2020

The following code from WEBrick's ssl.rb passes a block into the RSA constructor, which is supposed to be use to yield each generated byte in turn. We do not pass this block on to the generation logic, so this logic never runs and WEBrick's test_sni fails (as detailed in #6246).

https://github.com/ruby/webrick/blob/0ff321aa74a056faec3cfa77795003353fe8412d/lib/webrick/ssl.rb#L97-L136

@headius
Copy link
Member Author

headius commented Jun 1, 2020

I do not see a way to implement this with current Bouncy Castle.

The trouble spot is in BC's RSA KeyPairGeneratorSpi class, which does not provide a way to change the engine used to generate. That engine is hardcoded to an instance of RSAKeyPairGenerator.

The RSAKeyPairGenerator class can be extended, allowing us to at least override the chooseRandomPrime method with a callback, but it's not possible to make an instance of KeyPairGeneratorSpi that will use this customized class.

The only option based on BC would appear to be duplicating most of the key pair generation logic in our own implementation, possibly backed by the accessible bits of Bouncy Castle's generation logic.

My attempt to simply extend KeyPairGeneratorSpi is provided below, but it doesn't work because that class's fields are package-private.

package org.jruby.ext.openssl;

import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.bouncycastle.crypto.generators.RSAKeyPairGenerator;
import org.bouncycastle.crypto.params.RSAKeyGenerationParameters;
import org.bouncycastle.jcajce.provider.asymmetric.rsa.KeyPairGeneratorSpi;
import org.bouncycastle.jcajce.provider.asymmetric.util.PrimeCertaintyCalculator;

import java.math.BigInteger;

class CallbackRSAKeyPairGenerator extends KeyPairGeneratorSpi {
    final static BigInteger defaultPublicExponent = BigInteger.valueOf(0x10001);

    public interface Consumer<T> {
        void accept(T t);
    }

    public CallbackRSAKeyPairGenerator(final Consumer<BigInteger> callback) {
        super("RSA");

        engine = new RSAKeyPairGenerator() {
            protected BigInteger chooseRandomPrime(int bitlength, BigInteger e, BigInteger sqrdBound) {
                BigInteger bi = super.chooseRandomPrime(bitlength, e, sqrdBound);
                callback.accept(bi);
                return bi;
            }
        };
        param = new RSAKeyGenerationParameters(defaultPublicExponent,
                CryptoServicesRegistrar.getSecureRandom(), 2048, PrimeCertaintyCalculator.getDefaultCertainty(2048));
        engine.init(param);
    }
}

@headius
Copy link
Member Author

headius commented Jun 1, 2020

Additional diff for using the above class, if it worked:

diff --git a/src/main/java/org/jruby/ext/openssl/CallbackRSAKeyPairGenerator.java b/src/main/java/org/jruby/ext/openssl/CallbackRSAKeyPairGenerator.java
index 7ef3d4f..190cd30 100644
--- a/src/main/java/org/jruby/ext/openssl/CallbackRSAKeyPairGenerator.java
+++ b/src/main/java/org/jruby/ext/openssl/CallbackRSAKeyPairGenerator.java
@@ -1,4 +1,35 @@
 package org.jruby.ext.openssl;
 
-public class CallbackRSAKeyPairGenerator {
+import org.bouncycastle.crypto.CryptoServicesRegistrar;
+import org.bouncycastle.crypto.generators.RSAKeyPairGenerator;
+import org.bouncycastle.crypto.params.RSAKeyGenerationParameters;
+import org.bouncycastle.jcajce.provider.asymmetric.rsa.KeyPairGeneratorSpi;
+import org.bouncycastle.jcajce.provider.asymmetric.util.PrimeCertaintyCalculator;
+
+import java.math.BigInteger;
+
+class CallbackRSAKeyPairGenerator extends KeyPairGeneratorSpi {
+    final static BigInteger defaultPublicExponent = BigInteger.valueOf(0x10001);
+
+    RSAKeyGenerationParameters param;
+    RSAKeyPairGenerator engine;
+
+    public interface Consumer<T> {
+        void accept(T t);
+    }
+
+    public CallbackRSAKeyPairGenerator(final Consumer<BigInteger> callback) {
+        super("RSA");
+
+        engine = new RSAKeyPairGenerator() {
+            protected BigInteger chooseRandomPrime(int bitlength, BigInteger e, BigInteger sqrdBound) {
+                BigInteger bi = super.chooseRandomPrime(bitlength, e, sqrdBound);
+                callback.accept(bi);
+                return bi;
+            }
+        };
+        param = new RSAKeyGenerationParameters(defaultPublicExponent,
+                CryptoServicesRegistrar.getSecureRandom(), 2048, PrimeCertaintyCalculator.getDefaultCertainty(2048));
+        engine.init(param);
+    }
 }
diff --git a/src/main/java/org/jruby/ext/openssl/PKeyRSA.java b/src/main/java/org/jruby/ext/openssl/PKeyRSA.java
index b19d659..62dc3cc 100644
--- a/src/main/java/org/jruby/ext/openssl/PKeyRSA.java
+++ b/src/main/java/org/jruby/ext/openssl/PKeyRSA.java
@@ -39,8 +39,8 @@ import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.NoSuchAlgorithmException;
 import java.security.PrivateKey;
+import java.security.Provider;
 import java.security.PublicKey;
-import java.security.SecureRandom;
 import java.security.interfaces.RSAPrivateCrtKey;
 import java.security.interfaces.RSAPublicKey;
 import java.security.spec.InvalidKeySpecException;
@@ -171,7 +171,7 @@ public class PKeyRSA extends PKey {
     public String getAlgorithm() { return "RSA"; }
 
     @JRubyMethod(name = "generate", meta = true, rest = true)
-    public static IRubyObject generate(IRubyObject self, IRubyObject[] args) {
+    public static IRubyObject generate(ThreadContext context, IRubyObject self, IRubyObject[] args) {
         final Ruby runtime = self.getRuntime();
         BigInteger exp = RSAKeyGenParameterSpec.F4;
         if ( Arity.checkArgumentCount(runtime, args, 1, 2) == 2 ) {
@@ -182,17 +182,31 @@ public class PKeyRSA extends PKey {
             }
         }
         final int keySize = RubyNumeric.fix2int(args[0]);
-        return rsaGenerate(runtime, new PKeyRSA(runtime, (RubyClass) self), keySize, exp);
+        return rsaGenerate(context, new PKeyRSA(context.runtime, (RubyClass) self), keySize, exp, Block.NULL_BLOCK);
     }
 
     /*
      * c: rsa_generate
      */
-    private static PKeyRSA rsaGenerate(final Ruby runtime,
-        PKeyRSA rsa, int keySize, BigInteger exp) throws RaiseException {
+    private static PKeyRSA rsaGenerate(final ThreadContext context,
+        PKeyRSA rsa, int keySize, BigInteger exp, final Block block) throws RaiseException {
+
+        final Ruby runtime = context.runtime;
+
         try {
-            KeyPairGenerator gen = SecurityHelper.getKeyPairGenerator("RSA");
-            if ( "IBMJCEFIPS".equals( gen.getProvider().getName() ) ) {
+            KeyPairGenerator gen;
+            if (block.isGiven()) {
+                gen = new CallbackRSAKeyPairGenerator(new CallbackRSAKeyPairGenerator.Consumer<BigInteger>() {
+                    @Override
+                    public void accept(BigInteger bi) {
+                        block.yield(context, RubyBignum.newBignum(runtime, bi));
+                    }
+                });
+            } else {
+                gen = SecurityHelper.getKeyPairGenerator("RSA");
+            }
+            Provider provider = gen.getProvider();
+            if ( provider != null && "IBMJCEFIPS".equals( provider.getName() ) ) {
                 gen.initialize(keySize); // IBMJCEFIPS does not support parameters
             } else {
                 gen.initialize(new RSAKeyGenParameterSpec(keySize, exp), getSecureRandom(runtime));
@@ -208,6 +222,7 @@ public class PKeyRSA extends PKey {
             throw newRSAError(runtime, e.getMessage());
         }
         catch (RuntimeException e) {
+            e.printStackTrace();
             throw newRSAError(rsa.getRuntime(), e);
         }
         return rsa;
@@ -236,7 +251,7 @@ public class PKeyRSA extends PKey {
             if (arg1 != null && !arg1.isNil()) {
                 exp = BigInteger.valueOf(RubyNumeric.num2long(arg1));
             }
-            return rsaGenerate(runtime, this, keySize, exp);
+            return rsaGenerate(context, this, keySize, exp, block);
         }
 
         final char[] passwd = password(context, arg1, block);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant