[15] RFR 8216012: Infinite loop in RSA KeyPairGenerator

[Valerie Peng]

Will do, thanks for the confirmation~

Valerie

On 1/15/2020 12:08 AM, yano-masanori at fujitsu.com wrote:
> 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.peng at oracle.com>
>> Sent: Saturday, January 11, 2020 9:30 AM
>> To: security-dev at openjdk.java.net; yano-masanori at 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-masanori at 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