Hi Max,

My own concern is that you may be using an undocumented functionality which may break later.

Since I don't have a better solution/workaround than what you have here, I think we can just go by what you have.

Thanks,
Valerie
On 6/18/2019 8:44 PM, Weijun Wang wrote:
On Jun 19, 2019, at 8:16 AM, Valerie Peng <[email protected]> wrote:

Hi Max,

Is this due to the RSA KeyPairGenerator impl of MSCAPI provider?
No, SunMSCAPI is at the end of the providers list. SunRsaSign is chosen on 
Windows.

Is there documentation on how the patch version of this RSAKeyPairGenerator 
class is built and why it'd be placed under ${test.classes}/patches/java.base 
directory?
I don't know. it's just my observation. Another way is to find the 
--patch-module argument on the test itself and re-apply it to the keytool 
command, but I can only get it with jdk.internal.misc.VM.getRuntimeArguments() 
and a @modules java.base/jdk.internal.misc tag is needed.

I am somewhat unsure about this approach as this would mean that we are not 
using the real default RSA key pair generator functionality for ALL 
env/platforms. Is this not part of what this test is about?
This is a keytool test which is mainly about keytool recognizing the key 
algorithm name and how to derive a signature algorithm from the key algorithm 
and key size. I think it does not matter what the provider of the key pair 
generator is.

I saw comments about Solaris RSA key pair gen being so slow that this test is 
not run on Solaris.
Yes, back when Solaris was slow I decided not to run on Solaris because the 
test is not meant to be platform dependent. While I was fixing this one, I 
thought about bring Solaris back but realized PKCS11 is used there (esp, you 
recently added PSS support) and I don't want to patch PKCS11 too.

If this is window-specific, maybe avoid the 8192 key size or find a way to work 
around this.
I'll need 8192 to make sure SHA512withRSA is chosen for this key size.

How do user generate 8192-bit RSA keys on windows? Do they just wait?
On my Windows (it's just a VirtialBox VM) it is not so slow, but it looks like 
Windows test machines tend to be slower than others. Also, I am afraid some day 
another OS might be also slow and that's why I decided to patch the generation 
code itself.

Thanks,
Max

Thanks,
Valerie
On 6/18/2019 7:26 AM, Weijun Wang wrote:
I rethink about this. Since the patched class was meant to make the generation faster, it 
should never silently fallback to "real" generation. Otherwise, it could be 
slow again unnoticed.

Updated webrev at http://cr.openjdk.java.net/~weijun/8225257/webrev.01/.

Thanks,
Max

p.s. Again, the difference between the original class and my patches one is:

diff --git 
src/java.base/share/classes/sun/security/rsa/RSAKeyPairGenerator.java 
test/jdk/sun/security/tools/keytool/pss/java.base/sun/security/rsa/RSAKeyPairGenerator.java
index 07fd44f..d73308f 100644
--- src/java.base/share/classes/sun/security/rsa/RSAKeyPairGenerator.java
+++ 
test/jdk/sun/security/tools/keytool/pss/java.base/sun/security/rsa/RSAKeyPairGenerator.java
@@ -1,12 +1,10 @@
  /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
   * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.  Oracle designates this
- * particular file as subject to the "Classpath" exception as provided
- * by Oracle in the LICENSE file that accompanied this code.
+ * published by the Free Software Foundation.
   *
   * This code is distributed in the hope that it will be useful, but WITHOUT
   * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
@@ -32,19 +30,11 @@ import java.security.spec.AlgorithmParameterSpec;
  import java.security.spec.RSAKeyGenParameterSpec;
import sun.security.jca.JCAUtil;
-import static sun.security.util.SecurityProviderConstants.DEF_RSA_KEY_SIZE;
-import static 
sun.security.util.SecurityProviderConstants.DEF_RSASSA_PSS_KEY_SIZE;
  import sun.security.x509.AlgorithmId;
  import static sun.security.rsa.RSAUtil.KeyType;
/**
- * RSA keypair generation. Standard algorithm, minimum key length 512 bit.
- * We generate two random primes until we find two where phi is relative
- * prime to the public exponent. Default exponent is 65537. It has only bit 0
- * and bit 4 set, which makes it particularly efficient.
- *
- * @since   1.5
- * @author  Andreas Sterbenz
+ * Fake RSA keypair generation.
   */
  public abstract class RSAKeyPairGenerator extends KeyPairGeneratorSpi {
@@ -57,9 +47,6 @@ public abstract class RSAKeyPairGenerator extends KeyPairGeneratorSpi {
      private final KeyType type;
      private AlgorithmId rsaId;
- // PRNG to use
-    private SecureRandom random;
-
      RSAKeyPairGenerator(KeyType type, int defKeySize) {
          this.type = type;
          // initialize to default in case the app does not call initialize()
@@ -120,35 +107,135 @@ public abstract class RSAKeyPairGenerator extends 
KeyPairGeneratorSpi {
this.keySize = tmpKeySize;
          this.publicExponent = tmpPublicExponent;
-        this.random = random;
      }
// generate the keypair. See JCA doc
      public KeyPair generateKeyPair() {
+
          // accommodate odd key sizes in case anybody wants to use them
-        int lp = (keySize + 1) >> 1;
-        int lq = keySize - lp;
-        if (random == null) {
-            random = JCAUtil.getSecureRandom();
-        }
          BigInteger e = publicExponent;
-        while (true) {
-            // generate two random primes of size lp/lq
-            BigInteger p = BigInteger.probablePrime(lp, random);
-            BigInteger q, n;
-            do {
-                q = BigInteger.probablePrime(lq, random);
-                // convention is for p > q
-                if (p.compareTo(q) < 0) {
-                    BigInteger tmp = p;
-                    p = q;
-                    q = tmp;
-                }
-                // modulus n = p * q
+        if (!e.equals(RSAKeyGenParameterSpec.F4)) {
+            throw new AssertionError("Only support F4 now");
+        }
+        BigInteger p, q, n;
+
+        // Pre-calculated p and q for e == RSAKeyGenParameterSpec.F4
+        switch (keySize) {
+            case 2048:
+                p = new BigInteger("1600840041787354447543653385760927"
+                        + "2642568308955833364523274045522752644800599"
+                        + "8669541532595690224703734511692014533312515"
+                        + "1867029838883431415692353449578487671384896"
+                        + "6611685764860941767986520897595108597563035"
+                        + "4023785639802607792535812062420427283857665"
+                        + "9883578590844700707106157871508280052743363"
+                        + "65749456332400771");
+                q = new BigInteger("1303880717101677622201474394769850"
+                        + "7257196073324816341282215626935164930077468"
+                        + "5999131251387556761167658937349436378464220"
+                        + "4831804147777472146628148336776639855791417"
+                        + "3849903041999943901924899580268176393595653"
+                        + "7357080543898614581363167420619163047562600"
+                        + "6155574020606891195960345238780709194499010"
+                        + "43652862954645301");
+                break;
+            case 4096:
+                p = new BigInteger("2985635754414679487171962796211911"
+                        + "1563710734938215274736352092606404045130913"
+                        + "2477365484439939846705721840432140066578525"
+                        + "0762327458086280430118434094733412377416194"
+                        + "8736124795243564050755767519346747209606612"
+                        + "5835460937739428885308798309679495432910469"
+                        + "0294757621321446003970767164933974474924664"
+                        + "1513767092845098947552598109657871041666676"
+                        + "2945573325433283821164032766425479703026349"
+                        + "9433641551427112483593214628620450175257586"
+                        + "4350119143877183562692754400346175237007314"
+                        + "7121580349193179272551363894896336921717843"
+                        + "3734726842184251708799134654802475890197293"
+                        + "9094908310578403843742664173424031260840446"
+                        + "591633359364559754200663");
+                q = new BigInteger("2279248439141087793789384816271625"
+                        + "1304008816573950275844533962181244003563987"
+                        + "6638461665174020058827698592331066726709304"
+                        + "9231319346136709972639455506783245161859951"
+                        + "6191872757335765533547033659834427437142631"
+                        + "3801232751161907082392011429712327250253948"
+                        + "6012497852063361866175243227579880020724881"
+                        + "9393797645220239009219998518884396282407710"
+                        + "7199202450846395844337846503427790307364624"
+                        + "5124871273035872938616425951596065309519651"
+                        + "1519189356431513094684173807318945903212527"
+                        + "7712469749366620048658571121822171067675915"
+                        + "5479178304648399924549334007222294762969503"
+                        + "5341584429803583589276956979963609078497238"
+                        + "760757619468018224491053");
+                break;
+            case 8192:
+                p = new BigInteger("9821669838446774374944535804569858"
+                        + "0553278885576950130485823829973470553571905"
+                        + "3014418421996241500307589880457361653957913"
+                        + "9176499436767288125182942994089196450118944"
+                        + "8701794862752733776161684616570463744619126"
+                        + "4981622564763630694110472008409561205704867"
+                        + "0221819623405201369630462487520858670679048"
+                        + "5854008441429858453634949980424333056803703"
+                        + "1205609490778445762604050796894221725977551"
+                        + "1428887194691696420765173256600200430067305"
+                        + "4364524177041858044598166859757042904625691"
+                        + "4292728453597609683799189454690202563236931"
+                        + "8171122071288244573793276051041975005528757"
+                        + "0228306442708182141334279133965507583927772"
+                        + "9244311696220253059281524393613278272067808"
+                        + "7017494446447670799055720358621918361716353"
+                        + "5018317015764698318012095108914870478138809"
+                        + "8204738169777192718869484177321870413838036"
+                        + "8149216482968887382371881239714335470844573"
+                        + "1862934371951394070111726593305334971041399"
+                        + "5517260339034138718517336990212463882142363"
+                        + "9154412320743552301967162100734381046548816"
+                        + "3883737645359595416600487444018399886391071"
+                        + "3777667222706059170707223589163679915863781"
+                        + "4662302526078720977228426750718207481384357"
+                        + "7918717041190413457052439016978578217755022"
+                        + "7370720979516554707297685239584071755267452"
+                        + "6021894842754355160100506065457679069228273"
+                        + "95209345267367982516553449135291473361");
+                q = new BigInteger("7902448465953646210110784092684896"
+                        + "0265474424590294110174550047938700740921014"
+                        + "1981650823416127449143596912363210790070524"
+                        + "2903784112701128957948996730263815210531364"
+                        + "0489145287401377007608600217628773627723381"
+                        + "1194123533939872283952535576847014977682278"
+                        + "9332064706645169741712060131540562788886577"
+                        + "3762235020990267901959745687867018811088495"
+                        + "3716021011509120447248882358515954471433808"
+                        + "2782236662758287959413069553620728137831579"
+                        + "2321174813204514354999978428741310035945405"
+                        + "0226661395731921098764192439072425262100813"
+                        + "9732949866553839713092238096261034339815187"
+                        + "2832617055364163276140160068136296115910569"
+                        + "9466440903693740716929166334256441926903849"
+                        + "1082968246155177124035336609654226388424434"
+                        + "5775783323612758615407928446164631651292743"
+                        + "8428509642959278732826297890909454571009075"
+                        + "7836191622138731918099379467912681177757761"
+                        + "6141378131042432093843778753846726589215845"
+                        + "7402160146427434508515156204064224022904659"
+                        + "8645441448874409852211668374267341177082462"
+                        + "7341410218867175406105046487057429530801973"
+                        + "0931082058719258230993681115780999537424968"
+                        + "2385515792331573549935317407789344892257264"
+                        + "7464569110078675090194686816764429827739815"
+                        + "0566036514181547634372488184242167294602000"
+                        + "8232780963578241583529875079397308150506597"
+                        + "37190564909892937290776929541076192569");
+                break;
+            default:
+                throw new AssertionError("Unknown keySize " + keySize);
+        }
+
          n = p.multiply(q);
-                // even with correctly sized p and q, there is a chance that
-                // n will be one bit short. re-generate the smaller prime if so
-            } while (n.bitLength() < keySize);
// phi = (p - 1) * (q - 1) must be relative prime to e
          // otherwise RSA just won't work ;-)
@@ -158,7 +245,7 @@ public abstract class RSAKeyPairGenerator extends 
KeyPairGeneratorSpi {
          // generate new p and q until they work. typically
          // the first try will succeed when using F4
          if (e.gcd(phi).equals(BigInteger.ONE) == false) {
-                continue;
+            throw new AssertionError("Should not happen");
          }
// private exponent d is the inverse of e mod phi
@@ -184,16 +271,3 @@ public abstract class RSAKeyPairGenerator extends 
KeyPairGeneratorSpi {
          }
      }
  }
-
-    public static final class Legacy extends RSAKeyPairGenerator {
-        public Legacy() {
-            super(KeyType.RSA, DEF_RSA_KEY_SIZE);
-        }
-    }
-
-    public static final class PSS extends RSAKeyPairGenerator {
-        public PSS() {
-            super(KeyType.PSS, DEF_RSASSA_PSS_KEY_SIZE);
-        }
-    }
-}


On Jun 18, 2019, at 12:58 PM, Weijun Wang <[email protected]> wrote:

Please take a review at

    http://cr.openjdk.java.net/~weijun/8225257/webrev.00/

I've patched the internal java.base/sun.security.rsa.RSAKeyPairGenerator class 
so it returns hardcoded pre-calculated key pairs.

Thanks,
Max

Reply via email to