Code Review Request 8072452 Support DHE sizes up to 8192-bits

[Xuelei Fan]

Hi Valerie,

All comments are good to me and accepted in the new webrev:

  http://cr.openjdk.java.net/~xuelei/8072452/webrev.02/

On 4/13/2016 8:26 AM, Valerie Peng wrote:
> Hi Xuelei,
> 
> Mostly look good, just some comments in line...
> 
> <src/java.base/share/classes/com/sun/crypto/provider/DHKeyPairGenerator.java>
> 
> line 95-100, so essentially, we will use the built-in parameters as much
> as possible and if none available, we will try to generate the
> parameters for key sizes <= 1024, right? The current comment is a little
> hard to parse. Maybe we can just explain it as below?
>      // Use the built-in parameters (ranging from 512 to 8192) when
> available
>      this.params = ParameterCache.getCachedDHParameterSpec(keysize);
>     // Due to performance issue, only support DH parameters generation
>     // up to 1024 bits
>     if ((this.params == null) && (keysize > 1024)) {
> 
Updated.

> <src/java.base/share/classes/sun/security/provider/DSAParameterGenerator.java>
> 
> - line 212, redundant?
Yes.  Updated.

> <src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11KeyPairGenerator.java>
> 
> - line 277-303, the "params"argument for this particular method is
> always null for non-RSA algorithms based on the usage. It is commented
> on line 232 and several other places as indicated by the "// XXX sanity
> check params" comments. So, more code changes will be needed here.
> Either you can add another boolean hasParams and rely on that argument,
> or change the specified argument type from RSAKeyGenParameterSpec to a
> generic one and then cast it later when it is needed.
> 
Good catch!  Updated per the latter proposal.

> <src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java>
> - line 141 - 146 and line 1548 - 1554 are identical. Seems to me that
> they can be customized a little as they are for different variable
> assignment. The "customizedDHKeySize" variable on line 147 is from user,
> right? Maybe what you really mean is something like the following?
>     // DH parameter generation can be extremely slow, best to use one of
> the
>     // supported pre-computed DH parameters (see DHCrypt class)
> As for line 1548 - 1554, it can be as simple as:
>     // DH parameter generation can be extremely slow, make sure to use
> one of the
>     // supported pre-computed DH parameters (see DHCrypt class)
Updated.

> - line 148, can customizedDHKeySize be 1024? The old code accepts it but
> the new code will error out. Is there a reason for rejecting 1024? The
> check only rejects "<1024" but yet the exception message says the value
> should be larger than 1024.
> Lastly, "should larger" => "should be larger" (line 150).
> 
1024 bits should not be rejected.  Updated the exception message.

Thanks for the comments!

Xuelei

> I will look at the tests next and send u comments separately.
> Thanks,
> Valerie
> On 4/1/2016 8:53 AM, Xuelei Fan wrote:
>> Hi,
>>
>> Please review this improvement update to support DHE sizes up to
>> 8192-bits and DSA sizes up to 3072-bits:
>>
>>    http://cr.openjdk.java.net/~xuelei/8072452/webrev.00
>>
>> This updated extends to support 3072-bits DH and DSA parameters
>> generation, and pre-computed DH parameters up to 8192 bits and
>> pre-computed DSA parameters up to 3072-bits.
>>
>> Thanks,
>> Xuelei