Hi Valerie, Thank you for updating my test. It looks fine to me. Please start to integrate it into JDK15.
Regards, Masanori Yano > -----Original Message----- > From: Valerie Peng <valerie.p...@oracle.com> > Sent: Saturday, January 11, 2020 9:30 AM > To: security-dev@openjdk.java.net; yano-masan...@fujitsu.com > Subject: [15] RFR 8216012: Infinite loop in RSA KeyPairGenerator > > Hi Masanori Yano, > > Your fix looks fine, I just made some minor update to the regression test, > e.g. > add more debugging output, removing unnecessary code and cases, etc. > > I put your initial contribution at > http://cr.openjdk.java.net/~valeriep/8216012/webrev.00 > > I made some update and latest fix is at > http://cr.openjdk.java.net/~valeriep/8216012/webrev.01 > > Can you please take a look and let me know if you are ok with webrev.01? > If all are well, I will proceed to integrate it into JDK15. > > Thanks, > > Valerie > > On 12/2/2019 4:40 PM, Valerie Peng wrote: > > Hi Masanori Yano, > > > > I can help sponsoring this fix. However, as it's a P4, it may be > > targeted to 15 depending on the available cycles. > > > > Are you a contributor for OpenJDK? > > > > If not, please see http://openjdk.java.net/contribute/ for the process. > > > > Thanks, > > Valerie > > On 10/8/2019 8:10 PM, yano-masan...@fujitsu.com wrote: > >> Hello. > >> > >> I would like to contribute for JDK-8216012. > >> > >> The cause of this problem is RSAKeyPairGenerator that doesn't check > >> the public exponent even though the algorithm of rsa key generation > >> can use only odd exponent number. > >> > >> To generate a KeyPair, the RSAKeyPairGenerator finds two random > >> primes P and Q, and calculate Phi = (P - 1) * (Q - 1). If Phi is not > >> relative prime to exponent, RSAKeyPairGenerator retry from the first. > >> > >> The value of Phi must be an even number because P and Q are odd > numbers. > >> If exponent is an even number, the greatest common divisor cannot be > >> 1 because one of common divisors is 2 which is bigger than 1. > >> Therefore, generateKeyPair() method of RSAKeyPairGenerator cannot > >> exit the retrying loop. > >> > >> To solve this problem, I propose to check whether the public exponent > >> is even number. > >> > >> Please sponsor the following change. > >> > >> diff --git > >> a/src/java.base/share/classes/sun/security/rsa/RSAKeyPairGenerator.ja > >> va > >> b/src/java.base/share/classes/sun/security/rsa/RSAKeyPairGenerator.ja > >> va > >> --- > >> a/src/java.base/share/classes/sun/security/rsa/RSAKeyPairGenerator.ja > >> va > >> +++ > >> b/src/java.base/share/classes/sun/security/rsa/RSAKeyPairGenerator.ja > >> va > >> @@ -96,6 +96,10 @@ > >> throw new InvalidAlgorithmParameterException > >> ("Public exponent must be 3 or larger"); > >> } > >> + if (!tmpPublicExponent.testBit(0)) { > >> + throw new InvalidAlgorithmParameterException > >> + ("Public exponent must be an odd > number"); > >> + } > >> if (tmpPublicExponent.bitLength() > tmpKeySize) { > >> throw new InvalidAlgorithmParameterException > >> ("Public exponent must be smaller than > key > >> size"); diff --git > >> a/test/jdk/sun/security/rsa/TestKeyPairGeneratorExponent.java > >> b/test/jdk/sun/security/rsa/TestKeyPairGeneratorExponent.java > >> new file mode 100644 > >> --- /dev/null > >> +++ b/test/jdk/sun/security/rsa/TestKeyPairGeneratorExponent.java > >> @@ -0,0 +1,65 @@ > >> +import java.math.BigInteger; > >> + > >> +import java.security.*; > >> +import java.security.interfaces.*; > >> +import java.security.spec.*; > >> + > >> +/** > >> + * @test > >> + * @bug 8216012 > >> + * @summary Tests the RSA public key exponent for KeyPairGenerator > >> + * @run main/timeout=60 TestKeyPairGeneratorExponent */ public > >> +class TestKeyPairGeneratorExponent { > >> + private static int keyLen = 512; > >> + > >> + private static BigInteger[] validExponents = new BigInteger[] { > >> + RSAKeyGenParameterSpec.F0, > >> + RSAKeyGenParameterSpec.F4, > >> + // Since 512-bit exponent is larger than modulus, fails in > >> RSAPublicKeyImpl.checkExponentRange(). > >> + BigInteger.ONE.shiftLeft(keyLen - > >> +1).subtract(BigInteger.ONE) > >> + }; > >> + > >> + private static BigInteger[] invalidExponents = new BigInteger[] > >> +{ > >> + BigInteger.valueOf(-1), > >> + BigInteger.ZERO, > >> + BigInteger.ONE, > >> + // 8216012: An even number causes infinite loop. > >> + BigInteger.valueOf(4), > >> + BigInteger.ONE.shiftLeft(keyLen) > >> + }; > >> + > >> + public static void testValidExponents(KeyPairGenerator kpg, > >> BigInteger exponent) { > >> + try { > >> + kpg.initialize(new RSAKeyGenParameterSpec(keyLen, > >> exponent)); > >> + kpg.generateKeyPair(); > >> + } catch(InvalidAlgorithmParameterException iape){ > >> + throw new RuntimeException("Unexpected Exception: " > + > >> iape); > >> + } > >> + } > >> + > >> + public static void testInvalidExponents(KeyPairGenerator kpg, > >> BigInteger exponent) { > >> + try { > >> + kpg.initialize(new RSAKeyGenParameterSpec(keyLen, > >> exponent)); > >> + kpg.generateKeyPair(); > >> + throw new RuntimeException("Expected > >> InvalidAlgorithmParameterException was not thrown."); > >> + } catch(InvalidAlgorithmParameterException iape){ > >> + // Expected InvalidAlgorithmParameterException was > >> thrown.OK > >> + } > >> + } > >> + > >> + public static void main(String[] args) throws Exception { > >> + Provider provider = Security.getProvider("SunRsaSign"); > >> + KeyPairGenerator kpg; > >> + > >> + for(BigInteger validExponent : validExponents) { > >> + kpg = KeyPairGenerator.getInstance("RSA", provider); > >> + testValidExponents(kpg, validExponent); > >> + } > >> + > >> + for(BigInteger invalidExponent : invalidExponents) { > >> + kpg = KeyPairGenerator.getInstance("RSA", provider); > >> + testInvalidExponents(kpg, invalidExponent); > >> + } > >> + } > >> +} > >> > >> Regards, > >> Masanori Yano
